Skip to content

Commit

Permalink
Merge aggregate, structure shape grammar/parsing (#1790)
Browse files Browse the repository at this point in the history
This commit combines aggregate shapes (list, map, union) and
structure shapes in the grammar and parser. Previously, the way
these shapes were defined only differed by aggregate shapes not
allowing `for <resource>` syntax. With this change, aggregate
shapes can now use `for <resource>` syntax and elide resource
identifiers/properties.

The parser was also updated so parsing inline structures uses
the same method of parsing aggregate shapes.

Tests making sure `for <resource>` couldn't be used with non
structure shapes were removed, and tests were added for verifying
`for <resource>` behavior with lists, maps, and unions. The tests
added for list/map/union elided members in elided-members.smithy
mirror the tests for structure shapes in the same file.

This also updates the naming of shape productions in the grammar.
  • Loading branch information
milesziemer authored May 25, 2023
1 parent ead65d6 commit 4784858
Show file tree
Hide file tree
Showing 7 changed files with 365 additions and 75 deletions.
52 changes: 22 additions & 30 deletions docs/source-2.0/spec/idl.rst
Original file line number Diff line number Diff line change
Expand Up @@ -174,44 +174,36 @@ string support defined in :rfc:`7405`.
UseStatement :%s"use" `SP` `AbsoluteRootShapeId` `BR`
ShapeStatements :`ShapeOrApplyStatement` *(`BR` `ShapeOrApplyStatement`)
ShapeOrApplyStatement :`ShapeStatement` / `ApplyStatement`
ShapeStatement :`TraitStatements` `ShapeBody`
ShapeBody :`SimpleShapeStatement`
:/ `EnumStatement`
:/ `AggregateShapeStatement`
:/ `StructureStatement`
:/ `EntityStatement`
:/ `OperationStatement`
SimpleShapeStatement :`SimpleTypeName` `SP` `Identifier` [`Mixins`]
ShapeStatement :`TraitStatements` `Shape`
Shape :`SimpleShape` / `EnumShape` / `AggregateShape` / `EntityShape` / `OperationShape`
SimpleShape :`SimpleTypeName` `SP` `Identifier` [`Mixins`]
SimpleTypeName :%s"blob" / %s"boolean" / %s"document" / %s"string"
:/ %s"byte" / %s"short" / %s"integer" / %s"long"
:/ %s"float" / %s"double" / %s"bigInteger"
:/ %s"bigDecimal" / %s"timestamp"
Mixins :[`SP`] %s"with" [`WS`] "[" [`WS`] 1*(`ShapeId` [`WS`]) "]"
EnumStatement :`EnumTypeName` `SP` `Identifier` [`Mixins`] [`WS`] `EnumShapeMembers`
EnumShape :`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`
AggregateShapeStatement :`AggregateTypeName` `SP` `Identifier` [`Mixins`] `ShapeMembers`
AggregateTypeName :%s"list" / %s"map" / %s"union"
StructureStatement :%s"structure" `SP` `Identifier` [`StructureResource`]
: [`Mixins`] [`WS`] `ShapeMembers`
StructureResource :`SP` %s"for" `SP` `ShapeId`
AggregateShape :`AggregateTypeName` `SP` `Identifier` [`ForResource`] [`Mixins`] [`WS`] `ShapeMembers`
AggregateTypeName :%s"list" / %s"map" / %s"union" / %s"structure"
ForResource :`SP` %s"for" `SP` `ShapeId`
ShapeMembers :"{" [`WS`] *(`TraitStatements` `ShapeMember` [`WS`]) "}"
ShapeMember :(`ExplicitShapeMember` / `ElidedShapeMember`) [`ValueAssignment`]
ExplicitShapeMember :`Identifier` [`SP`] ":" [`SP`] `ShapeId`
ElidedShapeMember :"$" `Identifier`
EntityStatement :`EntityTypeName` `SP` `Identifier` [`Mixins`] [`WS`] `NodeObject`
EntityShape :`EntityTypeName` `SP` `Identifier` [`Mixins`] [`WS`] `NodeObject`
EntityTypeName :%s"service" / %s"resource"
OperationStatement :%s"operation" `SP` `Identifier` [`Mixins`] [`WS`] `OperationBody`
OperationShape :%s"operation" `SP` `Identifier` [`Mixins`] [`WS`] `OperationBody`
OperationBody :"{" [`WS`]
: *(`OperationInput` / `OperationOutput` / `OperationErrors`)
: [`WS`] "}"
: ; only one of each property can be specified.
OperationInput :%s"input" [`WS`] (`InlineStructure` / (":" [`WS`] `ShapeId`))
OperationOutput :%s"output" [`WS`] (`InlineStructure` / (":" [`WS`] `ShapeId`))
OperationInput :%s"input" [`WS`] (`InlineAggregateShape` / (":" [`WS`] `ShapeId`))
OperationOutput :%s"output" [`WS`] (`InlineAggregateShape` / (":" [`WS`] `ShapeId`))
OperationErrors :%s"errors" [`WS`] ":" [`WS`] "[" [`WS`] *(`ShapeId` [`WS`]) "]"
InlineStructure :":=" [`WS`] `TraitStatements` [`StructureResource`]
: [`Mixins`] [`WS`] `ShapeMembers`
InlineAggregateShape :":=" [`WS`] `TraitStatements` [`ForResource`] [`Mixins`] [`WS`] `ShapeMembers`
.. rubric:: Traits

Expand Down Expand Up @@ -705,7 +697,7 @@ Simple shapes
-------------

:ref:`Simple shapes <simple-types>` are defined using a
:token:`smithy:SimpleShapeStatement`.
:token:`smithy:SimpleShape`.

The following example defines a ``string`` shape:

Expand Down Expand Up @@ -768,7 +760,7 @@ The following example defines an ``integer`` shape with a :ref:`range-trait`:
Enum shapes
-----------

The :ref:`enum` shape is defined using an :token:`smithy:EnumStatement`.
The :ref:`enum` shape is defined using an :token:`smithy:EnumShape`.

The following example defines an :ref:`enum` shape:

Expand Down Expand Up @@ -831,7 +823,7 @@ IntEnum shapes
--------------

The :ref:`intEnum` shape is defined using an
:token:`smithy:EnumStatement`.
:token:`smithy:EnumShape`.

.. note::
The :ref:`enumValue trait <enumValue-trait>` is required on all
Expand Down Expand Up @@ -879,7 +871,7 @@ The above intEnum is exactly equivalent to the following intEnum:
List shapes
-----------

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

The following example defines a list with a string member from the
:ref:`prelude <prelude>`:
Expand Down Expand Up @@ -960,7 +952,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:AggregateShapeStatement`.
A :ref:`map <map>` shape is defined using a :token:`smithy:AggregateShape`.

The following example defines a map of strings to integers:

Expand Down Expand Up @@ -1057,7 +1049,7 @@ Structure shapes
----------------

A :ref:`structure <structure>` shape is defined using a
:token:`smithy:StructureStatement`.
:token:`smithy:AggregateShape`.

The following example defines a structure with two members:

Expand Down Expand Up @@ -1170,7 +1162,7 @@ Is exactly equivalent to:
Union shapes
------------

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

The following example defines a union shape with several members:

Expand Down Expand Up @@ -1224,7 +1216,7 @@ The following example defines a union shape with several members:
Service shape
-------------

A service shape is defined using a :token:`smithy:EntityStatement` and the provided
A service shape is defined using a :token:`smithy:EntityShape` and the provided
:token:`smithy:NodeObject` supports the same properties defined in the
:ref:`service specification <service>`.

Expand Down Expand Up @@ -1273,7 +1265,7 @@ a resource named ``Model`` and an operation named ``PingService``:
Operation shape
---------------

An operation shape is defined using an :token:`smithy:OperationStatement` and
An operation shape is defined using an :token:`smithy:OperationShape` and
the same properties defined in the :ref:`operation specification <operation>`.

The following example defines an operation shape that accepts an input
Expand Down Expand Up @@ -1430,7 +1422,7 @@ The suffixes for the generated names can be customized using the
Resource shape
--------------

A resource shape is defined using a :token:`smithy:EntityStatement` and the
A resource shape is defined using a :token:`smithy:EntityShape` and the
provided :token:`smithy:NodeObject` supports the same properties defined in the
:ref:`resource specification <resource>`.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
import software.amazon.smithy.model.shapes.ShapeId;
import software.amazon.smithy.model.shapes.ShapeType;
import software.amazon.smithy.model.shapes.StructureShape;
import software.amazon.smithy.model.shapes.UnionShape;
import software.amazon.smithy.model.traits.DefaultTrait;
import software.amazon.smithy.model.traits.DocumentationTrait;
import software.amazon.smithy.model.traits.EnumValueTrait;
Expand Down Expand Up @@ -558,6 +557,8 @@ private void parseShapeOrApply(List<IdlTraitParser.Result> traits) {
case LIST:
case SET:
case MAP:
case UNION:
case STRUCTURE:
parseAggregateShape(id, location, type.createBuilderForType());
break;
case ENUM:
Expand All @@ -566,12 +567,6 @@ private void parseShapeOrApply(List<IdlTraitParser.Result> traits) {
case INT_ENUM:
parseEnumShape(id, location, IntEnumShape.builder());
break;
case STRUCTURE:
parseStructuredShape(id, location, StructureShape.builder());
break;
case UNION:
parseStructuredShape(id, location, UnionShape.builder());
break;
case SERVICE:
parseServiceStatement(id, location);
break;
Expand Down Expand Up @@ -700,25 +695,7 @@ private void parseEnumShape(ShapeId id, SourceLocation location, AbstractShapeBu

private void parseAggregateShape(ShapeId id, SourceLocation location, AbstractShapeBuilder<?, ?> builder) {
LoadOperation.DefineShape operation = createShape(builder.id(id).source(location));
parseMixins(operation);
parseMembers(operation);
operations.accept(operation);
}

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

// If it's a structure, parse the optional "from" statement to enable
// eliding member targets for resource identifiers.
if (builder.getShapeType() == ShapeType.STRUCTURE) {
parseForResource(operation);
}

// Parse optional "with" statements to add mixins, but only if it's supported by the version.
parseForResource(operation);
parseMixins(operation);
parseMembers(operation);
operations.accept(operation);
Expand Down Expand Up @@ -999,12 +976,8 @@ private ShapeId parseInlineStructure(String name, IdlTraitParser.Result defaultT
ShapeId id = ShapeId.fromRelative(expectNamespace(), name);
SourceLocation location = tokenizer.getCurrentTokenLocation();
StructureShape.Builder builder = StructureShape.builder().id(id).source(location);
LoadOperation.DefineShape operation = createShape(builder);
parseMixins(operation);
parseForResource(operation);
parseMembers(operation);
parseAggregateShape(id, location, builder);
addTraits(id, traits);
operations.accept(operation);
return id;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[ERROR] com.test#WrongMemberList: Missing required member of shape `com.test#WrongMemberList`: member | Model
[ERROR] com.test#WrongMemberList: List shapes may only have a `member` member, but found `id` | Model
[ERROR] com.test#ExtraElidedMemberList: List shapes may only have a `member` member, but found `other` | Model
[ERROR] com.test#ConflictingTypesList$member: Member conflicts with an inherited mixin member: `com.test#MixinList$member` | Model
[ERROR] com.test#WrongMembersMap: Missing required members of shape `com.test#WrongMembersMap`: key, value | Model
[ERROR] com.test#WrongMembersMap: Map shapes may only have `key` and `value` members, but found `id` | Model
[ERROR] com.test#WrongMembersMap: Map shapes may only have `key` and `value` members, but found `other` | Model
[ERROR] com.test#ExtraMemberMap: Map shapes may only have `key` and `value` members, but found `other` | Model
[ERROR] com.test#ConflictingTypesMap$key: Member conflicts with an inherited mixin member: `com.test#MixinMap$key` | Model
[ERROR] com.test#ConflictingTypesMap$value: Member conflicts with an inherited mixin member: `com.test#MixinMap$value` | Model
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
$version: "2.0"

namespace com.test

resource MyResource {
identifiers: {
id: String
member: String
key: String
}
properties: {
value: String
other: String
}
}

list WrongMemberList for MyResource {
$id
}

list ExtraElidedMemberList for MyResource {
$member
$other
}

@mixin
list MixinList {
member: Integer
}

list ConflictingTypesList for MyResource with [MixinList] {
$member
}

map WrongMembersMap for MyResource {
$id
$other
}

map ExtraMemberMap for MyResource {
$key
$value
$other
}

@mixin
map MixinMap {
key: Integer
value: Integer
}

map ConflictingTypesMap for MyResource with [MixinMap] {
$key
$value
}

This file was deleted.

Loading

0 comments on commit 4784858

Please sign in to comment.