Allow replacement of simple shape types in ModelTransformer

Author: jdisanti

smithy-model/src/main/java/software/amazon/smithy/model/transform/ReplaceShapes.java

@@ -27,6 +27,7 @@
 import java.util.Set;
 import java.util.stream.Collectors;
 import software.amazon.smithy.model.Model;
+import software.amazon.smithy.model.shapes.CollectionShape;
 import software.amazon.smithy.model.shapes.ListShape;
 import software.amazon.smithy.model.shapes.MapShape;
 import software.amazon.smithy.model.shapes.MemberShape;
@@ -84,7 +85,7 @@ Model transform(ModelTransformer transformer, Model model) {
             return model;
         }
 
-        assertNoShapeChangedType(model, shouldReplace);
+        assertShapeTypeChangesSound(model, shouldReplace);
         Model.Builder builder = createReplacedModelBuilder(model, shouldReplace);
 
         // If a member shape changes, then ensure that the containing shape
@@ -121,18 +122,51 @@ private List<Shape> determineShapesToReplace(Model model) {
                 .collect(toList());
     }
 
-    private void assertNoShapeChangedType(Model model, List<Shape> shouldReplace) {
+    private void assertShapeTypeChangesSound(Model model, List<Shape> shouldReplace) {
         // Throws if any mappings attempted to change a shape's type.
         shouldReplace.stream()
                 .flatMap(previous -> Pair.flatMapStream(previous, p -> model.getShape(p.getId())))
                 .filter(pair -> pair.getLeft().getType() != pair.getRight().getType())
+                .filter(pair -> !isReplacementValid(model, pair.getLeft(), pair.getRight()))
                 .forEach(pair -> {
-                    throw new RuntimeException(String.format(
+                    throw new IllegalStateException(String.format(
                             "Cannot change the type of %s from %s to %s",
-                            pair.getLeft().getId(), pair.getLeft().getType(), pair.getRight().getType()));
+                            pair.getLeft().getId(), pair.getRight().getType(), pair.getLeft().getType()));
                 });
     }
 
+    private boolean isReplaceable(Shape shape) {
+        return !shape.isDocumentShape()
+                && !shape.isMapShape()
+                && !shape.isMemberShape()
+                && !shape.isOperationShape()
+                && !shape.isResourceShape()
+                && !shape.isServiceShape()
+                && !shape.isStructureShape()
+                && !shape.isUnionShape();
+    }
+
+    private boolean isReplacementValid(Model model, Shape left, Shape right) {
+        if (isListCollection(left) && isListCollection(right)) {
+            validateListCollectionReplacement(model, (CollectionShape) left, (CollectionShape) right);
+        }
+        return isReplaceable(left) && isReplaceable(right) && isListCollection(left) == isListCollection(right);
+    }
+
+    private boolean isListCollection(Shape shape) {
+        return shape.isListShape() || shape.isSetShape();
+    }
+
+    private void validateListCollectionReplacement(Model model, CollectionShape left, CollectionShape right) {
+        Shape leftTarget = model.expectShape(left.getMember().getTarget());
+        Shape rightTarget = model.expectShape(right.getMember().getTarget());
+        if (!isReplacementValid(model, leftTarget, rightTarget)) {
+            throw new IllegalStateException(String.format(
+                    "Cannot change the type of %s from %s to %s",
+                    left.getMember().getId(), rightTarget.getType(), leftTarget.getType()));
+        }
+    }
+
     private Model.Builder createReplacedModelBuilder(Model model, List<Shape> shouldReplace) {
         // Add member shapes to the builder. This builder is mutated
         // by the visitor, which will ensure that newly added members

smithy-model/src/test/java/software/amazon/smithy/model/transform/ReplaceShapesTest.java

@@ -16,20 +16,23 @@
 package software.amazon.smithy.model.transform;
 
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Optional;
 import org.hamcrest.Matchers;
-import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.SourceLocation;
 import software.amazon.smithy.model.shapes.IntegerShape;
 import software.amazon.smithy.model.shapes.ListShape;
+import software.amazon.smithy.model.shapes.LongShape;
 import software.amazon.smithy.model.shapes.MapShape;
 import software.amazon.smithy.model.shapes.MemberShape;
+import software.amazon.smithy.model.shapes.SetShape;
 import software.amazon.smithy.model.shapes.ShapeId;
 import software.amazon.smithy.model.shapes.StringShape;
 import software.amazon.smithy.model.shapes.StructureShape;
@@ -41,14 +44,85 @@
 public class ReplaceShapesTest {
 
     @Test
-    public void cannotChangeShapeTypes() {
-        Assertions.assertThrows(RuntimeException.class, () -> {
+    public void cannotChangeComplexShapeTypes() {
+        IllegalStateException ex = assertThrows(IllegalStateException.class, () -> {
             ShapeId shapeId = ShapeId.from("ns.foo#id1");
             StringShape shape = StringShape.builder().id(shapeId).build();
-            Model model = Model.builder().addShape(shape).build();
+            Model model = Model.builder()
+                    .addShape(shape)
+                    .addShape(LongShape.builder().id("ns.foo#id2").build())
+                    .build();
             ModelTransformer transformer = ModelTransformer.create();
-            transformer.mapShapes(model, s -> IntegerShape.builder().id(shapeId).build());
+            transformer.mapShapes(model, s -> {
+                if (s.getId().equals(shapeId)) {
+                    return StructureShape.builder()
+                        .id(shapeId)
+                        .addMember(
+                                MemberShape.builder()
+                                        .id(ShapeId.from("ns.foo#id1$member1"))
+                                        .target("ns.foo#id2")
+                                        .build()
+                        ).build();
+                }
+                return s;
+            });
         });
+        assertEquals("Cannot change the type of ns.foo#id1 from string to structure", ex.getMessage());
+    }
+
+    @Test
+    public void canChangeSimpleShapeTypes() {
+        ShapeId shapeId = ShapeId.from("ns.foo#id1");
+        IntegerShape shape = IntegerShape.builder().id(shapeId).build();
+        Model model = Model.builder().addShape(shape).build();
+        ModelTransformer transformer = ModelTransformer.create();
+        transformer.mapShapes(model, s -> LongShape.builder().id(shapeId).build());
+    }
+
+    @Test
+    public void canExchangeSimilarListCollectionTypes() {
+        ShapeId shapeId = ShapeId.from("ns.foo#id1");
+        ListShape shape = ListShape.builder().id(shapeId).member(ShapeId.from("smithy.api#Long")).build();
+        Model model = Model.builder()
+                .addShape(shape)
+                .addShape(LongShape.builder().id(ShapeId.from("smithy.api#Long")).build())
+                .build();
+        ModelTransformer transformer = ModelTransformer.create();
+        transformer.mapShapes(model, s -> {
+            if (s.getId().equals(shapeId)) {
+                return SetShape.builder().id(shapeId).member(ShapeId.from("smithy.api#Long")).build();
+            }
+            return s;
+        });
+    }
+
+    @Test
+    public void canNotExchangeListCollectionTypesWithNonReplaceableMembers() {
+        ShapeId shapeId = ShapeId.from("ns.foo#id1");
+        ShapeId structId = ShapeId.from("ns.foo#id2");
+        ListShape shape = ListShape.builder().id(shapeId).member(ShapeId.from("smithy.api#Long")).build();
+        Model model = Model.builder()
+                .addShape(shape)
+                .addShape(LongShape.builder().id(ShapeId.from("smithy.api#Long")).build())
+                .addShape(StructureShape.builder()
+                        .id(structId)
+                        .addMember(
+                                MemberShape.builder()
+                                        .id(structId.withMember("member1"))
+                                        .target("smithy.api#Long")
+                                        .build()
+                        ).build())
+                .build();
+        IllegalStateException ex = assertThrows(IllegalStateException.class, () -> {
+            ModelTransformer transformer = ModelTransformer.create();
+            transformer.mapShapes(model, s -> {
+                if (s.getId().equals(shapeId)) {
+                    return SetShape.builder().id(shapeId).member(structId).build();
+                }
+                return s;
+            });
+        });
+        assertEquals("Cannot change the type of ns.foo#id1$member from long to structure", ex.getMessage());
     }
 
     @Test