Merge pull request #89 from golangci/support/fix-mockgen-detecting

Write debug logs for autogen excluding for #86

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

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.

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.

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)
 	}
 }

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
+}

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 ""
 	}

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)
-		})
-	}
-}

test/testdata/autogenerated/autogenerated.go

@@ -1,4 +1,4 @@
 // Code generated by ... DO NOT EDIT.
-package testdata
+package p
 
 var vvv int

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

test/testdata/autogenerated/mockgen.go

@@ -3,4 +3,4 @@
 
 package p
 
-func unusedFunc2() {}
+func UnusedFunc2() {}

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