Tolerate missing ';' in enum elements (bufbuild#213)

While still producing an error, see bufbuild#200.
kralicky · Feb 7, 2024 · 885d1ff · 885d1ff
1 parent de1e17f
commit 885d1ff
Show file tree

Hide file tree

Showing 6 changed files with 607 additions and 519 deletions.
diff --git a/ast/enum.go b/ast/enum.go
@@ -131,10 +131,10 @@ func NewEnumValueNode(name *IdentNode, equals *RuneNode, number IntValueNode, op
 	if number == nil {
 		panic("number is nil")
 	}
-	if semicolon == nil {
-		panic("semicolon is nil")
+	numChildren := 3
+	if semicolon != nil {
+		numChildren++
 	}
-	numChildren := 4
 	if opts != nil {
 		numChildren++
 	}
@@ -143,7 +143,9 @@ func NewEnumValueNode(name *IdentNode, equals *RuneNode, number IntValueNode, op
 	if opts != nil {
 		children = append(children, opts)
 	}
-	children = append(children, semicolon)
+	if semicolon != nil {
+		children = append(children, semicolon)
+	}
 	return &EnumValueNode{
 		compositeNode: compositeNode{
 			children: children,

diff --git a/ast/ranges.go b/ast/ranges.go
@@ -290,16 +290,17 @@ func NewReservedNamesNode(keyword *KeywordNode, names []StringValueNode, commas
 	if keyword == nil {
 		panic("keyword is nil")
 	}
-	if semicolon == nil {
-		panic("semicolon is nil")
-	}
 	if len(names) == 0 {
 		panic("must have at least one name")
 	}
 	if len(commas) != len(names)-1 {
 		panic(fmt.Sprintf("%d names requires %d commas, not %d", len(names), len(names)-1, len(commas)))
 	}
-	children := make([]Node, 0, len(names)*2+1)
+	numChildren := len(names) * 2
+	if semicolon != nil {
+		numChildren++
+	}
+	children := make([]Node, 0, numChildren)
 	children = append(children, keyword)
 	for i, name := range names {
 		if i > 0 {
@@ -313,7 +314,9 @@ func NewReservedNamesNode(keyword *KeywordNode, names []StringValueNode, commas
 		}
 		children = append(children, name)
 	}
-	children = append(children, semicolon)
+	if semicolon != nil {
+		children = append(children, semicolon)
+	}
 	return &ReservedNode{
 		compositeNode: compositeNode{
 			children: children,

diff --git a/parser/ast.go b/parser/ast.go
@@ -126,6 +126,15 @@ func newFileElements(semicolons []*ast.RuneNode, elements []ast.FileElement) []a
 	return elems
 }
 
+func newEnumElements(semicolons []*ast.RuneNode, elements []ast.EnumElement) []ast.EnumElement {
+	elems := make([]ast.EnumElement, 0, len(semicolons)+len(elements))
+	for _, semicolon := range semicolons {
+		elems = append(elems, ast.NewEmptyDeclNode(semicolon))
+	}
+	elems = append(elems, elements...)
+	return elems
+}
+
 type nodeWithEmptyDecls[T ast.Node] struct {
 	Node       T
 	EmptyDecls []*ast.EmptyDeclNode
@@ -164,3 +173,12 @@ func toFileElements[T ast.FileElement](nodes nodeWithEmptyDecls[T]) []ast.FileEl
 	}
 	return elements
 }
+
+func toEnumElements[T ast.EnumElement](nodes nodeWithEmptyDecls[T]) []ast.EnumElement {
+	elements := make([]ast.EnumElement, 1+len(nodes.EmptyDecls))
+	elements[0] = nodes.Node
+	for i, emptyDecl := range nodes.EmptyDecls {
+		elements[i+1] = emptyDecl
+	}
+	return elements
+}
diff --git a/parser/parser_test.go b/parser/parser_test.go
@@ -140,7 +140,6 @@ func TestLenientParse_SemicolonLess(t *testing.T) {
 			Error: `syntax = "proto3";
 							service Foo {
 								rpc Bar (Baz) returns (Qux) {
-									;
 									option (foo) = { bar: 1 }
 								}
 							}`,
@@ -152,6 +151,49 @@ func TestLenientParse_SemicolonLess(t *testing.T) {
 									}
 								}`,
 		},
+		"enum-value": {
+			Error: `syntax = "proto3";
+							enum Foo {
+								FOO = 0
+							}`,
+			NoError: `syntax = "proto3";
+								enum Foo {
+									;
+									FOO = 0;;
+								}`,
+		},
+		"enum-value-options": {
+			Error: `syntax = "proto3";
+							enum Foo {
+								FOO = 0 [foo = 1]
+							}`,
+			NoError: `syntax = "proto3";
+								enum Foo {
+									FOO = 0 [foo = 1];
+								}`,
+		},
+		"enum-options": {
+			Error: `syntax = "proto3";
+							enum Foo {
+								option (foo) = 1
+							}`,
+			NoError: `syntax = "proto3";
+								enum Foo {
+									;
+									option (foo) = 1;;
+								}`,
+		},
+		"enum-reserved": {
+			Error: `syntax = "proto3";
+							enum Foo {
+								reserved "FOO"
+							}`,
+			NoError: `syntax = "proto3";
+								enum Foo {
+									;
+									reserved "FOO";;
+								}`,
+		},
 	}
 	for name, input := range inputs {
 		name, input := name, input

diff --git a/parser/proto.y b/parser/proto.y
@@ -33,11 +33,12 @@ import (
 	ooElements   []ast.OneofElement
 	ext          *ast.ExtensionRangeNode
 	resvd        *ast.ReservedNode
+	resvdN       nodeWithEmptyDecls[*ast.ReservedNode]
 	en           *ast.EnumNode
 	enN          nodeWithEmptyDecls[*ast.EnumNode]
 	enElement    ast.EnumElement
 	enElements   []ast.EnumElement
-	env          *ast.EnumValueNode
+	env          nodeWithEmptyDecls[*ast.EnumValueNode]
 	extend       *ast.ExtendNode
 	extendN      nodeWithEmptyDecls[*ast.ExtendNode]
 	extElement   ast.ExtendElement
@@ -75,6 +76,8 @@ import (
 
 // any non-terminal which returns a value needs a type, which is
 // really a field name in the above union struct
+// TODO: Remove all *N types by migrating all usage to them, then removing the old types
+// and renaming the *N types to the old types.
 %type <file>         file
 %type <syn>          syntaxDecl
 %type <ed>           editionDecl
@@ -107,14 +110,14 @@ import (
 %type <ooElement>    oneofElement
 %type <ooElements>   oneofElements oneofBody
 %type <names>        fieldNameStrings fieldNameIdents
-%type <resvd>        msgReserved enumReserved reservedNames
+%type <resvd>        msgReserved reservedNames
+%type <resvdN>       enumReserved reservedNamesWithEmptyDecls
 %type <rng>          tagRange enumValueRange
 %type <rngs>         tagRanges enumValueRanges
 %type <ext>          extensionRangeDecl
 %type <en>           enumDecl
 %type <enN>					 enumDeclWithEmptyDecls
-%type <enElement>    enumElement
-%type <enElements>   enumElements enumBody
+%type <enElements>   enumElement enumElements enumBody
 %type <env>          enumValueDecl
 %type <extend>       extensionDecl
 %type <extendN>      extensionDeclWithEmptyDecls
@@ -757,10 +760,12 @@ msgReserved : _RESERVED tagRanges ';' {
 	}
 	| reservedNames
 
-enumReserved : _RESERVED enumValueRanges ';' {
-		$$ = ast.NewReservedRangesNode($1.ToKeyword(), $2.ranges, $2.commas, $3)
+enumReserved : _RESERVED enumValueRanges ';' semicolons {
+	  // TODO: Tolerate a missing semicolon here. This currnelty creates a shift/reduce conflict
+		// between `reserved 1 to 10` and `reserved 1` followed by `to = 10`.
+		$$ = newNodeWithEmptyDecls(ast.NewReservedRangesNode($1.ToKeyword(), $2.ranges, $2.commas, $3), $4)
 	}
-	| reservedNames
+	| reservedNamesWithEmptyDecls
 
 reservedNames : _RESERVED fieldNameStrings ';' {
 		$$ = ast.NewReservedNamesNode($1.ToKeyword(), $2.names, $2.commas, $3)
@@ -769,6 +774,15 @@ reservedNames : _RESERVED fieldNameStrings ';' {
 		$$ = ast.NewReservedIdentifiersNode($1.ToKeyword(), $2.idents, $2.commas, $3)
 	}
 
+reservedNamesWithEmptyDecls : _RESERVED fieldNameStrings semicolons {
+	  semi, extra := protolex.(*protoLex).requireSemicolon($3)
+		$$ = newNodeWithEmptyDecls(ast.NewReservedNamesNode($1.ToKeyword(), $2.names, $2.commas, semi), extra)
+	}
+	| _RESERVED fieldNameIdents semicolons {
+		semi, extra := protolex.(*protoLex).requireSemicolon($3)
+		$$ = newNodeWithEmptyDecls(ast.NewReservedIdentifiersNode($1.ToKeyword(), $2.idents, $2.commas, semi), extra)
+	}
+
 fieldNameStrings : stringLit {
 		$$ = &nameSlices{names: []ast.StringValueNode{toStringValueNode($1)}}
 	}
@@ -795,47 +809,40 @@ enumDeclWithEmptyDecls : _ENUM identifier '{' enumBody '}' semicolons {
 		$$ = newNodeWithEmptyDecls(ast.NewEnumNode($1.ToKeyword(), $2, $3, $4, $5), $6)
 	}
 
-enumBody : {
-		$$ = nil
+enumBody : semicolons {
+		$$ = newEnumElements($1, nil)
+	}
+	| semicolons enumElements {
+		$$ = newEnumElements($1, $2)
 	}
-	| enumElements
 
 enumElements : enumElements enumElement {
-		if $2 != nil {
-			$$ = append($1, $2)
-		} else {
-			$$ = $1
-		}
+		$$ = append($1, $2...)
 	}
 	| enumElement {
-		if $1 != nil {
-			$$ = []ast.EnumElement{$1}
-		} else {
-			$$ = nil
-		}
+		$$ = $1
 	}
 
-enumElement : optionDecl {
-		$$ = $1
+enumElement : optionDeclWithEmptyDecls {
+		$$ = toEnumElements($1)
 	}
 	| enumValueDecl {
-		$$ = $1
+		$$ = toEnumElements($1)
 	}
 	| enumReserved {
-		$$ = $1
-	}
-	| ';' {
-		$$ = ast.NewEmptyDeclNode($1)
+		$$ = toEnumElements($1)
 	}
 	| error {
 		$$ = nil
 	}
 
-enumValueDecl : enumValueName '=' enumValueNumber ';' {
-		$$ = ast.NewEnumValueNode($1, $2, $3, nil, $4)
+enumValueDecl : enumValueName '=' enumValueNumber semicolons {
+		semi, extra := protolex.(*protoLex).requireSemicolon($4)
+		$$ = newNodeWithEmptyDecls(ast.NewEnumValueNode($1, $2, $3, nil, semi), extra)
 	}
-	|  enumValueName '=' enumValueNumber compactOptions ';' {
-		$$ = ast.NewEnumValueNode($1, $2, $3, $4, $5)
+	|  enumValueName '=' enumValueNumber compactOptions semicolons {
+		semi, extra := protolex.(*protoLex).requireSemicolon($5)
+		$$ = newNodeWithEmptyDecls(ast.NewEnumValueNode($1, $2, $3, $4, semi), extra)
 	}
 
 messageDecl : _MESSAGE identifier '{' messageBody '}' {