Add ability to elide targets for mixins/resources

This adds in syntactic sugar that allows modelers to elide targets
for members that are inherited from mixins or defined in resources.

This necessarily also adds the ability for a structure to indicate
a resource to pull potential targets from.

This is all syntactic sugar for the IDL, so the JSON AST isn't
impacted.

This is accomplished by refactoring the code used for mixins to
instead be a generic system for applying modifications to a shape
after some set of dependencies has been resolved.

Author: JordonPhillips

docs/source/1.0/guides/migrating-idl-1-to-2.rst

@@ -199,14 +199,6 @@ having to copy and paste them. The following model:
 
     namespace smithy.example
 
-    resource User {
-        identifiers: {
-            email: String,
-            id: String,
-        },
-        read: GetUser
-    }
-
     operation GetUser {
         input: GetUserInput,
         output: GetUserOutput
@@ -240,15 +232,6 @@ Can be updated to:
 
     namespace smithy.example
 
-    resource User {
-        identifiers: {
-            email: String
-            id: String
-            username: String
-        },
-        read: GetUser
-    }
-
     @mixin
     structure UserIdentifiers {
         @required
@@ -274,6 +257,66 @@ that otherwise have to be copied and pasted.
     work.
 
 
+Use the target elision syntax sugar to reduce boilerplate
+---------------------------------------------------------
+
+Resource shapes contain a set of identifiers, but when writing structures that
+contain those identifiers you have to duplicate those definitions entirely. In
+IDL 2.0, you can use the target elision syntax with a structure bound to a
+resource. For example:
+
+.. code-block:: smithy
+
+    $version: "2.0"
+
+    namespace smithy.example
+
+    resource User {
+        identifiers: {
+            id: String
+            email: String
+        }
+    }
+
+    // The `for` syntax here determines which resource should be checked.
+    structure UserDetails for User {
+        // With this syntax, the target is automatically inferred from the
+        // resource.
+        $id
+
+        // Uncomment this to include an email member. Unlike with mixins, you
+        // must opt in to the members that you want to include. This allows you
+        // to have partial views of a resource, such as in a create operation
+        // that does not bind all of the identifiers.
+        // $email
+
+        address: String
+    }
+
+This syntax can also be used with mixins to more succinctly add additional
+traits to included members.
+
+.. code-block:: smithy
+
+    $version: "2.0"
+
+    namespace smithy.example
+
+    @mixin
+    structure UserIdentifiers {
+        id: String
+        email: String
+    }
+
+    structure UserDetails with [UserIdentifiers] {
+        @required
+        $id
+
+        @required
+        $email
+    }
+
+
 Remove unsightly commas
 -----------------------
 

docs/source/1.0/spec/core/idl.rst

@@ -190,12 +190,14 @@ The Smithy IDL is defined by the following ABNF:
     enum_shape_statement   :`enum_type_name` `ws` `identifier` [`mixins`] `ws` `enum_shape_members`
     enum_type_name         :"enum" / "intEnum"
     enum_shape_members     :"{" `ws` 1*(`trait_statements` `identifier` `ws`) "}"
-    shape_members          :"{" `ws` *(`shape_member_kvp` `ws`) "}"
-    shape_member_kvp       :`trait_statements` `identifier` `ws` ":" `ws` `shape_id`
+    shape_members          :"{" `ws` *(`trait_statements` `shape_member` `ws`) "}"
+    shape_member           :`shape_member_kvp` / `shape_member_elided`
+    shape_member_kvp       :`identifier` `ws` ":" `ws` `shape_id`
+    shape_member_elided    :"$" `identifier`
     list_statement :"list" `ws` `identifier` [`mixins`] `ws` `shape_members`
     set_statement :"set" `ws` `identifier` [`mixins`] `ws` `shape_members`
     map_statement :"map" `ws` `identifier` [`mixins`] `ws` `shape_members`
-    structure_statement     :"structure" `ws` `identifier` [`mixins`] `ws` `shape_members`
+    structure_statement     :"structure" `ws` `identifier` ["for" `shape_id`] [`mixins`] `ws` `shape_members`
     union_statement :"union" `ws` `identifier` [`mixins`] `ws` `shape_members`
     service_statement :"service" `ws` `identifier` [`mixins`] `ws` `node_object`
     operation_statement :"operation" `ws` `identifier` [`mixins`] `ws` `inlineable_properties`
@@ -1408,6 +1410,11 @@ and defines a :ref:`read <read-lifecycle>` operation:
             }
         }
 
+.. seealso::
+
+    The :ref:`target elision syntax <idl-target-elision>` for an easy way to
+    define structures that reference resource identifiers without having to
+    repeat the target definition.
 
 .. _idl-mixins:
 
@@ -1438,6 +1445,109 @@ For example:
     string SensitiveText with [SensitiveString]
 
 
+.. _idl-target-elision:
+
+Target Elision
+--------------
+
+Having to completely redefine a :ref:`resource identifier <resource-identifiers>`
+to use it in a structure or redefine a member from a :ref:`mixin` to add
+additional traits can be cumbersome and potentially error-prone. The
+:token:`type elision syntax <smithy:shape_member_elided>` can be used to cut
+down on that repetition by prefixing the member name with a ``$``. If a member
+is prefixed this way, its target will automatically be set to the target of a
+mixin member with the same name. The following example shows how to elide the
+target for a member inherited from a mixin:
+
+.. code-block:: smithy
+
+    $version: "2.0"
+
+    namespace smithy.example
+
+    @mixin
+    structure IdBearer {
+        id: String
+    }
+
+    structure IdRequired with [IdBearer] {
+        @required
+        $id
+    }
+
+Additionally, structure shapes can reference a :ref:`resource <idl-resource>`
+shape to define members that represent the resource's identifiers without having
+to redefine the target shape. In addition to prefixing a member with ``$``, the
+structure must also add ``for`` followed by the resource referenced in
+the shape's definition before any mixins are specified.
+
+To resolve elided types, first check if any bound resource defines an
+identifier that case-sensitively matches the elided member name. If a match is
+found, the type targeted by that identifier is used for the elided type. If no
+identifier matches the elided member name, mixin members are case-sensitively
+checked, and if a match is found, the type targeted by the mixin member is
+used as the elided type. It is an error if neither the resource or mixin
+members matches an elided member name.
+
+The following example shows a structure reusing an identifier definition from
+a resource:
+
+.. code-block:: smithy
+
+    $version: "2.0"
+
+    namespace smithy.example
+
+    resource User {
+        identifiers: {
+            name: String
+            uuid: String
+        }
+    }
+
+    structure UserSummary for User {
+        $name
+        age: Short
+    }
+
+Note that the ``UserSummary`` structure does not attempt to define the
+``uuid`` identifier. When referencing a resource in this way, only the
+identifiers that are explicitly referenced are added to the structure. This
+allows structures to define subsets of identifiers, which can be useful for
+operations like create operations where some of those identifiers may be
+generated by the service.
+
+Structures may only reference one resource shape in this way.
+
+When using both mixins and a resource reference, the referenced resource will
+be checked first. The following example is invalid:
+
+.. code-block:: smithy
+
+    $version: "2.0"
+
+    namespace smithy.example
+
+    resource User {
+        identifiers: {
+            uuid: String
+        }
+    }
+
+    @mixin
+    structure UserIdentifiers {
+        uuid: Blob
+    }
+
+    // This is invalid because the `uuid` member's target is set to
+    // String, which then conflicts with the UserIdentifiers mixin.
+    structure UserSummary for User with [UserIdentifiers] {
+        $uuid
+    }
+
+
+
+
 .. _documentation-comment:
 
 Documentation comment

smithy-model/src/main/java/software/amazon/smithy/model/loader/AbstractMutableModelFile.java

@@ -50,7 +50,8 @@ abstract class AbstractMutableModelFile implements ModelFile {
     private final Set<ShapeId> allShapeIds = new HashSet<>();
     private final Map<ShapeId, AbstractShapeBuilder<?, ?>> shapes = new LinkedHashMap<>();
     private final Map<ShapeId, Map<String, MemberShape.Builder>> members = new HashMap<>();
-    private final Map<ShapeId, Set<ShapeId>> pendingShapes = new HashMap<>();
+    private final Map<ShapeId, List<PendingShapeModifier>> pendingModifications = new HashMap<>();
+    private final Map<ShapeId, Set<ShapeId>> pendingDependencies = new HashMap<>();
     private final List<ValidationEvent> events = new ArrayList<>();
     private final MetadataContainer metadata = new MetadataContainer(events);
     private final TraitFactory traitFactory;
@@ -106,7 +107,13 @@ void onShape(AbstractShapeBuilder<?, ?> builder) {
     }
 
     void addPendingMixin(ShapeId shape, ShapeId mixin) {
-        pendingShapes.computeIfAbsent(shape, id -> new LinkedHashSet<>()).add(mixin);
+        addPendingModification(shape, new ApplyMixin(mixin, events));
+    }
+
+    void addPendingModification(ShapeId shape, PendingShapeModifier pendingModification) {
+        pendingDependencies.computeIfAbsent(shape, id -> new LinkedHashSet<>())
+                .addAll(pendingModification.getDependencies());
+        pendingModifications.computeIfAbsent(shape, id -> new ArrayList<>()).add(pendingModification);
     }
 
     private SourceException onConflict(AbstractShapeBuilder<?, ?> builder, AbstractShapeBuilder<?, ?> previous) {
@@ -181,13 +188,15 @@ public final CreatedShapes createShapes(TraitContainer resolvedTraits) {
         List<Shape> resolvedShapes = new ArrayList<>(shapes.size());
         List<PendingShape> pendingMixins = new ArrayList<>();
 
-        for (Map.Entry<ShapeId, Set<ShapeId>> entry : pendingShapes.entrySet()) {
+        for (Map.Entry<ShapeId, List<PendingShapeModifier>> entry : this.pendingModifications.entrySet()) {
             ShapeId subject = entry.getKey();
-            Set<ShapeId> mixins = entry.getValue();
+            List<PendingShapeModifier> pendingModifications = entry.getValue();
+            Set<ShapeId> dependencies = pendingDependencies.getOrDefault(subject, Collections.emptySet());
             AbstractShapeBuilder<?, ?> builder = shapes.get(entry.getKey());
             Map<String, MemberShape.Builder> builderMembers = claimMembersOfContainer(builder.getId());
             shapes.remove(entry.getKey());
-            pendingMixins.add(createPendingShape(subject, builder, builderMembers, mixins, traitContainer));
+            pendingMixins.add(createPendingShape(
+                    subject, builder, builderMembers, dependencies, pendingModifications, traitContainer));
         }
 
         // Build members and add them to top-level shapes.
@@ -223,55 +232,21 @@ private PendingShape createPendingShape(
             AbstractShapeBuilder<?, ?> builder,
             Map<String, MemberShape.Builder> builderMembers,
             Set<ShapeId> mixins,
+            List<PendingShapeModifier> pendingModifications,
             TraitContainer resolvedTraits
     ) {
         return PendingShape.create(subject, builder, mixins, shapeMap -> {
-            // Build normal members first.
             for (MemberShape.Builder memberBuilder : builderMembers.values()) {
+                for (PendingShapeModifier pendingModification : pendingModifications) {
+                    pendingModification.modifyMember(builder, memberBuilder, resolvedTraits, shapeMap);
+                }
                 buildShape(memberBuilder, resolvedTraits).ifPresent(builder::addMember);
             }
-            // Add each mixin and ensure there are no member conflicts.
-            for (ShapeId mixin : mixins) {
-                Shape mixinShape = shapeMap.get(mixin);
-                for (MemberShape member : mixinShape.members()) {
-                    ShapeId targetId = builder.getId().withMember(member.getMemberName());
-                    Map<ShapeId, Trait> introducedTraits = traitContainer.getTraitsForShape(targetId);
-
-                    MemberShape introducedMember = null;
-                    if (builderMembers.containsKey(member.getMemberName())) {
-                        introducedMember = builderMembers.get(member.getMemberName())
-                                .addMixin(member)
-                                .build();
-
-                        if (!introducedMember.getTarget().equals(member.getTarget())) {
-                            // Members cannot be redefined if their targets conflict.
-                            MemberShape.Builder conflict = builderMembers.get(member.getMemberName());
-                            events.add(ValidationEvent.builder()
-                                    .severity(Severity.ERROR)
-                                    .id(Validator.MODEL_ERROR)
-                                    .shapeId(conflict.getId())
-                                    .sourceLocation(conflict.getSourceLocation())
-                                    .message("Member conflicts with an inherited mixin member: " + member.getId())
-                                    .build());
-                        }
-                    } else if (!introducedTraits.isEmpty()) {
-                        // Build local member copies before adding mixins if traits
-                        // were introduced to inherited mixin members.
-                        introducedMember = MemberShape.builder()
-                                .id(targetId)
-                                .target(member.getTarget())
-                                .source(member.getSourceLocation())
-                                .addTraits(introducedTraits.values())
-                                .addMixin(member)
-                                .build();
-                    }
-
-                    if (introducedMember != null) {
-                        builder.addMember(introducedMember);
-                    }
-                }
-                builder.addMixin(mixinShape);
+
+            for (PendingShapeModifier pendingModification : pendingModifications) {
+                pendingModification.modifyShape(builder, builderMembers, resolvedTraits, shapeMap);
             }
+
             buildShape(builder, resolvedTraits).ifPresent(result -> shapeMap.put(result.getId(), result));
         });
     }
@@ -285,10 +260,24 @@ private <S extends Shape, B extends AbstractShapeBuilder<? extends B, S>> Option
                 builder.addTrait(trait);
             }
             return Optional.of(builder.build());
+        } catch (IllegalStateException e) {
+            if (builder.getShapeType() == ShapeType.MEMBER && ((MemberShape.Builder) builder).getTarget() == null) {
+                events.add(ValidationEvent.builder()
+                        .severity(Severity.ERROR)
+                        .id(Validator.MODEL_ERROR)
+                        .shapeId(builder.getId())
+                        .sourceLocation(builder.getSourceLocation())
+                        .message("Member target was elided, but no bound resource or mixin contained a matching "
+                                + "identifier or member name.")
+                        .build());
+                return Optional.empty();
+            }
+            throw e;
         } catch (SourceException e) {
             events.add(ValidationEvent.fromSourceException(e, "", builder.getId()));
             resolvedTraits.clearTraitsForShape(builder.getId());
             return Optional.empty();
         }
     }
+
 }