Skip to content

Fix #72: match more autogenerated files patterns. #82

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 11, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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