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: x/tools: support for struct field's tag as string const #40247

Closed
amarjeetanandsingh opened this issue Jul 16, 2020 · 2 comments
Closed
Labels
FrozenDueToAge Proposal Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@amarjeetanandsingh
Copy link
Contributor

amarjeetanandsingh commented Jul 16, 2020

Problem

You can't use string var / const in tags. Below is an illegal syntax -

type user struct {
	name string `json:"` + FIELD_NAME + `"`
}

As @rsc told in this comment, I also feel tag syntax shouldn't be complicated.

However, this leads us to a situation where we need to carry a string (tag) value everywhere in the code. I will try to explain with an example. Below code tries to update a user.Name field in two different nosql databases. (However this issue remains valid in other scenarios too)

Suppose we have a user struct as :-

type user struct {
	ID   string `bson:"ID" json:"ID"`
	Name string `bson:"NAME"  json:"NAME"`
}

Code to update the user.Name in MongoDB will be like -

mongoFilter := map[string]string{"ID": "userID"}
mongoFieldVal := map[string]map[string]string{"$set": {"NAME": "new name"}}
userColl.UpdateOne(nil, mongoFilter, mongoFieldVal)

Code to update the user.Name in ArangoDB will be like -

arangoFieldVal := map[string]string{"NAME": "new name"}
userColl.UpdateDocument(nil, "user_ID", arangoFieldVal)

As you can see the problem is -

  • we need to carry the tag(json/bson) values "ID" and "NAME" as string every-time we need to interact with these fields.

Problem(1)

  • This increases the chances of doing mistakes because we need to type a string literal at different places.

Problem(2)

  • Also its difficult to maintain because in case somebody updates the json tag value, one might miss to update the string(tag) value at few places, leading to creepy unnoticeable bug that escapes the build time check.

Solution Proposed

In my opinion, there should be single point to manage the field's tag value. Since adding string const inside field's tag value will complicate things, we can go the other way around.

We can have a tool to go:generate the tag's value as string constant, and use these constants as the field's tag value.

  • This reduces the need to type the string value while accessing the field and solves the Problem(1).
  • In case somebody updates a field's name, the corresponding generated string constant will change, forcing the person doing the change to use the new string constant, at the build time itself. This solves the Problem(2).

Implementation

This is a very frequent usecase. So I feel, instead of using a third party code, there should be a support in the language tool itself.

If we eventually decide to implement this feature somewhere as a tool, there can be multiple ways to achieve this. At first, to start the discussion, a very raw idea is -

  • Recommend using field tags for consistency.(We may want to ignore the const generation for the fields without tags)
  • For every package, generate a struct_field_tag_const_gen.go file. This contains all structs'(in the current package) fields' tags as const string in the current package.

I have written a very raw code that generates a file with string constants for each package.
It uses ast.Inspect and reflect.StructTag api to access a struct's comments and its fields' tags.
It considers only those structs which have some magic comment on it. We can decide what mechanism to follow to filter out the structs. In case we decide this feature worth considering, I'll try to improve my current implementation.

/cc @rsc @alandonovan @mvdan

@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Jul 16, 2020
@gopherbot gopherbot added this to the Unreleased milestone Jul 16, 2020
@rsc
Copy link
Contributor

rsc commented Jul 16, 2020

I assume that "the language tool" here means gopls? We can't put everything anyone needs in gopls. Go will only scale to many developers with a variety of needs if people can write their own tools. If everyone has to put everything into a single shared tool, the coordination problem will overwhelm any work.

If you'd find that go:generate tool helpful, then by all means write it and use it. But I disagree that this is "a very frequent use case", and it almost certainly shouldn't go into any tool that the Go team maintains.


Separate from that point, if it were me I would structure the code to treat the struct tags as the truth and in the few cases where you need the tags for constructing maps (as opposed to using json.Marshal directly), fetch them from the struct, like in https://play.golang.org/p/n-vpYSZhkLK.

@andybons andybons changed the title x/tools: Proposal: support for struct field's tag as string const proposal: x/tools: support for struct field's tag as string const Jul 16, 2020
@amarjeetanandsingh
Copy link
Contributor Author

Thank you @rsc for your opinion and pointing out a different solution.
Closing it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge Proposal Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

3 participants