Skip to content

Commit

Permalink
Tolerate trailing '.' in option names (bufbuild#222)
Browse files Browse the repository at this point in the history
While still producing an error, see:
bufbuild#200
  • Loading branch information
Alfus authored and kralicky committed Feb 7, 2024
1 parent 7dcb3dc commit 7186508
Show file tree
Hide file tree
Showing 4 changed files with 764 additions and 657 deletions.
10 changes: 8 additions & 2 deletions ast/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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,
Expand Down
52 changes: 48 additions & 4 deletions parser/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}
Expand Down Expand Up @@ -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{}
Expand Down
49 changes: 35 additions & 14 deletions parser/proto.y
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
Loading

0 comments on commit 7186508

Please sign in to comment.