Linter to disallow struct initialization without field names

[arxeiss]

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]

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]

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]

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]

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]

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.

• If you have _ = X{} and it is empty, I don't care how many fields the struct X has.
• If you will write _ = X{"some value"} and X has only 1 field, it is OK too.
• If it has multiple fields, _ = X{"some value"} will raise an compile error as you have to define all or nothing. (unless you specify names of fields)

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

[ccoVeille]

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

[ccoVeille]

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 F

so 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.

[arxeiss]

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

[ccoVeille]

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.

[alexandear]

Goland supports detecting "Struct initialization without field names", see https://www.jetbrains.com/help/inspectopedia/GoStructInitializationWithoutFieldNames.html:

Struct initialization without field names

Reports structures that are initialized without specifying their field names. By default, the inspection is available only when you use the type that is defined in a different package.

When initializing a structure, it is better to explicitly state field names in order to ensure that in case of changes in order of these fields or in names of the fields, they will correctly continue to be addressed.

Example:

_ = io.LimitedReader{nil, 10}

The LimitedReader initialization will be highlighted because explicit names for struct fields are missing. You can apply the Add keys and delete zero values quick-fix to the struct initialization. After the quick-fix is applied, the code looks as follows:

_ = io.LimitedReader{N: 10}

---

I wanted to try out to implement this rule, but I have a feeling it is not possible with Revive.

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:

/Users/Oleksandr_Redko/src/github.com/mgechev/revive/formatter/friendly.go:118:33: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/formatter/friendly.go:118:39: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/formatter/sarif.go:50:3: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/formatter/sarif.go:51:3: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/formatter/sarif.go:52:3: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/bool_literal_in_expr.go:22:24: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/bool_literal_in_expr.go:22:33: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/call_to_gc.go:23:20: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/call_to_gc.go:23:31: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/constant_logical_expr.go:22:32: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/constant_logical_expr.go:22:41: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/empty_block.go:20:22: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/empty_block.go:20:49: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/empty_lines.go:21:22: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/empty_lines.go:21:28: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/empty_lines.go:21:69: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/flag_param.go:42:26: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/flag_param.go:42:38: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/flag_param.go:42:42: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/identical_branches.go:21:30: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/identical_branches.go:21:39: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/if_return.go:23:22: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/if_return.go:23:31: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/nested_structs.go:38:21: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/package_comments.go:35:28: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/package_comments.go:35:37: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/package_comments.go:35:43: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/package_comments.go:35:54: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/range.go:22:19: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/range.go:22:25: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/string_of_int.go:24:22: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/string_of_int.go:24:28: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/time_equal.go:22:22: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/time_equal.go:22:28: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/time_naming.go:23:22: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/time_naming.go:23:28: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/unconditional_recursion.go:72:31: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/unconditional_recursion.go:72:55: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/unconditional_recursion.go:72:41: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/unconditional_recursion.go:72:46: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/unconditional_recursion.go:101:43: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/unconditional_recursion.go:101:53: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/unexported_naming.go:21:34: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/unnecessary_stmt.go:20:31: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/unreachable_code.go:39:27: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/unreachable_code.go:39:38: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/useless_break.go:22:25: struct initialization without field names
/Users/Oleksandr_Redko/src/github.com/mgechev/revive/rule/useless_break.go:22:36: struct initialization without field names
exit status 3

[arxeiss]

@alexandear thank you for your comment. I totally missed that one. I think it is not possible with Revive as I wrote above. Revive works only with AST, while I need static analysis, which your snippet actually does. And it is pretty easy.

Wondering if you already did such a linter or not. And if not, I would probably start my own check. Lately I see that "anti-pattern" very often in our code base. So I really need to include such a check.

[alexandear]

@arxeiss Thanks for the follow-up! I haven't written such a linter myself, but I agree it's a pattern worth catching. Sounds like a good idea to implement a custom check — happy to help or review it if you move forward with it.

[denisvmedia]

I think it's the same as composites in govet.

[denisvmedia]

See here: https://pkg.go.dev/cmd/vet (can be also used in golangci-lint)

[arxeiss]

@denisvmedia Good point, it really does what I would like to have. The issue is, it skips local types. Meaning types defined in the same package.

So before jumping into my own implementation, I will try to ask for improvement. Thanks, I think we can close this issue, as we cannot implement that in Revive at all