Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: Go 2: ensure exhaustive struct filling #42940

Closed
phrounz opened this issue Dec 2, 2020 · 6 comments
Closed

proposal: Go 2: ensure exhaustive struct filling #42940

phrounz opened this issue Dec 2, 2020 · 6 comments
Labels
FrozenDueToAge LanguageChange Proposal v2 A language change or incompatible library change
Milestone

Comments

@phrounz
Copy link

phrounz commented Dec 2, 2020

Proposal in brief
Add a keyword like "exhaustive" to compile-check that all fields in a struct instance creation are informed/filled.

Example and related issue
In the corresponding code:

type foo struct {
  a string
  b string
  c string
}

func main() {
  var f = foo{
    a: "hello",
    b: "world",
  }
  fmt.Printf("%v\n", f)
}

I think there is a danger here (which is not specific to Go, it also exists in C,C++, or many object-oriented langages): "c" has not been filled and will have its default empty string value. It may perfectly be intentional (filling fields is optional and it allows flexibility), but in some cases it may also not be: maybe this "c" field has been added afterwards and the developer may have forgot to correct all corresponding struct creations.

Proposed solutions with examples

1. The proposal is to create a keyword like "exhaustive" which would cause a compilation check that that all fields are filled in
that struct creation statement.

type foo struct {
  a string
  b string
  c string
}

func main() {
  var f = exhaustive foo{
    a: "hello",
    b: "world",
  }
  fmt.Printf("%v\n", f)
}

This code would generate a compilation error: "exhaustive keyword with missing field in struct foo: c"

Of course, this keyword would also be accepted on function calls like any normal statement:

fmt.Printf("%v\n", myFunction(exhaustive foo{a: "hello"}))

2. An even bolder idea: the "exhaustive" keyword on the struct type declaration would forbid any struct creation where a field declaration is missing.

type foo exhaustive struct {
  a string
  b string
  c string
}

func main() {
  var f = foo{
    a: "hello",
    b: "world",
  }
  fmt.Printf("%v\n", f)
}

This code would generate the same compilation error.

3. Another totally different approach: a keyword for "compulsory" fields.

type foo struct {
  a string compulsory
  b string
  c string
}

func main() {
  var f = foo{
    b: "hello",
  }
  fmt.Printf("%v\n", f)
}

Compilation error, not because of "c" which remains optional, but because of "a".

Go 2 language change template

Would you consider yourself a novice, intermediate, or experienced Go programmer?
Intermediate, I don't know.

What other languages do you have experience with?
Perl, C, C++, PHP, Bash, Mysql, and a bit of Java, Javascript, Python

Would this change make Go easier or harder to learn, and why?
Slightly harder but the feature is kinda simple, and optional to use which makes it a less imperative feature to know.

Has this idea, or one like it, been proposed before?
Not found in github issues.

Who does this proposal help, and why?
Developers wanting the most robust code possible.

What is the proposed change?
Please describe as precisely as possible the change to the language.
What would change in the language spec?
Please also describe the change informally, as in a class teaching Go.
(see above).

Is this change backward compatible?
Would break code where the keyword "exhaustive"/"compulsory" is used as a type/variable, but maybe it could be possible to accept it nevertheless and distingish a type/variable from the keyword, depending of the implementation.

What is the cost of this proposal? (Every language change has a cost).
Slightly higher language maintenance.

How many tools (such as vet, gopls, gofmt, goimports, etc.) would be affected?
Several, depending on implementation.

What is the compile time cost?
Presumably low.

What is the run time cost?
Absolutely none. The binary program generated would be strictly the same provided that compilation passes.

Can you describe a possible implementation?
No. I just assume that it would impact the code that parses the struct statements.

Do you have a prototype? (This is not required.)
No.

Orthogonality: how does this change interact or overlap with existing features?
Interacts with the way struct and declaration works.

Is the goal of this change a performance improvement?
No.

Does this affect error handling?
No.

Is this about generics?
No.

@gopherbot gopherbot added this to the Proposal milestone Dec 2, 2020
@davecheney
Copy link
Contributor

Is this a widespread problem in general? I’m thinking something on the order of types nils or := shadowing variables in an outer scope.

@ianlancetaylor ianlancetaylor added v2 A language change or incompatible library change LanguageChange labels Dec 2, 2020
@ianlancetaylor
Copy link
Contributor

This seems better suited to a static analysis tool than to a language change. A new keyword seems like a very heavyweight mechanism for a feature like this.

@mvdan
Copy link
Member

mvdan commented Dec 2, 2020

To add to Ian's point, a static analysis tool could simply use struct field tags to see which fields are compulsory, so this is definitely doable without changing the Go language.

@DeedleFake
Copy link

If this became a feature, adding a field to a struct would always be a breaking change, since if a single person used this keyword somewhere it would cause it to break. This definitely seems to be better suited to a separate static analysis tool than a language-level feature.

@maja42
Copy link

maja42 commented Dec 5, 2020

I actually use something like that in unit tests. I have a small function that checks (using reflection) if all exported fields in a struct are set. And it allows me to specify fields for which it's okay to have the default/zero value.
This is possible today and superior to this proposal, because the fields that must be set are defined in unit tests and not globally at the struct definition. Hence, different tests can require different fields to be set.

I use it mostly to check if all protobuf fields are populated in an RPC to detect if the proto definition was changed without a corresponding code update.

@phrounz
Copy link
Author

phrounz commented Dec 14, 2020

Ok, since popular opinon is that it should not cause a language change, and could be in a separate static analysis check tool instead, I did implement such a tool, using go/parser and go/ast:

https://github.com/phrounz/go-parano

As of today it is currently experimental, though.

@phrounz phrounz closed this as completed Dec 14, 2020
@golang golang locked and limited conversation to collaborators Dec 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge LanguageChange Proposal v2 A language change or incompatible library change
Projects
None yet
Development

No branches or pull requests

7 participants