Update buf format to work correctly with editions #2969
Conversation
| if editionNode != nil { | ||
| f.writeEdition(editionNode) | ||
| } | ||
| if syntaxNode := f.fileNode.Syntax; syntaxNode != nil && editionNode == nil { |
There was a problem hiding this comment.
This would effectively wipe a syntax line from the file if an edition line exists, but that seems reasonable, right?
There was a problem hiding this comment.
They cannot both exist. You either specify syntax proto2 or proto3 or you specify an edition.
There was a problem hiding this comment.
makes sense — just wondering if we should leave it up to a linter or a buf build failing to point that out to the user, rather than removing it altogether. But I don't think this is unreasonable behavior, and should be a niche case anyway.
There was a problem hiding this comment.
It's an invariant of the AST. This logic is really here just to try to produce a valid source file even if there's something unexpectedly wrong with the AST. FWIW, we already do similar in the reserved nodes, where we prefer names, then identifiers, and then ranges. If a node had more than one set, the others would be ignored (which is correct to produce a valid source file since only one is allowed).
Thanks @stefanvanburen for noticing this!
The formatter needed to be updated to be aware of the
editionAST node (which appears instead ofsyntaxin Editions files) and also needed updates to reserved names, which use identifiers instead of string literals in Editions.