Repository: smithy-lang/smithy
Pull request #2287 (2024-05-13 14:45:13)

Fix docs for `aws.iam#serviceResolvedConditionKeys` and `aws.iam#conditionKeyValue`
Besides missing imports, `aws.iam#conditionKeyValue` was referring to an
undefined condition key.

Error if you build the current model:

```
Shape: smithy.example#OperationAInput$key
File:  /home/ANT.AMAZON.COM/davidpz/code/smithy-rs-main/codegen-server-test/../codegen-core/common-test-models/simple.smithy:22:9

19| @conditionKeys(["myservice:ActionContextKey1"])
20| operation OperationA {
21|     input := {
22|         @conditionKeyValue("ActionContextKey1")
  |         ^

This operation `smithy.example#OperationA` scoped within the
`smithy.example#MyService` service with member
`smithy.example#OperationAInput$key` refers to an undefined condition key
`ActionContextKey1`. Expected one of the following defined condition keys:
[`myservice:ActionContextKey1`]
```

---
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.


david-perez: Would have been caught by https://github.com/smithy-lang/smithy/issues/298.

Patch:
diff --git a/docs/source-2.0/aws/aws-iam.rst b/docs/source-2.0/aws/aws-iam.rst
index 91377a1fe..570036b42 100644
--- a/docs/source-2.0/aws/aws-iam.rst
+++ b/docs/source-2.0/aws/aws-iam.rst
@@ -545,6 +545,10 @@ The following example defines two service-specific condition keys:
 
     namespace smithy.example
 
+    use aws.api#service
+    use aws.iam#defineConditionKeys
+    use aws.iam#serviceResolvedConditionKeys
+
     @defineConditionKeys(
         "myservice:ActionContextKey1": { type: "String" },
         "myservice:ActionContextKey2": { type: "String" }
@@ -583,6 +587,11 @@ explicitly binds ``ActionContextKey1`` to the field ``key``.
 
     namespace smithy.example
 
+    use aws.api#service
+    use aws.iam#conditionKeys
+    use aws.iam#defineConditionKeys
+    use aws.iam#conditionKeyValue
+
     @defineConditionKeys(
         "myservice:ActionContextKey1": { type: "String" }
     )
@@ -595,7 +604,7 @@ explicitly binds ``ActionContextKey1`` to the field ``key``.
     @conditionKeys(["myservice:ActionContextKey1"])
     operation OperationA {
         input := {
-            @conditionKeyValue("ActionContextKey1")
+            @conditionKeyValue("myservice:ActionContextKey1")
             key: String
         }
         output := {
diff --git a/smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/ChangedEnumTrait.java b/smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/ChangedEnumTrait.java
index 72cba13cd..75a494e20 100644
--- a/smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/ChangedEnumTrait.java
+++ b/smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/ChangedEnumTrait.java
@@ -43,6 +43,7 @@ public final class ChangedEnumTrait extends AbstractDiffEvaluator {
     private static final String ORDER_CHANGED = ".OrderChanged.";
     private static final String NAME_CHANGED = ".NameChanged.";
     private static final String REMOVED = ".Removed.";
+    private static final String APPENDED = ".Appended.";
 
     @Override
     public List<ValidationEvent> evaluate(Differences differences) {
@@ -135,8 +136,14 @@ public final class ChangedEnumTrait extends AbstractDiffEvaluator {
                     if (newTrait.isSynthetic()) {
                         appendedSource = change.getNewShape().getSourceLocation();
                     }
-                    events.add(note(change.getNewShape(), appendedSource, String.format(
-                            "Enum value `%s` was appended", definition.getValue())));
+                    events.add(
+                            ValidationEvent.builder()
+                                    .severity(Severity.NOTE)
+                                    .message(String.format("Enum value `%s` was appended", definition.getValue()))
+                                    .shape(change.getNewShape())
+                                    .sourceLocation(appendedSource)
+                                    .id(getEventId() + APPENDED + newPosition)
+                                    .build());
                 }
             }
 


Test patch:
diff --git a/smithy-diff/src/test/java/software/amazon/smithy/diff/evaluators/ChangedEnumTraitTest.java b/smithy-diff/src/test/java/software/amazon/smithy/diff/evaluators/ChangedEnumTraitTest.java
index a1d6fc755..9907f615a 100644
--- a/smithy-diff/src/test/java/software/amazon/smithy/diff/evaluators/ChangedEnumTraitTest.java
+++ b/smithy-diff/src/test/java/software/amazon/smithy/diff/evaluators/ChangedEnumTraitTest.java
@@ -47,6 +47,7 @@ public class ChangedEnumTraitTest {
                 .addTrait(EnumTrait.builder()
                         .addEnum(EnumDefinition.builder().value("foo").build())
                         .addEnum(EnumDefinition.builder().value("baz").build())
+                        .addEnum(EnumDefinition.builder().value("bar").build())
                         .sourceLocation(source)
                         .build())
                 .build();
@@ -54,9 +55,14 @@ public class ChangedEnumTraitTest {
         Model modelB = Model.assembler().addShape(s2).assemble().unwrap();
         List<ValidationEvent> events = ModelDiff.compare(modelA, modelB);
 
-        assertThat(TestHelper.findEvents(events, "ChangedEnumTrait").size(), equalTo(1));
-        assertThat(TestHelper.findEvents(events, "ChangedEnumTrait").get(0).getSeverity(), equalTo(Severity.NOTE));
-        assertThat(TestHelper.findEvents(events, "ChangedEnumTrait").get(0).getSourceLocation(), equalTo(source));
+        assertThat(TestHelper.findEvents(events, "ChangedEnumTrait").size(), equalTo(2));
+        assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.Appended").size(), equalTo(2));
+        assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.Appended.1").size(), equalTo(1));
+        assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.Appended.2").size(), equalTo(1));
+        assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.Appended").stream()
+                .allMatch(e -> e.getSeverity() == Severity.NOTE), equalTo(true));
+        assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.Appended").stream()
+                .allMatch(e -> source.equals(e.getSourceLocation())), equalTo(true));
     }
 
     @Test
@@ -86,9 +92,9 @@ public class ChangedEnumTraitTest {
                 equalTo(Severity.ERROR));
         assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.NameChanged.0").get(0).getSourceLocation(),
                 equalTo(source));
-        assertThat(TestHelper.findEvents(events, "ChangedEnumTrait").get(1).getSeverity(),
+        assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.Appended.1").get(0).getSeverity(),
                 equalTo(Severity.NOTE));
-        assertThat(TestHelper.findEvents(events, "ChangedEnumTrait").get(1).getSourceLocation(),
+        assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.Appended.1").get(0).getSourceLocation(),
                 equalTo(source));
     }
 
@@ -115,10 +121,10 @@ public class ChangedEnumTraitTest {
         Model modelB = Model.assembler().addShape(s2).assemble().unwrap();
         List<ValidationEvent> events = ModelDiff.compare(modelA, modelB);
 
-        assertThat(TestHelper.findEvents(events, "ChangedEnumTrait").size(), equalTo(1));
+        assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.Appended.1").size(), equalTo(1));
         assertThat(TestHelper.findEvents(events, "ChangedEnumTrait").get(0).getSourceLocation(),
                 equalTo(source));
-        assertThat(TestHelper.findEvents(events, "ChangedEnumTrait").stream()
+        assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.Appended").stream()
                 .allMatch(e -> e.getSeverity() == Severity.NOTE), equalTo(true));
     }
 
@@ -455,16 +461,15 @@ public class ChangedEnumTraitTest {
 
         List<ValidationEvent> changeEvents = TestHelper.findEvents(allEvents, "ChangedEnumTrait");
         assertThat(changeEvents.size(), equalTo(2));
-        assertThat(TestHelper.findEvents(changeEvents, "ChangedEnumTrait.Removed.1").size(), equalTo(1));
-        assertThat(TestHelper.findEvents(changeEvents, "ChangedEnumTrait.Removed.1").get(0).getSourceLocation(),
-                equalTo(beforeSource));
 
-        ValidationEvent removedEvent = changeEvents.get(0);
+        assertThat(TestHelper.findEvents(changeEvents, "ChangedEnumTrait.Removed.1").size(), equalTo(1));
+        ValidationEvent removedEvent = TestHelper.findEvents(changeEvents, "ChangedEnumTrait.Removed.1").get(0);
         assertThat(removedEvent.getSeverity(), equalTo(Severity.ERROR));
         assertThat(removedEvent.getMessage(), stringContainsInOrder("Enum value `old2` was removed"));
         assertThat(removedEvent.getSourceLocation(), equalTo(beforeSource));
 
-        ValidationEvent appendedEvent = changeEvents.get(1);
+        assertThat(TestHelper.findEvents(changeEvents, "ChangedEnumTrait.Appended.2").size(), equalTo(1));
+        ValidationEvent appendedEvent = TestHelper.findEvents(changeEvents, "ChangedEnumTrait.Appended.2").get(0);
         assertThat(appendedEvent.getSeverity(), equalTo(Severity.NOTE));
         assertThat(appendedEvent.getMessage(), stringContainsInOrder("Enum value `new1` was appended"));
         assertThat(appendedEvent.getSourceLocation(), equalTo(afterSource));
@@ -512,7 +517,7 @@ public class ChangedEnumTraitTest {
                 equalTo(afterSource));
         assertThat(TestHelper.findEvents(events, "ChangedEnumTrait").subList(0, 3).stream()
                 .allMatch(e -> e.getSeverity() == Severity.ERROR), equalTo(true));
-        assertThat(TestHelper.findEvents(events, "ChangedEnumTrait").subList(3, 4).stream()
+        assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.Appended.2").stream()
                 .allMatch(e -> e.getSeverity() == Severity.NOTE), equalTo(true));
     }
 
@@ -553,7 +558,7 @@ public class ChangedEnumTraitTest {
                 equalTo(source));
         assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.Removed.1").stream()
                 .allMatch(e -> e.getSeverity() == Severity.ERROR), equalTo(true));
-        assertThat(TestHelper.findEvents(events, "ChangedEnumTrait").subList(1, 2).stream()
+        assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.Appended.2").stream()
                 .allMatch(e -> e.getSeverity() == Severity.NOTE), equalTo(true));
     }
 }