1
0
mirror of https://github.com/mgechev/revive.git synced 2024-11-30 08:57:07 +02:00

Merge pull request #3 from mgechev/minko/optional-type-checking

Introduce optional type checking
This commit is contained in:
Minko Gechev 2018-06-01 07:16:16 -07:00 committed by GitHub
commit 6182489121
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 60 additions and 53 deletions

View File

@ -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`.
<p align="center">
<img src="./assets/demo.svg" alt="" width="700">
@ -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

View File

@ -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)

View File

@ -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 {

View File

@ -24,6 +24,7 @@ func (r *ContextKeysType) Apply(file *lint.File, arguments lint.Arguments) []lin
},
}
file.Pkg.TypeCheck()
ast.Walk(walker, fileAst)
return failures

View File

@ -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

View File

@ -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
}

View File

@ -24,6 +24,7 @@ func (r *UnexportedReturnRule) Apply(file *lint.File, arguments lint.Arguments)
},
}
file.Pkg.TypeCheck()
ast.Walk(walker, fileAst)
return failures

View File

@ -25,6 +25,7 @@ func (r *VarDeclarationsRule) Apply(file *lint.File, arguments lint.Arguments) [
},
}
file.Pkg.TypeCheck()
ast.Walk(walker, fileAst)
return failures

View File

@ -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]