Ignore valid set->list type change in diff

It isn't an error to migrate a set to a list with the uniqueItems trait. This is a recommended change because sets are deprecated and should not be implemented by any tooling.
smithy-lang · Sep 2, 2022 · 319389f · 319389f
1 parent 8ac4ecd
commit 319389f
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 0 deletions.
diff --git a/smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/ChangedShapeType.java b/smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/ChangedShapeType.java
@@ -17,7 +17,11 @@
 
 import java.util.List;
 import java.util.stream.Collectors;
+import software.amazon.smithy.diff.ChangedShape;
 import software.amazon.smithy.diff.Differences;
+import software.amazon.smithy.model.shapes.Shape;
+import software.amazon.smithy.model.shapes.ShapeType;
+import software.amazon.smithy.model.traits.UniqueItemsTrait;
 import software.amazon.smithy.model.validation.ValidationEvent;
 
 /**
@@ -28,9 +32,23 @@ public final class ChangedShapeType extends AbstractDiffEvaluator {
     public List<ValidationEvent> evaluate(Differences differences) {
         return differences.changedShapes()
                 .filter(diff -> diff.getOldShape().getType() != diff.getNewShape().getType())
+                .filter(diff -> !expectedSetToListChange(diff))
                 .map(diff -> error(diff.getNewShape(), String.format(
                         "Shape `%s` type was changed from `%s` to `%s`.",
                         diff.getShapeId(), diff.getOldShape().getType(), diff.getNewShape().getType())))
                 .collect(Collectors.toList());
     }
+
+    private boolean expectedSetToListChange(ChangedShape<Shape> diff) {
+        ShapeType oldType = diff.getOldShape().getType();
+        ShapeType newType = diff.getNewShape().getType();
+
+        // Smithy diff doesn't raise an issue if a set is changed to a list and the list
+        // has the uniqueItems trait. Set is deprecated and this is a recommended change.
+        if (oldType == ShapeType.SET && newType == ShapeType.LIST) {
+            return diff.getNewShape().hasTrait(UniqueItemsTrait.class);
+        }
+
+        return false;
+    }
 }
diff --git a/smithy-diff/src/test/java/software/amazon/smithy/diff/evaluators/ChangedShapeTypeTest.java b/smithy-diff/src/test/java/software/amazon/smithy/diff/evaluators/ChangedShapeTypeTest.java
@@ -16,12 +16,15 @@
 package software.amazon.smithy.diff.evaluators;
 
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.equalTo;
 
 import java.util.List;
 import org.junit.jupiter.api.Test;
 import software.amazon.smithy.diff.ModelDiff;
 import software.amazon.smithy.model.Model;
+import software.amazon.smithy.model.node.Node;
+import software.amazon.smithy.model.shapes.ModelSerializer;
 import software.amazon.smithy.model.shapes.Shape;
 import software.amazon.smithy.model.shapes.StringShape;
 import software.amazon.smithy.model.shapes.TimestampShape;
@@ -40,4 +43,20 @@ public void detectsTypeChanges() {
         assertThat(TestHelper.findEvents(events, "ChangedShapeType").size(), equalTo(1));
         assertThat(TestHelper.findEvents(events, shapeA1.getId()).size(), equalTo(1));
     }
+
+    @Test
+    public void ignoresExpectedSetToListMigration() {
+        String rawModel = "$version: \"1.0\"\nnamespace smithy.example\nset Foo { member: String }\n";
+        Model oldModel = Model.assembler().addUnparsedModel("example.smithy", rawModel)
+                .assemble().unwrap();
+        Node serialized = ModelSerializer.builder().build().serialize(oldModel);
+        Model newModel = Model.assembler()
+                .addDocumentNode(serialized)
+                .assemble()
+                .unwrap();
+
+        List<ValidationEvent> events = ModelDiff.compare(oldModel, newModel);
+
+        assertThat(TestHelper.findEvents(events, "ChangedShapeType"), empty());
+    }
 }