diff --git a/README.md b/README.md index 36a5122..e40dfe5 100644 --- a/README.md +++ b/README.md @@ -12,15 +12,16 @@ Fast, configurable, extensible, flexible, and beautiful linter for Go. Drop-in r Here's how `revive` is different from `golint`: -- Allow us to enable or disable rules using a configuration file. -- Allows us to configure the linting rules with a TOML file. -- Provides functionality for disabling a specific rule or the entire linter for a file or a range of lines. - - `golint` allows this only for generated files. -- Provides multiple formatters which let us customize the output. -- Allows us to customize the return code for the entire linter or based on the failure of only some rules. -- *Everyone can extend it easily with custom rules or formatters.* -- `Revive` provides more rules compared to `golint`. -- Faster. It runs the rules over each file in a separate goroutine. +* Allows us to enable or disable rules using a configuration file. +* Allows us to configure the linting rules with a TOML file. +* 2x faster running the same rules as golint. +* Provides functionality for disabling a specific rule or the entire linter for a file or a range of lines. + * `golint` allows this only for generated files. +* Optional type checking. Most rules in golint do not require type checking. If you disable them in the config file, revive will run over 6x faster than golint. +* Provides multiple formatters which let us customize the output. +* Allows us to customize the return code for the entire linter or based on the failure of only some rules. +* _Everyone can extend it easily with custom rules or formatters._ +* `Revive` provides more rules compared to `golint`.

@@ -32,7 +33,7 @@ Since the default behavior of `revive` is compatible with `golint`, without prov ### Text Editors -- Support for VSCode in [vscode-go](https://github.com/Microsoft/vscode-go/pull/1699). +* Support for VSCode in [vscode-go](https://github.com/Microsoft/vscode-go/pull/1699). ### Installation @@ -116,31 +117,31 @@ This will use `config.toml`, the `friendly` formatter, and will run linting over List of all available rules. The rules ported from `golint` are left unchanged and indicated in the `golit` column. -| Name | Config | Description | `golint` | -| --------------------- | :----: | :--------------------------------------------------------------- | :------: | -| `blank-imports` | n/a | Disallows blank imports | yes | -| `context-as-argument` | n/a | `context.Context` should be the first argument of a function. | yes | -| `context-keys-type` | n/a | Disallows the usage of basic types in `context.WithValue`. | yes | -| `dot-imports` | n/a | Forbids `.` imports. | yes | -| `error-return` | n/a | The error return parameter should be last. | yes | -| `error-strings` | n/a | Conventions around error strings. | yes | -| `error-naming` | n/a | Naming of error variables. | yes | -| `exported` | n/a | Naming and commenting conventions on exported symbols. | yes | -| `if-return` | n/a | Redundant if when returning an error. | yes | -| `increment-decrement` | n/a | Use `i++` and `i--` instead of `i += 1` and `i -= 1`. | yes | -| `var-naming` | n/a | Naming rules. | yes | -| `var-declaration` | n/a | Reduces redundancies around variable declaration. | yes | -| `package-comments` | n/a | Package commenting conventions. | yes | -| `range` | n/a | Prevents redundant variables when iterating over a collection. | yes | -| `receiver-naming` | n/a | Conventions around the naming of receivers. | yes | -| `time-naming` | n/a | Conventions around the naming of time variables. | yes | -| `unexported-return` | n/a | Warns when a public return is from unexported type. | yes | -| `indent-error-flow` | n/a | Prevents redundant else statements. | yes | -| `errorf` | n/a | Should replace `error.New(fmt.Sprintf())` with `error.Errorf()` | yes | -| `argument-limit` | int | Specifies the maximum number of arguments a function can receive | no | -| `cyclomatic` | int | Sets restriction for maximum Cyclomatic complexity. | no | -| `max-public-structs` | int | The maximum number of public structs in a file. | no | -| `file-header` | string | Header which each file should have. | no | +| Name | Config | Description | `golint` | Typed | +| --------------------- | :----: | :--------------------------------------------------------------- | :------: | :---: | +| `context-keys-type` | n/a | Disallows the usage of basic types in `context.WithValue`. | yes | yes | +| `time-naming` | n/a | Conventions around the naming of time variables. | yes | yes | +| `var-declaration` | n/a | Reduces redundancies around variable declaration. | yes | yes | +| `unexported-return` | n/a | Warns when a public return is from unexported type. | yes | yes | +| `errorf` | n/a | Should replace `error.New(fmt.Sprintf())` with `error.Errorf()` | yes | yes | +| `blank-imports` | n/a | Disallows blank imports | yes | no | +| `context-as-argument` | n/a | `context.Context` should be the first argument of a function. | yes | no | +| `dot-imports` | n/a | Forbids `.` imports. | yes | no | +| `error-return` | n/a | The error return parameter should be last. | yes | no | +| `error-strings` | n/a | Conventions around error strings. | yes | no | +| `error-naming` | n/a | Naming of error variables. | yes | no | +| `exported` | n/a | Naming and commenting conventions on exported symbols. | yes | no | +| `if-return` | n/a | Redundant if when returning an error. | yes | no | +| `increment-decrement` | n/a | Use `i++` and `i--` instead of `i += 1` and `i -= 1`. | yes | no | +| `var-naming` | n/a | Naming rules. | yes | no | +| `package-comments` | n/a | Package commenting conventions. | yes | no | +| `range` | n/a | Prevents redundant variables when iterating over a collection. | yes | no | +| `receiver-naming` | n/a | Conventions around the naming of receivers. | yes | no | +| `indent-error-flow` | n/a | Prevents redundant else statements. | yes | no | +| `argument-limit` | int | Specifies the maximum number of arguments a function can receive | no | no | +| `cyclomatic` | int | Sets restriction for maximum Cyclomatic complexity. | no | no | +| `max-public-structs` | int | The maximum number of public structs in a file. | no | no | +| `file-header` | string | Header which each file should have. | no | no | ## Available Formatters @@ -226,14 +227,14 @@ sys 0m9.146s ```shell # no type checking -time revive kubernetes/... > /dev/null +time revive -config untyped.toml kubernetes/... > /dev/null -real 0m10.526s -user 0m54.228s -sys 0m3.534s +real 0m8.471s +user 0m40.721s +sys 0m3.262s ``` -Keep in mind that with type checking enabled, the performance may drop to twice faster than `golint`: +Keep in mind that if you use rules which require type checking, the performance may drop to 2x faster than `golint`: ```shell # type checking enabled @@ -244,7 +245,7 @@ user 2m6.708s sys 0m17.192s ``` -Currently, type checking is enabled by default. +Currently, type checking is enabled by default. If you want to run the linter without type checking, remove all typed rules from the configuration file. ## License diff --git a/lint/linter.go b/lint/linter.go index cad080b..cdca84f 100644 --- a/lint/linter.go +++ b/lint/linter.go @@ -57,6 +57,7 @@ func (l *Linter) lintPackage(filenames []string, ruleSet []Rule, config Config, pkg := &Package{ fset: token.NewFileSet(), files: map[string]*File{}, + mu: sync.Mutex{}, } for _, filename := range filenames { content, err := l.reader(filename) diff --git a/lint/package.go b/lint/package.go index 63d5892..4dbaa03 100644 --- a/lint/package.go +++ b/lint/package.go @@ -21,6 +21,7 @@ type Package struct { Sortable map[string]bool // main is whether this is a "main" package. main int + mu sync.Mutex } var newImporter = func(fset *token.FileSet) types.ImporterFrom { @@ -50,8 +51,15 @@ func (p *Package) IsMain() bool { return false } -// typeCheck performs type checking for given package. -func (p *Package) typeCheck() error { +// TypeCheck performs type checking for given package. +func (p *Package) TypeCheck() error { + p.mu.Lock() + // If type checking has already been performed + // skip it. + if p.TypesInfo != nil || p.TypesPkg != nil { + p.mu.Unlock() + return nil + } config := &types.Config{ // By setting a no-op error reporter, the type checker does as much work as possible. Error: func(error) {}, @@ -74,6 +82,7 @@ func (p *Package) typeCheck() error { // since we will get partial information. p.TypesPkg = typesPkg p.TypesInfo = info + p.mu.Unlock() return err } @@ -140,7 +149,6 @@ func receiverType(fn *ast.FuncDecl) string { } func (p *Package) lint(rules []Rule, config Config, failures chan Failure) { - p.typeCheck() p.scanSortable() var wg sync.WaitGroup for _, file := range p.files { diff --git a/rule/context-keys-type.go b/rule/context-keys-type.go index c310aa7..240a691 100644 --- a/rule/context-keys-type.go +++ b/rule/context-keys-type.go @@ -24,6 +24,7 @@ func (r *ContextKeysType) Apply(file *lint.File, arguments lint.Arguments) []lin }, } + file.Pkg.TypeCheck() ast.Walk(walker, fileAst) return failures diff --git a/rule/errorf.go b/rule/errorf.go index 2763069..4068022 100644 --- a/rule/errorf.go +++ b/rule/errorf.go @@ -25,6 +25,7 @@ func (r *ErrorfRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Fai }, } + file.Pkg.TypeCheck() ast.Walk(walker, fileAst) return failures diff --git a/rule/time-naming.go b/rule/time-naming.go index 85bf337..18dfaf1 100644 --- a/rule/time-naming.go +++ b/rule/time-naming.go @@ -21,6 +21,8 @@ func (r *TimeNamingRule) Apply(file *lint.File, arguments lint.Arguments) []lint } w := &lintTimeNames{file, onFailure} + + file.Pkg.TypeCheck() ast.Walk(w, file.AST) return failures } diff --git a/rule/unexported-return.go b/rule/unexported-return.go index 623dcc3..c14aff1 100644 --- a/rule/unexported-return.go +++ b/rule/unexported-return.go @@ -24,6 +24,7 @@ func (r *UnexportedReturnRule) Apply(file *lint.File, arguments lint.Arguments) }, } + file.Pkg.TypeCheck() ast.Walk(walker, fileAst) return failures diff --git a/rule/var-declarations.go b/rule/var-declarations.go index 0d7a038..c8e0993 100644 --- a/rule/var-declarations.go +++ b/rule/var-declarations.go @@ -25,6 +25,7 @@ func (r *VarDeclarationsRule) Apply(file *lint.File, arguments lint.Arguments) [ }, } + file.Pkg.TypeCheck() ast.Walk(walker, fileAst) return failures diff --git a/config.toml b/untyped.toml similarity index 62% rename from config.toml rename to untyped.toml index 5e9aac8..67307bb 100644 --- a/config.toml +++ b/untyped.toml @@ -1,10 +1,5 @@ -ignoreGeneratedHeader = true -severity = "error" -confidence = 0.8 - [rule.blank-imports] [rule.context-as-argument] -[rule.context-keys-type] [rule.dot-imports] [rule.error-return] [rule.error-strings] @@ -13,11 +8,7 @@ confidence = 0.8 [rule.if-return] [rule.increment-decrement] [rule.var-naming] -[rule.var-declaration] [rule.package-comments] [rule.range] [rule.receiver-naming] -[rule.time-naming] -[rule.unexported-return] [rule.indent-error-flow] -[rule.errorf]