Fix ModifiedTrait for traits with breaking change rules (#2249)

smithy-lang · Apr 19, 2024 · 2883dc7 · 2883dc7
1 parent d7a37af
commit 2883dc7
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 2 deletions.
diff --git a/smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/ModifiedTrait.java b/smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/ModifiedTrait.java
@@ -145,8 +145,10 @@ private static Map<ShapeId, List<DiffStrategy>> computeDiffStrategies(Model mode
             List<DiffStrategy> strategies = createStrategiesForShape(shape, true);
             if (!strategies.isEmpty()) {
                 result.put(shape.getId(), strategies);
-            } else if (definition.getBreakingChanges().isEmpty()) {
-                // Avoid duplicate validation events; only perform the default validation when there are no diff rules.
+            } else if (!definition.getBreakingChanges().isEmpty()) {
+                // Avoid duplicate validation events; delegate emitting events to TraitBreakingChange.
+                result.put(shape.getId(), Collections.emptyList());
+            } else {
                 result.put(shape.getId(), DEFAULT_STRATEGIES);
             }
         }

diff --git a/smithy-diff/src/test/java/software/amazon/smithy/diff/evaluators/ModifiedTraitTest.java b/smithy-diff/src/test/java/software/amazon/smithy/diff/evaluators/ModifiedTraitTest.java
@@ -36,6 +36,7 @@
 import software.amazon.smithy.model.shapes.ShapeId;
 import software.amazon.smithy.model.shapes.StringShape;
 import software.amazon.smithy.model.traits.DynamicTrait;
+import software.amazon.smithy.model.traits.JsonNameTrait;
 import software.amazon.smithy.model.traits.RequiredTrait;
 import software.amazon.smithy.model.traits.TagsTrait;
 import software.amazon.smithy.model.traits.TraitDefinition;
@@ -366,4 +367,23 @@ public void letsOtherValidatorsHandleRequiredTrait() {
 
         assertThat(TestHelper.findEvents(ModelDiff.compare(oldModel, newModel), "ModifiedTrait"), empty());
     }
+
+    @Test
+    public void letsTraitBreakingChangeHandleDiffEvents() {
+        String originalModel =
+            "$version: \"2.0\"\n"
+          + "namespace smithy.example\n"
+          + "structure Test {\n"
+          + "   @jsonName(\"a\")\n"
+          + "   member: String\n"
+          + "}";
+        Model oldModel = Model.assembler().addUnparsedModel("foo.smithy", originalModel).assemble().unwrap();
+        Model newModel = ModelTransformer.create().replaceShapes(oldModel, ListUtils.of(
+                Shape.shapeToBuilder(oldModel.expectShape(ShapeId.from("smithy.example#Test$member")))
+                        .removeTrait(JsonNameTrait.ID)
+                        .build()));
+
+        assertThat(TestHelper.findEvents(ModelDiff.compare(oldModel, newModel), "ModifiedTrait"), empty());
+        assertThat(TestHelper.findEvents(ModelDiff.compare(oldModel, newModel), "TraitBreakingChange"), hasSize(1));
+    }
 }
diff --git a/...t/resources/software/amazon/smithy/diff/evaluators/trait-modified-all-severities-a.smithy b/...t/resources/software/amazon/smithy/diff/evaluators/trait-modified-all-severities-a.smithy
@@ -56,4 +56,7 @@ structure aTrait {
 
     @tags(["diff.warning.const"])
     l: String,
+
+    @tags(["diff.contents"])
+    m: String,
 }
diff --git a/...t/resources/software/amazon/smithy/diff/evaluators/trait-modified-all-severities-b.smithy b/...t/resources/software/amazon/smithy/diff/evaluators/trait-modified-all-severities-b.smithy
@@ -56,4 +56,7 @@ structure aTrait {
 
     @tags(["diff.warning.const"])
     l: String,
+
+    @tags(["diff.contents"])
+    m: String,
 }