Add transform that can remove invalid defaults

If a default trait is incompatible with the range trait of the member
or the member target, then the shape is essentially in a state where
the member is invalid: omit a value for the member at it is
automatically incompatible with the range trait of the member.

This transformer finds such cases and removes invalid default values.
It is likely that services with such a model did not intend for the
members to actually have a default value, and this typically only
happens with members that have a default value of `0`.

Author: mtdowling

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

@@ -656,4 +656,14 @@ public Model addClientOptional(Model model, boolean applyWhenNoDefaultValue) {
     public Model downgradeToV1(Model model) {
         return new DowngradeToV1().transform(this, model);
     }
+
+    /**
+     * Remove default traits if the default conflicts with the range trait of the shape.
+     *
+     * @param model Model to transform.
+     * @return Returns the transformed model.
+     */
+    public Model removeInvalidDefaults(Model model) {
+        return new RemoveInvalidDefaults().transform(this, model);
+    }
 }

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

@@ -0,0 +1,75 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package software.amazon.smithy.model.transform;
+
+import java.util.HashSet;
+import java.util.Set;
+import java.util.logging.Logger;
+import software.amazon.smithy.model.Model;
+import software.amazon.smithy.model.node.Node;
+import software.amazon.smithy.model.shapes.MemberShape;
+import software.amazon.smithy.model.shapes.Shape;
+import software.amazon.smithy.model.traits.DefaultTrait;
+import software.amazon.smithy.model.traits.RangeTrait;
+
+/**
+ * Removes default values from shapes where the default value is incompatible with
+ * the constraint traits of the shape.
+ */
+final class RemoveInvalidDefaults {
+
+    private static final Logger LOGGER = Logger.getLogger(RemoveInvalidDefaults.class.getName());
+
+    Model transform(ModelTransformer transformer, Model model) {
+        Set<Shape> invalidDefaults = new HashSet<>();
+        Set<Shape> updates = new HashSet<>();
+
+        // First collect invalid shapes. Members with invalid defaults either need to remove the default or
+        // set the default to null if their target shape's default remains intact but the member is invalid.
+        for (Shape shape : model.getShapesWithTrait(DefaultTrait.class)) {
+            shape.getMemberTrait(model, RangeTrait.class).ifPresent(rangeTrait -> {
+                DefaultTrait defaultTrait = shape.expectTrait(DefaultTrait.class);
+                if (defaultTrait.toNode().isNumberNode()) {
+                    defaultTrait.toNode().expectNumberNode().asBigDecimal().ifPresent(value -> {
+                        if (rangeTrait.getMin().filter(min -> value.compareTo(min) < 0).isPresent()
+                                || rangeTrait.getMin().filter(max -> value.compareTo(max) > 0).isPresent()) {
+                            invalidDefaults.add(shape);
+                        }
+                    });
+                }
+            });
+        }
+
+        for (Shape shape : invalidDefaults) {
+            updates.add(modify(shape, model, invalidDefaults));
+        }
+
+        return transformer.replaceShapes(model, updates);
+    }
+
+    private Shape modify(Shape shape, Model model, Set<Shape> otherShapes) {
+        // To show up here, the shape has to have a range trait, or the target has to have one.
+        RangeTrait rangeTrait = shape.getMemberTrait(model, RangeTrait.class).get();
+        LOGGER.info(() -> "Removing default trait from " + shape.getId()
+                          + " because of an incompatible range trait: "
+                          + Node.printJson(rangeTrait.toNode()));
+
+        // Members that target a shape with a default value need to set their default to null to override it.
+        // Other members and other shapes can simply remove the default trait.
+        if (shape.isMemberShape()) {
+            MemberShape member = shape.asMemberShape().get();
+            boolean targetHasDefault = model.getShape(member.getTarget())
+                    // Treat target shapes that will have their default removed as if it doesn't have a default.
+                    .filter(target -> !otherShapes.contains(target) && target.hasTrait(DefaultTrait.class))
+                    .isPresent();
+            if (targetHasDefault) {
+                return member.toBuilder().addTrait(new DefaultTrait(Node.nullNode())).build();
+            }
+        }
+
+        return Shape.shapeToBuilder(shape).removeTrait(DefaultTrait.ID).build();
+    }
+}

smithy-model/src/main/java/software/amazon/smithy/model/validation/node/RangeTraitPlugin.java

@@ -15,7 +15,6 @@
 
 package software.amazon.smithy.model.validation.node;
 
-import java.math.BigDecimal;
 import software.amazon.smithy.model.node.Node;
 import software.amazon.smithy.model.node.Node.NonNumericFloat;
 import software.amazon.smithy.model.node.NumberNode;
@@ -70,27 +69,26 @@ private void checkNonNumeric(Shape shape, RangeTrait trait, StringNode node, Emi
     }
 
     protected void check(Shape shape, Context context, RangeTrait trait, NumberNode node, Emitter emitter) {
-        Number number = node.getValue();
-        BigDecimal decimal = number instanceof BigDecimal
-                ? (BigDecimal) number
-                : new BigDecimal(number.toString());
-
         trait.getMin().ifPresent(min -> {
-            if (decimal.compareTo(new BigDecimal(min.toString())) < 0) {
-                emitter.accept(node, getSeverity(node, context), String.format(
-                        "Value provided for `%s` must be greater than or equal to %s, but found %s",
-                        shape.getId(), min, number),
-                        shape.isMemberShape() ? MEMBER : TARGET);
-            }
+            node.asBigDecimal().ifPresent(decimal -> {
+                if (decimal.compareTo(min) < 0) {
+                    emitter.accept(node, getSeverity(node, context), String.format(
+                                           "Value provided for `%s` must be greater than or equal to %s, but found %s",
+                                           shape.getId(), min, decimal),
+                                   shape.isMemberShape() ? MEMBER : TARGET);
+                }
+            });
         });
 
         trait.getMax().ifPresent(max -> {
-            if (decimal.compareTo(new BigDecimal(max.toString())) > 0) {
-                emitter.accept(node, getSeverity(node, context), String.format(
-                        "Value provided for `%s` must be less than or equal to %s, but found %s",
-                        shape.getId(), max, number),
-                        shape.isMemberShape() ? MEMBER : TARGET);
-            }
+            node.asBigDecimal().ifPresent(decimal -> {
+                if (decimal.compareTo(max) > 0) {
+                    emitter.accept(node, getSeverity(node, context), String.format(
+                                           "Value provided for `%s` must be less than or equal to %s, but found %s",
+                                           shape.getId(), max, decimal),
+                                   shape.isMemberShape() ? MEMBER : TARGET);
+                }
+            });
         });
     }
 

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

@@ -0,0 +1,33 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package software.amazon.smithy.model.transform;
+
+import org.junit.jupiter.api.Test;
+import software.amazon.smithy.model.Model;
+import software.amazon.smithy.model.node.Node;
+import software.amazon.smithy.model.shapes.ModelSerializer;
+
+public class RemoveInvalidDefaultsTest {
+    @Test
+    public void removeInvalidDefaultsBasedOnRangeTrait() {
+        Model input = Model.assembler()
+                .addImport(getClass().getResource("bad-defaults-range-trait.smithy"))
+                .assemble()
+                .unwrap();
+        Model output = Model.assembler()
+                .addImport(getClass().getResource("bad-defaults-range-trait.fixed.smithy"))
+                .assemble()
+                .unwrap();
+
+        ModelTransformer transformer = ModelTransformer.create();
+
+        Model result = transformer.removeInvalidDefaults(input);
+
+        Node actual = ModelSerializer.builder().build().serialize(result);
+        Node expected = ModelSerializer.builder().build().serialize(output);
+        Node.assertEquals(actual, expected);
+    }
+}

smithy-model/src/test/resources/software/amazon/smithy/model/transform/bad-defaults-range-trait.fixed.smithy

@@ -0,0 +1,39 @@
+$version: "2.0"
+
+namespace smithy.example
+
+structure Foo {
+    // Default was removed.
+    @range(min: 1)
+    invalid1: Integer
+
+    // Default was removed.
+    invalid2: ValueGreaterThanZero
+
+    // The default of the target shape was removed, so it can be removed here too.
+    invalid3: ValueGreaterThanZeroWithDefault
+
+    // Cancel out the root level default.
+    @range(min: 1)
+    invalid4: PrimitiveInteger = null
+
+    valid1: ValueGreaterThanZero = 1
+
+    valid2: ValueGreaterThanZero
+
+    valid3: Integer
+
+    valid4: Integer = 0
+
+    @range(min: 1)
+    valid5: Integer
+
+    @range(min: 1)
+    valid6: Integer = 1
+}
+
+@range(min: 1)
+integer ValueGreaterThanZero
+
+@range(min: 1)
+integer ValueGreaterThanZeroWithDefault // default was removed

smithy-model/src/test/resources/software/amazon/smithy/model/transform/bad-defaults-range-trait.smithy

@@ -0,0 +1,41 @@
+$version: "2.0"
+
+namespace smithy.example
+
+structure Foo {
+    // The member itself creates an invalid combination of the range trait and default value.
+    @range(min: 1)
+    invalid1: Integer = 0
+
+    // The member adds a default value that is incompatible with the target shape.
+    invalid2: ValueGreaterThanZero = 0
+
+    // The member targets a shape where the range trait is incompatible with the default of the member.
+    invalid3: ValueGreaterThanZeroWithDefault = 0
+
+    // The range trait here makes the default value invalid. This member targets a root shape with a default, so the
+    // default has to be set to null to cancel out the root level default.
+    @range(min: 1)
+    invalid4: PrimitiveInteger = 0
+
+    valid1: ValueGreaterThanZero = 1
+
+    valid2: ValueGreaterThanZero
+
+    valid3: Integer
+
+    valid4: Integer = 0
+
+    @range(min: 1)
+    valid5: Integer
+
+    @range(min: 1)
+    valid6: Integer = 1
+}
+
+@range(min: 1)
+integer ValueGreaterThanZero
+
+@range(min: 1)
+@default(0) // bad default and range trait combination.
+integer ValueGreaterThanZeroWithDefault