Skip to content

Commit

Permalink
Tolerate missing semicolons in the service body (bufbuild#206)
Browse files Browse the repository at this point in the history
While still producing an error, allowing the rpc or option ast nodes to
still be generated. See
bufbuild#200

Will follow up with diffs of a similar pattern for the other body
contexts.
  • Loading branch information
Alfus authored and kralicky committed Feb 7, 2024
1 parent 2a0d7e7 commit b8ca64e
Show file tree
Hide file tree
Showing 9 changed files with 787 additions and 645 deletions.
6 changes: 4 additions & 2 deletions ast/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,12 @@ func NewSyntaxNode(keyword *KeywordNode, equals *RuneNode, syntax StringValueNod
if syntax == nil {
panic("syntax is nil")
}
var children []Node
if semicolon == nil {
panic("semicolon is nil")
children = []Node{keyword, equals, syntax}
} else {
children = []Node{keyword, equals, syntax, semicolon}
}
children := []Node{keyword, equals, syntax, semicolon}
return &SyntaxNode{
compositeNode: compositeNode{
children: children,
Expand Down
7 changes: 5 additions & 2 deletions ast/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,13 @@ func NewOptionNode(keyword *KeywordNode, name *OptionNameNode, equals *RuneNode,
if val == nil {
panic("val is nil")
}
var children []Node
if semicolon == nil {
panic("semicolon is nil")
children = []Node{keyword, name, equals, val}
} else {
children = []Node{keyword, name, equals, val, semicolon}
}
children := []Node{keyword, name, equals, val, semicolon}

return &OptionNode{
compositeNode: compositeNode{
children: children,
Expand Down
6 changes: 4 additions & 2 deletions ast/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,12 @@ func NewRPCNode(keyword *KeywordNode, name *IdentNode, input *RPCTypeNode, retur
if output == nil {
panic("output is nil")
}
var children []Node
if semicolon == nil {
panic("semicolon is nil")
children = []Node{keyword, name, input, returns, output}
} else {
children = []Node{keyword, name, input, returns, output, semicolon}
}
children := []Node{keyword, name, input, returns, output, semicolon}
return &RPCNode{
compositeNode: compositeNode{
children: children,
Expand Down
38 changes: 38 additions & 0 deletions parser/ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,3 +90,41 @@ func (list *messageFieldList) toNodes() ([]*ast.MessageFieldNode, []*ast.RuneNod
}
return fields, delimiters
}

func newEmptyDeclNodes(semicolons []*ast.RuneNode) []*ast.EmptyDeclNode {
emptyDecls := make([]*ast.EmptyDeclNode, len(semicolons))
for i, semicolon := range semicolons {
emptyDecls[i] = ast.NewEmptyDeclNode(semicolon)
}
return emptyDecls
}

func newServiceElements(semicolons []*ast.RuneNode, elements []ast.ServiceElement) []ast.ServiceElement {
elems := make([]ast.ServiceElement, 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
}

func newNodeWithEmptyDecls[T ast.Node](node T, extraSemicolons []*ast.RuneNode) nodeWithEmptyDecls[T] {
return nodeWithEmptyDecls[T]{
Node: node,
EmptyDecls: newEmptyDeclNodes(extraSemicolons),
}
}

func toServiceElements[T ast.ServiceElement](nodes nodeWithEmptyDecls[T]) []ast.ServiceElement {
serviceElements := make([]ast.ServiceElement, 1+len(nodes.EmptyDecls))
serviceElements[0] = nodes.Node
for i, emptyDecl := range nodes.EmptyDecls {
serviceElements[i+1] = emptyDecl
}
return serviceElements
}
8 changes: 8 additions & 0 deletions parser/lexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -761,3 +761,11 @@ func (l *protoLex) errWithCurrentPos(err error, offset int) reporter.ErrorWithPo
pos := l.info.SourcePos(l.input.offset() + offset)
return reporter.Error(ast.NewSourceSpan(pos, pos), err)
}

func (l *protoLex) requireSemicolon(semicolons []*ast.RuneNode) (*ast.RuneNode, []*ast.RuneNode) {
if len(semicolons) == 0 {
l.Error("expected ';'")
return nil, nil
}
return semicolons[0], semicolons[1:]
}
45 changes: 45 additions & 0 deletions parser/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,51 @@ func TestJunkParse(t *testing.T) {
}
}

func TestLenientParse_SemicolonLess(t *testing.T) {
t.Parallel()
inputs := map[string]struct {
Error string
NoError string
}{
"method": {
Error: `syntax = "proto3";
service Foo {
;
rpc Bar (Baz) returns (Qux)
rpc Qux (Baz) returns (Qux);;
}`,
NoError: `syntax = "proto3";
service Foo {
;
rpc Bar (Baz) returns (Qux);
rpc Qux (Baz) returns (Qux);;
}`,
},
"service-options": {
Error: `syntax = "proto3";
service Foo {
option (foo) = { bar: 1 }
}`,
NoError: `syntax = "proto3";
service Foo {
option (foo) = { bar: 1 };
}`,
},
}
for name, input := range inputs {
name, input := name, input
t.Run(name, func(t *testing.T) {
t.Parallel()
errHandler := reporter.NewHandler(nil)
protoName := fmt.Sprintf("%s.proto", name)
_, err := Parse(protoName, strings.NewReader(input.NoError), errHandler)
require.NoError(t, err)
_, err = Parse(protoName, strings.NewReader(input.Error), errHandler)
require.ErrorContains(t, err, "expected ';'")
})
}
}

func TestSimpleParse(t *testing.T) {
t.Parallel()
protos := map[string]Result{}
Expand Down
67 changes: 41 additions & 26 deletions parser/proto.y
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,12 @@ import (
svc *ast.ServiceNode
svcElement ast.ServiceElement
svcElements []ast.ServiceElement
mtd *ast.RPCNode
mtd nodeWithEmptyDecls[*ast.RPCNode]
mtdMsgType *ast.RPCTypeNode
mtdElement ast.RPCElement
mtdElements []ast.RPCElement
opt *ast.OptionNode
optN nodeWithEmptyDecls[*ast.OptionNode]
opts *compactOptionSlices
ref *ast.FieldReferenceNode
optNms *fieldRefSlices
Expand All @@ -68,6 +69,7 @@ import (
f *ast.FloatLiteralNode
id *ast.IdentNode
b *ast.RuneNode
bs []*ast.RuneNode
err error
}

Expand All @@ -81,6 +83,7 @@ import (
%type <imprt> importDecl
%type <pkg> packageDecl
%type <opt> optionDecl compactOption
%type <optN> optionDeclWithEmptyDecls
%type <opts> compactOptionDecls
%type <ref> extensionName messageLiteralFieldName
%type <optNms> optionName
Expand Down Expand Up @@ -117,12 +120,12 @@ import (
%type <extElements> extensionElements extensionBody
%type <str> stringLit
%type <svc> serviceDecl
%type <svcElement> serviceElement
%type <svcElements> serviceElements serviceBody
%type <svcElements> serviceElement serviceElements serviceBody
%type <mtd> methodDecl
%type <mtdElement> methodElement
%type <mtdElements> methodElements methodBody
%type <mtdMsgType> methodMessageType
%type <bs> semicolons semicolonList

// same for terminals
%token <s> _STRING_LIT
Expand Down Expand Up @@ -214,6 +217,20 @@ fileElement : importDecl {
$$ = nil
}

semicolonList : ';' {
$$ = []*ast.RuneNode{$1}
}
| semicolonList ';' {
$$ = append($1, $2)
}

semicolons : semicolonList {
$$ = $1
}
| {
$$ = nil
}

syntaxDecl : _SYNTAX '=' stringLit ';' {
$$ = ast.NewSyntaxNode($1.ToKeyword(), $2, toStringValueNode($3), $4)
}
Expand Down Expand Up @@ -299,6 +316,12 @@ optionDecl : _OPTION optionName '=' optionValue ';' {
$$ = ast.NewOptionNode($1.ToKeyword(), optName, $3, $4, $5)
}

optionDeclWithEmptyDecls : _OPTION optionName '=' optionValue semicolons {
optName := ast.NewOptionNameNode($2.refs, $2.dots)
semi, extra := protolex.(*protoLex).requireSemicolon($5)
$$ = newNodeWithEmptyDecls(ast.NewOptionNode($1.ToKeyword(), optName, $3, $4, semi), extra)
}

optionName : identifier {
fieldReferenceNode := ast.NewFieldReferenceNode($1)
$$ = &fieldRefSlices{refs: []*ast.FieldReferenceNode{fieldReferenceNode}}
Expand Down Expand Up @@ -937,47 +960,39 @@ serviceDecl : _SERVICE identifier '{' serviceBody '}' {
$$ = ast.NewServiceNode($1.ToKeyword(), $2, $3, $4, $5)
}

serviceBody : {
$$ = nil
serviceBody : semicolons {
$$ = newServiceElements($1, nil)
}
| semicolons serviceElements {
$$ = newServiceElements($1, $2)
}
| serviceElements

serviceElements : serviceElements serviceElement {
if $2 != nil {
$$ = append($1, $2)
} else {
$$ = $1
}
$$ = append($1, $2...)
}
| serviceElement {
if $1 != nil {
$$ = []ast.ServiceElement{$1}
} else {
$$ = nil
}
$$ = $1
}

// NB: doc suggests support for "stream" declaration, separate from "rpc", but
// it does not appear to be supported in protoc (doc is likely from grammar for
// Google-internal version of protoc, with support for streaming stubby)
serviceElement : optionDecl {
$$ = $1
serviceElement : optionDeclWithEmptyDecls {
$$ = toServiceElements($1)
}
| methodDecl {
$$ = $1
}
| ';' {
$$ = ast.NewEmptyDeclNode($1)
$$ = toServiceElements($1)
}
| error {
$$ = nil
}

methodDecl : _RPC identifier methodMessageType _RETURNS methodMessageType ';' {
$$ = ast.NewRPCNode($1.ToKeyword(), $2, $3, $4.ToKeyword(), $5, $6)
methodDecl : _RPC identifier methodMessageType _RETURNS methodMessageType semicolons {
semi, extra := protolex.(*protoLex).requireSemicolon($6)
$$ = newNodeWithEmptyDecls(ast.NewRPCNode($1.ToKeyword(), $2, $3, $4.ToKeyword(), $5, semi), extra)
}
| _RPC identifier methodMessageType _RETURNS methodMessageType '{' methodBody '}' {
$$ = ast.NewRPCNodeWithBody($1.ToKeyword(), $2, $3, $4.ToKeyword(), $5, $6, $7, $8)
| _RPC identifier methodMessageType _RETURNS methodMessageType '{' methodBody '}' semicolons {
$$ = newNodeWithEmptyDecls(ast.NewRPCNodeWithBody($1.ToKeyword(), $2, $3, $4.ToKeyword(), $5, $6, $7, $8), $9)
}

methodMessageType : '(' _STREAM typeName ')' {
Expand Down
Loading

0 comments on commit b8ca64e

Please sign in to comment.