From 3e427a10e7710c2a18a62764f8bfaa82158781f2 Mon Sep 17 00:00:00 2001 From: chavacava Date: Thu, 21 Jul 2022 19:54:56 +0000 Subject: [PATCH] working version needs to fix tests data needs to cleanup code --- lint/file.go | 23 +++++---- lint/linter.go | 69 ++++++++++++++------------- lint/package.go | 88 +++++++++++++++++++---------------- revivelib/core.go | 24 +++++++++- rule/time-equal.go | 2 +- test/utils.go | 25 +++++++++- testdata/add-constant.go | 4 +- testdata/banned-characters.go | 7 +-- testdata/bare-return.go | 12 +++-- 9 files changed, 156 insertions(+), 98 deletions(-) diff --git a/lint/file.go b/lint/file.go index dcf0e608f..a08bdd64a 100644 --- a/lint/file.go +++ b/lint/file.go @@ -3,7 +3,6 @@ package lint import ( "bytes" "go/ast" - "go/parser" "go/printer" "go/token" "go/types" @@ -14,10 +13,9 @@ import ( // File abstraction used for representing files. type File struct { - Name string - Pkg *Package - content []byte - AST *ast.File + Name string + Pkg *Package + AST *ast.File } // IsTest returns if the file contains tests. @@ -25,9 +23,13 @@ func (f *File) IsTest() bool { return strings.HasSuffix(f.Name, "_test.go") } // Content returns the file's content. func (f *File) Content() []byte { - return f.content + buf := bytes.Buffer{} + fs := token.NewFileSet() + printer.Fprint(&buf, fs, f.AST) + return buf.Bytes() } +/* // NewFile creates a new file func NewFile(name string, content []byte, pkg *Package) (*File, error) { f, err := parser.ParseFile(pkg.fset, name, content, parser.ParseComments) @@ -41,16 +43,17 @@ func NewFile(name string, content []byte, pkg *Package) (*File, error) { AST: f, }, nil } +*/ // ToPosition returns line and column for given position. func (f *File) ToPosition(pos token.Pos) token.Position { - return f.Pkg.fset.Position(pos) + return f.Pkg.goPkg.Fset.Position(pos) } // Render renders a node. func (f *File) Render(x interface{}) string { var buf bytes.Buffer - if err := printer.Fprint(&buf, f.Pkg.fset, x); err != nil { + if err := printer.Fprint(&buf, f.Pkg.goPkg.Fset, x); err != nil { panic(err) } return buf.String() @@ -58,7 +61,7 @@ func (f *File) Render(x interface{}) string { // CommentMap builds a comment map for the file. func (f *File) CommentMap() ast.CommentMap { - return ast.NewCommentMap(f.Pkg.fset, f.AST, f.AST.Comments) + return ast.NewCommentMap(f.Pkg.goPkg.Fset, f.AST, f.AST.Comments) } var basicTypeKinds = map[types.BasicKind]string{ @@ -77,7 +80,7 @@ func (f *File) IsUntypedConst(expr ast.Expr) (defType string, ok bool) { // Re-evaluate expr outside its context to see if it's untyped. // (An expr evaluated within, for example, an assignment context will get the type of the LHS.) exprStr := f.Render(expr) - tv, err := types.Eval(f.Pkg.fset, f.Pkg.TypesPkg(), expr.Pos(), exprStr) + tv, err := types.Eval(f.Pkg.goPkg.Fset, f.Pkg.goPkg.Types, expr.Pos(), exprStr) if err != nil { return "", false } diff --git a/lint/linter.go b/lint/linter.go index fb1ab6f28..881d2255f 100644 --- a/lint/linter.go +++ b/lint/linter.go @@ -1,14 +1,15 @@ package lint import ( - "bufio" - "bytes" "fmt" "go/token" "os" "regexp" "strconv" + "strings" "sync" + + "golang.org/x/tools/go/packages" ) // ReadFile defines an abstraction for reading files. @@ -49,18 +50,18 @@ func (l Linter) readFile(path string) (result []byte, err error) { } var ( - genHdr = []byte("// Code generated ") - genFtr = []byte(" DO NOT EDIT.") + genHdr = "// Code generated " + genFtr = " DO NOT EDIT." ) // Lint lints a set of files with the specified rule. -func (l *Linter) Lint(packages [][]string, ruleSet []Rule, config Config) (<-chan Failure, error) { +func (l *Linter) Lint(pckgs []*packages.Package, ruleSet []Rule, config Config) (<-chan Failure, error) { failures := make(chan Failure) var wg sync.WaitGroup - for _, pkg := range packages { + for _, pkg := range pckgs { wg.Add(1) - go func(pkg []string) { + go func(pkg *packages.Package) { if err := l.lintPackage(pkg, ruleSet, config, failures); err != nil { fmt.Fprintln(os.Stderr, err) os.Exit(1) @@ -77,33 +78,38 @@ func (l *Linter) Lint(packages [][]string, ruleSet []Rule, config Config) (<-cha return failures, nil } -func (l *Linter) lintPackage(filenames []string, ruleSet []Rule, config Config, failures chan Failure) error { - pkg := &Package{ - fset: token.NewFileSet(), - files: map[string]*File{}, - } - for _, filename := range filenames { - content, err := l.readFile(filename) - if err != nil { - return err - } - if !config.IgnoreGeneratedHeader && isGenerated(content) { - continue - } +func (l *Linter) lintPackage(goPkg *packages.Package, ruleSet []Rule, config Config, failures chan Failure) error { + lintPkg := NewPackage(goPkg) + for _, file := range goPkg.Syntax { + /* + content, err := l.readFile(filename) + if err != nil { + return err + }*/ - file, err := NewFile(filename, content, pkg) - if err != nil { - addInvalidFileFailure(filename, err.Error(), failures) + if !config.IgnoreGeneratedHeader && isGenerated(file.Doc.Text()) { continue } - pkg.files[filename] = file + /* + file, err := NewFile(filename, content, pkg) + if err != nil { + addInvalidFileFailure(filename, err.Error(), failures) + continue + } + */ + newFile := File{ + Name: file.Name.Name, + Pkg: &lintPkg, + AST: file, + } + lintPkg.files[file.Name.String()] = &newFile } - if len(pkg.files) == 0 { + if len(lintPkg.files) == 0 { return nil } - pkg.lint(ruleSet, config, failures) + lintPkg.lint(ruleSet, config, failures) return nil } @@ -111,15 +117,8 @@ func (l *Linter) lintPackage(filenames []string, ruleSet []Rule, config Config, // isGenerated reports whether the source file is generated code // according the rules from https://golang.org/s/generatedcode. // This is inherited from the original go lint. -func isGenerated(src []byte) bool { - sc := bufio.NewScanner(bytes.NewReader(src)) - for sc.Scan() { - b := sc.Bytes() - if bytes.HasPrefix(b, genHdr) && bytes.HasSuffix(b, genFtr) && len(b) >= len(genHdr)+len(genFtr) { - return true - } - } - return false +func isGenerated(fileComment string) bool { + return strings.HasPrefix(fileComment, genHdr) && strings.HasSuffix(fileComment, genFtr) && len(fileComment) >= len(genHdr)+len(genFtr) } // addInvalidFileFailure adds a failure for an invalid formatted file diff --git a/lint/package.go b/lint/package.go index 47e5b6f85..bdfc9e98d 100644 --- a/lint/package.go +++ b/lint/package.go @@ -7,18 +7,16 @@ import ( "sync" "golang.org/x/tools/go/gcexportdata" + "golang.org/x/tools/go/packages" "github.com/mgechev/revive/internal/typeparams" ) // Package represents a package in the project. type Package struct { - fset *token.FileSet + goPkg *packages.Package files map[string]*File - typesPkg *types.Package - typesInfo *types.Info - // sortable is the set of types in the package that implement sort.Interface. sortable map[string]bool // main is whether this is a "main" package. @@ -26,6 +24,14 @@ type Package struct { sync.RWMutex } +func NewPackage(pkg *packages.Package) Package { + return Package{ + goPkg: pkg, + files: map[string]*File{}, + sortable: map[string]bool{}, + } +} + var newImporter = func(fset *token.FileSet) types.ImporterFrom { return gcexportdata.NewImporter(fset, make(map[string]*types.Package)) } @@ -65,14 +71,14 @@ func (p *Package) IsMain() bool { func (p *Package) TypesPkg() *types.Package { p.RLock() defer p.RUnlock() - return p.typesPkg + return p.goPkg.Types } // TypesInfo yields type information of this package identifiers func (p *Package) TypesInfo() *types.Info { p.RLock() defer p.RUnlock() - return p.typesInfo + return p.goPkg.TypesInfo } // Sortable yields a map of sortable types in this package @@ -83,41 +89,45 @@ func (p *Package) Sortable() map[string]bool { } // TypeCheck performs type checking for given package. -func (p *Package) TypeCheck() error { - p.Lock() - defer p.Unlock() - // If type checking has already been performed - // skip it. - if p.typesInfo != nil || p.typesPkg != nil { - return nil - } - config := &types.Config{ - // By setting a no-op error reporter, the type checker does as much work as possible. - Error: func(error) {}, - Importer: newImporter(p.fset), - } - info := &types.Info{ - Types: make(map[ast.Expr]types.TypeAndValue), - Defs: make(map[*ast.Ident]types.Object), - Uses: make(map[*ast.Ident]types.Object), - Scopes: make(map[ast.Node]*types.Scope), - } - var anyFile *File - var astFiles []*ast.File - for _, f := range p.files { - anyFile = f - astFiles = append(astFiles, f.AST) - } +func (p *Package) TypeCheck() error { + return nil // TODO delete this function + /* + p.Lock() + defer p.Unlock() + + // If type checking has already been performed + // skip it. + if p.typesInfo != nil || p.typesPkg != nil { + return nil + } + config := &types.Config{ + // By setting a no-op error reporter, the type checker does as much work as possible. + Error: func(error) {}, + Importer: newImporter(p.fset), + } + info := &types.Info{ + Types: make(map[ast.Expr]types.TypeAndValue), + Defs: make(map[*ast.Ident]types.Object), + Uses: make(map[*ast.Ident]types.Object), + Scopes: make(map[ast.Node]*types.Scope), + } + var anyFile *File + var astFiles []*ast.File + for _, f := range p.files { + anyFile = f + astFiles = append(astFiles, f.AST) + } - typesPkg, err := check(config, anyFile.AST.Name.Name, p.fset, astFiles, info) + typesPkg, err := check(config, anyFile.AST.Name.Name, p.fset, astFiles, info) - // Remember the typechecking info, even if config.Check failed, - // since we will get partial information. - p.typesPkg = typesPkg - p.typesInfo = info + // Remember the typechecking info, even if config.Check failed, + // since we will get partial information. + p.typesPkg = typesPkg + p.typesInfo = info - return err + return err + */ } // check function encapsulates the call to go/types.Config.Check method and @@ -136,10 +146,10 @@ func check(config *types.Config, n string, fset *token.FileSet, astFiles []*ast. // TypeOf returns the type of an expression. func (p *Package) TypeOf(expr ast.Expr) types.Type { - if p.typesInfo == nil { + if p.goPkg.TypesInfo == nil { return nil } - return p.typesInfo.TypeOf(expr) + return p.goPkg.TypesInfo.TypeOf(expr) } type walker struct { diff --git a/revivelib/core.go b/revivelib/core.go index d7397e7ad..d31ddb5ce 100644 --- a/revivelib/core.go +++ b/revivelib/core.go @@ -1,11 +1,13 @@ package revivelib import ( + "flag" "io/ioutil" "log" "strings" - "github.com/mgechev/dots" + "golang.org/x/tools/go/packages" + "github.com/mgechev/revive/config" "github.com/mgechev/revive/lint" "github.com/mgechev/revive/logging" @@ -162,6 +164,7 @@ func (r *Revive) Format( return output, exitCode, nil } +/* func getPackages(includePatterns []string, excludePatterns ArrayFlags) ([][]string, error) { globs := normalizeSplit(includePatterns) if len(globs) == 0 { @@ -175,6 +178,25 @@ func getPackages(includePatterns []string, excludePatterns ArrayFlags) ([][]stri return packages, nil } +*/ + +func getPackages(includePatterns []string, excludePatterns ArrayFlags) ([]*packages.Package, error) { + globs := normalizeSplit(flag.Args()) + if len(globs) == 0 { + globs = append(globs, ".") + } + + cfg := &packages.Config{Mode: packages.LoadSyntax} + pckgs, err := packages.Load(cfg, globs...) + if err != nil { + return nil, errors.Wrap(err, "loading packages") + } + if packages.PrintErrors(pckgs) > 0 { + return nil, errors.Wrap(err, "loading packages") + } + + return pckgs, nil +} func normalizeSplit(strs []string) []string { res := []string{} diff --git a/rule/time-equal.go b/rule/time-equal.go index 72ecf26fe..894925d9c 100644 --- a/rule/time-equal.go +++ b/rule/time-equal.go @@ -21,7 +21,7 @@ func (*TimeEqualRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { w := &lintTimeEqual{file, onFailure} if w.file.Pkg.TypeCheck() != nil { - return nil + panic(1) } ast.Walk(w, file.AST) diff --git a/test/utils.go b/test/utils.go index 536e8cdbe..6bc7d31f2 100644 --- a/test/utils.go +++ b/test/utils.go @@ -16,6 +16,7 @@ import ( "github.com/mgechev/revive/lint" "github.com/pkg/errors" + "golang.org/x/tools/go/packages" ) func testRule(t *testing.T, filename string, rule lint.Rule, config ...*lint.RuleConfig) { @@ -45,7 +46,11 @@ func assertSuccess(t *testing.T, baseDir string, fi os.FileInfo, rules []lint.Ru return ioutil.ReadFile(baseDir + file) }, 0) - ps, err := l.Lint([][]string{{fi.Name()}}, rules, lint.Config{ + pkgs, err := loadFileAsPackage(`../testdata/` + fi.Name()) + if err != nil { + return err + } + ps, err := l.Lint(pkgs, rules, lint.Config{ Rules: config, }) if err != nil { @@ -62,6 +67,18 @@ func assertSuccess(t *testing.T, baseDir string, fi os.FileInfo, rules []lint.Ru return nil } +func loadFileAsPackage(filename string) ([]*packages.Package, error) { + cfg := &packages.Config{Mode: packages.LoadSyntax} + pckgs, err := packages.Load(cfg, filename) + if err != nil { + return nil, errors.Wrap(err, "loading packages") + } + if packages.PrintErrors(pckgs) > 0 { + return nil, errors.Wrap(err, "loading packages") + } + return pckgs, nil +} + func assertFailures(t *testing.T, baseDir string, fi os.FileInfo, src []byte, rules []lint.Rule, config map[string]lint.RuleConfig) error { l := lint.New(func(file string) ([]byte, error) { return ioutil.ReadFile(baseDir + file) @@ -72,7 +89,11 @@ func assertFailures(t *testing.T, baseDir string, fi os.FileInfo, src []byte, ru return errors.Errorf("Test file %v does not have instructions", fi.Name()) } - ps, err := l.Lint([][]string{{fi.Name()}}, rules, lint.Config{ + pkgs, err := loadFileAsPackage(`../testdata/` + fi.Name()) + if err != nil { + return err + } + ps, err := l.Lint(pkgs, rules, lint.Config{ Rules: config, }) if err != nil { diff --git a/testdata/add-constant.go b/testdata/add-constant.go index d240f4023..b56f03c49 100644 --- a/testdata/add-constant.go +++ b/testdata/add-constant.go @@ -1,13 +1,13 @@ package fixtures -func foo(a, b, c, d int) { +func foo(a float32, b string, c, d int) { a = 1.0 // ignore b = "ignore" c = 2 // ignore println("lit", 12) // MATCH /avoid magic numbers like '12', create a named constant for it/ if a == 12.50 { // MATCH /avoid magic numbers like '12.50', create a named constant for it/ if b == "lit" { - c = "lit" // MATCH /string literal "lit" appears, at least, 3 times, create a named constant for it/ + b = "lit" // MATCH /string literal "lit" appears, at least, 3 times, create a named constant for it/ } for i := 0; i < 1; i++ { println("lit") diff --git a/testdata/banned-characters.go b/testdata/banned-characters.go index e10c9f222..77d6a9b6a 100644 --- a/testdata/banned-characters.go +++ b/testdata/banned-characters.go @@ -1,9 +1,10 @@ package fixtures -const Ω = "Omega" // MATCH:3 /banned character found: Ω/ +const Ω = "Omega" // MATCH /banned character found: Ω/ // func contains banned characters Ω // authorized banned chars in comment -func funcΣ() error { // MATCH:6 /banned character found: Σ/ - var charσhid string // MATCH:7 /banned character found: σ/ +func funcΣ() error { // MATCH /banned character found: Σ/ + var charσhid string // MATCH /banned character found: σ/ + _ = charσhid // MATCH /banned character found: σ/ return nil } diff --git a/testdata/bare-return.go b/testdata/bare-return.go index 243deb8c2..6f5226ef2 100644 --- a/testdata/bare-return.go +++ b/testdata/bare-return.go @@ -3,23 +3,25 @@ package fixtures func bare1() (int, int, error) { go func() (a int) { return // MATCH /avoid using bare returns, please add return expressions/ - }(5) + }() + return 1, 1, nil } func bare2(a, b int) (int, error, int) { defer func() (a int) { return // MATCH /avoid using bare returns, please add return expressions/ - }(5) + }() + return 1, nil, 1 } -func bare3(a string, b int) (a int, b float32, c string, d string) { +func bare3(a string, b int) (ra int, rb float32, rc string, rd string) { go func() (a int, b int) { return a, b - }(5, 6) + }() defer func() (a int) { return a - }(5) + }() return // MATCH /avoid using bare returns, please add return expressions/ }