Skip to content

Commit 319389f

Browse files
mtdowlingMichael Dowling
authored and
Michael Dowling
committed
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.
1 parent 8ac4ecd commit 319389f

File tree

2 files changed

+37
-0
lines changed

2 files changed

+37
-0
lines changed

smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/ChangedShapeType.java

+18
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@
1717

1818
import java.util.List;
1919
import java.util.stream.Collectors;
20+
import software.amazon.smithy.diff.ChangedShape;
2021
import software.amazon.smithy.diff.Differences;
22+
import software.amazon.smithy.model.shapes.Shape;
23+
import software.amazon.smithy.model.shapes.ShapeType;
24+
import software.amazon.smithy.model.traits.UniqueItemsTrait;
2125
import software.amazon.smithy.model.validation.ValidationEvent;
2226

2327
/**
@@ -28,9 +32,23 @@ public final class ChangedShapeType extends AbstractDiffEvaluator {
2832
public List<ValidationEvent> evaluate(Differences differences) {
2933
return differences.changedShapes()
3034
.filter(diff -> diff.getOldShape().getType() != diff.getNewShape().getType())
35+
.filter(diff -> !expectedSetToListChange(diff))
3136
.map(diff -> error(diff.getNewShape(), String.format(
3237
"Shape `%s` type was changed from `%s` to `%s`.",
3338
diff.getShapeId(), diff.getOldShape().getType(), diff.getNewShape().getType())))
3439
.collect(Collectors.toList());
3540
}
41+
42+
private boolean expectedSetToListChange(ChangedShape<Shape> diff) {
43+
ShapeType oldType = diff.getOldShape().getType();
44+
ShapeType newType = diff.getNewShape().getType();
45+
46+
// Smithy diff doesn't raise an issue if a set is changed to a list and the list
47+
// has the uniqueItems trait. Set is deprecated and this is a recommended change.
48+
if (oldType == ShapeType.SET && newType == ShapeType.LIST) {
49+
return diff.getNewShape().hasTrait(UniqueItemsTrait.class);
50+
}
51+
52+
return false;
53+
}
3654
}

smithy-diff/src/test/java/software/amazon/smithy/diff/evaluators/ChangedShapeTypeTest.java

+19
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,15 @@
1616
package software.amazon.smithy.diff.evaluators;
1717

1818
import static org.hamcrest.MatcherAssert.assertThat;
19+
import static org.hamcrest.Matchers.empty;
1920
import static org.hamcrest.Matchers.equalTo;
2021

2122
import java.util.List;
2223
import org.junit.jupiter.api.Test;
2324
import software.amazon.smithy.diff.ModelDiff;
2425
import software.amazon.smithy.model.Model;
26+
import software.amazon.smithy.model.node.Node;
27+
import software.amazon.smithy.model.shapes.ModelSerializer;
2528
import software.amazon.smithy.model.shapes.Shape;
2629
import software.amazon.smithy.model.shapes.StringShape;
2730
import software.amazon.smithy.model.shapes.TimestampShape;
@@ -40,4 +43,20 @@ public void detectsTypeChanges() {
4043
assertThat(TestHelper.findEvents(events, "ChangedShapeType").size(), equalTo(1));
4144
assertThat(TestHelper.findEvents(events, shapeA1.getId()).size(), equalTo(1));
4245
}
46+
47+
@Test
48+
public void ignoresExpectedSetToListMigration() {
49+
String rawModel = "$version: \"1.0\"\nnamespace smithy.example\nset Foo { member: String }\n";
50+
Model oldModel = Model.assembler().addUnparsedModel("example.smithy", rawModel)
51+
.assemble().unwrap();
52+
Node serialized = ModelSerializer.builder().build().serialize(oldModel);
53+
Model newModel = Model.assembler()
54+
.addDocumentNode(serialized)
55+
.assemble()
56+
.unwrap();
57+
58+
List<ValidationEvent> events = ModelDiff.compare(oldModel, newModel);
59+
60+
assertThat(TestHelper.findEvents(events, "ChangedShapeType"), empty());
61+
}
4362
}

0 commit comments

Comments
 (0)