x/tools/go/analysis/passes/composite: flag to require keys within same package

[arxeiss]

I would like to suggest improvement to existing Vet Composite linter.

Currently, composite requires struct field names to be specified only, if given type is defined in different package. However, I believe, same restrictions should be done for type within same package. Because if some struct is used in many places, adding 1 field requires changed everywhere. Even, if it is irrelevant and I would be specifying only empty value.

Not to introduce breaking change, it would be possible to turn this on by flag.

Meaning all those selected lines (30 - 68) would be raising issue https://cs.opensource.google/go/x/tools/+/effd83eb4d44f4443100debb7ede6619a4ac6528:go/analysis/passes/composite/testdata/src/a/a.go;l=30-68

[gabyhelp]

Related Issues

• x/tools/go/analysis/passes/composite: check locally defined structs #54823 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[arxeiss]

My bad I missed the issue #54823. But it was closed by author. However, I still think it could be implemented.

1. Turned on only when developer wants with flag - not turned on by default
2. Don't sure if gopls still doesn't support adding flags, but even if not, it would be raised during explicit linter run
3. I think go vet is supressing errors outside current module. Otherwise we would see tons of false positives. I doubt all developers are following same rules as Go standards.

[adonovan]

I think a flag is the wrong UX: it's global, whereas what you want is something package-local or even struct local. It seems to me that a more useful behavior would be for the composite analyzer to honor a directive within a particular struct or package that tells it to be stricter. For example, something like this:

type Point struct {
    //go:vet composite strict
    X, Y int
}
...
var _ = Point{1, 2} // error: composite: unkeyed Point struct literal

The directive syntax has only relatively recently been formalized and has so far been used (outside the compiler) only once, for the //go:fix inline directive, so we'll likely need some discussion about how it should work in light of the precedents it will set.

Immediate questions:

• how should it be spelled?
• should it be allowed within (or before) a particular struct, at package level, or both?

@dominikh

[jimmyfrasche]

I would expect a directive to go above the struct it applied to as it would with a func. If I saw random code with a directive above fields I would assume it was setting something at the field level.

//go:vet composite strict
type Point struct {
    X, Y int
}