Skip to content

Commit

Permalink
Fix #72: match more autogenerated files patterns.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jirfag committed Jun 10, 2018
1 parent 46088de commit adb6be7
Show file tree
Hide file tree
Showing 11 changed files with 232 additions and 87 deletions.
10 changes: 3 additions & 7 deletions pkg/commands/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package commands
import (
"context"
"fmt"
"go/token"
"io/ioutil"
"log"
"os"
Expand Down Expand Up @@ -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 {
Expand All @@ -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),
Expand Down
40 changes: 32 additions & 8 deletions pkg/lint/astcache/astcache.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -79,6 +96,20 @@ func LoadFromProgram(prog *loader.Program) (*Cache, error) {
return c, nil
}

func (c *Cache) parseFile(filePath string, fset *token.FileSet) {
if fset == nil {
fset = token.NewFileSet()
}

f, err := parser.ParseFile(fset, filePath, nil, parser.ParseComments) // comments needed by e.g. golint
c.m[filePath] = &File{
F: f,
Fset: fset,
Err: err,
Name: filePath,
}
}

func LoadFromFiles(files []string) (*Cache, error) {
c := &Cache{
m: map[string]*File{},
Expand All @@ -87,14 +118,7 @@ func LoadFromFiles(files []string) (*Cache, error) {
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,
Fset: fset,
Err: err,
Name: filePath,
}
c.parseFile(filePath, fset)
}

c.prepareValidFiles()
Expand Down
88 changes: 88 additions & 0 deletions pkg/result/processors/autogenerated_exclude.go
Original file line number Diff line number Diff line change
@@ -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() {}
88 changes: 88 additions & 0 deletions pkg/result/processors/autogenerated_exclude_test.go

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

61 changes: 10 additions & 51 deletions pkg/result/processors/nolint.go
Original file line number Diff line number Diff line change
@@ -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"
)

Expand Down Expand Up @@ -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
cache filesCache
astCache *astcache.Cache
}

func NewNolint(fset *token.FileSet) *Nolint {
func NewNolint(astCache *astcache.Cache) *Nolint {
return &Nolint{
fset: fset,
cache: filesCache{},
cache: filesCache{},
astCache: astCache,
}
}

Expand All @@ -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 {
Expand All @@ -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)
}

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())
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.ignoredRanges = buildIgnoredRangesForFile(file, p.fset)
fd.ignoredRanges = buildIgnoredRangesForFile(file.F, file.Fset)
return fd, nil
}

Expand All @@ -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
Expand Down
Loading

0 comments on commit adb6be7

Please sign in to comment.