Improve diff, rename ephemeral to synthetic

This commit improves how Smithy diff reports adding/removing/changing
shapes and traits so that it's more readable. This was something I had
to do in order to make sense of the impact of this transform on AWS
models, so it's included in this PR. I added logging to the transform,
and renamed "ephemeral" traits to "synthetic" traits to better match
other existing Smithy nomenclature.

smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/AddedShape.java

@@ -18,6 +18,7 @@
 import java.util.List;
 import java.util.stream.Collectors;
 import software.amazon.smithy.diff.Differences;
+import software.amazon.smithy.model.shapes.Shape;
 import software.amazon.smithy.model.validation.ValidationEvent;
 
 /**
@@ -27,9 +28,14 @@ public final class AddedShape extends AbstractDiffEvaluator {
     @Override
     public List<ValidationEvent> evaluate(Differences differences) {
         return differences.addedShapes()
-                .map(shape -> note(shape, String.format(
-                        "Shape `%s` of type `%s` was added to the model",
-                        shape.getId(), shape.getType())))
+                .filter(shape -> !isMemberOfAddedShape(shape, differences))
+                .map(shape -> note(shape, String.format("Added %s `%s`", shape.getType(), shape.getId())))
                 .collect(Collectors.toList());
     }
+
+    private boolean isMemberOfAddedShape(Shape shape, Differences differences) {
+        return shape.asMemberShape()
+                .filter(member -> !differences.getOldModel().getShapeIds().contains(member.getContainer()))
+                .isPresent();
+    }
 }

smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/ChangedOperationInput.java

@@ -31,10 +31,10 @@ public List<ValidationEvent> evaluate(Differences differences) {
         return differences.changedShapes(OperationShape.class)
                 .filter(change -> !change.getOldShape().getInputShape().equals(change.getNewShape().getInputShape()))
                 .map(change -> error(change.getNewShape(), String.format(
-                        "Input shape of `%s` changed from `%s` to `%s`",
+                        "Changed operation input of `%s` from `%s` to `%s`",
                         change.getShapeId(),
                         change.getOldShape().getInputShape(),
-                        change.getNewShape().getOutputShape())))
+                        change.getNewShape().getInputShape())))
                 .collect(Collectors.toList());
     }
 }

smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/ChangedOperationOutput.java

@@ -31,7 +31,7 @@ public List<ValidationEvent> evaluate(Differences differences) {
         return differences.changedShapes(OperationShape.class)
                 .filter(change -> !change.getOldShape().getOutputShape().equals(change.getNewShape().getOutputShape()))
                 .map(change -> error(change.getNewShape(), String.format(
-                        "Output shape of `%s` changed from `%s` to `%s`",
+                        "Changed operation output of `%s` from `%s` to `%s`",
                         change.getShapeId(),
                         change.getOldShape().getOutputShape(),
                         change.getNewShape().getOutputShape())))

smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/ModifiedTrait.java

@@ -209,13 +209,10 @@ List<ValidationEvent> validate(
                 String message;
                 String pretty = Node.prettyPrintJson(right.toNode());
                 if (path.isEmpty()) {
-                    String template = severity == Severity.DANGER || severity == Severity.ERROR
-                                      ? "It is a breaking change to add the `%s` trait. The added trait value is: %s"
-                                      : "The `%s` trait was added with the value: %s";
-                    message = String.format(template, trait, pretty);
+                    message = String.format("Added trait `%s` with value %s", trait, pretty);
                 } else {
-                    message = String.format("`%s` was added to the `%s` trait with a value of %s",
-                                            path, trait, pretty);
+                    message = String.format("Added trait contents to `%s` at path `%s` with value %s",
+                                            trait, path, pretty);
                 }
 
                 return Collections.singletonList(ValidationEvent.builder()
@@ -246,13 +243,10 @@ List<ValidationEvent> validate(
                 String pretty = Node.prettyPrintJson(left.toNode());
                 String message;
                 if (path.isEmpty()) {
-                    String template = severity == Severity.DANGER || severity == Severity.ERROR
-                            ? "It is a breaking change to remove the `%s` trait. The removed trait value was: %s"
-                            : "The `%s` trait was removed. The removed trait value was: %s";
-                    message = String.format(template, trait, pretty);
+                    message = String.format("Removed trait `%s`. Removed value: %s", trait, pretty);
                 } else {
-                    message = String.format("`%s` was removed from the `%s` trait. The removed value was: %s",
-                                            path, trait, pretty);
+                    message = String.format("Removed trait contents from `%s` at path `%s`. Removed value: %s",
+                                            trait, path, pretty);
                 }
 
                 return Collections.singletonList(ValidationEvent.builder()
@@ -283,14 +277,10 @@ List<ValidationEvent> validate(
                 String rightPretty = Node.prettyPrintJson(right.toNode());
                 String message;
                 if (path.isEmpty()) {
-                    String template = severity == Severity.DANGER || severity == Severity.ERROR
-                                      ? "It is a breaking change to change the value of the `%s` trait. The value "
-                                        + "changed from %s to %s"
-                                      : "The `%s` trait value changed from %s to %s";
-                    message = String.format(template, trait, leftPretty, rightPretty);
+                    message = String.format("Changed trait `%s` from %s to %s", trait, leftPretty, rightPretty);
                 } else {
-                    message = String.format("`%s` was changed on the `%s` trait from %s to %s",
-                                            path, trait, leftPretty, rightPretty);
+                    message = String.format("Changed trait contents of `%s` at path `%s` from %s to %s",
+                                            trait, path, leftPretty, rightPretty);
                 }
 
                 return Collections.singletonList(ValidationEvent.builder()

smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/RemovedShape.java

@@ -18,6 +18,7 @@
 import java.util.List;
 import java.util.stream.Collectors;
 import software.amazon.smithy.diff.Differences;
+import software.amazon.smithy.model.shapes.Shape;
 import software.amazon.smithy.model.traits.PrivateTrait;
 import software.amazon.smithy.model.validation.ValidationEvent;
 
@@ -29,9 +30,14 @@ public final class RemovedShape extends AbstractDiffEvaluator {
     public List<ValidationEvent> evaluate(Differences differences) {
         return differences.removedShapes()
                 .filter(shape -> !shape.hasTrait(PrivateTrait.class))
-                .map(shape -> error(shape, String.format(
-                        "Shape `%s` of type `%s` was removed from the model: %s",
-                        shape.getId(), shape.getType(), shape)))
+                .filter(shape -> !isMemberOfRemovedShape(shape, differences))
+                .map(shape -> error(shape, String.format("Removed %s `%s`", shape.getType(), shape.getId())))
                 .collect(Collectors.toList());
     }
+
+    private boolean isMemberOfRemovedShape(Shape shape, Differences differences) {
+        return shape.asMemberShape()
+                .filter(member -> !differences.getNewModel().getShapeIds().contains(member.getContainer()))
+                .isPresent();
+    }
 }

smithy-diff/src/test/java/software/amazon/smithy/diff/evaluators/AddedShapeTest.java

@@ -22,6 +22,8 @@
 import org.junit.jupiter.api.Test;
 import software.amazon.smithy.diff.ModelDiff;
 import software.amazon.smithy.model.Model;
+import software.amazon.smithy.model.shapes.ListShape;
+import software.amazon.smithy.model.shapes.MemberShape;
 import software.amazon.smithy.model.shapes.Shape;
 import software.amazon.smithy.model.shapes.StringShape;
 import software.amazon.smithy.model.validation.ValidationEvent;
@@ -37,4 +39,17 @@ public void detectsShapeAdded() {
 
         assertThat(TestHelper.findEvents(events, "AddedShape").size(), equalTo(1));
     }
+
+    @Test
+    public void doesNotEmitForMembersOfAddedContainerShapes() {
+        Shape string = StringShape.builder().id("foo.baz#Baz").build();
+        MemberShape member = MemberShape.builder().id("foo.baz#Bam$member").target(string).build();
+        ListShape list = ListShape.builder().id("foo.baz#Bam").addMember(member).build();
+        Model modelA = Model.assembler().addShapes(string).assemble().unwrap();
+        Model modelB = Model.assembler().addShapes(list, string).assemble().unwrap();
+        List<ValidationEvent> events = ModelDiff.compare(modelA, modelB);
+
+        assertThat(TestHelper.findEvents(events, "AddedShape").size(), equalTo(1));
+        assertThat(TestHelper.findEvents(events, list.getId()).size(), equalTo(1));
+    }
 }

smithy-diff/src/test/java/software/amazon/smithy/diff/evaluators/ChangedPaginatedTraitTest.java

@@ -74,7 +74,7 @@ public void detectsRemovalOfItems() {
 
         assertThat(changedTraitEvents.size(), equalTo(1));
         assertThat(changedTraitEvents.get(0).getMessage(),
-                   containsString("`/items` was removed from the `smithy.api#paginated` trait. The removed value was: \"things\""));
+                   containsString("Removed trait contents from `smithy.api#paginated` at path `/items`. Removed value: \"things\""));
     }
 
     @Test
@@ -91,7 +91,7 @@ public void detectsAdditionOfItems() {
 
         assertThat(changedTraitEvents.size(), equalTo(1));
         assertThat(changedTraitEvents.get(0).getMessage(),
-                   containsString("`/items` was added to the `smithy.api#paginated` trait with a value of \"things\""));
+                   containsString("Added trait contents to `smithy.api#paginated` at path `/items` with value \"things\""));
     }
 
     @Test
@@ -108,7 +108,7 @@ public void detectsChangeToItems() {
 
         assertThat(changedTraitEvents.size(), equalTo(1));
         assertThat(changedTraitEvents.get(0).getMessage(),
-                   containsString("`/items` was changed on the `smithy.api#paginated` trait from \"things\" to \"otherThings\""));
+                   containsString("Changed trait contents of `smithy.api#paginated` at path `/items` from \"things\" to \"otherThings\""));
     }
 
     @Test
@@ -125,7 +125,7 @@ public void detectsRemovalOfPageSize() {
 
         assertThat(changedTraitEvents.size(), equalTo(1));
         assertThat(changedTraitEvents.get(0).getMessage(),
-                   containsString("`/pageSize` was removed from the `smithy.api#paginated` trait. The removed value was: \"maxResults\""));
+                   containsString("Removed trait contents from `smithy.api#paginated` at path `/pageSize`. Removed value: \"maxResults\""));
     }
 
     @Test
@@ -157,7 +157,7 @@ public void detectsChangeToPageSize() {
 
         assertThat(changedTraitEvents.size(), equalTo(1));
         assertThat(changedTraitEvents.get(0).getMessage(),
-                   containsString("`/pageSize` was changed on the `smithy.api#paginated` trait from \"maxResults\" to \"otherMaxResults\""));
+                   containsString("Changed trait contents of `smithy.api#paginated` at path `/pageSize` from \"maxResults\" to \"otherMaxResults\""));
     }
 
     @Test
@@ -173,7 +173,7 @@ public void detectsAnyChangeToInputToken() {
 
         assertThat(changedTraitEvents.size(), equalTo(1));
         assertThat(changedTraitEvents.get(0).getMessage(),
-                   containsString("`/inputToken` was changed on the `smithy.api#paginated` trait from \"token\" to \"otherToken\""));
+                   containsString("Changed trait contents of `smithy.api#paginated` at path `/inputToken` from \"token\" to \"otherToken\""));
     }
 
     @Test
@@ -189,6 +189,6 @@ public void detectsAnyChangeToOutputToken() {
 
         assertThat(changedTraitEvents.size(), equalTo(1));
         assertThat(changedTraitEvents.get(0).getMessage(),
-                   containsString("`/outputToken` was changed on the `smithy.api#paginated` trait from \"token\" to \"otherToken\""));
+                   containsString("Changed trait contents of `smithy.api#paginated` at path `/outputToken` from \"token\" to \"otherToken\""));
     }
 }