diff --git a/Makefile b/Makefile index f168813fec6b..0e5aad6597a2 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,5 @@ test: - go install -i ./cmd/... # needed for govet and golint + go install ./cmd/... # needed for govet and golint golangci-lint run -v golangci-lint run --fast --no-config -v golangci-lint run --no-config -v diff --git a/README.md b/README.md index c27c6087917e..83eee5470746 100644 --- a/README.md +++ b/README.md @@ -371,18 +371,18 @@ nolint comment, diff, regexps; prettify paths, etc. We use `cobra` for command-line options. # FAQ -**Q: How do you add a custom linter?** +**How do you add a custom linter?** -A: You can integrate it yourself, see this [wiki page](https://github.com/golangci/golangci-lint/wiki/How-to-add-a-custom-linter) with documentation. Or you can create a [GitHub Issue](https://github.com/golangci/golangci-lint/issues/new) and we will integrate when time permits. +You can integrate it yourself, see this [wiki page](https://github.com/golangci/golangci-lint/wiki/How-to-add-a-custom-linter) with documentation. Or you can create a [GitHub Issue](https://github.com/golangci/golangci-lint/issues/new) and we will integrate when time permits. -**Q: It's cool to use `golangci-lint` when starting a project, but what about existing projects with large codebase? It will take days to fix all found issues** +**It's cool to use `golangci-lint` when starting a project, but what about existing projects with large codebase? It will take days to fix all found issues** -A: We are sure that every project can easily integrate `golangci-lint`, even the large one. The idea is to not fix all existing issues. Fix only newly added issue: issues in new code. To do this setup CI (or better use [GolangCI](https://golangci.com) to run `golangci-lint` with option `--new-from-rev=origin/master`. Also, take a look at option `-n`. +We are sure that every project can easily integrate `golangci-lint`, even the large one. The idea is to not fix all existing issues. Fix only newly added issue: issues in new code. To do this setup CI (or better use [GolangCI](https://golangci.com) to run `golangci-lint` with option `--new-from-rev=origin/master`. Also, take a look at option `-n`. By doing this you won't create new issues in your code and can choose fix existing issues (or not). -**Q: How to use `golangci-lint` in CI (Continuous Integration)?** +**How to use `golangci-lint` in CI (Continuous Integration)?** -A: You have 2 choices: +You have 2 choices: 1. Use [GolangCI](https://golangci.com): this service is highly integrated with GitHub (issues are commented in the pull request) and uses a `golangci-lint` tool. For configuration use `.golangci.yml` (or toml/json). 2. Use custom CI: just run `golangci-lint` in CI and check the exit code. If it's non-zero - fail the build. The main disadvantage is that you can't see issues in pull request code and would need to view the build log, then open the referenced source file to see the context. If you'd like to vendor `golangci-lint` in your repo, run: @@ -403,12 +403,14 @@ go install ./vendor/github.com/golangci/golangci-lint/cmd/golangci-lint/` ``` Vendoring `golangci-lint` saves a network request, potentially making your CI system a little more reliable. -**Q: `govet` or `golint` reports false-positives or false-negatives** +**`govet` or `golint` reports false-positives or false-negatives** + These linters obtain type information from installed package files. The same issue you will encounter if will run `govet` or `golint` without `golangci-lint`. -Try `go install -i ./...` (or `go install -i ./cmd/...`: depends on the project) first. +Try `go install ./...` (or `go install ./cmd/...`: depends on the project) first. + +**`golangci-lint` doesn't work** -**Q: `golangci-lint` doesn't work** 1. Update it: `go get -u github.com/golangci/golangci-lint/cmd/golangci-lint` 2. Run it with `-v` option and check the output. 3. If it doesn't help create a [GitHub issue](https://github.com/golangci/golangci-lint/issues/new) with the output from the error and #2 above. diff --git a/README.md.tmpl b/README.md.tmpl index 0c0816855d3f..cdf190a5b0dd 100644 --- a/README.md.tmpl +++ b/README.md.tmpl @@ -243,18 +243,18 @@ nolint comment, diff, regexps; prettify paths, etc. We use `cobra` for command-line options. # FAQ -**Q: How do you add a custom linter?** +**How do you add a custom linter?** -A: You can integrate it yourself, see this [wiki page](https://github.com/golangci/golangci-lint/wiki/How-to-add-a-custom-linter) with documentation. Or you can create a [GitHub Issue](https://github.com/golangci/golangci-lint/issues/new) and we will integrate when time permits. +You can integrate it yourself, see this [wiki page](https://github.com/golangci/golangci-lint/wiki/How-to-add-a-custom-linter) with documentation. Or you can create a [GitHub Issue](https://github.com/golangci/golangci-lint/issues/new) and we will integrate when time permits. -**Q: It's cool to use `golangci-lint` when starting a project, but what about existing projects with large codebase? It will take days to fix all found issues** +**It's cool to use `golangci-lint` when starting a project, but what about existing projects with large codebase? It will take days to fix all found issues** -A: We are sure that every project can easily integrate `golangci-lint`, even the large one. The idea is to not fix all existing issues. Fix only newly added issue: issues in new code. To do this setup CI (or better use [GolangCI](https://golangci.com) to run `golangci-lint` with option `--new-from-rev=origin/master`. Also, take a look at option `-n`. +We are sure that every project can easily integrate `golangci-lint`, even the large one. The idea is to not fix all existing issues. Fix only newly added issue: issues in new code. To do this setup CI (or better use [GolangCI](https://golangci.com) to run `golangci-lint` with option `--new-from-rev=origin/master`. Also, take a look at option `-n`. By doing this you won't create new issues in your code and can choose fix existing issues (or not). -**Q: How to use `golangci-lint` in CI (Continuous Integration)?** +**How to use `golangci-lint` in CI (Continuous Integration)?** -A: You have 2 choices: +You have 2 choices: 1. Use [GolangCI](https://golangci.com): this service is highly integrated with GitHub (issues are commented in the pull request) and uses a `golangci-lint` tool. For configuration use `.golangci.yml` (or toml/json). 2. Use custom CI: just run `golangci-lint` in CI and check the exit code. If it's non-zero - fail the build. The main disadvantage is that you can't see issues in pull request code and would need to view the build log, then open the referenced source file to see the context. If you'd like to vendor `golangci-lint` in your repo, run: @@ -275,12 +275,14 @@ go install ./vendor/github.com/golangci/golangci-lint/cmd/golangci-lint/` ``` Vendoring `golangci-lint` saves a network request, potentially making your CI system a little more reliable. -**Q: `govet` or `golint` reports false-positives or false-negatives** +**`govet` or `golint` reports false-positives or false-negatives** + These linters obtain type information from installed package files. The same issue you will encounter if will run `govet` or `golint` without `golangci-lint`. -Try `go install -i ./...` (or `go install -i ./cmd/...`: depends on the project) first. +Try `go install ./...` (or `go install ./cmd/...`: depends on the project) first. + +**`golangci-lint` doesn't work** -**Q: `golangci-lint` doesn't work** 1. Update it: `go get -u github.com/golangci/golangci-lint/cmd/golangci-lint` 2. Run it with `-v` option and check the output. 3. If it doesn't help create a [GitHub issue](https://github.com/golangci/golangci-lint/issues/new) with the output from the error and #2 above. diff --git a/pkg/commands/root.go b/pkg/commands/root.go index 92e491ad848a..d4ac1b725e5a 100644 --- a/pkg/commands/root.go +++ b/pkg/commands/root.go @@ -8,6 +8,7 @@ import ( "runtime/pprof" "github.com/golangci/golangci-lint/pkg/config" + "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/printers" "github.com/sirupsen/logrus" "github.com/spf13/cobra" @@ -16,7 +17,9 @@ import ( func setupLog(isVerbose bool) { log.SetFlags(0) // don't print time - if isVerbose { + if logutils.IsDebugEnabled() { + logrus.SetLevel(logrus.DebugLevel) + } else if isVerbose { logrus.SetLevel(logrus.InfoLevel) } } diff --git a/pkg/logutils/logutils.go b/pkg/logutils/logutils.go index 4661bd5f33e7..affd07d5fca1 100644 --- a/pkg/logutils/logutils.go +++ b/pkg/logutils/logutils.go @@ -2,6 +2,7 @@ package logutils import ( "os" + "strings" "github.com/sirupsen/logrus" ) @@ -15,3 +16,38 @@ func HiddenWarnf(format string, args ...interface{}) { logrus.Infof(format, args...) } } + +func getEnabledDebugs() map[string]bool { + ret := map[string]bool{} + debugVar := os.Getenv("GL_DEBUG") + if debugVar == "" { + return ret + } + + for _, tag := range strings.Split(debugVar, ",") { + ret[tag] = true + } + + return ret +} + +var enabledDebugs = getEnabledDebugs() + +type DebugFunc func(format string, args ...interface{}) + +func nopDebugf(format string, args ...interface{}) {} + +func Debug(tag string) DebugFunc { + if !enabledDebugs[tag] { + return nopDebugf + } + + return func(format string, args ...interface{}) { + newArgs := append([]interface{}{tag}, args...) + logrus.Debugf("%s: "+format, newArgs...) + } +} + +func IsDebugEnabled() bool { + return len(enabledDebugs) != 0 +} diff --git a/pkg/result/processors/autogenerated_exclude.go b/pkg/result/processors/autogenerated_exclude.go index 276ce05e95b9..3a12b5891d8a 100644 --- a/pkg/result/processors/autogenerated_exclude.go +++ b/pkg/result/processors/autogenerated_exclude.go @@ -7,9 +7,12 @@ import ( "strings" "github.com/golangci/golangci-lint/pkg/lint/astcache" + "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" ) +var autogenDebugf = logutils.Debug("autogen_exclude") + type ageFileSummary struct { isGenerated bool } @@ -62,10 +65,12 @@ func isGeneratedFileByComment(doc string) bool { doc = strings.ToLower(doc) for _, marker := range markers { if strings.Contains(doc, marker) { + autogenDebugf("doc contains marker %q: file is generated", marker) return true } } + autogenDebugf("doc of len %d doesn't contain any of markers: %s", len(doc), markers) return false } @@ -83,27 +88,43 @@ func (p *AutogeneratedExclude) getOrCreateFileSummary(i *result.Issue) (*ageFile return nil, fmt.Errorf("can't parse file %s: %s", i.FilePath(), f.Err) } - doc := getDoc(f.F, f.Fset) + autogenDebugf("file %q: astcache file is %+v", i.FilePath(), *f) + + doc := getDoc(f.F, f.Fset, i.FilePath()) fs.isGenerated = isGeneratedFileByComment(doc) + autogenDebugf("file %q is generated: %t", i.FilePath(), fs.isGenerated) return fs, nil } -func getDoc(f *ast.File, fset *token.FileSet) string { +func getDoc(f *ast.File, fset *token.FileSet, filePath string) string { // don't use just f.Doc: e.g. mockgen leaves extra line between comment and package name - importPos := f.End() + var importPos token.Pos if len(f.Imports) != 0 { importPos = f.Imports[0].Pos() + autogenDebugf("file %q: search comments until first import pos %d (%s)", filePath, importPos, fset.Position(importPos)) + } else { + importPos = f.End() + autogenDebugf("file %q: search comments until EOF pos %d (%s)", filePath, importPos, fset.Position(importPos)) } var neededComments []string for _, g := range f.Comments { - if g.Pos() < importPos && fset.Position(g.Pos()).Column == 1 { - neededComments = append(neededComments, g.Text()) + pos := g.Pos() + filePos := fset.Position(pos) + text := g.Text() + isAllowed := pos < importPos && filePos.Column == 1 + if isAllowed { + autogenDebugf("file %q: pos=%d, filePos=%s: comment %q: it's allowed", filePath, pos, filePos, text) + neededComments = append(neededComments, text) + } else { + autogenDebugf("file %q: pos=%d, filePos=%s: comment %q: it's NOT allowed", filePath, pos, filePos, text) } } + autogenDebugf("file %q: got %d allowed comments", filePath, len(neededComments)) + if len(neededComments) == 0 { return "" } diff --git a/pkg/result/processors/autogenerated_exclude_test.go b/pkg/result/processors/autogenerated_exclude_test.go index a8820dcb8a82..e0ca23642d91 100644 --- a/pkg/result/processors/autogenerated_exclude_test.go +++ b/pkg/result/processors/autogenerated_exclude_test.go @@ -1,13 +1,9 @@ package processors import ( - "go/token" - "path/filepath" "strings" "testing" - "github.com/golangci/golangci-lint/pkg/lint/astcache" - "github.com/golangci/golangci-lint/pkg/result" "github.com/stretchr/testify/assert" ) @@ -66,23 +62,3 @@ func TestIsAutogeneratedDetection(t *testing.T) { assert.False(t, isGenerated) } } - -func TestNoIssuesInAutogeneratedFiles(t *testing.T) { - files := []string{ - "autogenerated.go", - "autogenerated_alt_hdr.go", - "autogenerated_alt_hdr2.go", - } - for _, file := range files { - t.Run(file, func(t *testing.T) { - i := result.Issue{ - Pos: token.Position{ - Filename: filepath.Join("testdata", file), - Line: 4, - }, - } - p := NewAutogeneratedExclude(astcache.NewCache()) - processAssertEmpty(t, p, i) - }) - } -} diff --git a/pkg/result/processors/testdata/autogenerated.go b/test/testdata/autogenerated/autogenerated.go similarity index 75% rename from pkg/result/processors/testdata/autogenerated.go rename to test/testdata/autogenerated/autogenerated.go index d4878924040b..bbf25c842379 100644 --- a/pkg/result/processors/testdata/autogenerated.go +++ b/test/testdata/autogenerated/autogenerated.go @@ -1,4 +1,4 @@ // Code generated by ... DO NOT EDIT. -package testdata +package p var vvv int diff --git a/test/testdata/autogenerated/gen.go b/test/testdata/autogenerated/do_not_edit.go similarity index 100% rename from test/testdata/autogenerated/gen.go rename to test/testdata/autogenerated/do_not_edit.go diff --git a/pkg/result/processors/testdata/autogenerated_alt_hdr2.go b/test/testdata/autogenerated/go_bindata.go similarity index 84% rename from pkg/result/processors/testdata/autogenerated_alt_hdr2.go rename to test/testdata/autogenerated/go_bindata.go index 8739d8daae75..164e7ba13e89 100644 --- a/pkg/result/processors/testdata/autogenerated_alt_hdr2.go +++ b/test/testdata/autogenerated/go_bindata.go @@ -3,6 +3,6 @@ // bar.baz // x/y.z // DO NOT EDIT! -package testdata +package p var vv int diff --git a/test/testdata/autogenerated/mockgen.go b/test/testdata/autogenerated/mockgen.go index 0d02e1d3e4e2..6e147bd60bf8 100644 --- a/test/testdata/autogenerated/mockgen.go +++ b/test/testdata/autogenerated/mockgen.go @@ -3,4 +3,4 @@ package p -func unusedFunc2() {} +func UnusedFunc2() {} diff --git a/pkg/result/processors/testdata/autogenerated_alt_hdr.go b/test/testdata/autogenerated/protoc_gen_foo.go similarity index 83% rename from pkg/result/processors/testdata/autogenerated_alt_hdr.go rename to test/testdata/autogenerated/protoc_gen_foo.go index a88b80492370..f772fabb2703 100644 --- a/pkg/result/processors/testdata/autogenerated_alt_hdr.go +++ b/test/testdata/autogenerated/protoc_gen_foo.go @@ -1,6 +1,6 @@ // Code generated by protoc-gen-foo // source: bar.proto // DO NOT EDIT!!! -package testdata +package p var v int