golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
122.6k stars 17.48k forks source link

proposal: Go 2: require field name for struct composite literal when the struct reference is with a selector #27423

Closed maruel closed 5 years ago

maruel commented 6 years ago

Background

Adding a new field to a struct in a package is currently a breaking change. This means that adding a field to any public struct in a package requires a new major package version, creating a fair amount of churn for users when coupled with modules.

One major reason this is a breaking change is because of the second rule of Composite literals that allows to not specify each individual key name, and use a list instead, i.e. image.Point{10, 20}.

Adding methods to a struct is generally not considered a breaking change (even if it is) and it is frequently done in the Go standard library.

The net result is that it is very hard to write future proof structs, as adding a struct fields is a generally considered a breaking change due to the allowance of keyless composite literal rule for struct imported from an external package.

Precedent

go vet already warns on keyless struct composite literal.

Proposal

When a struct composite literal appears in the code, and the struct symbol reference uses a selector, that is, it is not in the current package, then field names are required.

This means changing the composite literal spec from:

For struct literals the following rules apply:

to

For struct literals the following rules apply:

This is effectively turning the go vet composites rule into a compile rule, removing the need for the go vet check.

Rationale

Enable package authors to add fields to struct the same way they feel safe to add methods to struct.

Automatic fix

While this is a breaking change in the language, as current code that use struct composite literal from imported packages with keyless list would stop compiling, it is a relatively easy fix that could be 100% automated.

Caveats

Breaking change due to embedding

Adding a field to a struct would still be a breaking change when the struct is embedded into another one, as described in this post. Still, the cases where this occurs are expected to be significantly lesser than struct composite literal, resulting in a net benefits.

Verbosity

The rationale to disallow non-keyed list only when a selector is used enables using non-keyed list in unit tests, which are often verbose.

The obvious downside is that other packages will be more verbose in the struct composite, but long term forward compatibility is worth the price.

Example

Synthetic

Source foo.go

package foo

type struct T {
  A string
}

In foo_test.go, it would be allowed to use the short form T{"hi"} because the type T is referred to without a selector.

On the other hand, in source bar.go:

package bar

import "foo"

var t = foo.T{A: "hi"}

Using foo.T{"hi"} would be disallowed because type T is referred to via selector foo..

Real world

I'd like to add a new field to the struct physic.Env as the BME680 sensor supports gas measurement, but I can't add a field to this struct without bumping periph's major version from 3 to 4. This is because I don't know which code will break.

ianlancetaylor commented 6 years ago

I think it's worth mentioning the section on struct literals in the Go 1 compatibility doc: https://golang.org/doc/go1compat#expectations .

maruel commented 6 years ago

Thanks, that's a good point. The proposal is essentially replacing the recommendation with enforcement, for better future proofing for package authors, including the stdlib.

dsnet commented 6 years ago

This seems to be a duplicate of #2794, but has more prose written for the same issue.

I should note that compatibility (as defined by the standard library and reasonably by most packages) is not defined by whether your dependents continue to build or not. For example, the standard library reserves the right to change testing.MainStart even it means breaking someone's build.

As another real-world example: In protocol buffers, we had to start generating an XXX_NoUnkeyedLiteral field to force users to use keyed literals. It broke people, but was a necessary step to prevent further misuses.

drakmaniso commented 6 years ago

IMHO this proposition would make code harder to read in a few domains that use short vector-like structs (for coordinates, colors...), especially for graphics programming, indie game development, and probably other math-heavy domains.

These tend to have not only quite a few literals in the code, but also conversion to/from separate variables, leading to stuttering: p = image.Point{X: x, Y:y}

Maybe unkeyed literals could be allowed only for structs where all field names are one-letter long?

Note that, in these cases, using arrays instead of structs is not a good option either, because it obfuscates the code that make use of the fields: c = color.RGBA{nc.R*nc.A, nc.G*nc.A, nc.B*nc.A} is much easier to understand than c = color.RGBA{nc[0]*nc[3], nc[1]*nc[3], nc[2]*nc[3]}

ianlancetaylor commented 5 years ago

Closing as dup of #2794.