Changed scalar removal to WARNING

rchache · Jan 2, 2024 · 965b13e · 965b13e
1 parent dbab6d2
commit 965b13e
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 20 deletions.
diff --git a/smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/RemovedShape.java b/smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/RemovedShape.java
@@ -19,22 +19,30 @@
 import java.util.stream.Collectors;
 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.EnumTrait;
 import software.amazon.smithy.model.traits.PrivateTrait;
+import software.amazon.smithy.model.validation.Severity;
 import software.amazon.smithy.model.validation.ValidationEvent;
 
 /**
  * Creates an ERROR event when a non-private non-scalar shape is removed.
- * Creates a NOTE event when a non-private scalar shape is removed.
+ * Creates a WARNING event when a non-private scalar shape is removed.
  */
 public final class RemovedShape extends AbstractDiffEvaluator {
     @Override
     public List<ValidationEvent> evaluate(Differences differences) {
         return differences.removedShapes()
                 .filter(shape -> !shape.hasTrait(PrivateTrait.class))
                 .filter(shape -> !isMemberOfRemovedShape(shape, differences))
-                .map(shape -> isScalarType(shape)
-                        ? note(shape, String.format("Removed %s `%s`", shape.getType(), shape.getId()))
+                .map(shape -> isInconsequentialType(shape)
+                        ? ValidationEvent.builder()
+                            .severity(Severity.WARNING)
+                            .message(String.format("Removed %s `%s`", shape.getType(), shape.getId()))
+                            .shapeId(shape.getId())
+                            .id(getEventId() + ".ScalarShape")
+                            .sourceLocation(shape.getSourceLocation())
+                            .build()
                         : error(shape, String.format("Removed %s `%s`", shape.getType(), shape.getId())))
                 .collect(Collectors.toList());
     }
@@ -45,18 +53,19 @@ private boolean isMemberOfRemovedShape(Shape shape, Differences differences) {
                 .isPresent();
     }
 
-    private boolean isScalarType(Shape shape) {
-        return shape.isBigDecimalShape()
-            || shape.isBigIntegerShape()
-            || shape.isBlobShape()
-            || shape.isBooleanShape()
-            || shape.isByteShape()
-            || shape.isDoubleShape()
-            || shape.isFloatShape()
-            || shape.isShortShape()
-            || shape.isTimestampShape()
-            || shape.isLongShape()
-            || (shape.isStringShape() && !shape.hasTrait(EnumTrait.class))
-            || (shape.isIntegerShape() && !shape.isIntEnumShape());
+    private boolean isInconsequentialType(Shape shape) {
+        ShapeType shapeType = shape.getType();
+        return shapeType == ShapeType.BIG_DECIMAL
+                || shapeType == ShapeType.BIG_INTEGER
+                || shapeType == ShapeType.BLOB
+                || shapeType == ShapeType.BOOLEAN
+                || shapeType == ShapeType.BYTE
+                || shapeType == ShapeType.DOUBLE
+                || shapeType == ShapeType.FLOAT
+                || shapeType == ShapeType.SHORT
+                || shapeType == ShapeType.TIMESTAMP
+                || shapeType == ShapeType.LONG
+                || ((shapeType == ShapeType.STRING) && !shape.hasTrait(EnumTrait.class))
+                || (shapeType == ShapeType.INTEGER);
     }
 }
diff --git a/smithy-diff/src/test/java/software/amazon/smithy/diff/evaluators/RemovedShapeTest.java b/smithy-diff/src/test/java/software/amazon/smithy/diff/evaluators/RemovedShapeTest.java
@@ -62,7 +62,7 @@ public void detectsShapeRemoval() {
     }
 
     @Test
-    public void emitsNotesForScalarShapes() {
+    public void emitsWarningsForScalarShapes() {
         Shape[] scalarShapes = new Shape[] {
                 IntegerShape.builder().id("foo.baz#BazOne").build(),
                 BigDecimalShape.builder().id("foo.baz#BazTwo").build(),
@@ -81,9 +81,9 @@ public void emitsNotesForScalarShapes() {
         Model modelB = Model.assembler().assemble().unwrap();
         List<ValidationEvent> events = ModelDiff.compare(modelA, modelB);
 
-        assertThat(TestHelper.findEvents(events, "RemovedShape").size(), equalTo(12));
-        assertThat("Scalar removals should be NOTE severity",
-                events.stream().allMatch(event -> Severity.NOTE.equals(event.getSeverity())));
+        assertThat(TestHelper.findEvents(events, "RemovedShape.ScalarShape").size(), equalTo(12));
+        assertThat("Scalar removals should be WARNING severity",
+                events.stream().allMatch(event -> Severity.WARNING.equals(event.getSeverity())));
     }
 
     @Test