Fix false positives with unused identifiers

Issue #265: don't report unused warning when an identifier is
used only in tests.
This commit is contained in:
Denis Isaev 2018-11-05 14:00:48 +03:00 committed by Isaev Denis
parent 3345c7136f
commit ccac35a87e
5 changed files with 84 additions and 14 deletions

View File

@ -2,5 +2,6 @@ Thank you for creating the issue!
Please include the following information:
1. Version of golangci-lint: `golangci-lint --version` (or git commit if you don't use binary distribution)
2. Go environment: `go version && go env`
3. Verbose output of running: `golangci-lint run -v`
2. Config file: `cat .golangci.yml`
3. Go environment: `go version && go env`
4. Verbose output of running: `golangci-lint run -v`

View File

@ -7,6 +7,7 @@ import (
"go/types"
"os"
"path/filepath"
"regexp"
"strings"
"time"
@ -25,18 +26,20 @@ import (
)
type ContextLoader struct {
cfg *config.Config
log logutils.Log
debugf logutils.DebugFunc
goenv *goutil.Env
cfg *config.Config
log logutils.Log
debugf logutils.DebugFunc
goenv *goutil.Env
pkgTestIDRe *regexp.Regexp
}
func NewContextLoader(cfg *config.Config, log logutils.Log, goenv *goutil.Env) *ContextLoader {
return &ContextLoader{
cfg: cfg,
log: log,
debugf: logutils.Debug("loader"),
goenv: goenv,
cfg: cfg,
log: log,
debugf: logutils.Debug("loader"),
goenv: goenv,
pkgTestIDRe: regexp.MustCompile(`^(.*) \[(.*)\.test\]`),
}
}
@ -222,19 +225,65 @@ func (cl ContextLoader) loadPackages(ctx context.Context, loadMode packages.Load
i, pkg.ID, pkg.GoFiles, pkg.CompiledGoFiles, syntaxFiles)
}
var retPkgs []*packages.Package
for _, pkg := range pkgs {
for _, err := range pkg.Errors {
if strings.Contains(err.Msg, "no Go files") {
return nil, errors.Wrapf(exitcodes.ErrNoGoFiles, "package %s", pkg.PkgPath)
}
}
if !shouldSkipPkg(pkg) {
retPkgs = append(retPkgs, pkg)
}
return cl.filterPackages(pkgs), nil
}
func (cl ContextLoader) tryParseTestPackage(pkg *packages.Package) (name, testName string, isTest bool) {
matches := cl.pkgTestIDRe.FindStringSubmatch(pkg.ID)
if matches == nil {
return "", "", false
}
return matches[1], matches[2], true
}
func (cl ContextLoader) filterPackages(pkgs []*packages.Package) []*packages.Package {
packagesWithTests := map[string]bool{}
for _, pkg := range pkgs {
name, testName, isTest := cl.tryParseTestPackage(pkg)
if !isTest {
continue
}
packagesWithTests[name] = true
if name != testName {
cl.log.Infof("pkg ID=%s: %s != %s: %#v", pkg.ID, name, testName, pkg)
}
}
return retPkgs, nil
cl.debugf("package with tests: %#v", packagesWithTests)
var retPkgs []*packages.Package
for _, pkg := range pkgs {
if shouldSkipPkg(pkg) {
cl.debugf("skip pkg ID=%s", pkg.ID)
continue
}
_, _, isTest := cl.tryParseTestPackage(pkg)
if !isTest && packagesWithTests[pkg.PkgPath] {
// If tests loading is enabled,
// for package with files a.go and a_test.go go/packages loads two packages:
// 1. ID=".../a" GoFiles=[a.go]
// 2. ID=".../a [.../a.test]" GoFiles=[a.go a_test.go]
// We need only the second package, otherwise we can get warnings about unused variables/fields/functions
// in a.go if they are used only in a_test.go.
cl.debugf("skip pkg ID=%s because we load it with test package", pkg.ID)
continue
}
retPkgs = append(retPkgs, pkg)
}
return retPkgs
}
//nolint:gocyclo

View File

@ -163,6 +163,12 @@ func TestDeadcodeNoFalsePositivesInMainPkg(t *testing.T) {
checkNoIssuesRun(t, out, exitCode)
}
func TestIdentifierUsedOnlyInTests(t *testing.T) {
out, exitCode := runGolangciLint(t, "--no-config", "--disable-all", "-Eunused",
filepath.Join(testdataDir, "used_only_in_tests"))
checkNoIssuesRun(t, out, exitCode)
}
func TestConfigFileIsDetected(t *testing.T) {
checkGotConfig := func(out string, exitCode int) {
assert.Equal(t, exitcodes.Success, exitCode, out)

5
test/testdata/used_only_in_tests/a.go vendored Normal file
View File

@ -0,0 +1,5 @@
package p
func f() bool {
return true
}

View File

@ -0,0 +1,9 @@
package p
import "testing"
func TestF(t *testing.T) {
if !f() {
t.Fail()
}
}