Repository: smithy-lang/smithy
Pull request #1972 (2023-09-06 14:24:09)

Correct smithy-diff nullability error messages
These messages reflect a previous design in Smithy that was refactored to rename @nullable to @clientOptional, but these error messages and IDs were missed in Smithy-Diff. This change also simplifies the error message for adding a required trait to remove extra detail (this message is only presented when relevant so the detail isn't needed).

*Issue #, if available:*

*Description of changes:*


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.



Patch:
diff --git a/smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/ChangedNullability.java b/smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/ChangedNullability.java
index 0831aaae4..8d2963ee5 100644
--- a/smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/ChangedNullability.java
+++ b/smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/ChangedNullability.java
@@ -110,15 +110,15 @@ public class ChangedNullability extends AbstractDiffEvaluator {
             eventsToAdd.add(emit(Severity.DANGER, "AddedInputTrait", shape, message,
                             "The @input trait was added to " + newMember.getContainer()));
         } else if (!newHasInput) {
-            // Can't add nullable to a preexisting required member.
+            // Can't add clientOptional to a preexisting required member.
             if (change.isTraitAdded(ClientOptionalTrait.ID) && change.isTraitInBoth(RequiredTrait.ID)) {
-                eventsToAdd.add(emit(Severity.ERROR, "AddedNullableTrait", shape, message,
-                                "The @nullable trait was added to a @required member."));
+                eventsToAdd.add(emit(Severity.ERROR, "AddedClientOptionalTrait", shape, message,
+                                "The @clientOptional trait was added to a @required member."));
             }
-            // Can't add required to a member unless the member is marked as nullable.
+            // Can't add required to a member unless the member is marked as @clientOptional or part of @input.
             if (change.isTraitAdded(RequiredTrait.ID) && !newMember.hasTrait(ClientOptionalTrait.ID)) {
                 eventsToAdd.add(emit(Severity.ERROR, "AddedRequiredTrait", shape, message,
-                                "The @required trait was added to a member that is not marked as @nullable."));
+                                "The @required trait was added to a member."));
             }
             // Can't add the default trait to a member unless the member was previously required.
             if (change.isTraitAdded(DefaultTrait.ID) && !change.isTraitRemoved(RequiredTrait.ID)) {


Test patch:
diff --git a/smithy-diff/src/test/java/software/amazon/smithy/diff/evaluators/ChangedNullabilityTest.java b/smithy-diff/src/test/java/software/amazon/smithy/diff/evaluators/ChangedNullabilityTest.java
index b18d07acc..0205a2990 100644
--- a/smithy-diff/src/test/java/software/amazon/smithy/diff/evaluators/ChangedNullabilityTest.java
+++ b/smithy-diff/src/test/java/software/amazon/smithy/diff/evaluators/ChangedNullabilityTest.java
@@ -159,13 +159,12 @@ public class ChangedNullabilityTest {
         assertThat(events.stream()
                            .filter(event -> event.getSeverity() == Severity.ERROR)
                            .filter(event -> event.getId().equals("ChangedNullability.AddedRequiredTrait"))
-                           .filter(event -> event.getMessage().contains("The @required trait was added to a member "
-                                                                        + "that is not marked as @nullable"))
+                           .filter(event -> event.getMessage().contains("The @required trait was added to a member"))
                            .count(), equalTo(1L));
     }
 
     @Test
-    public void detectAdditionOfNullableTrait() {
+    public void detectAdditionOfClientOptionalTrait() {
         MemberShape member1 = MemberShape.builder()
                 .id("foo.baz#Baz$bam")
                 .target("foo.baz#String")
@@ -181,8 +180,8 @@ public class ChangedNullabilityTest {
 
         assertThat(events.stream()
                            .filter(event -> event.getSeverity() == Severity.ERROR)
-                           .filter(event -> event.getId().equals("ChangedNullability.AddedNullableTrait"))
-                           .filter(event -> event.getMessage().contains("The @nullable trait was added to a "
+                           .filter(event -> event.getId().equals("ChangedNullability.AddedClientOptionalTrait"))
+                           .filter(event -> event.getMessage().contains("The @clientOptional trait was added to a "
                                                                         + "@required member"))
                            .count(), equalTo(1L));
     }