Skip to content

Commit

Permalink
Throw syntax exceptions on duplicate bindings
Browse files Browse the repository at this point in the history
This commit updates the Smithy IDL parsing to throw when duplicate
bindings or members are found in the following declarations: service,
resource, and operation shapes, applied structure traits, and applied
traits with members that are structures. This behavior is unsafe for
model writers and shouldn't be valid.
  • Loading branch information
kstich committed Feb 18, 2021
1 parent 8f548cb commit 33050df
Show file tree
Hide file tree
Showing 13 changed files with 62 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ private void parseStructuredShape(ShapeId id, SourceLocation location, AbstractS
private void parseOperationStatement(ShapeId id, SourceLocation location) {
ws();
OperationShape.Builder builder = OperationShape.builder().id(id).source(location);
ObjectNode node = IdlNodeParser.parseObjectNode(this);
ObjectNode node = IdlNodeParser.parseObjectNode(this, id.toString());
LoaderUtils.checkForAdditionalProperties(node, id, OPERATION_PROPERTY_NAMES, modelFile.events());
modelFile.onShape(builder);
optionalId(node, "input", builder::input);
Expand All @@ -553,7 +553,7 @@ private void parseOperationStatement(ShapeId id, SourceLocation location) {
private void parseServiceStatement(ShapeId id, SourceLocation location) {
ws();
ServiceShape.Builder builder = new ServiceShape.Builder().id(id).source(location);
ObjectNode shapeNode = IdlNodeParser.parseObjectNode(this);
ObjectNode shapeNode = IdlNodeParser.parseObjectNode(this, id.toString());
LoaderUtils.checkForAdditionalProperties(shapeNode, id, SERVICE_PROPERTY_NAMES, modelFile.events());
builder.version(shapeNode.expectStringMember(VERSION_KEY).getValue());
modelFile.onShape(builder);
Expand All @@ -580,7 +580,7 @@ private void parseResourceStatement(ShapeId id, SourceLocation location) {
ws();
ResourceShape.Builder builder = ResourceShape.builder().id(id).source(location);
modelFile.onShape(builder);
ObjectNode shapeNode = IdlNodeParser.parseObjectNode(this);
ObjectNode shapeNode = IdlNodeParser.parseObjectNode(this, id.toString());

LoaderUtils.checkForAdditionalProperties(shapeNode, id, RESOURCE_PROPERTY_NAMES, modelFile.events());
optionalId(shapeNode, PUT_KEY, builder::put);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ static Node parseNode(IdlModelParser parser) {
char c = parser.peek();
switch (c) {
case '{':
return parseObjectNode(parser);
return parseObjectNode(parser, "object node");
case '[':
return parseArrayNode(parser);
case '"': {
Expand Down Expand Up @@ -119,7 +119,7 @@ static Node parseTextBlock(IdlModelParser parser) {
return new StringNode(IdlTextParser.parseQuotedTextAndTextBlock(parser, true), location);
}

static ObjectNode parseObjectNode(IdlModelParser parser) {
static ObjectNode parseObjectNode(IdlModelParser parser, String parent) {
parser.increaseNestingLevel();
SourceLocation location = parser.currentLocation();
Map<StringNode, Node> entries = new LinkedHashMap<>();
Expand All @@ -137,7 +137,11 @@ static ObjectNode parseObjectNode(IdlModelParser parser) {
parser.expect(':');
parser.ws();
Node value = parseNode(parser);
entries.put(new StringNode(key, keyLocation), value);
StringNode keyNode = new StringNode(key, keyLocation);
Node previous = entries.put(keyNode, value);
if (previous != null) {
throw parser.syntax("Duplicate member of " + parent + ": '" + keyNode.getValue() + '\'');
}
parser.ws();
if (parser.peek() == ',') {
parser.skip();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ private static Node parseTraitValueBodyIdentifierOrQuotedString(
private static ObjectNode parseStructuredTrait(IdlModelParser parser, StringNode startingKey) {
Map<StringNode, Node> entries = new LinkedHashMap<>();
Node firstValue = IdlNodeParser.parseNode(parser);
// This put call can be done safely without checking for duplicates,
// as it's always the first member of the trait.
entries.put(startingKey, firstValue);
parser.ws();

Expand Down Expand Up @@ -150,6 +152,9 @@ private static void parseTraitStructureKvp(IdlModelParser parser, Map<StringNode
parser.ws();
Node nextValue = IdlNodeParser.parseNode(parser);
parser.ws();
entries.put(nextKey, nextValue);
Node previous = entries.put(nextKey, nextValue);
if (previous != null) {
throw parser.syntax("Duplicate member of trait: '" + nextKey.getValue() + '\'');
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[ERROR] -: Parse error at line 5, column 23 near `, | Model
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
namespace com.foo

operation GetFoo {
input: GetFooInput,
input: GetFooInput,
}

structure GetFooInput {}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[ERROR] -: Parse error at line 8, column 22 near `, | Model
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
namespace com.foo

resource Foo {
identifiers: {
id: String,
},
create: createFoo,
create: createFoo,
}

operation createFoo {}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[ERROR] -: Parse error at line 5, column 26 near `, | Model
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
namespace com.foo

service Foo {
version: "2020-07-02",
version: "2020-07-02",
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[ERROR] -: Parse error at line 5, column 19 near `, | Model
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
namespace com.foo

@deprecated(
message: "Foo",
message: "Foo",
)
structure Foo {}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[ERROR] -: Parse error at line 6, column 21 near `, | Model
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
namespace com.foo

@enum([
{
value: "foo",
value: "foo",
},
])
string Foo

0 comments on commit 33050df

Please sign in to comment.