Tolerate trailing '.' in option names (#222)

While still producing an error, see: #200
bufbuild · Dec 8, 2023 · 4c57c26 · 4c57c26
1 parent 508b83b
commit 4c57c26
Show file tree

Hide file tree

Showing 4 changed files with 764 additions and 657 deletions.
diff --git a/ast/options.go b/ast/options.go
@@ -159,10 +159,10 @@ func NewOptionNameNode(parts []*FieldReferenceNode, dots []*RuneNode) *OptionNam
 	if len(parts) == 0 {
 		panic("must have at least one part")
 	}
-	if len(dots) != len(parts)-1 {
+	if len(dots) != len(parts)-1 && len(dots) != len(parts) {
 		panic(fmt.Sprintf("%d parts requires %d dots, not %d", len(parts), len(parts)-1, len(dots)))
 	}
-	children := make([]Node, 0, len(parts)*2-1)
+	children := make([]Node, 0, len(parts)+len(dots))
 	for i, part := range parts {
 		if part == nil {
 			panic(fmt.Sprintf("parts[%d] is nil", i))
@@ -175,6 +175,12 @@ func NewOptionNameNode(parts []*FieldReferenceNode, dots []*RuneNode) *OptionNam
 		}
 		children = append(children, part)
 	}
+	if len(dots) == len(parts) { // Add the erroneous, but tolerated trailing dot.
+		if dots[len(dots)-1] == nil {
+			panic(fmt.Sprintf("dots[%d] is nil", len(dots)-1))
+		}
+		children = append(children, dots[len(dots)-1])
+	}
 	return &OptionNameNode{
 		compositeNode: compositeNode{
 			children: children,

diff --git a/parser/parser_test.go b/parser/parser_test.go
@@ -92,12 +92,19 @@ func runParseErrorTestCases(t *testing.T, testCases map[string]parseErrorTestCas
 		name, testCase := name, testCase
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
-			errHandler := reporter.NewHandler(nil)
 			protoName := fmt.Sprintf("%s.proto", name)
-			_, err := Parse(protoName, strings.NewReader(testCase.NoError), errHandler)
+			_, err := Parse(protoName, strings.NewReader(testCase.NoError), reporter.NewHandler(nil))
 			require.NoError(t, err)
-			_, err = Parse(protoName, strings.NewReader(testCase.Error), errHandler)
-			require.ErrorContains(t, err, expected)
+
+			producedError := false
+			errHandler := reporter.NewHandler(reporter.NewReporter(func(err reporter.ErrorWithPos) error {
+				if strings.Contains(err.Error(), expected) {
+					producedError = true
+				}
+				return nil
+			}, nil))
+			_, _ = Parse(protoName, strings.NewReader(testCase.Error), errHandler)
+			require.Truef(t, producedError, "expected error containing %q", expected)
 		})
 	}
 }
@@ -444,6 +451,43 @@ func TestLenientParse_OptionsTrailingComma(t *testing.T) {
 	runParseErrorTestCases(t, inputs, "unexpected ','")
 }
 
+func TestLenientParse_OptionNameTrailingDot(t *testing.T) {
+	t.Parallel()
+	inputs := map[string]parseErrorTestCase{
+		"field-options": {
+			Error: `syntax = "proto3";
+							message Foo {
+								int32 bar = 1 [default.=1];
+							}`,
+			NoError: `syntax = "proto3";
+								message Foo {
+									int32 bar = 1 [default=1];
+								}`,
+		},
+		"field-options-ext": {
+			Error: `syntax = "proto3";
+							message Foo {
+								int32 bar = 1 [(foo).=1];
+							}`,
+			NoError: `syntax = "proto3";
+								message Foo {
+									int32 bar = 1 [(foo)=1];
+								}`,
+		},
+		"field-options-both": {
+			Error: `syntax = "proto3";
+							message Foo {
+								int32 bar = 1 [baz.(foo).=1];
+							}`,
+			NoError: `syntax = "proto3";
+								message Foo {
+									int32 bar = 1 [baz.(foo)=1];
+								}`,
+		},
+	}
+	runParseErrorTestCases(t, inputs, "unexpected '.'")
+}
+
 func TestSimpleParse(t *testing.T) {
 	t.Parallel()
 	protos := map[string]Result{}

diff --git a/parser/proto.y b/parser/proto.y
@@ -47,7 +47,8 @@ import (
 	optRaw       *ast.OptionNode
 	opt          nodeWithRunes[*ast.OptionNode]
 	opts         *compactOptionSlices
-	ref          *ast.FieldReferenceNode
+	refRaw       *ast.FieldReferenceNode
+	ref			     nodeWithRunes[*ast.FieldReferenceNode]
 	optNms       *fieldRefSlices
 	cmpctOpts    *ast.CompactOptionsNode
 	rng          *ast.RangeNode
@@ -81,8 +82,9 @@ import (
 %type <optRaw>       compactOption oneofOptionDecl
 %type <opt>          optionDecl compactOptionEntry compactOptionFinal
 %type <opts>         compactOptionDecls compactOptionLeadingDecls
-%type <ref>          extensionName messageLiteralFieldName
-%type <optNms>       optionName
+%type <refRaw>       extensionName messageLiteralFieldName optionNamePart
+%type <ref>          optionNameEntry optionNameFinal
+%type <optNms>       optionName optionNameLeading
 %type <cmpctOpts>    compactOptions
 %type <v>            value optionValue scalarValue messageLiteralWithBraces messageLiteral numLit listLiteral listElement listOfMessagesLiteral messageValue
 %type <il>           enumValueNumber
@@ -323,21 +325,40 @@ optionDecl : _OPTION optionName '=' optionValue semicolons {
 		$$ = newNodeWithRunes(ast.NewOptionNode($1.ToKeyword(), optName, $3, $4, semi), extra...)
 	}
 
-optionName : identifier {
-		fieldReferenceNode := ast.NewFieldReferenceNode($1)
-		$$ = &fieldRefSlices{refs: []*ast.FieldReferenceNode{fieldReferenceNode}}
+optionNamePart : identifier {
+		$$ = ast.NewFieldReferenceNode($1)
 	}
-	| optionName '.' identifier {
-		$1.refs = append($1.refs, ast.NewFieldReferenceNode($3))
-		$1.dots = append($1.dots, $2)
+	| extensionName {
 		$$ = $1
 	}
-	| extensionName {
-		$$ = &fieldRefSlices{refs: []*ast.FieldReferenceNode{$1}}
+
+optionNameEntry : optionNamePart '.' {
+		$$ = newNodeWithRunes($1, $2)
 	}
-	| optionName '.' extensionName {
-		$1.refs = append($1.refs, $3)
-		$1.dots = append($1.dots, $2)
+
+optionNameFinal : optionNamePart {
+		$$ = newNodeWithRunes($1)
+	}
+	| optionNameEntry {
+		protolex.(*protoLex).Error("unexpected '.'")
+		$$ = $1
+	}
+
+optionNameLeading : optionNameEntry {
+	  $$ = &fieldRefSlices{refs: []*ast.FieldReferenceNode{$1.Node}, dots: $1.Runes}
+  }
+	| optionNameLeading optionNameEntry {
+		$1.refs = append($1.refs, $2.Node)
+		$1.dots = append($1.dots, $2.Runes...)
+		$$ = $1
+	}
+
+optionName : optionNameFinal {
+	  $$ = &fieldRefSlices{refs: []*ast.FieldReferenceNode{$1.Node}, dots: $1.Runes}
+	}
+	| optionNameLeading optionNameFinal {
+		$1.refs = append($1.refs, $2.Node)
+		$1.dots = append($1.dots, $2.Runes...)
 		$$ = $1
 	}