Skip to content

Commit

Permalink
Merge pull request #82 from golangci/feature/match-more-autogenerated…
Browse files Browse the repository at this point in the history
…-files

Fix #72: match more autogenerated files patterns.
  • Loading branch information
jirfag authored Jun 11, 2018
2 parents 46088de + adb6be7 commit 34fa0b0
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 34fa0b0

Please sign in to comment.