Linter to disallow struct initialization without field names #1059
Comments
|
I wanted to try out to implement this rule, but I have a feeling it is not possible with Revive. type A []string
type X struct {
x string
y string
}
func DoSomething() {
_ = A{"xval", "yval"}
_ = X{"xval", "yval"}
} |
|
Hi @arxeiss thanks for the proposal. |
You can leverage type information to make the difference between arrays and structs (as in string-of-int) That said, you could try implementing a naive version (no type info) to see if there are too much false positives when analyzing real code bases. |
|
BTW, a language hack to force using field names when instantiation a struct is to declare a blank identified type position struct {
_ struct{} // just to force named fields in instantiation
x int
y int
}(again, it's a hack) |
This is not an issue actually.
So to detect when to raise an error and when not you don't need to know how many fields struct has. The real issue is, that on AST you don't know what type it is. In my example above it is visible. but it that struct would be defined on different package, you are lost. Even what you link (string-of-int rule) will not help in that case. |
|
This is worth trying if there is no govet, gocritic about this This the reason why I maintain the following .golangci-lint file to what could be reported https://github.com/ccoVeille/golangci-lint-config-examples/blob/main/90-daredevil/.golangci.yml |
|
I checked and unfortunately nothing detect it. So yes, adding it in revive makes sense. So here exhauststruct will report some of them package foo
type F struct {
A int
B int8
}
var (
_ = F{} // exhauststruct reports this: foo.F is missing fields A, B
_ = F{A: 1} // exhauststruct reports this: foo.F is missing field B
_ = F{B: 2} // exhauststruct reports this: foo.F is missing field A
_ = F{A: 1, B: 2} // valid
// valid but should be discouraged with a revive rule
// will break if someone play with fields ordering
_ = F{1, 2}
)Also, Go reports issues about this notation when they are partial as "error with struct literal" package foo
type F struct {
A int
B int
}
// typecheck errors
var _ = F{A: 1, 2} // compiler reports: mixture of field:value and value elements in struct literal
var _ = F{1} // compiler reports: too few values in struct literal of type Fso exhaustruct, could be a candidate for implementing something about struct literals, so out of revive. But exhauststruct is about something different that people wants to use. Something that is somehow against the "Go zero logic" mentioned above. I would go for a rule that will be named around struct literal and only this. |
|
@ccoVeille I agree with you. But as I said above, you are not able to implement that rule only on top of AST. You need some static analysis in order to know of which type the identifier is. And that is something that Revive cannot do. Or can you? |
|
I'm confident I cannot reply you 😰, I'm sorry. It's a point of detail out of my knowledge, even if I understood your point. Someone else, maybe @chavacava or @alexandear would be able to reply you. |
|
Goland supports detecting "Struct initialization without field names", see https://www.jetbrains.com/help/inspectopedia/GoStructInitializationWithoutFieldNames.html:
You can try to implement this with revive. If it's not possible, you can create your own linter to detect struct initialization without field names and add it to the golangci-lint. Possible implementation
package main
import (
"go/ast"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/singlechecker"
)
var Analyzer = &analysis.Analyzer{
Name: "fieldnames",
Doc: "check for struct initializations without field names",
Run: run,
}
func run(pass *analysis.Pass) (interface{}, error) {
for _, file := range pass.Files {
ast.Inspect(file, func(n ast.Node) bool {
compLit, ok := n.(*ast.CompositeLit)
if !ok {
return true
}
if _, ok := compLit.Type.(*ast.Ident); !ok {
return true
}
for _, elt := range compLit.Elts {
if _, ok := elt.(*ast.KeyValueExpr); !ok {
pass.Reportf(elt.Pos(), "struct initialization without field names")
}
}
return true
})
}
return nil, nil
}
func main() {
singlechecker.Main(Analyzer)
}Running the linter above on revive's codebase: |
Is your feature request related to a problem? Please describe.
Go allows you to initialize struct only by values, if they follow same order as in structure definition.
This can be useful if structure has 1 field, but otherwise it can lead to syntax errors if order is changed. And even worse, to hidden bugs, if there are fields with same type, and their order is changed.
Describe the solution you'd like
Do a linter, which will catch that and require to have names of fields inside struct initialization.
Describe alternatives you've considered
I have a feeling, that there was such linter. But I searched for that (not only in Revive) and I cannot find anything.
Additional context
I can try to prepare PR, but it is suggested to create issue first.
The text was updated successfully, but these errors were encountered: