Add missing mixin relationships

smithy-model/src/main/java/software/amazon/smithy/model/neighbor/NeighborVisitor.java

@@ -16,6 +16,7 @@
 package software.amazon.smithy.model.neighbor;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.shapes.EntityShape;
@@ -33,7 +34,6 @@
 import software.amazon.smithy.model.shapes.ShapeVisitor;
 import software.amazon.smithy.model.shapes.StructureShape;
 import software.amazon.smithy.model.shapes.UnionShape;
-import software.amazon.smithy.utils.ListUtils;
 
 /**
  * Finds all neighbors of a shape, returning them as a list of
@@ -59,12 +59,32 @@ public List<Relationship> getNeighbors(Shape shape) {
 
     @Override
     public List<Relationship> getDefault(Shape shape) {
-        return ListUtils.of();
+        return shape.getMixins().isEmpty()
+               ? Collections.emptyList()
+               : initializeRelationships(shape, 0);
+    }
+
+    private List<Relationship> initializeRelationships(Shape shape, int knownMemberCount) {
+        if (shape.isMemberShape()) {
+            // Members have mixins but shouldn't contribute a relationship.
+            return new ArrayList<>(knownMemberCount);
+        } else {
+            knownMemberCount += shape.getMixins().size();
+            List<Relationship> result = new ArrayList<>(knownMemberCount);
+            for (ShapeId mixin : shape.getMixins()) {
+                result.add(relationship(shape, RelationshipType.MIXIN, mixin));
+            }
+            return result;
+        }
     }
 
     @Override
     public List<Relationship> serviceShape(ServiceShape shape) {
-        List<Relationship> result = new ArrayList<>();
+        int operationAndResourceRelationships = 2 * (shape.getAllOperations().size() + shape.getResources().size());
+        int errorSize = shape.getErrors().size();
+        int neededSize = operationAndResourceRelationships + errorSize;
+        List<Relationship> result = initializeRelationships(shape, neededSize);
+
         // Add OPERATION from service -> operation. Add BINDING from operation -> service.
         for (ShapeId operation : shape.getOperations()) {
             addBinding(result, shape, operation, RelationshipType.OPERATION);
@@ -92,7 +112,12 @@ private void addBound(List<Relationship> result, Shape container, ShapeId bindin
 
     @Override
     public List<Relationship> resourceShape(ResourceShape shape) {
-        List<Relationship> result = new ArrayList<>();
+        // This is a rough estimate for the number of needed relationships.
+        int operationAndResourceRelationships = 2 * (shape.getAllOperations().size() + shape.getResources().size());
+        int identifierSize = shape.getIdentifiers().size();
+        int neededSize = (operationAndResourceRelationships * 2) + identifierSize;
+        List<Relationship> result = initializeRelationships(shape, neededSize);
+
         // Add IDENTIFIER relationships.
         shape.getIdentifiers().forEach((k, v) -> result.add(relationship(shape, RelationshipType.IDENTIFIER, v)));
         // Add RESOURCE from resourceA -> resourceB and BOUND from resourceB -> resourceA
@@ -154,10 +179,22 @@ private void addServiceAndResourceBindings(List<Relationship> result, ResourceSh
 
     @Override
     public List<Relationship> operationShape(OperationShape shape) {
-        List<Relationship> result = new ArrayList<>();
+        ShapeId input = shape.getInput().orElse(null);
+        ShapeId output = shape.getOutput().orElse(null);
+
+        // Calculate the number of relationships up front.
+        int assumedRelationshipCount = shape.getErrors().size()
+                + (input == null ? 0 : 1)
+                + (output == null ? 0 : 1);
+        List<Relationship> result = initializeRelationships(shape, assumedRelationshipCount);
 
-        shape.getInput().ifPresent(input -> result.add(relationship(shape, RelationshipType.INPUT, input)));
-        shape.getOutput().ifPresent(output -> result.add(relationship(shape, RelationshipType.OUTPUT, output)));
+        if (input != null) {
+            result.add(relationship(shape, RelationshipType.INPUT, input));
+        }
+
+        if (output != null) {
+            result.add(relationship(shape, RelationshipType.OUTPUT, output));
+        }
 
         for (ShapeId errorId : shape.getErrors()) {
             result.add(relationship(shape, RelationshipType.ERROR, errorId));
@@ -167,15 +204,15 @@ public List<Relationship> operationShape(OperationShape shape) {
 
     @Override
     public List<Relationship> memberShape(MemberShape shape) {
-        List<Relationship> result = new ArrayList<>(2);
+        List<Relationship> result = initializeRelationships(shape, 2);
         result.add(relationship(shape, RelationshipType.MEMBER_CONTAINER, shape.getContainer()));
         result.add(relationship(shape, RelationshipType.MEMBER_TARGET, shape.getTarget()));
         return result;
     }
 
     @Override
     public List<Relationship> enumShape(EnumShape shape) {
-        List<Relationship> result = new ArrayList<>();
+        List<Relationship> result = initializeRelationships(shape, shape.getAllMembers().size());
         for (MemberShape member : shape.getAllMembers().values()) {
             result.add(relationship(shape, RelationshipType.ENUM_MEMBER, member));
         }
@@ -184,7 +221,7 @@ public List<Relationship> enumShape(EnumShape shape) {
 
     @Override
     public List<Relationship> intEnumShape(IntEnumShape shape) {
-        List<Relationship> result = new ArrayList<>();
+        List<Relationship> result = initializeRelationships(shape, shape.getAllMembers().size());
         for (MemberShape member : shape.getAllMembers().values()) {
             result.add(relationship(shape, RelationshipType.INT_ENUM_MEMBER, member));
         }
@@ -193,28 +230,29 @@ public List<Relationship> intEnumShape(IntEnumShape shape) {
 
     @Override
     public List<Relationship> listShape(ListShape shape) {
-        return ListUtils.of(relationship(shape, RelationshipType.LIST_MEMBER, shape.getMember()));
+        List<Relationship> result = initializeRelationships(shape, 1);
+        result.add(relationship(shape, RelationshipType.LIST_MEMBER, shape.getMember()));
+        return result;
     }
 
     @Override
     public List<Relationship> setShape(SetShape shape) {
-        return ListUtils.of(relationship(shape, RelationshipType.SET_MEMBER, shape.getMember()));
+        List<Relationship> result = initializeRelationships(shape, 1);
+        result.add(relationship(shape, RelationshipType.SET_MEMBER, shape.getMember()));
+        return result;
     }
 
     @Override
     public List<Relationship> mapShape(MapShape shape) {
-        List<Relationship> result = new ArrayList<>(2);
+        List<Relationship> result = initializeRelationships(shape, 2);
         result.add(relationship(shape, RelationshipType.MAP_KEY, shape.getKey()));
         result.add(relationship(shape, RelationshipType.MAP_VALUE, shape.getValue()));
         return result;
     }
 
     @Override
     public List<Relationship> structureShape(StructureShape shape) {
-        List<Relationship> result = new ArrayList<>();
-        for (ShapeId mixin : shape.getMixins()) {
-            result.add(relationship(shape, RelationshipType.MIXIN, mixin));
-        }
+        List<Relationship> result = initializeRelationships(shape, shape.getAllMembers().size());
         for (MemberShape member : shape.getAllMembers().values()) {
             result.add(Relationship.create(shape, RelationshipType.STRUCTURE_MEMBER, member));
         }
@@ -223,10 +261,7 @@ public List<Relationship> structureShape(StructureShape shape) {
 
     @Override
     public List<Relationship> unionShape(UnionShape shape) {
-        List<Relationship> result = new ArrayList<>();
-        for (ShapeId mixin : shape.getMixins()) {
-            result.add(relationship(shape, RelationshipType.MIXIN, mixin));
-        }
+        List<Relationship> result = initializeRelationships(shape, shape.getAllMembers().size());
         for (MemberShape member : shape.getAllMembers().values()) {
             result.add(Relationship.create(shape, RelationshipType.UNION_MEMBER, member));
         }

smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/TargetValidator.java

@@ -102,6 +102,7 @@ private Optional<ValidationEvent> validateTarget(Model model, Shape shape, Shape
                     return Optional.of(error(shape, format(
                             "Members cannot target %s shapes, but found %s", target.getType(), target)));
                 }
+                return Optional.empty();
             case MAP_KEY:
                 return target.asMemberShape().flatMap(m -> validateMapKey(shape, m.getTarget(), model));
             case RESOURCE:

smithy-model/src/test/java/software/amazon/smithy/model/neighbor/NeighborVisitorTest.java

@@ -21,6 +21,7 @@
 import static org.hamcrest.Matchers.empty;
 
 import java.util.List;
+import java.util.stream.Collectors;
 import org.junit.jupiter.api.Test;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.shapes.BlobShape;
@@ -58,6 +59,26 @@ public void blobShape() {
         assertThat(relationships, empty());
     }
 
+    @Test
+    public void findsMixinsOnThingsOtherThanStructAndUnion() {
+        Shape blobMixin = BlobShape.builder().id("smithy.example#BlobMixin")
+                .addTrait(MixinTrait.builder().build())
+                .build();
+        Shape shape = BlobShape.builder()
+                .id("ns.foo#name")
+                .addMixin(blobMixin)
+                .build();
+        Model model = Model.builder().addShapes(shape, blobMixin).build();
+        NeighborVisitor neighborVisitor = new NeighborVisitor(model);
+        List<Relationship> relationships = shape.accept(neighborVisitor);
+
+        List<RelationshipType> types = relationships.stream()
+                .map(Relationship::getRelationshipType)
+                .collect(Collectors.toList());
+
+        assertThat(types, contains(RelationshipType.MIXIN));
+    }
+
     @Test
     public void booleanShape() {
         Shape shape = BooleanShape.builder().id("ns.foo#name").build();