Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use structure member parsing for lists and maps #1782

Merged
merged 10 commits into from
May 24, 2023
Merged

Use structure member parsing for lists and maps #1782

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a36a67c
Use structure member parsing for lists and maps
milesziemer May 18, 2023
69c6d61
merge aggregate shapes grammar
milesziemer May 16, 2023
584502b
Use shape type name in error message
milesziemer May 23, 2023
2350d81
Use single method for map/list/set parsing
milesziemer May 23, 2023
3d0ef43
emit both wrong and missing member errors
milesziemer May 23, 2023
6ee7914
Remove extra shape id checks in list/map ctor
milesziemer May 23, 2023
51d766f
Emit error for all missing members
milesziemer May 24, 2023
47036f1
Add test cases for invalid list/map mixins
milesziemer May 24, 2023
093a8ac
Use array to return required members
milesziemer May 24, 2023
9777699
Change to AggregateTypeName grammar production
milesziemer May 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 13 additions & 29 deletions docs/source-2.0/spec/idl.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,10 +177,8 @@ string support defined in :rfc:`7405`.
ShapeStatement :`TraitStatements` `ShapeBody`
ShapeBody :`SimpleShapeStatement`
:/ `EnumStatement`
:/ `ListStatement`
:/ `MapStatement`
:/ `AggregateShapeStatement`
:/ `StructureStatement`
:/ `UnionStatement`
:/ `ServiceStatement`
:/ `OperationStatement`
:/ `ResourceStatement`
Expand All @@ -193,30 +191,16 @@ string support defined in :rfc:`7405`.
EnumStatement :`EnumTypeName` `SP` `Identifier` [`Mixins`] [`WS`] `EnumShapeMembers`
EnumTypeName :%s"enum" / %s"intEnum"
EnumShapeMembers :"{" [`WS`] 1*(`TraitStatements` `Identifier` [`ValueAssignment`] [`WS`]) "}"
ValueAssignment :[`SP`] "=" [`SP`] `NodeValue` [`SP`] [`Comma`] `BR`
ListStatement :%s"list" `SP` `Identifier` [`Mixins`] [`WS`] `ListMembers`
ListMembers :"{" [`WS`] [`ListMember`] [`WS`] "}"
ListMember :`TraitStatements` (`ElidedListMember` / `ExplicitListMember`)
ElidedListMember :%s"$member"
ExplicitListMember :%s"member" [`SP`] ":" [`SP`] `ShapeId`
MapStatement :%s"map" `SP` `Identifier` [`Mixins`] [`WS`] `MapMembers`
MapMembers :"{" [`WS`] [`MapKey` / `MapValue` / (`MapKey` `WS` `MapValue`)] [`WS`] "}"
MapKey :`TraitStatements` (`ElidedMapKey` / `ExplicitMapKey`)
MapValue :`TraitStatements` (`ElidedMapValue` / `ExplicitMapValue`)
ElidedMapKey :%s"$key"
ExplicitMapKey :%s"key" [`SP`] ":" [`SP`] `ShapeId`
ElidedMapValue :%s"$value"
ExplicitMapValue :%s"value" [`SP`] ":" [`SP`] `ShapeId`
ValueAssignment :[`SP`] "=" [`SP`] `NodeValue` `BR`
AggregateShapeStatement :`AggregateTypeName` `SP` `Identifier` [`Mixins`] `ShapeMembers`
AggregateTypeName :%s"list" / %s"map" / %s"union"
StructureStatement :%s"structure" `SP` `Identifier` [`StructureResource`]
: [`Mixins`] [`WS`] `StructureMembers`
: [`Mixins`] [`WS`] `ShapeMembers`
StructureResource :`SP` %s"for" `SP` `ShapeId`
StructureMembers :"{" [`WS`] *(`TraitStatements` `StructureMember` [`WS`]) "}"
StructureMember :(`ExplicitStructureMember` / `ElidedStructureMember`) [`ValueAssignment`]
ExplicitStructureMember :`Identifier` [`SP`] ":" [`SP`] `ShapeId`
ElidedStructureMember :"$" `Identifier`
UnionStatement :%s"union" `SP` `Identifier` [`Mixins`] [`WS`] `UnionMembers`
UnionMembers :"{" [`WS`] *(`TraitStatements` `UnionMember` [`WS`]) "}"
UnionMember :(`ExplicitStructureMember` / `ElidedStructureMember`)
ShapeMembers :"{" [`WS`] *(`TraitStatements` `ShapeMember` [`WS`]) "}"
ShapeMember :(`ExplicitShapeMember` / `ElidedShapeMember`) [`ValueAssignment`]
ExplicitShapeMember :`Identifier` [`SP`] ":" [`SP`] `ShapeId`
ElidedShapeMember :"$" `Identifier`
ServiceStatement :%s"service" `SP` `Identifier` [`Mixins`] [`WS`] `NodeObject`
ResourceStatement :%s"resource" `SP` `Identifier` [`Mixins`] [`WS`] `NodeObject`
OperationStatement :%s"operation" `SP` `Identifier` [`Mixins`] [`WS`] `OperationBody`
Expand All @@ -228,7 +212,7 @@ string support defined in :rfc:`7405`.
OperationOutput :%s"output" [`WS`] (`InlineStructure` / (":" [`WS`] `ShapeId`))
OperationErrors :%s"errors" [`WS`] ":" [`WS`] "[" [`WS`] *(`ShapeId` [`WS`]) "]"
InlineStructure :":=" [`WS`] `TraitStatements` [`StructureResource`]
: [`Mixins`] [`WS`] `StructureMembers`
: [`Mixins`] [`WS`] `ShapeMembers`
.. rubric:: Traits

Expand Down Expand Up @@ -896,7 +880,7 @@ The above intEnum is exactly equivalent to the following intEnum:
List shapes
-----------

A :ref:`list <list>` shape is defined using a :token:`smithy:ListStatement`.
A :ref:`list <list>` shape is defined using a :token:`smithy:AggregateShapeStatement`.

The following example defines a list with a string member from the
:ref:`prelude <prelude>`:
Expand Down Expand Up @@ -977,7 +961,7 @@ Traits can be applied to the list shape and its member:
Map shapes
----------

A :ref:`map <map>` shape is defined using a :token:`smithy:MapStatement`.
A :ref:`map <map>` shape is defined using a :token:`smithy:AggregateShapeStatement`.

The following example defines a map of strings to integers:

Expand Down Expand Up @@ -1187,7 +1171,7 @@ Is exactly equivalent to:
Union shapes
------------

A :ref:`union <union>` shape is defined using a :token:`smithy:UnionStatement`.
A :ref:`union <union>` shape is defined using a :token:`smithy:AggregateShapeStatement`.

The following example defines a union shape with several members:

Expand Down
134 changes: 14 additions & 120 deletions smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlModelLoader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,12 @@
import software.amazon.smithy.model.node.ObjectNode;
import software.amazon.smithy.model.node.StringNode;
import software.amazon.smithy.model.shapes.AbstractShapeBuilder;
import software.amazon.smithy.model.shapes.CollectionShape;
import software.amazon.smithy.model.shapes.EnumShape;
import software.amazon.smithy.model.shapes.IntEnumShape;
import software.amazon.smithy.model.shapes.ListShape;
import software.amazon.smithy.model.shapes.MapShape;
import software.amazon.smithy.model.shapes.MemberShape;
import software.amazon.smithy.model.shapes.OperationShape;
import software.amazon.smithy.model.shapes.ResourceShape;
import software.amazon.smithy.model.shapes.ServiceShape;
import software.amazon.smithy.model.shapes.SetShape;
import software.amazon.smithy.model.shapes.ShapeId;
import software.amazon.smithy.model.shapes.ShapeType;
import software.amazon.smithy.model.shapes.StructureShape;
Expand All @@ -52,7 +48,6 @@
import software.amazon.smithy.model.traits.EnumValueTrait;
import software.amazon.smithy.model.traits.InputTrait;
import software.amazon.smithy.model.traits.OutputTrait;
import software.amazon.smithy.model.traits.Trait;
import software.amazon.smithy.model.traits.UnitTypeTrait;
import software.amazon.smithy.model.validation.Severity;
import software.amazon.smithy.model.validation.ValidationEvent;
Expand Down Expand Up @@ -561,13 +556,9 @@ private void parseShapeOrApply(List<IdlTraitParser.Result> traits) {
parseSimpleShape(id, location, type.createBuilderForType());
break;
case LIST:
parseCollection(id, location, ListShape.builder());
break;
case SET:
parseCollection(id, location, SetShape.builder());
break;
case MAP:
parseMapStatement(id, location);
parseAggregateShape(id, location, type.createBuilderForType());
break;
case ENUM:
parseEnumShape(id, location, EnumShape.builder());
Expand All @@ -576,10 +567,10 @@ private void parseShapeOrApply(List<IdlTraitParser.Result> traits) {
parseEnumShape(id, location, IntEnumShape.builder());
break;
case STRUCTURE:
parseStructuredShape(id, location, StructureShape.builder(), MemberParsing.PARSING_STRUCTURE_MEMBER);
parseStructuredShape(id, location, StructureShape.builder());
break;
case UNION:
parseStructuredShape(id, location, UnionShape.builder(), MemberParsing.PARSING_MEMBER);
parseStructuredShape(id, location, UnionShape.builder());
break;
case SERVICE:
parseServiceStatement(id, location);
Expand Down Expand Up @@ -665,64 +656,6 @@ private void parseMixins(LoadOperation.DefineShape operation) {
tokenizer.next();
}

// See parseMap for information on why members are parsed before the list/set is registered with the ModelFile.
private void parseCollection(ShapeId id, SourceLocation location, CollectionShape.Builder<?, ?> builder) {
LoadOperation.DefineShape operation = createShape(builder.id(id).source(location));
parseMixins(operation);
tokenizer.skipWsAndDocs();
tokenizer.expect(IdlToken.LBRACE);
tokenizer.next();
tokenizer.skipWs();
parsePossiblyElidedMember(operation, "member");
tokenizer.skipWsAndDocs();
tokenizer.expect(IdlToken.RBRACE);
tokenizer.next();
operations.accept(operation);
}

// Parsed list, set, and map members.
private void parsePossiblyElidedMember(LoadOperation.DefineShape operation, String memberName) {
boolean isElided = false;
List<IdlTraitParser.Result> memberTraits = IdlTraitParser.parseDocsAndTraitsBeforeShape(tokenizer, resolver);
SourceLocation location = tokenizer.getCurrentTokenLocation();

if (tokenizer.getCurrentToken() == IdlToken.DOLLAR) {
isElided = true;
if (!modelVersion.supportsTargetElision()) {
throw syntax(operation.toShapeId().withMember(memberName),
"Members can only elide targets in IDL version 2 or later");
}
tokenizer.next();
tokenizer.expect(IdlToken.IDENTIFIER);
} else {
if (!tokenizer.doesCurrentIdentifierStartWith(memberName.charAt(0))) {
if (!memberTraits.isEmpty()) {
throw syntax("Expected member definition to follow traits");
}
return;
}
}

MemberShape.Builder memberBuilder = MemberShape.builder()
.id(operation.toShapeId().withMember(memberName))
.source(location);

tokenizer.expectCurrentLexeme(memberName);
tokenizer.next();

if (!isElided) {
tokenizer.skipWsAndDocs();
tokenizer.expect(IdlToken.COLON);
tokenizer.next();
tokenizer.skipWsAndDocs();
String id = tokenizer.internString(IdlShapeIdParser.expectAndSkipShapeId(tokenizer));
addForwardReference(id, memberBuilder::target);
}

operation.addMember(memberBuilder);
addTraits(memberBuilder.getId(), memberTraits);
}

private void parseEnumShape(ShapeId id, SourceLocation location, AbstractShapeBuilder<?, ?> builder) {
LoadOperation.DefineShape operation = createShape(builder.id(id).source(location));
parseMixins(operation);
Expand Down Expand Up @@ -765,27 +698,17 @@ private void parseEnumShape(ShapeId id, SourceLocation location, AbstractShapeBu
operations.accept(operation);
}

private void parseMapStatement(ShapeId id, SourceLocation location) {
LoadOperation.DefineShape operation = createShape(MapShape.builder().id(id).source(location));
private void parseAggregateShape(ShapeId id, SourceLocation location, AbstractShapeBuilder<?, ?> builder) {
LoadOperation.DefineShape operation = createShape(builder.id(id).source(location));
parseMixins(operation);
tokenizer.skipWsAndDocs();
tokenizer.expect(IdlToken.LBRACE);
tokenizer.next();
tokenizer.skipWs();
parsePossiblyElidedMember(operation, "key");
tokenizer.skipWs();
parsePossiblyElidedMember(operation, "value");
tokenizer.skipWsAndDocs();
tokenizer.expect(IdlToken.RBRACE);
tokenizer.next();
parseMembers(operation);
operations.accept(operation);
}

private void parseStructuredShape(
ShapeId id,
SourceLocation location,
AbstractShapeBuilder<?, ?> builder,
MemberParsing memberParsing
AbstractShapeBuilder<?, ?> builder
) {
LoadOperation.DefineShape operation = createShape(builder.id(id).source(location));

Expand All @@ -797,40 +720,11 @@ private void parseStructuredShape(

// Parse optional "with" statements to add mixins, but only if it's supported by the version.
parseMixins(operation);
parseMembers(operation, memberParsing);
parseMembers(operation);
operations.accept(operation);
}

private enum MemberParsing {
PARSING_STRUCTURE_MEMBER {
@Override
boolean supportsAssignment() {
return true;
}

@Override
Trait createAssignmentTrait(ShapeId id, Node value) {
return new DefaultTrait(value);
}
},
PARSING_MEMBER {
@Override
boolean supportsAssignment() {
return false;
}

@Override
Trait createAssignmentTrait(ShapeId id, Node value) {
throw new UnsupportedOperationException();
}
};

abstract boolean supportsAssignment();

abstract Trait createAssignmentTrait(ShapeId id, Node value);
}

private void parseMembers(LoadOperation.DefineShape op, MemberParsing memberParsing) {
private void parseMembers(LoadOperation.DefineShape op) {
Set<String> definedMembers = new HashSet<>();

tokenizer.skipWsAndDocs();
Expand All @@ -839,15 +733,15 @@ private void parseMembers(LoadOperation.DefineShape op, MemberParsing memberPars
tokenizer.skipWs();

while (tokenizer.hasNext() && tokenizer.getCurrentToken() != IdlToken.RBRACE) {
parseMember(op, definedMembers, memberParsing);
parseMember(op, definedMembers);
tokenizer.skipWs();
}

tokenizer.expect(IdlToken.RBRACE);
tokenizer.next();
}

private void parseMember(LoadOperation.DefineShape operation, Set<String> defined, MemberParsing memberParsing) {
private void parseMember(LoadOperation.DefineShape operation, Set<String> defined) {
ShapeId parent = operation.toShapeId();

// Parse optional member traits.
Expand Down Expand Up @@ -905,15 +799,15 @@ private void parseMember(LoadOperation.DefineShape operation, Set<String> define
// Skip spaces to check if there is default trait sugar.
tokenizer.skipSpaces();

if (memberParsing.supportsAssignment() && tokenizer.getCurrentToken() == IdlToken.EQUAL) {
if (tokenizer.getCurrentToken() == IdlToken.EQUAL) {
if (!modelVersion.isDefaultSupported()) {
throw syntax("@default assignment is only supported in IDL version 2 or later");
}
tokenizer.expect(IdlToken.EQUAL);
tokenizer.next();
tokenizer.skipSpaces();
Node node = IdlNodeParser.expectAndSkipNode(tokenizer, resolver);
memberBuilder.addTrait(memberParsing.createAssignmentTrait(memberId, node));
memberBuilder.addTrait(new DefaultTrait(node));
tokenizer.expectAndSkipBr();
}

Expand Down Expand Up @@ -1108,7 +1002,7 @@ private ShapeId parseInlineStructure(String name, IdlTraitParser.Result defaultT
LoadOperation.DefineShape operation = createShape(builder);
parseMixins(operation);
parseForResource(operation);
parseMembers(operation, MemberParsing.PARSING_STRUCTURE_MEMBER);
parseMembers(operation);
addTraits(id, traits);
operations.accept(operation);
return id;
Expand Down
8 changes: 7 additions & 1 deletion smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderShapeMap.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,13 @@ private Shape buildShape(
}
MemberShape member = buildMember(memberBuilder);
if (member != null) {
builder.addMember(member);
// Adding a member may throw, but we want to continue execution, so we collect all
// errors that occur.
try {
builder.addMember(member);
} catch (SourceException e) {
events.add(ValidationEvent.fromSourceException(e, "", builder.getId()));
}
}
}

Expand Down
28 changes: 20 additions & 8 deletions smithy-model/src/main/java/software/amazon/smithy/model/shapes/CollectionShape.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@
import java.util.Objects;
import java.util.Optional;
import java.util.function.Consumer;
import software.amazon.smithy.model.SourceException;
import software.amazon.smithy.model.SourceLocation;
import software.amazon.smithy.model.traits.Trait;
import software.amazon.smithy.utils.StringUtils;

/**
* Abstract class representing Set and List shapes.
Expand All @@ -34,14 +36,9 @@ public abstract class CollectionShape extends Shape {

CollectionShape(Builder<?, ?> builder) {
super(builder, false);
member = getRequiredMixinMember(builder, builder.member, "member");

ShapeId expected = getId().withMember("member");
if (!member.getId().equals(expected)) {
throw new IllegalArgumentException(String.format(
"Expected member of `%s` to have an ID of `%s` but found `%s`",
getId(), expected, member.getId()));
}
MemberShape[] members = getRequiredMembers(builder, "member");
member = members[0];
validateMemberShapeIds();
}

/**
Expand Down Expand Up @@ -81,6 +78,15 @@ public abstract static class Builder<B extends Builder<B, S>, S extends Collecti

private MemberShape member;

@Override
public Optional<MemberShape> getMember(String memberName) {
if ("member".equals(memberName)) {
return Optional.ofNullable(member);
} else {
return Optional.empty();
}
}

@Override
public B id(ShapeId shapeId) {
if (member != null) {
Expand All @@ -97,6 +103,12 @@ public B id(ShapeId shapeId) {
*/
@SuppressWarnings("unchecked")
public B member(MemberShape member) {
if (member != null && !member.getMemberName().equals("member")) {
String shapeTypeName = StringUtils.capitalize(this.getShapeType().toString());
String message = String.format("%s shapes may only have a `member` member, but found `%s`",
shapeTypeName, member.getMemberName());
throw new SourceException(message, member);
}
this.member = Objects.requireNonNull(member);
return (B) this;
}
Expand Down
Loading