Skip to content

Commit

Permalink
Tolerate trailing '.' in unambiguous type name cases (bufbuild#224)
Browse files Browse the repository at this point in the history 
While still reporting an error. The important case for code completion
is extension type names. See
bufbuild#200
  • Loading branch information
@Alfus @kralicky
Alfus authored and kralicky committed Feb 7, 2024
1 parent 7186508 commit 3e95be1
Show file tree
Hide file tree
Showing 4 changed files with 914 additions and 652 deletions.
9 changes: 7 additions & 2 deletions ast/identifiers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,10 @@ func NewCompoundIdentNode(leadingDot *RuneNode, components []*IdentNode, dots []
if len(components) == 0 {
panic("must have at least one component")
}
if len(dots) != len(components)-1 {
if len(dots) != len(components)-1 && len(dots) != len(components) {
panic(fmt.Sprintf("%d components requires %d dots, not %d", len(components), len(components)-1, len(dots)))
}
numChildren := len(components)*2 - 1
numChildren := len(components) + len(dots)
if leadingDot != nil {
numChildren++
}
Expand All @@ -113,6 +113,11 @@ func NewCompoundIdentNode(leadingDot *RuneNode, components []*IdentNode, dots []
children = append(children, comp)
b.WriteString(comp.Val)
}
if len(dots) == len(components) {
dot := dots[len(dots)-1]
children = append(children, dot)
b.WriteRune(dot.Rune)
}
return &CompoundIdentNode{
compositeNode: compositeNode{
children: children,
Expand Down
106 changes: 105 additions & 1 deletion parser/parser_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,16 @@ func runParseErrorTestCases(t *testing.T, testCases map[string]parseErrorTestCas
require.NoError(t, err)

producedError := false
var errs []error
errHandler := reporter.NewHandler(reporter.NewReporter(func(err reporter.ErrorWithPos) error {
errs = append(errs, err)
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)
require.Truef(t, producedError, "expected error containing %q, got %v", expected, errs)
})
}
}
Expand Down Expand Up @@ -484,6 +486,108 @@ func TestLenientParse_OptionNameTrailingDot(t *testing.T) {
int32 bar = 1 [baz.(foo)=1];
}`,
},
"field-options-type": {
Error: `syntax = "proto3";
message Foo {
int32 bar = 1 [(type.)=1];
}`,
NoError: `syntax = "proto3";
message Foo {
int32 bar = 1 [(type)=1];
}`,
},
"message-options": {
Error: `syntax = "proto3";
message Foo {
option (foo.) = 1;
}`,
NoError: `syntax = "proto3";
message Foo {
option (foo) = 1;
}`,
},
"message-option-field": {
Error: `syntax = "proto3";
message Foo {
option (foo.) = {
[type.]: <a: 1>
};
}`,
NoError: `syntax = "proto3";
message Foo {
option (foo) = {
[type]: <a: 1>
};
}`,
},
"message-option-any-field": {
Error: `syntax = "proto3";
message Foo {
option (foo) = {
[url.com/type.]: <a: 1>
};
}`,
NoError: `syntax = "proto3";
message Foo {
option (foo) = {
[url.com/type]: <a: 1>
};
}`,
},
"message-option-any-field-url": {
Error: `syntax = "proto3";
message Foo {
option (foo) = {
[url.com./type]: <a: 1>
};
}`,
NoError: `syntax = "proto3";
message Foo {
option (foo) = {
[url.com/type]: <a: 1>
};
}`,
},
"map-value-type": {
Error: `syntax = "proto3";
message Foo {
map<string, type.> bar = 1;
}`,
NoError: `syntax = "proto3";
message Foo {
map<string, type> bar = 1;
}`,
},
"extension": {
Error: `syntax = "proto3";
extend Foo. {
int32 bar = 1;
}`,
NoError: `syntax = "proto3";
extend Foo {
int32 bar = 1;
}`,
},
"method-type": {
Error: `syntax = "proto3";
service Foo {
rpc Bar(type.) returns (type);
}`,
NoError: `syntax = "proto3";
service Foo {
rpc Bar(type) returns (type);
}`,
},
"method-stream-type": {
Error: `syntax = "proto3";
service Foo {
rpc Bar(stream type) returns (stream A.B.);
}`,
NoError: `syntax = "proto3";
service Foo {
rpc Bar(stream type) returns (stream A.B);
}`,
},
}
runParseErrorTestCases(t, inputs, "unexpected '.'")
}
Expand Down
75 changes: 64 additions & 11 deletions parser/proto.y
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ import (
rng *ast.RangeNode
rngs *rangeSlices
names *nameSlices
cidPart nodeWithRunes[*ast.IdentNode]
cid *identSlices
tid ast.IdentValueNode
sl *valueSlices
Expand Down Expand Up @@ -89,7 +90,8 @@ import (
%type <v> value optionValue scalarValue messageLiteralWithBraces messageLiteral numLit listLiteral listElement listOfMessagesLiteral messageValue
%type <il> enumValueNumber
%type <id> identifier mapKeyType msgElementName extElementName oneofElementName notGroupElementName mtdElementName enumValueName fieldCardinality
%type <cid> qualifiedIdentifier msgElementIdent extElementIdent oneofElementIdent notGroupElementIdent mtdElementIdent
%type <cidPart> qualifiedIdentifierEntry qualifiedIdentifierFinal mtdElementIdentEntry mtdElementIdentFinal
%type <cid> qualifiedIdentifier msgElementIdent extElementIdent oneofElementIdent notGroupElementIdent mtdElementIdent qualifiedIdentifierDot qualifiedIdentifierLeading mtdElementIdentLeading
%type <tid> typeName msgElementTypeIdent extElementTypeIdent oneofElementTypeIdent notGroupElementTypeIdent mtdElementTypeIdent
%type <sl> listElements messageLiterals
%type <msgLitFlds> messageLiteralFieldEntry messageLiteralFields messageTextFormat
Expand Down Expand Up @@ -265,6 +267,36 @@ qualifiedIdentifier : identifier {
$$ = $1
}

qualifiedIdentifierDot : qualifiedIdentifierFinal {
$$ = &identSlices{idents: []*ast.IdentNode{$1.Node}, dots: $1.Runes}
}
| qualifiedIdentifierLeading qualifiedIdentifierFinal {
$1.idents = append($1.idents, $2.Node)
$1.dots = append($1.dots, $2.Runes...)
$$ = $1
}

qualifiedIdentifierLeading : qualifiedIdentifierEntry {
$$ = &identSlices{idents: []*ast.IdentNode{$1.Node}, dots: $1.Runes}
}
| qualifiedIdentifierLeading qualifiedIdentifierEntry {
$1.idents = append($1.idents, $2.Node)
$1.dots = append($1.dots, $2.Runes...)
$$ = $1
}

qualifiedIdentifierFinal : identifier {
$$ = newNodeWithRunes($1)
}
| qualifiedIdentifierEntry {
protolex.(*protoLex).Error("unexpected '.'")
$$ = $1
}

qualifiedIdentifierEntry : identifier '.' {
$$ = newNodeWithRunes($1, $2)
}

// to mimic limitations of protoc recursive-descent parser,
// we don't allowed message statement keywords as identifiers
// (or oneof statement keywords [e.g. "option"] below)
Expand Down Expand Up @@ -305,15 +337,36 @@ notGroupElementIdent : notGroupElementName {
$$ = $1
}
mtdElementIdent : mtdElementName {
$$ = &identSlices{idents: []*ast.IdentNode{$1}}
mtdElementIdent : mtdElementIdentFinal {
$$ = &identSlices{idents: []*ast.IdentNode{$1.Node}, dots: $1.Runes}
}
| mtdElementIdent '.' identifier {
$1.idents = append($1.idents, $3)
$1.dots = append($1.dots, $2)
| mtdElementIdentLeading mtdElementIdentFinal {
$1.idents = append($1.idents, $2.Node)
$1.dots = append($1.dots, $2.Runes...)
$$ = $1
}
mtdElementIdentLeading : mtdElementIdentEntry {
$$ = &identSlices{idents: []*ast.IdentNode{$1.Node}, dots: $1.Runes}
}
| mtdElementIdentLeading mtdElementIdentEntry {
$1.idents = append($1.idents, $2.Node)
$1.dots = append($1.dots, $2.Runes...)
$$ = $1
}
mtdElementIdentFinal : mtdElementName {
$$ = newNodeWithRunes($1)
}
| mtdElementIdentEntry {
protolex.(*protoLex).Error("unexpected '.'")
$$ = $1
}
mtdElementIdentEntry : mtdElementName '.' {
$$ = newNodeWithRunes($1, $2)
}
oneofOptionDecl : _OPTION optionName '=' optionValue semicolon {
optName := ast.NewOptionNameNode($2.refs, $2.dots)
$$ = ast.NewOptionNode($1.ToKeyword(), optName, $3, $4, $5)
Expand Down Expand Up @@ -492,10 +545,10 @@ messageLiteralField : messageLiteralFieldName ':' value {
messageLiteralFieldName : identifier {
$$ = ast.NewFieldReferenceNode($1)
}
| '[' qualifiedIdentifier ']' {
| '[' qualifiedIdentifierDot ']' {
$$ = ast.NewExtensionFieldReferenceNode($1, $2.toIdentValueNode(nil), $3)
}
| '[' qualifiedIdentifier '/' qualifiedIdentifier ']' {
| '[' qualifiedIdentifierDot '/' qualifiedIdentifierDot ']' {
$$ = ast.NewAnyTypeReferenceNode($1, $2.toIdentValueNode(nil), $3, $4.toIdentValueNode(nil), $5)
}
| '[' error ']' {
Expand Down Expand Up @@ -571,10 +624,10 @@ messageLiterals : messageLiteral {
$$ = $1
}

typeName : qualifiedIdentifier {
typeName : qualifiedIdentifierDot {
$$ = $1.toIdentValueNode(nil)
}
| '.' qualifiedIdentifier {
| '.' qualifiedIdentifierDot {
$$ = $2.toIdentValueNode($1)
}

Expand Down Expand Up @@ -609,7 +662,7 @@ notGroupElementTypeIdent : notGroupElementIdent {
mtdElementTypeIdent : mtdElementIdent {
$$ = $1.toIdentValueNode(nil)
}
| '.' qualifiedIdentifier {
| '.' qualifiedIdentifierDot {
$$ = $2.toIdentValueNode($1)
}

Expand Down
Loading

0 comments on commit 3e95be1

Please sign in to comment.