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/printer: align multiple struct tags per field for readability #44650

Closed
jpap opened this issue Feb 26, 2021 · 6 comments
Closed

Comments

@jpap
Copy link
Contributor

jpap commented Feb 26, 2021

Motivation

I am repeatedly having to use multiple struct field tags and they are very difficult to read quickly while scanning through code, especially older or foreign code that is less familiar.

We currently write multiple tags using the "optionally space-separated key:"value" pairs" convention, but it boils down to a string that gofmt cannot not format nicely.

Issue #16918 gives an illustrative example:

type User struct {
    Name      string `json:"name" bson:"name" validate:"required"`
    LegalName string `json:"legalName" bson:"legalName" validate:"required"`
    Email     string `json:"email" bson:"email" validate:"required"`
        ....
}

I'd love to see multiple tag definitions like the above more readable: see the example in the proposed section below.

Proposal

A previous proposal #16918 was declined because the consensus was that tools like gofmt should not modify strings within a source file. This proposal differs by allowing multiple struct tags per field, which then allows gofmt to align using whitespace and without modifying the contents of each string component.

The above example can now be formatted so it becomes very easy to read:

type User struct {
    Name      string `json:"name"`      `bson:"name"`      `validate:"required"`
    LegalName string `json:"legalName"` `bson:"legalName"` `validate:"required"`
    Email     string `json:"email"`     `bson:"email"`     `validate:"required"`
        ...
}

We can support this syntax and avoid breaking API changes as follows:

  1. Define multiple tags as being components of a single "concrete" field tag string, where each tag component is separated by a single space. You can think of this solution as notionally similar to how the C pre-processor treats multiple string segments: they get concatenated and fed to the compiler as a single string.

    This change is likely contained to the Go scanner (+gofmt).

  2. Like the previous option, except components are concatenated without the separating space character. (We now behave exactly like C concatenating string segments.)

    In addition to the Go scanner (+gofmt) change, the existing reflect.StructTag convention of having a space separating multiple key:"value" pairs would need to be loosened, so as not require the space separator, but this does not impact its API. (I assume that convention was established so multiple pairs are more human-readable, rather than a constraint on the parser.)

  3. Like the previous option, but loosen the existing reflect.StructTag convention to include tabs, where multiple tags are separated by a tab. The motivating use-case for a tab would be to simultaneously enable an author wanting to manually align multiple tags within the same string by hand. (You can think of the above proposed syntax as a "compiler generated" version of this.)

Option 2 is my preference.

@gopherbot gopherbot added this to the Proposal milestone Feb 26, 2021
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Feb 27, 2021
@ianlancetaylor ianlancetaylor changed the title proposal: cmd/go multiple struct field tags and gofmt for alignment/readability proposal: allow multiple struct tags per field aligned for readability Feb 27, 2021
@dsnet
Copy link
Member

dsnet commented Mar 8, 2021

What's the expected output of formatting:

type User struct {
    Name      string `json:"name" bson:"name" validate:"required"`
    LegalName string `validate:"required" json:"legalName" bson:"legalName" `
    Email     string `bson:"email" validate:"required" json:"email"`
        ....
}

does it reorder the tags as well to achieve alignment?

@jpap
Copy link
Contributor Author

jpap commented Mar 9, 2021

Under the current proposal, your sample remains unchanged because each field has a single contiguous tag.

If you were to use multiple string fragments,

type User struct {
    Name      string `json:"name"` `bson:"name"` `validate:"required"`
    LegalName string `validate:"required"` `json:"legalName"` `bson:"legalName" `
    Email     string `bson:"email"` `validate:"required"` `json:"email"`
        ....
}

the current proposal would allow it to be formatted as:

type User struct {
    Name      string `json:"name"`         `bson:"name"`         `validate:"required"`
    LegalName string `validate:"required"` `json:"legalName"`    `bson:"legalName" `
    Email     string `bson:"email"`        `validate:"required"` `json:"email"`
        ....
}

(Which is still easier to read than the original, even if not re-arranged into columns.)

The reasons for this are as follows:

  • Proposal proposal: cmd/gofmt formatting/alignment of multiple struct tags #16918 was declined because the consensus was that gofmt shouldn't change the contents of strings.
  • This proposal departs from the previous by allowing struct field tags be comprised of string fragments: gofmt can format those fragments by inserting whitespace between them while preserving the contents of the strings without change. If gofmt could reorder each tag component, it would effectively be changing the concrete string tag.
  • The proposal aims to maintain backward-compatibility with existing APIs by maintaining the field tag concept as a single string.

Having said all that, I wonder whether your example would be frequent in practice:

  • When a developer first writes the User struct, they might start without tags, or tags for one key.
  • The developer might add additional tags to User in subsequent revisions: as a typist, it is easier and more natural for such edits to append the new key across all fields in the same sitting, maintaining the key-name ordering.
    • For example, when I do this, I typically use VSCode with multiple cursors so that I type out the key:"" part simultaneously for all lines before I fill in the double-quoted text on a line-by-line basis: it's easiest to add multiple carets to the end of each line with a keyboard shortcut than it is to randomly mix them across lines.

Perhaps a more interesting question might be what to do if there are "gaps",

type User struct {
    Name      string `validate:"required"` `json:"name"`      `bson:"name"`      
    LegalName string `validate:"required"` `json:"legalName"` `bson:"legalName"` 
    Email     string                       `json:"email"`     `bson:"email"`     
        ...
}

for which it would be nice arrange the spacing as shown, where possible.

@rsc
Copy link
Contributor

rsc commented Jul 14, 2021

This seems too specialized for gofmt.

It also seems like a separate formatter could do this easily (built with go/parser and go/printer), and gofmt would not get in the way.

@rsc rsc changed the title proposal: allow multiple struct tags per field aligned for readability proposal: go/printer: align multiple struct tags per field for readability Jul 14, 2021
@rsc
Copy link
Contributor

rsc commented Jul 21, 2021

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc rsc moved this from Incoming to Active in Proposals (old) Jul 21, 2021
@rsc
Copy link
Contributor

rsc commented Jul 28, 2021

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

@rsc rsc moved this from Active to Likely Decline in Proposals (old) Jul 28, 2021
@rsc rsc moved this from Likely Decline to Declined in Proposals (old) Aug 4, 2021
@rsc
Copy link
Contributor

rsc commented Aug 4, 2021

No change in consensus, so declined.
— rsc for the proposal review group

@rsc rsc closed this as completed Aug 4, 2021
@golang golang locked and limited conversation to collaborators Aug 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

No branches or pull requests

4 participants