proposal: go/parser: add a mode flag to disallow the new syntax for type parameters / instantiation

[findleyr]

In #47781, we propose to add a new node type to go/ast to support type and function instantiation. One of our concerns with this change is that there is code in the wild that assumes the completeness of the current set of go/ast nodes, and that this code will now panic when encountering an unknown node type (see golang/vscode-go#1551, for example). This concern was also raised in the monthly tools call.

Whatever final form the new APIs take, it would be helpful to provide a way for tool authors to prevent go/parser from producing the new syntax nodes. By analogy, users of go/types can set the Config.GoVersion field (see #46648).

While it might be a good idea to eventually add a go/parser.Config type with a GoVersion field. That is a large change that would be a burden on calling code: users would have to switch away from the parser.Parse* APIs. It is much easier for users to add a mode flag.

Therefore this issue proposes to add a new mode flag that tells go/parser not to accept the new syntax for type parameters and instantiation. This is actually already implemented via an unexported flag value, so the hardest part of implementing this proposal would be coming up with a name.

I suggest NoTypeParameters, but I don't love it. Other ideas are welcome, though note that we have been trying to avoid the word 'generics' in favor of the more precise 'type parameter', 'parameterized', etc.

CC @griesemer

[rsc]

As long as the default behavior is to accept the full Go language, then it seems fine to introduce DisallowTypeParameters as a temporary bandaid that old tools can adopt as a quick fix before they add proper support.

[griesemer]

For reference, the syntax parser has a mode called AllowGenerics. Since the default is now to allow them, we might invert that (internal) mode and rename it to DisallowGenerics (or DisallowTypeParameters).

[rsc]

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

[mvdan]

I'm not sure I understand the worry that leads to this proposal. Tools or libraries that don't yet support type parameters might panic or error if they see the new go/ast node types in Go 1.18. Presumably, that would lead to them either adding support for the new feature, or to add a few lines of code that gives an error like "type parameters not supported" in those cases.

How would a NoTypeParameters mode help in that situation? The end user would see some form of parse error, so it's about the same end result. The tool or library still needs to be modified to start using the mode, so in my eyes it's not making the transition smoother either.

I'm also slightly worried about scenarios where the code calling the Parse APIs is in a different module than the code that uses type switches on go/ast node types. For example: gopls parses code, and feeds an *ast.File to an external module, like https://pkg.go.dev/mvdan.cc/gofumpt/format#File. From the point of view of the library, it's too late to say if it wants to avoid type parameters or not. And if it wishes to simply not support type parameters, it can return a user-friendly error message without this proposed API.

[findleyr]

@mvdan these are great points.

The intent of this flag is to help with cases where it might not be trivial to update code to support type parameters, and a parser error is preferable to a panic. But as you point out, this can only ever really be a temporary workaround, and is only helpful in certain conditions. Maybe these conditions are rare enough that it is not worth a new mode flag.

CC @marwan-at-work who had brought this up in the tools call.

[mvdan]

I get that some tools or libraries might take effort to support type parameters. They could return a temporary error message in the meantime, perhaps pointing to an issue tracking the progress. Or if they don't support type parameters by definition, perhaps link to a page explaining the reason why. Both of those seem more useful to me than a blanket "no type parameters" mode, do not require the Parse and type switch to happen in the same module, and do not require new API :)

[findleyr]

Yes, in that context this would only be helpful where setting this mode flag is significantly easier than finding and updating all the places that might panic or behave incorrectly when encountering the new syntax.

That's the critical coefficient that justifies the new mode flag, and perhaps I overestimated it.

[mvdan]

Ah I see, that makes sense. To provide a data point, I just did some quick grepping over my 6 modules using go/ast, and they seem to have about 2-5 node type switches each. About a quarter of them are "exhaustive" with a default case that panics; the others just care about a few node types in particular.

So, at least for my software, I likely would only need to update one or two type switches per module to add the error returns. I wouldn't mind that, given that introducing new AST node types should be rare. And I'd have to edit those files anyway, if I wanted to add support for type parameters, which I hope we all do :)

[rsc]

It sounds like maybe this flag doesn't add as much as we thought.
Certainly having generics off by default and opt-in would avoid breaking users,
but that's not the right default for the next ten years.
Having the default be on with an opt-out, the opt-out seems like maybe not significantly less work than putting a new case in a type switch here and there to say "no generics yet".

Do I have that right?

[mvdan]

That seems like a good summary. My only brief addition would be: some tools expose themselves as libraries taking go/ast nodes, so they don't have the ability to provide go/parser options. https://pkg.go.dev/golang.org/x/tools/go/analysis is a good example; an analysis simply receives a bunch of ASTs.

[findleyr]

Unless anyone has concerns, I'll retract this proposal.

I'll leave it open until Tuesday (before the next proposal meeting) in case anyone wants to object.

[scott-cotton]

It sounds like maybe this flag doesn't add as much as we thought.
Certainly having generics off by default and opt-in would avoid breaking users,
but that's not the right default for the next ten years.
Having the default be on with an opt-out, the opt-out seems like maybe not significantly less work than putting a new case in a type switch here and there to say "no generics yet".

Do I have that right?

It may be nice to document "new in 1.18" or exactly which cases of switches suffice to check for type parameters.

Scott

[findleyr]

It may be nice to document "new in 1.18" or exactly which cases of switches suffice to check for type parameters.

Ack. We'll be sure to highlight this in the 1.18 release notes, perhaps linking to the go/ast proposal.

[findleyr]

Closing as nobody has objected. Please ping this issue if you have any concerns.

[rsc]

This proposal has been declined as retracted.
— rsc for the proposal review group