Fix linting of preprocessed files

Preprocessed files like .qtpl.go quicktemplate Go files can have
//line directives. They map to a source .qtpl file.
This commit fixes linting of such files:
1. don't fail on AST cache loading
2. output Go filename not .qtpl or similar

Also, here we update golint to the upstream version.

Relates: #316, #466, #467, #468

Author: jirfag

.travis.yml

@@ -3,6 +3,10 @@ language: go
 go:
   - 1.11.x
   - 1.12.x
+
+before_script:
+  - go get github.com/valyala/quicktemplate
+
 script: make check_generated test
 
 after_success:

go.mod

@@ -20,7 +20,7 @@ require (
 	github.com/golangci/gofmt v0.0.0-20181105071733-0b8337e80d98
 	github.com/golangci/gosec v0.0.0-20180901114220-66fb7fc33547
 	github.com/golangci/ineffassign v0.0.0-20180808204949-2ee8f2867dde
-	github.com/golangci/lint-1 v0.0.0-20180610141402-4bf9709227d1
+	github.com/golangci/lint-1 v0.0.0-20180610141402-ee948d087217
 	github.com/golangci/maligned v0.0.0-20180506175553-b1d89398deca
 	github.com/golangci/misspell v0.0.0-20180809174111-950f5d19e770
 	github.com/golangci/prealloc v0.0.0-20180630174525-215b22d4de21

go.sum

@@ -60,8 +60,8 @@ github.com/golangci/gosec v0.0.0-20180901114220-66fb7fc33547 h1:qMomh8bv+kDazm1d
 github.com/golangci/gosec v0.0.0-20180901114220-66fb7fc33547/go.mod h1:0qUabqiIQgfmlAmulqxyiGkkyF6/tOGSnY2cnPVwrzU=
 github.com/golangci/ineffassign v0.0.0-20180808204949-2ee8f2867dde h1:qEGp3ZF1Qw6TkbWKn6GdJ12Ssu/CpJBaBcJ4hrUjrSo=
 github.com/golangci/ineffassign v0.0.0-20180808204949-2ee8f2867dde/go.mod h1:e5tpTHCfVze+7EpLEozzMB3eafxo2KT5veNg1k6byQU=
-github.com/golangci/lint-1 v0.0.0-20180610141402-4bf9709227d1 h1:PHK2kIh21Zt4IcG0bBRzQwEDVKF64LnkoSXnm8lfJUk=
-github.com/golangci/lint-1 v0.0.0-20180610141402-4bf9709227d1/go.mod h1:/X8TswGSh1pIozq4ZwCfxS0WA5JGXguxk94ar/4c87Y=
+github.com/golangci/lint-1 v0.0.0-20180610141402-ee948d087217 h1:r7vyX+SN24x6+5AnpnrRn/bdwBb7U+McZqCHOVtXDuk=
+github.com/golangci/lint-1 v0.0.0-20180610141402-ee948d087217/go.mod h1:66R6K6P6VWk9I95jvqGxkqJxVWGFy9XlDwLwVz1RCFg=
 github.com/golangci/maligned v0.0.0-20180506175553-b1d89398deca h1:kNY3/svz5T29MYHubXix4aDDuE3RWHkPvopM/EDv/MA=
 github.com/golangci/maligned v0.0.0-20180506175553-b1d89398deca/go.mod h1:tvlJhZqDe4LMs4ZHD0oMUlt9G2LWuDGoisJTBzLMV9o=
 github.com/golangci/misspell v0.0.0-20180809174111-950f5d19e770 h1:EL/O5HGrF7Jaq0yNhBLucz9hTuRzj2LdwGBOaENgxIk=
@@ -171,6 +171,7 @@ golang.org/x/tools v0.0.0-20180221164845-07fd8470d635/go.mod h1:n7NCudcB/nEzxVGm
 golang.org/x/tools v0.0.0-20181117154741-2ddaf7f79a09/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 golang.org/x/tools v0.0.0-20181205014116-22934f0fdb62/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 golang.org/x/tools v0.0.0-20190121143147-24cd39ecf745/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
+golang.org/x/tools v0.0.0-20190311212946-11955173bddd/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs=
 golang.org/x/tools v0.0.0-20190420000508-685fecacd0a0 h1:pa1CyBALPFjblgkNQp7T7gEcFcG/GOG5Ck8IcnSVWGs=
 golang.org/x/tools v0.0.0-20190420000508-685fecacd0a0/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs=
 gopkg.in/airbrake/gobrake.v2 v2.0.9 h1:7z2uVWwn7oVeeugY1DtlPAy5H+KYgB1KeKTnqjNatLo=

pkg/lint/astcache/astcache.go

@@ -5,7 +5,6 @@ import (
 	"go/parser"
 	"go/token"
 	"path/filepath"
-	"time"
 
 	"golang.org/x/tools/go/packages"
 
@@ -107,34 +106,37 @@ func LoadFromPackages(pkgs []*packages.Package, log logutils.Log) (*Cache, error
 	return c, nil
 }
 
-func (c *Cache) loadFromPackage(pkg *packages.Package) {
-	if len(pkg.Syntax) == 0 || len(pkg.GoFiles) != len(pkg.CompiledGoFiles) {
-		// len(pkg.Syntax) == 0 if only filenames are loaded
-		// lengths aren't equal if there are preprocessed files (cgo)
-		startedAt := time.Now()
-
-		// can't use pkg.Fset: it will overwrite offsets by preprocessed files
-		fset := token.NewFileSet()
-		for _, f := range pkg.GoFiles {
-			c.parseFile(f, fset)
-		}
+func (c *Cache) extractFilenamesForAstFile(fset *token.FileSet, f *ast.File) []string {
+	var ret []string
 
-		c.log.Infof("Parsed AST of all pkg.GoFiles: %s for %s", pkg.GoFiles, time.Since(startedAt))
-		return
+	// false ignores //line comments: name can be incorrect for generated files with //line directives
+	// mapping e.g. from .rl to .go files.
+	pos := fset.PositionFor(f.Pos(), false)
+	if pos.Filename != "" {
+		ret = append(ret, pos.Filename)
 	}
 
+	return ret
+}
+
+func (c *Cache) loadFromPackage(pkg *packages.Package) {
 	for _, f := range pkg.Syntax {
-		pos := pkg.Fset.Position(f.Pos())
-		if pos.Filename == "" {
-			continue
+		for _, filename := range c.extractFilenamesForAstFile(pkg.Fset, f) {
+			filePath := c.normalizeFilename(filename)
+			c.m[filePath] = &File{
+				F:    f,
+				Fset: pkg.Fset,
+				Name: filePath,
+			}
 		}
+	}
 
-		filePath := c.normalizeFilename(pos.Filename)
-
-		c.m[filePath] = &File{
-			F:    f,
-			Fset: pkg.Fset,
-			Name: filePath,
+	// some Go files sometimes aren't present in pkg.Syntax
+	fset := token.NewFileSet() // can't use pkg.Fset: it will overwrite offsets by preprocessed files
+	for _, filePath := range pkg.GoFiles {
+		filePath = c.normalizeFilename(filePath)
+		if c.m[filePath] == nil {
+			c.parseFile(filePath, fset)
 		}
 	}
 }

pkg/lint/runner.go

@@ -67,8 +67,9 @@ func NewRunner(astCache *astcache.Cache, cfg *config.Config, log logutils.Log, g
 
 	return &Runner{
 		Processors: []processors.Processor{
-			processors.NewPathPrettifier(), // must be before diff, nolint and exclude autogenerated processor at least
 			processors.NewCgo(goenv),
+			processors.NewFilenameUnadjuster(astCache, log.Child("filename_unadjuster")), // must go after Cgo
+			processors.NewPathPrettifier(), // must be before diff, nolint and exclude autogenerated processor at least
 			skipFilesProcessor,
 			skipDirsProcessor, // must be after path prettifier
 

pkg/result/processors/filename_unadjuster.go

@@ -0,0 +1,87 @@
+package processors
+
+import (
+	"go/token"
+	"path/filepath"
+	"strings"
+
+	"github.com/golangci/golangci-lint/pkg/logutils"
+
+	"github.com/golangci/golangci-lint/pkg/lint/astcache"
+
+	"github.com/golangci/golangci-lint/pkg/result"
+)
+
+type posMapper func(pos token.Position) token.Position
+
+// FilenameUnadjuster is needed because a lot of linters use fset.Position(f.Pos())
+// to get filename. And they return adjusted filename (e.g. *.qtpl) for an issue. We need
+// restore real .go filename to properly output it, parse it, etc.
+type FilenameUnadjuster struct {
+	m   map[string]posMapper // map from adjusted filename to position mapper: adjusted -> unadjusted position
+	log logutils.Log
+}
+
+var _ Processor = FilenameUnadjuster{}
+
+func NewFilenameUnadjuster(cache *astcache.Cache, log logutils.Log) *FilenameUnadjuster {
+	m := map[string]posMapper{}
+	for _, f := range cache.GetAllValidFiles() {
+		adjustedFilename := f.Fset.PositionFor(f.F.Pos(), true).Filename
+		if adjustedFilename == "" {
+			continue
+		}
+		unadjustedFilename := f.Fset.PositionFor(f.F.Pos(), false).Filename
+		if unadjustedFilename == "" || unadjustedFilename == adjustedFilename {
+			continue
+		}
+		if !strings.HasSuffix(unadjustedFilename, ".go") {
+			continue // file.go -> /caches/cgo-xxx
+		}
+
+		f := f
+		m[adjustedFilename] = func(adjustedPos token.Position) token.Position {
+			tokenFile := f.Fset.File(f.F.Pos())
+			if tokenFile == nil {
+				log.Warnf("Failed to get token file for %s", adjustedFilename)
+				return adjustedPos
+			}
+			return f.Fset.PositionFor(tokenFile.Pos(adjustedPos.Offset), false)
+		}
+	}
+
+	return &FilenameUnadjuster{
+		m:   m,
+		log: log,
+	}
+}
+
+func (p FilenameUnadjuster) Name() string {
+	return "filename_unadjuster"
+}
+
+func (p FilenameUnadjuster) Process(issues []result.Issue) ([]result.Issue, error) {
+	return transformIssues(issues, func(i *result.Issue) *result.Issue {
+		issueFilePath := i.FilePath()
+		if !filepath.IsAbs(i.FilePath()) {
+			absPath, err := filepath.Abs(i.FilePath())
+			if err != nil {
+				p.log.Warnf("failed to build abs path for %q: %s", i.FilePath(), err)
+				return i
+			}
+			issueFilePath = absPath
+		}
+
+		mapper := p.m[issueFilePath]
+		if mapper == nil {
+			return i
+		}
+
+		newI := *i
+		newI.Pos = mapper(i.Pos)
+		p.log.Infof("Unadjusted from %v to %v", i.Pos, newI.Pos)
+		return &newI
+	}), nil
+}
+
+func (FilenameUnadjuster) Finish() {}

pkg/result/processors/nolint.go

@@ -7,6 +7,8 @@ import (
 	"sort"
 	"strings"
 
+	"github.com/pkg/errors"
+
 	"github.com/golangci/golangci-lint/pkg/lint/astcache"
 	"github.com/golangci/golangci-lint/pkg/lint/lintersdb"
 	"github.com/golangci/golangci-lint/pkg/logutils"
@@ -88,8 +90,12 @@ func (p *Nolint) getOrCreateFileData(i *result.Issue) (*fileData, error) {
 	}
 
 	file := p.astCache.Get(i.FilePath())
-	if file == nil || file.Err != nil {
-		return nil, fmt.Errorf("can't parse file %s: %v, astcache is %v", i.FilePath(), file, p.astCache.ParsedFilenames())
+	if file == nil {
+		return nil, fmt.Errorf("no file %s in ast cache %v",
+			i.FilePath(), p.astCache.ParsedFilenames())
+	}
+	if file.Err != nil {
+		return nil, errors.Wrapf(file.Err, "can't parse file %s", i.FilePath())
 	}
 
 	fd.ignoredRanges = p.buildIgnoredRangesForFile(file.F, file.Fset, i.FilePath())

test/run_test.go

@@ -2,6 +2,7 @@ package test
 
 import (
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -67,6 +68,30 @@ func TestGovetCustomFormatter(t *testing.T) {
 	testshared.NewLintRunner(t).Run(getTestDataDir("govet_custom_formatter")).ExpectNoIssues()
 }
 
+func TestLineDirectiveProcessedFilesLiteLoading(t *testing.T) {
+	r := testshared.NewLintRunner(t).Run("--print-issued-lines=false", "--no-config",
+		"--exclude-use-default=false", "-Egolint", getTestDataDir("quicktemplate"))
+
+	output := strings.Join([]string{
+		"testdata/quicktemplate/hello.qtpl.go:26:1: exported function `StreamHello` should have comment or be unexported (golint)",
+		"testdata/quicktemplate/hello.qtpl.go:50:1: exported function `Hello` should have comment or be unexported (golint)",
+		"testdata/quicktemplate/hello.qtpl.go:39:1: exported function `WriteHello` should have comment or be unexported (golint)",
+	}, "\n")
+	r.ExpectExitCode(exitcodes.IssuesFound).ExpectOutputEq(output + "\n")
+}
+
+func TestLineDirectiveProcessedFilesFullLoading(t *testing.T) {
+	r := testshared.NewLintRunner(t).Run("--print-issued-lines=false", "--no-config",
+		"--exclude-use-default=false", "-Egolint,govet", getTestDataDir("quicktemplate"))
+
+	output := strings.Join([]string{
+		"testdata/quicktemplate/hello.qtpl.go:26:1: exported function `StreamHello` should have comment or be unexported (golint)",
+		"testdata/quicktemplate/hello.qtpl.go:50:1: exported function `Hello` should have comment or be unexported (golint)",
+		"testdata/quicktemplate/hello.qtpl.go:39:1: exported function `WriteHello` should have comment or be unexported (golint)",
+	}, "\n")
+	r.ExpectExitCode(exitcodes.IssuesFound).ExpectOutputEq(output + "\n")
+}
+
 func TestSkippedDirsNoMatchArg(t *testing.T) {
 	dir := getTestDataDir("skipdirs", "skip_me", "nested")
 	r := testshared.NewLintRunner(t).Run("--print-issued-lines=false", "--no-config", "--skip-dirs", dir, "-Egolint", dir)

test/testdata/quicktemplate/hello.qtpl

@@ -0,0 +1,7 @@
+All text outside function templates is treated as comments,
+i.e. it is just ignored by quicktemplate compiler (`qtc`). It is for humans.
+
+Hello is a simple template function.
+{% func Hello(name string) %}
+	Hello, {%s name %}!
+{% endfunc %}

test/testdata/quicktemplate/hello.qtpl.go

@@ -0,0 +1,62 @@
+// This file is automatically generated by qtc from "hello.qtpl".
+// See https://github.com/valyala/quicktemplate for details.
+
+// All text outside function templates is treated as comments,
+// i.e. it is just ignored by quicktemplate compiler (`qtc`). It is for humans.
+//
+// Hello is a simple template function.
+
+//line hello.qtpl:5
+package quicktemplate
+
+//line hello.qtpl:5
+import (
+	qtio422016 "io"
+
+	qt422016 "github.com/valyala/quicktemplate"
+)
+
+//line hello.qtpl:5
+var (
+	_ = qtio422016.Copy
+	_ = qt422016.AcquireByteBuffer
+)
+
+//line hello.qtpl:5
+func StreamHello(qw422016 *qt422016.Writer, name string) {
+	//line hello.qtpl:5
+	qw422016.N().S(`
+	Hello, `)
+	//line hello.qtpl:6
+	qw422016.E().S(name)
+	//line hello.qtpl:6
+	qw422016.N().S(`!
+`)
+//line hello.qtpl:7
+}
+
+//line hello.qtpl:7
+func WriteHello(qq422016 qtio422016.Writer, name string) {
+	//line hello.qtpl:7
+	qw422016 := qt422016.AcquireWriter(qq422016)
+	//line hello.qtpl:7
+	StreamHello(qw422016, name)
+	//line hello.qtpl:7
+	qt422016.ReleaseWriter(qw422016)
+//line hello.qtpl:7
+}
+
+//line hello.qtpl:7
+func Hello(name string) string {
+	//line hello.qtpl:7
+	qb422016 := qt422016.AcquireByteBuffer()
+	//line hello.qtpl:7
+	WriteHello(qb422016, name)
+	//line hello.qtpl:7
+	qs422016 := string(qb422016.B)
+	//line hello.qtpl:7
+	qt422016.ReleaseByteBuffer(qb422016)
+	//line hello.qtpl:7
+	return qs422016
+//line hello.qtpl:7
+}