mgechev / revive

🔥 ~6x faster, stricter, configurable, extensible, and beautiful drop-in replacement for golint
https://revive.run
MIT License
4.79k stars 277 forks source link

Linter to disallow struct initialization without field names #1059

Open arxeiss opened 5 days ago

arxeiss commented 5 days ago

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.

type X struct {
    x string
    y string
}

func DoSomething() {
    l := X{"xval", "yval"}
    fmt.Printf("%+v\n", l)
}

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.

arxeiss commented 4 days ago

I wanted to try out to implement this rule, but I have a feeling it is not possible with Revive. It would require reflect package, or something more advanced. Because I cannot differentiate struct vs array initialization.

type A []string

type X struct {
    x string
    y string
}

func DoSomething() {
    _ = A{"xval", "yval"}
    _ = X{"xval", "yval"}
}
chavacava commented 4 days ago

Hi @arxeiss thanks for the proposal. Struct initialization was discussed at #682, your proposal is not exactly the same. IMO it's something that we could add. There are some cases for which initialization without names should be allowed: empty structs and structs with only one field. This might need access to the definition of the structure, something that is not necessarily available in all contexts

chavacava commented 4 days ago

I wanted to try out to implement this rule, but I have a feeling it is not possible with Revive. It would require reflect package, or something more advanced. Because I cannot differentiate struct vs array initialization.

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.

chavacava commented 4 days ago

BTW, a language hack to force using field names when instantiation a struct is to declare a blank identified _ field as the first field of the struct:

type position struct {
    _ struct{}  // just to force named fields in instantiation
    x int
    y int
}

(again, it's a hack)

arxeiss commented 4 days ago

There are some cases for which initialization without names should be allowed: empty structs and structs with only one field. This might need access to the definition of the structure, something that is not necessarily available in all contexts

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.

func DoSomething() {
    _ = pkg1.Values{"xval", "yval"} // is this definition of slice or struct?
    _ = pkg2.Values{"xval", "yval"}
}