Fix #72: match more autogenerated files patterns.

We skip all issues from autogenerated files.
Also reuse AST parsing for nolint and autogenerated exclude processors:
decrease processing time on golang source code from 3s to 800ms.
This commit is contained in:
Denis Isaev 2018-06-11 00:50:31 +03:00
parent 46088deacf
commit adb6be78bb
No known key found for this signature in database
GPG Key ID: A36A0EC8E27A1A01
11 changed files with 232 additions and 87 deletions

View File

@ -3,7 +3,6 @@ package commands
import (
"context"
"fmt"
"go/token"
"io/ioutil"
"log"
"os"
@ -192,10 +191,6 @@ func (e *Executor) runAnalysis(ctx context.Context, args []string) (<-chan resul
if len(excludePatterns) != 0 {
excludeTotalPattern = fmt.Sprintf("(%s)", strings.Join(excludePatterns, "|"))
}
fset := token.NewFileSet()
if lintCtx.Program != nil {
fset = lintCtx.Program.Fset
}
skipFilesProcessor, err := processors.NewSkipFiles(e.cfg.Run.SkipFiles)
if err != nil {
@ -204,12 +199,13 @@ func (e *Executor) runAnalysis(ctx context.Context, args []string) (<-chan resul
runner := lint.SimpleRunner{
Processors: []processors.Processor{
processors.NewPathPrettifier(), // must be before diff processor at least
processors.NewPathPrettifier(), // must be before diff, nolint and exclude autogenerated processor at least
processors.NewCgo(),
skipFilesProcessor,
processors.NewAutogeneratedExclude(lintCtx.ASTCache),
processors.NewExclude(excludeTotalPattern),
processors.NewNolint(fset),
processors.NewNolint(lintCtx.ASTCache),
processors.NewUniqByLine(),
processors.NewDiff(e.cfg.Issues.Diff, e.cfg.Issues.DiffFromRevision, e.cfg.Issues.DiffPatchFilePath),

View File

@ -24,10 +24,27 @@ type Cache struct {
s []*File
}
func NewCache() *Cache {
return &Cache{
m: map[string]*File{},
}
}
func (c Cache) Get(filename string) *File {
return c.m[filepath.Clean(filename)]
}
func (c Cache) GetOrParse(filename string) *File {
f := c.m[filename]
if f != nil {
return f
}
logrus.Infof("Parse AST for file %s on demand", filename)
c.parseFile(filename, nil)
return c.m[filename]
}
func (c Cache) GetAllValidFiles() []*File {
return c.s
}
@ -79,15 +96,11 @@ func LoadFromProgram(prog *loader.Program) (*Cache, error) {
return c, nil
}
func LoadFromFiles(files []string) (*Cache, error) {
c := &Cache{
m: map[string]*File{},
func (c *Cache) parseFile(filePath string, fset *token.FileSet) {
if fset == nil {
fset = token.NewFileSet()
}
fset := token.NewFileSet()
for _, filePath := range files {
filePath = filepath.Clean(filePath)
f, err := parser.ParseFile(fset, filePath, nil, parser.ParseComments) // comments needed by e.g. golint
c.m[filePath] = &File{
F: f,
@ -95,6 +108,17 @@ func LoadFromFiles(files []string) (*Cache, error) {
Err: err,
Name: filePath,
}
}
func LoadFromFiles(files []string) (*Cache, error) {
c := &Cache{
m: map[string]*File{},
}
fset := token.NewFileSet()
for _, filePath := range files {
filePath = filepath.Clean(filePath)
c.parseFile(filePath, fset)
}
c.prepareValidFiles()

View File

@ -0,0 +1,88 @@
package processors
import (
"fmt"
"strings"
"github.com/golangci/golangci-lint/pkg/lint/astcache"
"github.com/golangci/golangci-lint/pkg/result"
)
type ageFileSummary struct {
isGenerated bool
}
type ageFileSummaryCache map[string]*ageFileSummary
type AutogeneratedExclude struct {
fileSummaryCache ageFileSummaryCache
astCache *astcache.Cache
}
func NewAutogeneratedExclude(astCache *astcache.Cache) *AutogeneratedExclude {
return &AutogeneratedExclude{
fileSummaryCache: ageFileSummaryCache{},
astCache: astCache,
}
}
var _ Processor = &AutogeneratedExclude{}
func (p AutogeneratedExclude) Name() string {
return "autogenerated_exclude"
}
func (p *AutogeneratedExclude) Process(issues []result.Issue) ([]result.Issue, error) {
return filterIssuesErr(issues, p.shouldPassIssue)
}
func (p *AutogeneratedExclude) shouldPassIssue(i *result.Issue) (bool, error) {
fs, err := p.getOrCreateFileSummary(i)
if err != nil {
return false, err
}
// don't report issues for autogenerated files
return !fs.isGenerated, nil
}
// isGenerated reports whether the source file is generated code.
// Using a bit laxer rules than https://golang.org/s/generatedcode to
// match more generated code. See #48 and #72.
func isGeneratedFileByComment(doc string) bool {
const (
genCodeGenerated = "code generated"
genDoNotEdit = "do not edit"
genAutoFile = "autogenerated file" // easyjson
)
markers := []string{genCodeGenerated, genDoNotEdit, genAutoFile}
doc = strings.ToLower(doc)
for _, marker := range markers {
if strings.Contains(doc, marker) {
return true
}
}
return false
}
func (p *AutogeneratedExclude) getOrCreateFileSummary(i *result.Issue) (*ageFileSummary, error) {
fs := p.fileSummaryCache[i.FilePath()]
if fs != nil {
return fs, nil
}
fs = &ageFileSummary{}
p.fileSummaryCache[i.FilePath()] = fs
f := p.astCache.GetOrParse(i.FilePath())
if f.Err != nil {
return nil, fmt.Errorf("can't parse file %s: %s", i.FilePath(), f.Err)
}
fs.isGenerated = isGeneratedFileByComment(f.F.Doc.Text())
return fs, nil
}
func (p AutogeneratedExclude) Finish() {}

View File

@ -0,0 +1,88 @@
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"
)
func TestIsAutogeneratedDetection(t *testing.T) {
all := `
// generated by stringer -type Pill pill.go; DO NOT EDIT
// Code generated by "stringer -type Pill pill.go"; DO NOT EDIT
// Code generated by vfsgen; DO NOT EDIT
// Created by cgo -godefs - DO NOT EDIT
/* Created by cgo - DO NOT EDIT. */
// Generated by stringer -i a.out.go -o anames.go -p ppc64
// Do not edit.
// DO NOT EDIT
// generated by: x86map -fmt=decoder ../x86.csv
// DO NOT EDIT.
// Generate with: go run gen.go -full -output md5block.go
// generated by "go run gen.go". DO NOT EDIT.
// DO NOT EDIT. This file is generated by mksyntaxgo from the RE2 distribution.
// GENERATED BY make_perl_groups.pl; DO NOT EDIT.
// generated by mknacl.sh - do not edit
// DO NOT EDIT ** This file was generated with the bake tool ** DO NOT EDIT //
// Generated by running
// maketables --tables=all --data=http://www.unicode.org/Public/8.0.0/ucd/UnicodeData.txt --casefolding=http://www.unicode.org/Public/8.0.0/ucd/CaseFolding.txt
// DO NOT EDIT
/*
* CODE GENERATED AUTOMATICALLY WITH github.com/ernesto-jimenez/gogen/unmarshalmap
* THIS FILE SHOULD NOT BE EDITED BY HAND
*/
// AUTOGENERATED FILE: easyjson file.go
`
generatedCases := strings.Split(all, "\n\n")
for _, gc := range generatedCases {
isGenerated := isGeneratedFileByComment(gc)
assert.True(t, isGenerated)
}
notGeneratedCases := []string{"code not generated by", "test"}
for _, ngc := range notGeneratedCases {
isGenerated := isGeneratedFileByComment(ngc)
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)
})
}
}

View File

@ -1,16 +1,13 @@
package processors
import (
"bufio"
"bytes"
"fmt"
"go/ast"
"go/parser"
"go/token"
"io/ioutil"
"sort"
"strings"
"github.com/golangci/golangci-lint/pkg/lint/astcache"
"github.com/golangci/golangci-lint/pkg/result"
)
@ -44,20 +41,19 @@ func (i *ignoredRange) doesMatch(issue *result.Issue) bool {
type fileData struct {
ignoredRanges []ignoredRange
isGenerated bool
}
type filesCache map[string]*fileData
type Nolint struct {
fset *token.FileSet
cache filesCache
astCache *astcache.Cache
}
func NewNolint(fset *token.FileSet) *Nolint {
func NewNolint(astCache *astcache.Cache) *Nolint {
return &Nolint{
fset: fset,
cache: filesCache{},
astCache: astCache,
}
}
@ -71,29 +67,6 @@ func (p *Nolint) Process(issues []result.Issue) ([]result.Issue, error) {
return filterIssuesErr(issues, p.shouldPassIssue)
}
var (
genHdr = []byte("// Code generated")
genFtr = []byte("DO NOT EDIT")
)
// isGenerated reports whether the source file is generated code.
// Using a bit laxer rules than https://golang.org/s/generatedcode to
// match more generated code.
func isGenerated(src []byte) bool {
sc := bufio.NewScanner(bytes.NewReader(src))
var hdr, ftr bool
for sc.Scan() {
b := sc.Bytes()
if bytes.HasPrefix(b, genHdr) {
hdr = true
}
if bytes.Contains(b, genFtr) {
ftr = true
}
}
return hdr && ftr
}
func (p *Nolint) getOrCreateFileData(i *result.Issue) (*fileData, error) {
fd := p.cache[i.FilePath()]
if fd != nil {
@ -103,22 +76,12 @@ func (p *Nolint) getOrCreateFileData(i *result.Issue) (*fileData, error) {
fd = &fileData{}
p.cache[i.FilePath()] = fd
src, err := ioutil.ReadFile(i.FilePath())
if err != nil {
return nil, fmt.Errorf("can't read file %s: %s", i.FilePath(), err)
file := p.astCache.GetOrParse(i.FilePath())
if file.Err != nil {
return nil, fmt.Errorf("can't parse file %s: %s", i.FilePath(), file.Err)
}
fd.isGenerated = isGenerated(src)
if fd.isGenerated { // don't report issues for autogenerated files
return fd, nil
}
file, err := parser.ParseFile(p.fset, i.FilePath(), src, parser.ParseComments)
if err != nil {
return nil, fmt.Errorf("can't parse file %s", i.FilePath())
}
fd.ignoredRanges = buildIgnoredRangesForFile(file, p.fset)
fd.ignoredRanges = buildIgnoredRangesForFile(file.F, file.Fset)
return fd, nil
}
@ -145,10 +108,6 @@ func (p *Nolint) shouldPassIssue(i *result.Issue) (bool, error) {
return false, err
}
if fd.isGenerated { // don't report issues for autogenerated files
return false, nil
}
for _, ir := range fd.ignoredRanges {
if ir.doesMatch(i) {
return false, nil

View File

@ -5,6 +5,7 @@ import (
"path/filepath"
"testing"
"github.com/golangci/golangci-lint/pkg/lint/astcache"
"github.com/golangci/golangci-lint/pkg/result"
"github.com/stretchr/testify/assert"
)
@ -20,7 +21,7 @@ func newNolintFileIssue(line int, fromLinter string) result.Issue {
}
func TestNolint(t *testing.T) {
p := NewNolint(token.NewFileSet())
p := NewNolint(astcache.NewCache())
// test inline comments
processAssertEmpty(t, p, newNolintFileIssue(3, "gofmt"))
@ -76,26 +77,6 @@ func TestNolint(t *testing.T) {
}
}
func TestNoIssuesInAutogeneratedFiles(t *testing.T) {
files := []string{
"nolint_autogenerated.go",
"nolint_autogenerated_alt_hdr.go",
"nolint_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 := NewNolint(token.NewFileSet())
processAssertEmpty(t, p, i)
})
}
}
func TestIgnoredRangeMatches(t *testing.T) {
var testcases = []struct {
doc string

View File

@ -37,6 +37,11 @@ func TestCongratsMessageIfNoIssues(t *testing.T) {
checkNoIssuesRun(t, out, exitCode)
}
func TestAutogeneratedNoIssues(t *testing.T) {
out, exitCode := runGolangciLint(t, filepath.Join(testdataDir, "autogenerated"))
checkNoIssuesRun(t, out, exitCode)
}
func TestSymlinkLoop(t *testing.T) {
out, exitCode := runGolangciLint(t, filepath.Join(testdataDir, "symlink_loop", "..."))
checkNoIssuesRun(t, out, exitCode)

4
test/testdata/autogenerated/gen.go vendored Normal file
View File

@ -0,0 +1,4 @@
// DO NOT EDIT Code generated by something
package p
func unusedFunc() {}