lint/parenthesis-checker.go: add variable types check #31
Conversation
lint/parenthesis-checker.go
Outdated
| // TODO improve suggestions for complex cases like (func([](func()))) | ||
| // TODO improve linter output to write full type, not just place | ||
| // whereit could be simplified |
There was a problem hiding this comment.
We may want to disable interfacer in megacheck inside our CI script.
Method says validateExpr, but it accepts any ast.Node.
And that is not quite good, because ast.Inspect inside it will never handle anything except expressions.
This is not the first time interfacer bites us.
We should treat it as a suggestion, not obligation that breaks out build.
There was a problem hiding this comment.
What do you think about s/validateExpr/validateType/?
Types are expressions in AST terms, but not all expressions are types.
There was a problem hiding this comment.
SGTM, i ll rename it in a moment.
CI script is out of scope of that pr. I ll create issue for that.
lint/parenthesis-checker.go
Outdated
| } | ||
| case *ast.GenDecl: | ||
| for _, spec := range decl.Specs { | ||
| spec, ok := spec.(*ast.ValueSpec) |
There was a problem hiding this comment.
This will panic for specs that are not var or const.
Add this to your tests and linter will (most likely) panic:
type foo intThere was a problem hiding this comment.
What you may want to do for now is:
if decl.Tok != token.VAR || decl.Tok != token.CONST {
continue
}And only then iterate over decl.Specs assertion all it's elements to *ast.ValueSpec.
Just for the reference:
token.IMPORT *ImportSpec
token.CONST *ValueSpec
token.TYPE *TypeSpec
token.VAR *ValueSpec
There was a problem hiding this comment.
When you will handle token.TYPE, it's still required to skip token.IMPORT, as import specs are not interesting for your checker.
|
|
||
| var badVar [](func()) | ||
|
|
||
| var badVar2 [5](*int) |
There was a problem hiding this comment.
Add at least one case for const and type declarations.
There was a problem hiding this comment.
@quasilyte, could you please show, how this check could be used in type declarations?
There was a problem hiding this comment.
type foo [](func())For struct types, you need to walk all fields:
type bar struct {
bad1 [2](int)
bad2 []([](float64))
}
…into parenthesis_v2
| _ [](func()) | ||
| ) | ||
|
|
||
| const _ (int) = 5 |
There was a problem hiding this comment.
Add 1 type decl here and make sure that checker does not panic.
We will merge after that and you can continue to work on ast.TypeSpec in a separate PR.
|
|
||
| ast.Inspect(f.Type, func(n ast.Node) bool { | ||
| fmt.Printf("%s\n", nodeString(c.ctx.FileSet, n)) |
There was a problem hiding this comment.
Missed this one.
fmt.Printf("%s\n", nodeString(c.ctx.FileSet, n)) Should be removed.
No description provided.