Skip to content

Fix a false-positive from 'unused' #585

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ require (
github.com/golangci/dupl v0.0.0-20180902072040-3e9179ac440a
github.com/golangci/errcheck v0.0.0-20181003203344-ef45e06d44b6
github.com/golangci/go-misc v0.0.0-20180628070357-927a3d87b613
github.com/golangci/go-tools v0.0.0-20180109140146-af6baa5dc196
github.com/golangci/go-tools v0.0.0-20190318055746-e32c54105b7c
github.com/golangci/goconst v0.0.0-20180610141641-041c5f2b40f3
github.com/golangci/gocyclo v0.0.0-20180528134321-2becd97e67ee
github.com/golangci/gofmt v0.0.0-20181105071733-0b8337e80d98
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ github.com/golangci/errcheck v0.0.0-20181003203344-ef45e06d44b6 h1:i2jIkQFb8RG45
github.com/golangci/errcheck v0.0.0-20181003203344-ef45e06d44b6/go.mod h1:DbHgvLiFKX1Sh2T1w8Q/h4NAI8MHIpzCdnBUDTXU3I0=
github.com/golangci/go-misc v0.0.0-20180628070357-927a3d87b613 h1:9kfjN3AdxcbsZBf8NjltjWihK2QfBBBZuv91cMFfDHw=
github.com/golangci/go-misc v0.0.0-20180628070357-927a3d87b613/go.mod h1:SyvUF2NxV+sN8upjjeVYr5W7tyxaT1JVtvhKhOn2ii8=
github.com/golangci/go-tools v0.0.0-20180109140146-af6baa5dc196 h1:9rtVlONXLF1rJZzvLt4tfOXtnAFUEhxCJ64Ibzj6ECo=
github.com/golangci/go-tools v0.0.0-20180109140146-af6baa5dc196/go.mod h1:unzUULGw35sjyOYjUt0jMTXqHlZPpPc6e+xfO4cd6mM=
github.com/golangci/go-tools v0.0.0-20190318055746-e32c54105b7c h1:/7detzz5stiXWPzkTlPTzkBEIIE4WGpppBJYjKqBiPI=
github.com/golangci/go-tools v0.0.0-20190318055746-e32c54105b7c/go.mod h1:unzUULGw35sjyOYjUt0jMTXqHlZPpPc6e+xfO4cd6mM=
github.com/golangci/goconst v0.0.0-20180610141641-041c5f2b40f3 h1:pe9JHs3cHHDQgOFXJJdYkK6fLz2PWyYtP4hthoCMvs8=
github.com/golangci/goconst v0.0.0-20180610141641-041c5f2b40f3/go.mod h1:JXrF4TWy4tXYn62/9x8Wm/K/dm06p8tCKwFRDPZG/1o=
github.com/golangci/gocyclo v0.0.0-20180528134321-2becd97e67ee h1:J2XAy40+7yz70uaOiMbNnluTg7gyQhtGqLQncQh+4J8=
Expand Down
18 changes: 15 additions & 3 deletions pkg/golinters/megacheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,10 +237,11 @@ func (m megacheck) runMegacheck(workingPkgs []*packages.Package, checkExportedUn
// TODO: support Ignores option
}

return runMegacheckCheckers(checkers, opts, workingPkgs)
return runMegacheckCheckers(checkers, workingPkgs, opts)
}

// parseIgnore is a copy from megacheck code just to not fork megacheck
// parseIgnore is a copy from megacheck honnef.co/go/tools/lint/lintutil.parseIgnore
// just to not fork megacheck.
func parseIgnore(s string) ([]lint.Ignore, error) {
var out []lint.Ignore
if s == "" {
Expand All @@ -258,17 +259,28 @@ func parseIgnore(s string) ([]lint.Ignore, error) {
return out, nil
}

func runMegacheckCheckers(cs []lint.Checker, opt *lintutil.Options, workingPkgs []*packages.Package) ([]lint.Problem, error) {
// runMegacheckCheckers is like megacheck honnef.co/go/tools/lint/lintutil.Lint,
// but takes a list of already-parsed packages instead of a list of
// package-paths to parse.
func runMegacheckCheckers(cs []lint.Checker, workingPkgs []*packages.Package, opt *lintutil.Options) ([]lint.Problem, error) {
stats := lint.PerfStats{
CheckerInits: map[string]time.Duration{},
}

if opt == nil {
opt = &lintutil.Options{}
}
ignores, err := parseIgnore(opt.Ignores)
if err != nil {
return nil, err
}

// package-parsing elided here
stats.PackageLoading = 0

var problems []lint.Problem
// populating 'problems' with parser-problems elided here

if len(workingPkgs) == 0 {
return problems, nil
}
Expand Down
39 changes: 23 additions & 16 deletions pkg/lint/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,6 @@ func (cl ContextLoader) makeFakeLoaderPackageInfo(pkg *packages.Package) *loader
}
}

func shouldSkipPkg(pkg *packages.Package) bool {
// it's an implicit testmain package
return pkg.Name == "main" && strings.HasSuffix(pkg.PkgPath, ".test")
}

func (cl ContextLoader) makeFakeLoaderProgram(pkgs []*packages.Package) *loader.Program {
var createdPkgs []*loader.PackageInfo
for _, pkg := range pkgs {
Expand Down Expand Up @@ -296,7 +291,7 @@ func (cl ContextLoader) loadPackages(ctx context.Context, loadMode packages.Load
return nil, err
}

return cl.filterPackages(pkgs), nil
return cl.filterTestMainPackages(pkgs), nil
}

func (cl ContextLoader) tryParseTestPackage(pkg *packages.Package) (name, testName string, isTest bool) {
Expand All @@ -308,7 +303,22 @@ func (cl ContextLoader) tryParseTestPackage(pkg *packages.Package) (name, testNa
return matches[1], matches[2], true
}

func (cl ContextLoader) filterPackages(pkgs []*packages.Package) []*packages.Package {
func (cl ContextLoader) filterTestMainPackages(pkgs []*packages.Package) []*packages.Package {
var retPkgs []*packages.Package
for _, pkg := range pkgs {
if pkg.Name == "main" && strings.HasSuffix(pkg.PkgPath, ".test") {
// it's an implicit testmain package
cl.debugf("skip pkg ID=%s", pkg.ID)
continue
}

retPkgs = append(retPkgs, pkg)
}

return retPkgs
}

func (cl ContextLoader) filterDuplicatePackages(pkgs []*packages.Package) []*packages.Package {
packagesWithTests := map[string]bool{}
for _, pkg := range pkgs {
name, _, isTest := cl.tryParseTestPackage(pkg)
Expand All @@ -322,11 +332,6 @@ func (cl ContextLoader) filterPackages(pkgs []*packages.Package) []*packages.Pac

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,
Expand All @@ -353,22 +358,24 @@ func (cl ContextLoader) Load(ctx context.Context, linters []*linter.Config) (*li
return nil, err
}

if len(pkgs) == 0 {
deduplicatedPkgs := cl.filterDuplicatePackages(pkgs)

if len(deduplicatedPkgs) == 0 {
return nil, exitcodes.ErrNoGoFiles
}

var prog *loader.Program
if loadMode&packages.NeedTypes != 0 {
prog = cl.makeFakeLoaderProgram(pkgs)
prog = cl.makeFakeLoaderProgram(deduplicatedPkgs)
}

var ssaProg *ssa.Program
if loadMode&packages.NeedDeps != 0 {
ssaProg = cl.buildSSAProgram(pkgs)
ssaProg = cl.buildSSAProgram(deduplicatedPkgs)
}

astLog := cl.log.Child("astcache")
astCache, err := astcache.LoadFromPackages(pkgs, astLog)
astCache, err := astcache.LoadFromPackages(deduplicatedPkgs, astLog)
if err != nil {
return nil, err
}
Expand Down
4 changes: 4 additions & 0 deletions test/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ func TestIdentifierUsedOnlyInTests(t *testing.T) {
testshared.NewLintRunner(t).Run("--no-config", "--disable-all", "-Eunused", getTestDataDir("used_only_in_tests")).ExpectNoIssues()
}

func TestUnusedCheckExported(t *testing.T) {
testshared.NewLintRunner(t).Run("-c", "testdata_etc/unused_exported/golangci.yml", "testdata_etc/unused_exported/...").ExpectNoIssues()
}

func TestConfigFileIsDetected(t *testing.T) {
checkGotConfig := func(r *testshared.RunResult) {
r.ExpectExitCode(exitcodes.Success).
Expand Down
7 changes: 7 additions & 0 deletions test/testdata_etc/unused_exported/golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
linters:
disable-all: true
enable:
- unused
linters-settings:
unused:
check-exported: true
1 change: 1 addition & 0 deletions test/testdata_etc/unused_exported/lib/bar_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
package lib
13 changes: 13 additions & 0 deletions test/testdata_etc/unused_exported/lib/foo.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package lib

import (
"fmt"
)

func PublicFunc() {
privateFunc()
}

func privateFunc() {
fmt.Println("side effect")
}
9 changes: 9 additions & 0 deletions test/testdata_etc/unused_exported/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package main

import (
"github.com/golangci/golangci-lint/test/testdata_etc/unused_exported/lib"
)

func main() {
lib.PublicFunc()
}
13 changes: 1 addition & 12 deletions vendor/github.com/golangci/go-tools/lint/lint.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion vendor/modules.txt
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ github.com/golangci/errcheck/golangci
github.com/golangci/errcheck/internal/errcheck
# github.com/golangci/go-misc v0.0.0-20180628070357-927a3d87b613
github.com/golangci/go-misc/deadcode
# github.com/golangci/go-tools v0.0.0-20180109140146-af6baa5dc196
# github.com/golangci/go-tools v0.0.0-20190318055746-e32c54105b7c
github.com/golangci/go-tools/config
github.com/golangci/go-tools/lint
github.com/golangci/go-tools/lint/lintutil
Expand Down