From e6850d072833221afd974a7f03d97a9453374193 Mon Sep 17 00:00:00 2001 From: JordonPhillips Date: Thu, 16 Oct 2025 14:43:31 +0200 Subject: [PATCH 1/3] Enable running ModelDiff on specific changes This introduces a builder to the Differences class that allows it to be constructed with only a specific, limited set of changes. This enables comparison of two separate shapes. --- ...bfc100966ed570480edb0fc5d91a2f2cdf347.json | 7 + .../amazon/smithy/diff/Differences.java | 300 ++++++++++++++++-- .../amazon/smithy/diff/ModelDiff.java | 13 +- 3 files changed, 289 insertions(+), 31 deletions(-) create mode 100644 .changes/next-release/feature-213bfc100966ed570480edb0fc5d91a2f2cdf347.json diff --git a/.changes/next-release/feature-213bfc100966ed570480edb0fc5d91a2f2cdf347.json b/.changes/next-release/feature-213bfc100966ed570480edb0fc5d91a2f2cdf347.json new file mode 100644 index 00000000000..0663848e4d6 --- /dev/null +++ b/.changes/next-release/feature-213bfc100966ed570480edb0fc5d91a2f2cdf347.json @@ -0,0 +1,7 @@ +{ + "type": "feature", + "description": "Added a builder to `Differences` in smithy-diff that allows constructing an instance that only contains a set of hand-curated changes. This enables making comparisons that weren't or wouldn't be auto-detected, such as comparing two shapes of the same type to see if they are compatible with each other.", + "pull_requests": [ + "[#2796](https://github.com/smithy-lang/smithy/pull/2796/)" + ] +} diff --git a/smithy-diff/src/main/java/software/amazon/smithy/diff/Differences.java b/smithy-diff/src/main/java/software/amazon/smithy/diff/Differences.java index 2cdfb41ff5b..907080704ad 100644 --- a/smithy-diff/src/main/java/software/amazon/smithy/diff/Differences.java +++ b/smithy-diff/src/main/java/software/amazon/smithy/diff/Differences.java @@ -4,33 +4,60 @@ */ package software.amazon.smithy.diff; -import java.util.ArrayList; +import java.util.Collection; import java.util.List; +import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.stream.Stream; import software.amazon.smithy.model.Model; import software.amazon.smithy.model.node.Node; import software.amazon.smithy.model.shapes.Shape; +import software.amazon.smithy.utils.BuilderRef; import software.amazon.smithy.utils.Pair; +import software.amazon.smithy.utils.SmithyBuilder; +import software.amazon.smithy.utils.ToSmithyBuilder; /** * Queryable container for detected structural differences between two models. */ -public final class Differences { +public final class Differences implements ToSmithyBuilder { private final Model oldModel; private final Model newModel; - private final List> changedShapes = new ArrayList<>(); - private final List changedMetadata = new ArrayList<>(); - private Differences(Model oldModel, Model newModel) { - this.oldModel = oldModel; - this.newModel = newModel; - detectMetadataChanges(oldModel, newModel, this); - detectShapeChanges(oldModel, newModel, this); + private final List addedShapes; + private final List removedShapes; + private final List> changedShapes; + + private final List> addedMetadata; + private final List> removedMetadata; + private final List changedMetadata; + + private Differences(Builder builder) { + this.oldModel = SmithyBuilder.requiredState("oldModel", builder.oldModel); + this.newModel = SmithyBuilder.requiredState("newModel", builder.newModel); + this.addedShapes = builder.addedShapes.copy(); + this.removedShapes = builder.removedShapes.copy(); + this.changedShapes = builder.changedShapes.copy(); + this.addedMetadata = builder.addedMetadata.copy(); + this.removedMetadata = builder.removedMetadata.copy(); + this.changedMetadata = builder.changedMetadata.copy(); } - static Differences detect(Model oldModel, Model newModel) { - return new Differences(oldModel, newModel); + /** + * Detects all differences between two models. + * + * @param oldModel The previous state of the model. + * @param newModel The new state of the model. + * @return The set of differences between the two models. + */ + public static Differences detect(Model oldModel, Model newModel) { + return builder() + .oldModel(oldModel) + .newModel(newModel) + .detectShapeChanges() + .detectMetadataChanges() + .build(); } /** @@ -57,7 +84,7 @@ public Model getNewModel() { * @return Returns a stream of each added shape. */ public Stream addedShapes() { - return newModel.shapes().filter(shape -> !oldModel.getShape(shape.getId()).isPresent()); + return addedShapes.stream(); } /** @@ -93,7 +120,7 @@ public Stream> addedMetadata() { * @return Returns a stream of each removed shape. */ public Stream removedShapes() { - return oldModel.shapes().filter(shape -> !newModel.getShape(shape.getId()).isPresent()); + return removedShapes.stream(); } /** @@ -116,11 +143,7 @@ public Stream removedShapes(Class shapeType) { * @return Returns a stream of removed metadata. */ public Stream> removedMetadata() { - return oldModel.getMetadata() - .entrySet() - .stream() - .filter(entry -> !newModel.getMetadata().containsKey(entry.getKey())) - .map(entry -> Pair.of(entry.getKey(), entry.getValue())); + return removedMetadata.stream(); } /** @@ -174,21 +197,238 @@ public int hashCode() { return Objects.hash(getOldModel(), getNewModel()); } - private static void detectShapeChanges(Model oldModel, Model newModel, Differences differences) { - for (Shape oldShape : oldModel.toSet()) { - newModel.getShape(oldShape.getId()).ifPresent(newShape -> { - if (!oldShape.equals(newShape)) { - differences.changedShapes.add(new ChangedShape<>(oldShape, newShape)); + + /** + * Constructs a Builder for {@link Differences}. + * + *

For most uses of {@link Differences}, it should be constructed with {@link Differences#detect}. + */ + public static Builder builder() { + return new Builder(); + } + + @Override + public SmithyBuilder toBuilder() { + return builder() + .oldModel(oldModel) + .newModel(newModel) + .addedShapes(addedShapes) + .removedShapes(removedShapes) + .changedShapes(changedShapes) + .addedMetadata(addedMetadata) + .removedMetadata(removedMetadata) + .changedMetadata(changedMetadata); + } + + /** + * A Builder for {@link Differences}. + * + *

For most uses of {@link Differences}, it should be constructed with {@link Differences#detect}. + * + *

This is intended to be used for evaluating subsets of the models or synthetic + * differences between them. For example, two completely different shapes could be + * evaluated against each other to see what the differences are. + */ + public static final class Builder implements SmithyBuilder { + private Model oldModel; + private Model newModel; + + private final BuilderRef> addedShapes = BuilderRef.forList(); + private final BuilderRef> removedShapes = BuilderRef.forList(); + private final BuilderRef>> changedShapes = BuilderRef.forList(); + + private final BuilderRef>> addedMetadata = BuilderRef.forList(); + private final BuilderRef>> removedMetadata = BuilderRef.forList(); + private final BuilderRef> changedMetadata = BuilderRef.forList(); + + @Override + public Differences build() { + return new Differences(this); + } + + /** + * Sets the model to be used as the base model. + * + * @param oldModel The model to base changes on. + * @return Returns the builder. + */ + public Builder oldModel(Model oldModel) { + this.oldModel = oldModel; + return this; + } + + /** + * Sets the model to be used as the new model state. + * + * @param newModel The model to use as the new state. + * @return Returns the builder. + */ + public Builder newModel(Model newModel) { + this.newModel = newModel; + return this; + } + + /** + * Sets what shapes have been added. + * + *

For most uses, {@link #detectShapeChanges()} or {@link Differences#detect(Model, Model)} + * should be used instead. + * + * @param addedShapes The shapes to consider as having been added. + * @return Returns the builder. + */ + public Builder addedShapes(Collection addedShapes) { + this.addedShapes.clear(); + this.addedShapes.get().addAll(addedShapes); + return this; + } + + /** + * Sets what shapes have been removed. + * + *

For most uses, {@link #detectShapeChanges()} or {@link Differences#detect(Model, Model)} + * should be used instead. + * + * @param removedShapes The shapes to consider as having been removed. + * @return Returns the builder. + */ + public Builder removedShapes(Collection removedShapes) { + this.removedShapes.clear(); + this.removedShapes.get().addAll(removedShapes); + return this; + } + + /** + * Sets what shapes have been changed. + * + *

For most uses, {@link #detectShapeChanges()} or {@link Differences#detect(Model, Model)} + * should be used instead. + * + * @param changedShapes The shapes to consider as having changed. + * @return Returns the builder. + */ + public Builder changedShapes(Collection> changedShapes) { + this.changedShapes.clear(); + this.changedShapes.get().addAll(changedShapes); + return this; + } + + /** + * Adds a shape to the set of shapes that have been changed. + * + *

For most uses, {@link #detectShapeChanges()} or {@link Differences#detect(Model, Model)} + * should be used instead. + * + * @param changedShape A shape to consider as having changed. + * @return Returns the builder. + */ + public Builder changedShape(ChangedShape changedShape) { + this.changedShapes.get().add(changedShape); + return this; + } + + /** + * Sets the metadata that is considered to have been added. + * + *

For most uses, {@link #detectMetadataChanges()} or {@link Differences#detect(Model, Model)} + * should be used instead. + * + * @param addedMetadata The metadata to consider as having been added. + * @return Returns the builder. + */ + public Builder addedMetadata(Collection> addedMetadata) { + this.addedMetadata.clear(); + this.addedMetadata.get().addAll(addedMetadata); + return this; + } + + /** + * Sets the metadata that is considered to have been removed. + * + *

For most uses, {@link #detectMetadataChanges()} or {@link Differences#detect(Model, Model)} + * should be used instead. + * + * @param removedMetadata The metadata to consider as having been removed. + * @return Returns the builder. + */ + public Builder removedMetadata(Collection> removedMetadata) { + this.removedMetadata.clear(); + this.removedMetadata.get().addAll(removedMetadata); + return this; + } + + /** + * Sets the metadata that is considered to have been changed. + * + *

For most uses, {@link #detectMetadataChanges()} or {@link Differences#detect(Model, Model)} + * should be used instead. + * + * @param changedMetadata The metadata to consider as having been changed. + * @return Returns the builder. + */ + public Builder changedMetadata(Collection changedMetadata) { + this.changedMetadata.clear(); + this.changedMetadata.get().addAll(changedMetadata); + return this; + } + + /** + * Detects all shape additions, removals, and changes. + * + * @return Returns the builder. + */ + public Builder detectShapeChanges() { + addedShapes.clear(); + removedShapes.clear(); + changedShapes.clear(); + for (Shape oldShape : oldModel.toSet()) { + Optional newShape = newModel.getShape(oldShape.getId()); + if (newShape.isPresent()) { + if (!oldShape.equals(newShape.get())) { + changedShapes.get().add(new ChangedShape<>(oldShape, newShape.get())); + } + } else { + removedShapes.get().add(oldShape); } - }); + } + + for (Shape newShape : newModel.toSet()) { + if (!oldModel.getShape(newShape.getId()).isPresent()) { + addedShapes.get().add(newShape); + } + } + + return this; } - } - private static void detectMetadataChanges(Model oldModel, Model newModel, Differences differences) { - oldModel.getMetadata().forEach((k, v) -> { - if (newModel.getMetadata().containsKey(k) && !newModel.getMetadata().get(k).equals(v)) { - differences.changedMetadata.add(new ChangedMetadata(k, v, newModel.getMetadata().get(k))); + /** + * Detects all metadata additions, removals, and changes. + * + * @return Returns the builder. + */ + public Builder detectMetadataChanges() { + addedMetadata.clear(); + removedMetadata.clear(); + changedMetadata.clear(); + for (Map.Entry entry : oldModel.getMetadata().entrySet()) { + String k = entry.getKey(); + Node v = entry.getValue(); + if (newModel.getMetadata().containsKey(k)) { + if (!newModel.getMetadata().get(k).equals(v)) { + changedMetadata.get().add(new ChangedMetadata(k, v, newModel.getMetadata().get(k))); + } + } else { + removedMetadata.get().add(Pair.of(k, v)); + } } - }); + + for (Map.Entry entry : newModel.getMetadata().entrySet()) { + if (!oldModel.getMetadata().containsKey(entry.getKey())) { + addedMetadata.get().add(Pair.of(entry.getKey(), entry.getValue())); + } + } + + return this; + } } } diff --git a/smithy-diff/src/main/java/software/amazon/smithy/diff/ModelDiff.java b/smithy-diff/src/main/java/software/amazon/smithy/diff/ModelDiff.java index bc3c82696a6..adb4be71c08 100644 --- a/smithy-diff/src/main/java/software/amazon/smithy/diff/ModelDiff.java +++ b/smithy-diff/src/main/java/software/amazon/smithy/diff/ModelDiff.java @@ -265,12 +265,23 @@ public Builder newModel(ValidatedResult newModel) { * @throws IllegalStateException if {@code oldModel} and {@code newModel} are not set. */ public Result compare() { + return compare(Differences.detect(oldModel, newModel)); + } + + /** + * Performs an evaluation of specific differences between models. + * + * @param differences A specific set of differences to evaluate. + * + * @return Returns the diff {@link Result}. + * @throws IllegalStateException if {@code oldModel} and {@code newModel} are not set. + */ + public Result compare(Differences differences) { SmithyBuilder.requiredState("oldModel", oldModel); SmithyBuilder.requiredState("newModel", newModel); List evaluators = new ArrayList<>(); ServiceLoader.load(DiffEvaluator.class, classLoader).forEach(evaluators::add); - Differences differences = Differences.detect(oldModel, newModel); // Applies suppressions and elevates event severities. ValidationEventDecorator decoratorResult = new ModelBasedEventDecorator() From 6a0a6584047b205eb118a1a43544d9442a64af46 Mon Sep 17 00:00:00 2001 From: JordonPhillips Date: Thu, 16 Oct 2025 14:51:13 +0200 Subject: [PATCH 2/3] Add DiffEvaluator method that accepts ClassLoader This adds a new, defaulted method to DiffEvaluator that additionally takes a ClassLoader. This may be used for a number of reasons, but was added particularly to allow running ModelDiff on synthetic changes from an evaluator. --- .../feature-6d5a718d854e66c3e6b5d7dc2622a8d17e14f9b9.json | 7 +++++++ .../java/software/amazon/smithy/diff/DiffEvaluator.java | 4 ++++ 2 files changed, 11 insertions(+) create mode 100644 .changes/next-release/feature-6d5a718d854e66c3e6b5d7dc2622a8d17e14f9b9.json diff --git a/.changes/next-release/feature-6d5a718d854e66c3e6b5d7dc2622a8d17e14f9b9.json b/.changes/next-release/feature-6d5a718d854e66c3e6b5d7dc2622a8d17e14f9b9.json new file mode 100644 index 00000000000..b2d476a93cc --- /dev/null +++ b/.changes/next-release/feature-6d5a718d854e66c3e6b5d7dc2622a8d17e14f9b9.json @@ -0,0 +1,7 @@ +{ + "type": "feature", + "description": "Updated `DiffEvaluator` interface to include an optional method that additionally accepts a `ClassLoader`. This ensures that the loader the evaluation is configured with can be used in any evaluators that may need it.", + "pull_requests": [ + "[#2796](https://github.com/smithy-lang/smithy/pull/2796)" + ] +} diff --git a/smithy-diff/src/main/java/software/amazon/smithy/diff/DiffEvaluator.java b/smithy-diff/src/main/java/software/amazon/smithy/diff/DiffEvaluator.java index 710b120ef86..84909282428 100644 --- a/smithy-diff/src/main/java/software/amazon/smithy/diff/DiffEvaluator.java +++ b/smithy-diff/src/main/java/software/amazon/smithy/diff/DiffEvaluator.java @@ -29,4 +29,8 @@ public interface DiffEvaluator { * @return Returns validation events that are relative to the new model. */ List evaluate(Differences differences); + + default List evaluate(ClassLoader classLoader, Differences differences) { + return evaluate(differences); + } } From 1488f719699aca218e928895f6590852df8f587d Mon Sep 17 00:00:00 2001 From: JordonPhillips Date: Tue, 14 Oct 2025 16:46:51 +0200 Subject: [PATCH 3/3] Loosen ChangedMemberTarget evaluator This updates the ChangedMemberTarget diff evaulator to emit a warning instead of an error when the new target has a different set of traits. It now is expected to only emit an error when the new target is not expected to be generated as a compatible type. The messages generally have been updated. It now generates messages about traits by generating a syntehtic ChangedShape and calling ModelDiff on that. This is also how it deals with knowing whether a trait change is compatible or not - it just checks if there's any DANGER or worse events in the new set of events. --- ...abc83aaa1939d12a03ffe4d22799a9873fa78.json | 7 + .../amazon/smithy/diff/ChangedShape.java | 2 +- .../amazon/smithy/diff/Differences.java | 1 - .../diff/evaluators/ChangedMemberTarget.java | 244 +++++++------ .../evaluators/ChangedMemberTargetTest.java | 329 ++++++++++++++---- .../double-nesting-a.smithy | 16 + .../double-nesting-b.smithy | 17 + ...st-added-compatible-member-trait-a.smithy} | 0 ...st-added-compatible-member-trait-b.smithy} | 0 ...-added-incompatible-member-trait-a.smithy} | 0 ...t-added-incompatible-member-trait-b.smithy | 23 ++ ...d-list-compatible-changed-member-a.smithy} | 0 ...d-list-compatible-changed-member-b.smithy} | 0 ...-list-incompatible-changed-member-a.smithy | 14 + ...-list-incompatible-changed-member-b.smithy | 14 + .../nested-list-unchanged-member-a.smithy | 14 + .../nested-list-unchanged-member-b.smithy} | 0 ...d-map-key-added-compatible-trait-a.smithy} | 0 ...d-map-key-added-compatible-trait-b.smithy} | 0 ...map-key-added-incompatible-trait-a.smithy} | 0 ...-map-key-added-incompatible-trait-b.smithy | 25 ++ ...ap-key-compatible-changed-target-a.smithy} | 0 ...ap-key-compatible-changed-target-b.smithy} | 0 ...-key-incompatible-changed-target-a.smithy} | 0 ...p-key-incompatible-changed-target-b.smithy | 19 + .../nested-map-unchanged-member-a.smithy} | 0 .../nested-map-unchanged-member-b.smithy} | 0 ...-map-value-added-compatible-trait-a.smithy | 15 + ...map-value-added-compatible-trait-b.smithy} | 0 ...ap-value-added-incompatible-trait-a.smithy | 15 + ...ap-value-added-incompatible-trait-b.smithy | 25 ++ ...p-value-compatible-changed-target-a.smithy | 15 + ...-value-compatible-changed-target-b.smithy} | 0 ...value-incompatible-changed-target-a.smithy | 15 + ...value-incompatible-changed-target-b.smithy | 17 + 35 files changed, 654 insertions(+), 173 deletions(-) create mode 100644 .changes/next-release/feature-b23abc83aaa1939d12a03ffe4d22799a9873fa78.json create mode 100644 smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/double-nesting-a.smithy create mode 100644 smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/double-nesting-b.smithy rename smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/{changed-member-target-invalid-nested1-a.smithy => changed-member-target/nested-list-added-compatible-member-trait-a.smithy} (100%) rename smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/{changed-member-target-invalid-nested1-b.smithy => changed-member-target/nested-list-added-compatible-member-trait-b.smithy} (100%) rename smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/{changed-member-target-invalid-nested2-a.smithy => changed-member-target/nested-list-added-incompatible-member-trait-a.smithy} (100%) create mode 100644 smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-list-added-incompatible-member-trait-b.smithy rename smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/{changed-member-target-valid-nested1-a.smithy => changed-member-target/nested-list-compatible-changed-member-a.smithy} (100%) rename smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/{changed-member-target-invalid-nested2-b.smithy => changed-member-target/nested-list-compatible-changed-member-b.smithy} (100%) create mode 100644 smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-list-incompatible-changed-member-a.smithy create mode 100644 smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-list-incompatible-changed-member-b.smithy create mode 100644 smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-list-unchanged-member-a.smithy rename smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/{changed-member-target-valid-nested1-b.smithy => changed-member-target/nested-list-unchanged-member-b.smithy} (100%) rename smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/{changed-member-target-invalid-nested-mapkey1-a.smithy => changed-member-target/nested-map-key-added-compatible-trait-a.smithy} (100%) rename smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/{changed-member-target-invalid-nested-mapkey1-b.smithy => changed-member-target/nested-map-key-added-compatible-trait-b.smithy} (100%) rename smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/{changed-member-target-invalid-nested-mapkey2-a.smithy => changed-member-target/nested-map-key-added-incompatible-trait-a.smithy} (100%) create mode 100644 smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-key-added-incompatible-trait-b.smithy rename smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/{changed-member-target-invalid-nested-mapvalue1-a.smithy => changed-member-target/nested-map-key-compatible-changed-target-a.smithy} (100%) rename smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/{changed-member-target-invalid-nested-mapkey2-b.smithy => changed-member-target/nested-map-key-compatible-changed-target-b.smithy} (100%) rename smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/{changed-member-target-invalid-nested-mapvalue2-a.smithy => changed-member-target/nested-map-key-incompatible-changed-target-a.smithy} (100%) create mode 100644 smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-key-incompatible-changed-target-b.smithy rename smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/{changed-member-target-valid-nested2-a.smithy => changed-member-target/nested-map-unchanged-member-a.smithy} (100%) rename smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/{changed-member-target-valid-nested2-b.smithy => changed-member-target/nested-map-unchanged-member-b.smithy} (100%) create mode 100644 smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-value-added-compatible-trait-a.smithy rename smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/{changed-member-target-invalid-nested-mapvalue1-b.smithy => changed-member-target/nested-map-value-added-compatible-trait-b.smithy} (100%) create mode 100644 smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-value-added-incompatible-trait-a.smithy create mode 100644 smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-value-added-incompatible-trait-b.smithy create mode 100644 smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-value-compatible-changed-target-a.smithy rename smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/{changed-member-target-invalid-nested-mapvalue2-b.smithy => changed-member-target/nested-map-value-compatible-changed-target-b.smithy} (100%) create mode 100644 smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-value-incompatible-changed-target-a.smithy create mode 100644 smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-value-incompatible-changed-target-b.smithy diff --git a/.changes/next-release/feature-b23abc83aaa1939d12a03ffe4d22799a9873fa78.json b/.changes/next-release/feature-b23abc83aaa1939d12a03ffe4d22799a9873fa78.json new file mode 100644 index 00000000000..b05287782c1 --- /dev/null +++ b/.changes/next-release/feature-b23abc83aaa1939d12a03ffe4d22799a9873fa78.json @@ -0,0 +1,7 @@ +{ + "type": "feature", + "description": "Loosened the `ChangedMemberTarget` diff evaluator to not report `ERROR` level events so often. Now it will always report an `ERROR` if the change is expected to result in codegen type errors, but otherwise it will default to `WARNING` unless differing traits between the targets would result in a higher severity event had those trait changes been applied to the original target.", + "pull_requests": [ + "[#2796](https://github.com/smithy-lang/smithy/pull/2796)" + ] +} diff --git a/smithy-diff/src/main/java/software/amazon/smithy/diff/ChangedShape.java b/smithy-diff/src/main/java/software/amazon/smithy/diff/ChangedShape.java index 1e2db50b856..761785efe56 100644 --- a/smithy-diff/src/main/java/software/amazon/smithy/diff/ChangedShape.java +++ b/smithy-diff/src/main/java/software/amazon/smithy/diff/ChangedShape.java @@ -63,7 +63,7 @@ public SourceLocation getSourceLocation() { * @return Return the shape ID. */ public ShapeId getShapeId() { - return newShape.getId(); + return oldShape.getId(); } /** diff --git a/smithy-diff/src/main/java/software/amazon/smithy/diff/Differences.java b/smithy-diff/src/main/java/software/amazon/smithy/diff/Differences.java index 907080704ad..81bc94a11a0 100644 --- a/smithy-diff/src/main/java/software/amazon/smithy/diff/Differences.java +++ b/smithy-diff/src/main/java/software/amazon/smithy/diff/Differences.java @@ -197,7 +197,6 @@ public int hashCode() { return Objects.hash(getOldModel(), getNewModel()); } - /** * Constructs a Builder for {@link Differences}. * diff --git a/smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/ChangedMemberTarget.java b/smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/ChangedMemberTarget.java index ae45831d8ee..b239d4dbade 100644 --- a/smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/ChangedMemberTarget.java +++ b/smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/ChangedMemberTarget.java @@ -7,36 +7,42 @@ import java.util.ArrayList; import java.util.List; import java.util.Set; -import java.util.StringJoiner; -import java.util.TreeSet; import java.util.stream.Collectors; import software.amazon.smithy.diff.ChangedShape; import software.amazon.smithy.diff.Differences; +import software.amazon.smithy.diff.ModelDiff; import software.amazon.smithy.model.Model; import software.amazon.smithy.model.shapes.CollectionShape; import software.amazon.smithy.model.shapes.MapShape; import software.amazon.smithy.model.shapes.MemberShape; import software.amazon.smithy.model.shapes.Shape; import software.amazon.smithy.model.shapes.ShapeId; -import software.amazon.smithy.model.shapes.ShapeType; import software.amazon.smithy.model.shapes.SimpleShape; import software.amazon.smithy.model.traits.EnumTrait; -import software.amazon.smithy.model.traits.Trait; import software.amazon.smithy.model.validation.Severity; import software.amazon.smithy.model.validation.ValidationEvent; -import software.amazon.smithy.utils.ListUtils; +import software.amazon.smithy.utils.Pair; import software.amazon.smithy.utils.SetUtils; +import software.amazon.smithy.utils.StringUtils; /** * Checks for changes in the shapes targeted by a member. * - *

If the shape targeted by the member changes from a simple shape to - * a simple shape of the same type with the same traits, or a list or set - * that has a member that targets the shame exact shape and has the same - * traits, then the emitted event is a WARNING. If an enum trait is - * found on the old or newly targeted shape, then the event is an ERROR, - * because enum traits typically materialize as named types in codegen. - * All other changes are ERROR events. + *

If the new target is not a compatible type, the emitted event will be + * an ERROR. The new target is not compatible if any of the following are true: + * + *

    + *
  • The new target is a different shape type than the old target.
    • + *
  • The target is a shape type whose name is significant to code generation, + * such as structures and enums.
    • + *
  • The new target is a list whose member is not a compatible type with the + * old target's member.
    • + *
  • The new target is a map whose key or value is not a compatible type with + * the old target's key or value.
    • + *
+ * + *

If the types are compatible, the emitted event will default to a WARNING. This + * is elevated if any trait changes would result in a higher severity. */ public final class ChangedMemberTarget extends AbstractDiffEvaluator { @@ -47,29 +53,49 @@ public final class ChangedMemberTarget extends AbstractDiffEvaluator { */ private static final Set SIGNIFICANT_CODEGEN_TRAITS = SetUtils.of(EnumTrait.ID); + private static final Pair COMPATIBLE = + Pair.of(Severity.WARNING, "This was determined backward compatible."); + @Override public List evaluate(Differences differences) { + return evaluate(ChangedMemberTarget.class.getClassLoader(), differences); + } + + @Override + public List evaluate(ClassLoader classLoader, Differences differences) { return differences.changedShapes(MemberShape.class) .filter(change -> !change.getOldShape().getTarget().equals(change.getNewShape().getTarget())) - .map(change -> createChangeEvent(differences, change)) + .map(change -> createChangeEvent(classLoader, differences, change)) .collect(Collectors.toList()); } - private ValidationEvent createChangeEvent(Differences differences, ChangedShape change) { - Shape oldTarget = getShapeTarget(differences.getOldModel(), change.getOldShape().getTarget()); - Shape newTarget = getShapeTarget(differences.getNewModel(), change.getNewShape().getTarget()); - List issues = areShapesCompatible(oldTarget, newTarget); - Severity severity = issues.isEmpty() ? Severity.WARNING : Severity.ERROR; - - String message = createSimpleMessage(change, oldTarget, newTarget); - if (severity == Severity.WARNING) { - message += "This was determined backward compatible."; - } else { - message += String.join(". ", issues) + "."; - } + private ValidationEvent createChangeEvent( + ClassLoader classLoader, + Differences differences, + ChangedShape change + ) { + return createChangeEvent(classLoader, differences.getOldModel(), differences.getNewModel(), change); + } + + private ValidationEvent createChangeEvent( + ClassLoader classLoader, + Model oldModel, + Model newModel, + ChangedShape change + ) { + Shape oldTarget = getShapeTarget(oldModel, change.getOldShape().getTarget()); + Shape newTarget = getShapeTarget(newModel, change.getNewShape().getTarget()); + + Pair evaluation = evaluateShape( + classLoader, + oldModel, + newModel, + oldTarget, + newTarget); + String message = createSimpleMessage(change, oldTarget, newTarget) + evaluation.getRight(); return ValidationEvent.builder() - .severity(severity) + .severity(evaluation.getLeft()) .id(getEventId()) .shape(change.getNewShape()) .message(message) @@ -80,77 +106,107 @@ private Shape getShapeTarget(Model model, ShapeId id) { return model.getShape(id).orElse(null); } - private static List areShapesCompatible(Shape oldShape, Shape newShape) { + private Pair evaluateShape( + ClassLoader classLoader, + Model oldModel, + Model newModel, + Shape oldShape, + Shape newShape + ) { if (oldShape == null || newShape == null) { - return ListUtils.of(); + return COMPATIBLE; } if (oldShape.getType() != newShape.getType()) { - return ListUtils.of(String.format("The type of the targeted shape changed from %s to %s", - oldShape.getType(), - newShape.getType())); + return Pair.of( + Severity.ERROR, + String.format( + "The type of the targeted shape changed from %s to %s.", + oldShape.getType(), + newShape.getType())); } - if (!(oldShape instanceof SimpleShape || oldShape instanceof CollectionShape || oldShape instanceof MapShape)) { - return ListUtils.of(String.format("The name of a %s is significant", oldShape.getType())); + if (!(oldShape instanceof SimpleShape || oldShape instanceof CollectionShape || oldShape.isMapShape()) + || oldShape.isIntEnumShape() + || oldShape.isEnumShape()) { + return Pair.of( + Severity.ERROR, + String.format("The name of a %s is significant.", oldShape.getType())); } - List results = new ArrayList<>(); for (ShapeId significantCodegenTrait : SIGNIFICANT_CODEGEN_TRAITS) { if (oldShape.hasTrait(significantCodegenTrait)) { - results.add(String.format("The `%s` trait was found on the target, so the name of the targeted " - + "shape matters for codegen", - significantCodegenTrait)); + return Pair.of( + Severity.ERROR, + String.format("The `%s` trait was found on the target, so the name of the targeted " + + "shape matters for codegen.", + significantCodegenTrait)); } } - if (!oldShape.getAllTraits().equals(newShape.getAllTraits())) { - results.add(createTraitDiffMessage(oldShape, newShape)); - } - + // Now that we've checked several terminal conditions, we need to evaluate traits and + // collection/map member targets. To evaluate traits, we will create a synthetic diff + // set to re-run the diff evaluator on. That will ensure that any differences are + // given the proper severity and context rather than simply returning an ERROR for any + // difference. + Differences.Builder differences = Differences.builder() + .oldModel(oldModel) + .newModel(newModel) + .changedShape(new ChangedShape<>(oldShape, newShape)); + + // Add any list / map members to the set of differences to check, and potentially + // recurse if this evaluator needs to be run on them. Note that this can't recurse + // infinitely, even without any specific checks here. That's because to get to this + // point a member target had to change without changing shape type and without being + // a structure, union, or enum. Neither maps nor lists can recurse by themselves or + // with each other, there MUST be a structure or union in the path for recursion to + // happen in a way that Smithy will allow. Therefore, when the structure or union + // in the path is hit, it'll get caught in the terminal conditions above. if (oldShape instanceof CollectionShape) { - evaluateMember(oldShape.getType(), - results, - ((CollectionShape) oldShape).getMember(), - ((CollectionShape) newShape).getMember()); + MemberShape oldMember = ((CollectionShape) oldShape).getMember(); + MemberShape newMember = ((CollectionShape) newShape).getMember(); + differences.changedShape(new ChangedShape<>(oldMember, newMember)); } else if (oldShape instanceof MapShape) { - MapShape oldMapShape = (MapShape) oldShape; - MapShape newMapShape = (MapShape) newShape; - // Both the key and value need to be evaluated for maps. - evaluateMember(oldShape.getType(), - results, - oldMapShape.getKey(), - newMapShape.getKey()); - evaluateMember(oldShape.getType(), - results, - oldMapShape.getValue(), - newMapShape.getValue()); + MapShape oldMap = (MapShape) oldShape; + MapShape newMap = (MapShape) newShape; + differences.changedShape(new ChangedShape<>(oldMap.getKey(), newMap.getKey())); + differences.changedShape(new ChangedShape<>(oldMap.getValue(), newMap.getValue())); } - return results; - } + // Re-run the diff evaluator with this changed shape and any changed members. + ModelDiff.Result result = ModelDiff.builder() + .oldModel(oldModel) + .newModel(newModel) + .classLoader(classLoader) + .compare(differences.build()); + List diffEvents = new ArrayList<>(result.getDiffEvents()); - private static void evaluateMember( - ShapeType oldShapeType, - List results, - MemberShape oldMember, - MemberShape newMember - ) { - String memberSlug = oldShapeType == ShapeType.MAP ? oldMember.getMemberName() + " " : ""; - if (!oldMember.getTarget().equals(newMember.getTarget())) { - results.add(String.format("Both the old and new shapes are a %s, but the old shape %stargeted " - + "`%s` while the new shape targets `%s`", - oldShapeType, - memberSlug, - oldMember.getTarget(), - newMember.getTarget())); - } else if (!oldMember.getAllTraits().equals(newMember.getAllTraits())) { - results.add(String.format("Both the old and new shapes are a %s, but their %smembers have " - + "differing traits. %s", - oldShapeType, - memberSlug, - createTraitDiffMessage(oldMember, newMember))); + if (diffEvents.isEmpty()) { + return COMPATIBLE; + } + + Severity severity = Severity.WARNING; + StringBuilder message = new StringBuilder("This will result in the following effective differences:") + .append(System.lineSeparator()) + .append(System.lineSeparator()); + + for (ValidationEvent event : diffEvents) { + // If the severity in any event is greater than the current severity, elevate it + // to that level. + severity = severity.compareTo(event.getSeverity()) > 0 ? severity : event.getSeverity(); + + // Add the event to a list and indent the message in case it also spans + // multiple lines. + String eventMessage = StringUtils.indent(event.getMessage(), 2).trim(); + message.append(String.format("- [%s] %s%n", event.getSeverity(), eventMessage)); + } + + // If there are only warnings or less, + if (severity.compareTo(Severity.WARNING) <= 0) { + message.insert(0, "This was determined backward compatible. "); } + + return Pair.of(severity, message.toString().trim()); } private static String createSimpleMessage(ChangedShape change, Shape oldTarget, Shape newTarget) { @@ -162,36 +218,4 @@ private static String createSimpleMessage(ChangedShape change, Shap change.getNewShape().getTarget(), newTarget.getType()); } - - private static String createTraitDiffMessage(Shape oldShape, Shape newShape) { - StringJoiner joiner = new StringJoiner(". "); - ChangedShape targetChange = new ChangedShape<>(oldShape, newShape); - - Set removedTraits = targetChange.removedTraits() - .map(Trait::toShapeId) - .collect(Collectors.toCollection(TreeSet::new)); - - if (!removedTraits.isEmpty()) { - joiner.add("The targeted shape no longer has the following traits: " + removedTraits); - } - - Set addedTraits = targetChange.addedTraits() - .map(Trait::toShapeId) - .collect(Collectors.toCollection(TreeSet::new)); - - if (!addedTraits.isEmpty()) { - joiner.add("The newly targeted shape now has the following additional traits: " + addedTraits); - } - - // Only select the traits that exist in both placed but changed. - Set changedTraits = new TreeSet<>(targetChange.getTraitDifferences().keySet()); - changedTraits.removeAll(addedTraits); - changedTraits.removeAll(removedTraits); - - if (!changedTraits.isEmpty()) { - joiner.add("The newly targeted shape has traits that differ from the previous shape: " + changedTraits); - } - - return joiner.toString(); - } } diff --git a/smithy-diff/src/test/java/software/amazon/smithy/diff/evaluators/ChangedMemberTargetTest.java b/smithy-diff/src/test/java/software/amazon/smithy/diff/evaluators/ChangedMemberTargetTest.java index 4b8543b72c4..e3d0d485f29 100644 --- a/smithy-diff/src/test/java/software/amazon/smithy/diff/evaluators/ChangedMemberTargetTest.java +++ b/smithy-diff/src/test/java/software/amazon/smithy/diff/evaluators/ChangedMemberTargetTest.java @@ -143,8 +143,7 @@ public void detectsTraitRemovalOnMemberTarget() { assertThat(TestHelper.findEvents(events, "ChangedMemberTarget").get(0).getMessage(), equalTo("The shape targeted by the member `foo.baz#List$member` changed from " + "`foo.baz#String1` (string) to `foo.baz#String2` (string). The `smithy.api#enum` trait " - + "was found on the target, so the name of the targeted shape matters for codegen. " - + "The targeted shape no longer has the following traits: [smithy.api#enum].")); + + "was found on the target, so the name of the targeted shape matters for codegen.")); } @Test @@ -163,11 +162,13 @@ public void detectsTraitAddedToMemberTarget() { assertThat(TestHelper.findEvents(events, "ChangedMemberTarget").size(), equalTo(1)); assertThat(TestHelper.findEvents(events, member2.getId()).size(), equalTo(1)); - assertThat(TestHelper.findEvents(events, Severity.ERROR).size(), equalTo(1)); + assertThat(TestHelper.findEvents(events, Severity.WARNING).size(), equalTo(1)); assertThat(TestHelper.findEvents(events, "ChangedMemberTarget").get(0).getMessage(), - equalTo("The shape targeted by the member `foo.baz#List$member` changed from `foo.baz#String1` " - + "(string) to `foo.baz#String2` (string). The newly targeted shape now has the " - + "following additional traits: [smithy.api#sensitive].")); + equalTo(String + .format("The shape targeted by the member `foo.baz#List$member` changed from `foo.baz#String1` " + + "(string) to `foo.baz#String2` (string). This was determined backward compatible. This will " + + "result in the following effective differences:%n%n" + + "- [NOTE] Added trait `smithy.api#sensitive` with value `{}`"))); } @Test @@ -186,21 +187,23 @@ public void detectsTraitChangedOnMemberTarget() { assertThat(TestHelper.findEvents(events, "ChangedMemberTarget").size(), equalTo(1)); assertThat(TestHelper.findEvents(events, member2.getId()).size(), equalTo(1)); - assertThat(TestHelper.findEvents(events, Severity.ERROR).size(), equalTo(1)); + assertThat(TestHelper.findEvents(events, Severity.WARNING).size(), equalTo(1)); assertThat(TestHelper.findEvents(events, "ChangedMemberTarget").get(0).getMessage(), - equalTo("The shape targeted by the member `foo.baz#List$member` changed from `foo.baz#String1` " - + "(string) to `foo.baz#String2` (string). The newly targeted shape has traits that " - + "differ from the previous shape: [smithy.api#documentation].")); + equalTo(String + .format("The shape targeted by the member `foo.baz#List$member` changed from `foo.baz#String1` " + + "(string) to `foo.baz#String2` (string). This was determined backward compatible. This will " + + "result in the following effective differences:%n%n" + + "- [NOTE] Changed trait `smithy.api#documentation` from `a` to `b`"))); } @Test - public void detectsAcceptableListMemberChangesInNestedTargets() { + public void detectsNestedListWithUnchangedMemberTarget() { Model modelA = Model.assembler() - .addImport(getClass().getResource("changed-member-target-valid-nested1-a.smithy")) + .addImport(getClass().getResource("changed-member-target/nested-list-unchanged-member-a.smithy")) .assemble() .unwrap(); Model modelB = Model.assembler() - .addImport(getClass().getResource("changed-member-target-valid-nested1-b.smithy")) + .addImport(getClass().getResource("changed-member-target/nested-list-unchanged-member-b.smithy")) .assemble() .unwrap(); List events = ModelDiff.compare(modelA, modelB); @@ -209,24 +212,24 @@ public void detectsAcceptableListMemberChangesInNestedTargets() { assertThat(TestHelper.findEvents(events, Severity.WARNING).size(), equalTo(1)); assertThat(TestHelper.findEvents(events, "ChangedMemberTarget").get(0).getMessage(), equalTo("The shape targeted by the member `smithy.example#A$member` changed from " - + "`smithy.example#B1` (list) to `smithy.example#B2` (list). This was determined " - + "backward compatible.")); + + "`smithy.example#B1` (list) to `smithy.example#B2` (list). This was determined backward " + + "compatible.")); } @Test - public void detectsAcceptableMapMemberChangesInNestedTargets() { + public void detectsNestedMapWithUnchangedValueTarget() { Model modelA = Model.assembler() - .addImport(getClass().getResource("changed-member-target-valid-nested2-a.smithy")) + .addImport(getClass().getResource("changed-member-target/nested-map-unchanged-member-a.smithy")) .assemble() .unwrap(); Model modelB = Model.assembler() - .addImport(getClass().getResource("changed-member-target-valid-nested2-b.smithy")) + .addImport(getClass().getResource("changed-member-target/nested-map-unchanged-member-b.smithy")) .assemble() .unwrap(); List events = ModelDiff.compare(modelA, modelB); assertThat(TestHelper.findEvents(events, "ChangedMemberTarget").size(), equalTo(1)); - assertThat(TestHelper.findEvents(events, Severity.WARNING).size(), equalTo(1)); + assertThat(TestHelper.findEvents(events, Severity.ERROR).size(), equalTo(1)); assertThat(TestHelper.findEvents(events, "ChangedMemberTarget").get(0).getMessage(), equalTo("The shape targeted by the member `smithy.example#A$member` changed from " + "`smithy.example#B1` (map) to `smithy.example#B2` (map). This was determined " @@ -234,13 +237,40 @@ public void detectsAcceptableMapMemberChangesInNestedTargets() { } @Test - public void detectsInvalidListMemberChangesInNestedTargets() { + public void detectsCompatibleTraitsAddedToNestedListMembers() { Model modelA = Model.assembler() - .addImport(getClass().getResource("changed-member-target-invalid-nested1-a.smithy")) + .addImport(getClass() + .getResource("changed-member-target/nested-list-added-compatible-member-trait-a.smithy")) .assemble() .unwrap(); Model modelB = Model.assembler() - .addImport(getClass().getResource("changed-member-target-invalid-nested1-b.smithy")) + .addImport(getClass() + .getResource("changed-member-target/nested-list-added-compatible-member-trait-b.smithy")) + .assemble() + .unwrap(); + List events = ModelDiff.compare(modelA, modelB); + + assertThat(TestHelper.findEvents(events, "ChangedMemberTarget").size(), equalTo(1)); + ValidationEvent event = TestHelper.findEvents(events, "ChangedMemberTarget").get(0); + assertThat(event.getSeverity(), equalTo(Severity.WARNING)); + assertThat(event.getMessage(), + equalTo(String.format("The shape targeted by the member `smithy.example#A$member` changed from " + + "`smithy.example#B1` (list) to `smithy.example#B2` (list). This was determined backward " + + "compatible. This will result in the following effective differences:%n%n" + + "- [WARNING] Added trait `smithy.api#pattern` with value `^[a-z]+$`; The @pattern trait " + + "should only be added if the string already had adhered to the pattern."))); + } + + @Test + public void detectsIncompatibleTraitsAddedToNestedListMembers() { + Model modelA = Model.assembler() + .addImport(getClass() + .getResource("changed-member-target/nested-list-added-incompatible-member-trait-a.smithy")) + .assemble() + .unwrap(); + Model modelB = Model.assembler() + .addImport(getClass() + .getResource("changed-member-target/nested-list-added-incompatible-member-trait-b.smithy")) .assemble() .unwrap(); List events = ModelDiff.compare(modelA, modelB); @@ -249,20 +279,50 @@ public void detectsInvalidListMemberChangesInNestedTargets() { ValidationEvent event = TestHelper.findEvents(events, "ChangedMemberTarget").get(0); assertThat(event.getSeverity(), equalTo(Severity.ERROR)); assertThat(event.getMessage(), - equalTo("The shape targeted by the member `smithy.example#A$member` changed from " - + "`smithy.example#B1` (list) to `smithy.example#B2` (list). Both the old and new " - + "shapes are a list, but their members have differing traits. The newly targeted " - + "shape now has the following additional traits: [smithy.api#pattern].")); + equalTo(String.format("The shape targeted by the member `smithy.example#A$member` changed from " + + "`smithy.example#B1` (list) to `smithy.example#B2` (list). This will result in the following " + + "effective differences:%n%n" + + "- [ERROR] Added trait `smithy.example#noAddingTrait`"))); } @Test - public void detectsInvalidListMemberTargetChange() { + public void detectsNestedListMemberChangedToCompatibleTarget() { Model modelA = Model.assembler() - .addImport(getClass().getResource("changed-member-target-invalid-nested2-a.smithy")) + .addImport( + getClass().getResource("changed-member-target/nested-list-compatible-changed-member-a.smithy")) .assemble() .unwrap(); Model modelB = Model.assembler() - .addImport(getClass().getResource("changed-member-target-invalid-nested2-b.smithy")) + .addImport( + getClass().getResource("changed-member-target/nested-list-compatible-changed-member-b.smithy")) + .assemble() + .unwrap(); + List events = ModelDiff.compare(modelA, modelB); + + assertThat(TestHelper.findEvents(events, "ChangedMemberTarget").size(), equalTo(1)); + ValidationEvent event = TestHelper.findEvents(events, "ChangedMemberTarget").get(0); + assertThat(event.getSeverity(), equalTo(Severity.WARNING)); + assertThat(event.getMessage(), + equalTo(String.format("The shape targeted by the member `smithy.example#A$member` changed from " + + "`smithy.example#B1` (list) to `smithy.example#B2` (list). This was determined backward " + + "compatible. This will result in the following effective differences:%n%n" + + "- [WARNING] The shape targeted by the member `smithy.example#B1$member` changed from " + + "`smithy.example#MyString` (string) to `smithy.example#MyString2` (string). This was " + + "determined backward compatible."))); + } + + @Test + public void detectsNestedListMemberChangedToIncompatibleTarget() { + Model modelA = Model.assembler() + .addImport( + getClass() + .getResource("changed-member-target/nested-list-incompatible-changed-member-a.smithy")) + .assemble() + .unwrap(); + Model modelB = Model.assembler() + .addImport( + getClass() + .getResource("changed-member-target/nested-list-incompatible-changed-member-b.smithy")) .assemble() .unwrap(); List events = ModelDiff.compare(modelA, modelB); @@ -271,20 +331,51 @@ public void detectsInvalidListMemberTargetChange() { ValidationEvent event = TestHelper.findEvents(events, "ChangedMemberTarget").get(0); assertThat(event.getSeverity(), equalTo(Severity.ERROR)); assertThat(event.getMessage(), - equalTo("The shape targeted by the member `smithy.example#A$member` changed from " - + "`smithy.example#B1` (list) to `smithy.example#B2` (list). Both the old and new " - + "shapes are a list, but the old shape targeted `smithy.example#MyString` while " - + "the new shape targets `smithy.example#MyString2`.")); + equalTo(String.format("The shape targeted by the member `smithy.example#A$member` changed from " + + "`smithy.example#B1` (list) to `smithy.example#B2` (list). This will result in the following " + + "effective differences:%n%n" + + "- [ERROR] The shape targeted by the member `smithy.example#B1$member` changed from " + + "`smithy.example#MyString` (string) to `smithy.example#MyInteger` (integer). The type of the " + + "targeted shape changed from string to integer."))); + } + + @Test + public void detectsCompatibleTraitsAddedToNestedMapKey() { + Model modelA = Model.assembler() + .addImport( + getClass().getResource("changed-member-target/nested-map-key-added-compatible-trait-a.smithy")) + .assemble() + .unwrap(); + Model modelB = Model.assembler() + .addImport( + getClass().getResource("changed-member-target/nested-map-key-added-compatible-trait-b.smithy")) + .assemble() + .unwrap(); + List events = ModelDiff.compare(modelA, modelB); + + assertThat(TestHelper.findEvents(events, "ChangedMemberTarget").size(), equalTo(1)); + ValidationEvent event = TestHelper.findEvents(events, "ChangedMemberTarget").get(0); + assertThat(event.getSeverity(), equalTo(Severity.WARNING)); + assertThat(event.getMessage(), + equalTo(String.format("The shape targeted by the member `smithy.example#A$member` changed from " + + "`smithy.example#B1` (map) to `smithy.example#B2` (map). This was determined backward " + + "compatible. This will result in the following effective differences:%n%n" + + "- [WARNING] Added trait `smithy.api#pattern` with value `^[a-z]+$`; The @pattern trait " + + "should only be added if the string already had adhered to the pattern."))); } @Test - public void detectsInvalidMapKeyChangesInNestedTargets() { + public void detectsIncompatibleTraitsAddedToNestedMapKey() { Model modelA = Model.assembler() - .addImport(getClass().getResource("changed-member-target-invalid-nested-mapkey1-a.smithy")) + .addImport( + getClass() + .getResource("changed-member-target/nested-map-key-added-incompatible-trait-a.smithy")) .assemble() .unwrap(); Model modelB = Model.assembler() - .addImport(getClass().getResource("changed-member-target-invalid-nested-mapkey1-b.smithy")) + .addImport( + getClass() + .getResource("changed-member-target/nested-map-key-added-incompatible-trait-b.smithy")) .assemble() .unwrap(); List events = ModelDiff.compare(modelA, modelB); @@ -293,20 +384,48 @@ public void detectsInvalidMapKeyChangesInNestedTargets() { ValidationEvent event = TestHelper.findEvents(events, "ChangedMemberTarget").get(0); assertThat(event.getSeverity(), equalTo(Severity.ERROR)); assertThat(event.getMessage(), - equalTo("The shape targeted by the member `smithy.example#A$member` changed from " - + "`smithy.example#B1` (map) to `smithy.example#B2` (map). Both the old and new " - + "shapes are a map, but their key members have differing traits. The newly targeted " - + "shape now has the following additional traits: [smithy.api#pattern].")); + equalTo(String.format("The shape targeted by the member `smithy.example#A$member` changed from " + + "`smithy.example#B1` (map) to `smithy.example#B2` (map). This will result in the following " + + "effective differences:%n%n" + + "- [ERROR] Added trait `smithy.example#noAddingTrait`"))); } @Test - public void detectsInvalidMapKeyTargetChange() { + public void detectsNestedMapKeyChangedToCompatibleTarget() { Model modelA = Model.assembler() - .addImport(getClass().getResource("changed-member-target-invalid-nested-mapkey2-a.smithy")) + .addImport(getClass() + .getResource("changed-member-target/nested-map-key-compatible-changed-target-a.smithy")) .assemble() .unwrap(); Model modelB = Model.assembler() - .addImport(getClass().getResource("changed-member-target-invalid-nested-mapkey2-b.smithy")) + .addImport(getClass() + .getResource("changed-member-target/nested-map-key-compatible-changed-target-b.smithy")) + .assemble() + .unwrap(); + List events = ModelDiff.compare(modelA, modelB); + + assertThat(TestHelper.findEvents(events, "ChangedMemberTarget").size(), equalTo(1)); + ValidationEvent event = TestHelper.findEvents(events, "ChangedMemberTarget").get(0); + assertThat(event.getSeverity(), equalTo(Severity.WARNING)); + assertThat(event.getMessage(), + equalTo(String.format("The shape targeted by the member `smithy.example#A$member` changed from " + + "`smithy.example#B1` (map) to `smithy.example#B2` (map). This was determined backward " + + "compatible. This will result in the following effective differences:%n%n" + + "- [WARNING] The shape targeted by the member `smithy.example#B1$key` changed from " + + "`smithy.example#MyString` (string) to `smithy.example#MyString2` (string). This was " + + "determined backward compatible."))); + } + + @Test + public void detectsNestedMapKeyChangedToIncompatibleTarget() { + Model modelA = Model.assembler() + .addImport(getClass() + .getResource("changed-member-target/nested-map-key-incompatible-changed-target-a.smithy")) + .assemble() + .unwrap(); + Model modelB = Model.assembler() + .addImport(getClass() + .getResource("changed-member-target/nested-map-key-incompatible-changed-target-b.smithy")) .assemble() .unwrap(); List events = ModelDiff.compare(modelA, modelB); @@ -315,20 +434,49 @@ public void detectsInvalidMapKeyTargetChange() { ValidationEvent event = TestHelper.findEvents(events, "ChangedMemberTarget").get(0); assertThat(event.getSeverity(), equalTo(Severity.ERROR)); assertThat(event.getMessage(), - equalTo("The shape targeted by the member `smithy.example#A$member` changed from " - + "`smithy.example#B1` (map) to `smithy.example#B2` (map). Both the old and new " - + "shapes are a map, but the old shape key targeted `smithy.example#MyString` while " - + "the new shape targets `smithy.example#MyString2`.")); + equalTo(String.format("The shape targeted by the member `smithy.example#A$member` changed from " + + "`smithy.example#B1` (map) to `smithy.example#B2` (map). This will result in the following " + + "effective differences:%n%n" + + "- [ERROR] The shape targeted by the member `smithy.example#B1$key` changed from " + + "`smithy.example#MyString` (string) to `smithy.example#MyEnum` (enum). The type of the " + + "targeted shape changed from string to enum."))); + } + + @Test + public void detectsCompatibleTraitsAddedToNestedMapValue() { + Model modelA = Model.assembler() + .addImport(getClass() + .getResource("changed-member-target/nested-map-value-added-compatible-trait-a.smithy")) + .assemble() + .unwrap(); + Model modelB = Model.assembler() + .addImport(getClass() + .getResource("changed-member-target/nested-map-value-added-compatible-trait-b.smithy")) + .assemble() + .unwrap(); + List events = ModelDiff.compare(modelA, modelB); + + assertThat(TestHelper.findEvents(events, "ChangedMemberTarget").size(), equalTo(1)); + ValidationEvent event = TestHelper.findEvents(events, "ChangedMemberTarget").get(0); + assertThat(event.getSeverity(), equalTo(Severity.WARNING)); + assertThat(event.getMessage(), + equalTo(String.format("The shape targeted by the member `smithy.example#A$member` changed from " + + "`smithy.example#B1` (map) to `smithy.example#B2` (map). This was determined backward " + + "compatible. This will result in the following effective differences:%n%n" + + "- [WARNING] Added trait `smithy.api#pattern` with value `^[a-z]+$`; The @pattern trait " + + "should only be added if the string already had adhered to the pattern."))); } @Test - public void detectsInvalidMapValueChangesInNestedTargets() { + public void detectsIncompatibleTraitsAddedToNestedMapValue() { Model modelA = Model.assembler() - .addImport(getClass().getResource("changed-member-target-invalid-nested-mapvalue1-a.smithy")) + .addImport(getClass() + .getResource("changed-member-target/nested-map-value-added-incompatible-trait-a.smithy")) .assemble() .unwrap(); Model modelB = Model.assembler() - .addImport(getClass().getResource("changed-member-target-invalid-nested-mapvalue1-b.smithy")) + .addImport(getClass() + .getResource("changed-member-target/nested-map-value-added-incompatible-trait-b.smithy")) .assemble() .unwrap(); List events = ModelDiff.compare(modelA, modelB); @@ -337,20 +485,48 @@ public void detectsInvalidMapValueChangesInNestedTargets() { ValidationEvent event = TestHelper.findEvents(events, "ChangedMemberTarget").get(0); assertThat(event.getSeverity(), equalTo(Severity.ERROR)); assertThat(event.getMessage(), - equalTo("The shape targeted by the member `smithy.example#A$member` changed from " - + "`smithy.example#B1` (map) to `smithy.example#B2` (map). Both the old and new " - + "shapes are a map, but their value members have differing traits. The newly targeted " - + "shape now has the following additional traits: [smithy.api#pattern].")); + equalTo(String.format("The shape targeted by the member `smithy.example#A$member` changed from " + + "`smithy.example#B1` (map) to `smithy.example#B2` (map). This will result in the following " + + "effective differences:%n%n" + + "- [ERROR] Added trait `smithy.example#noAddingTrait`"))); + } + + @Test + public void detectsNestedMapValueChangedToCompatibleTarget() { + Model modelA = Model.assembler() + .addImport(getClass() + .getResource("changed-member-target/nested-map-value-compatible-changed-target-a.smithy")) + .assemble() + .unwrap(); + Model modelB = Model.assembler() + .addImport(getClass() + .getResource("changed-member-target/nested-map-value-compatible-changed-target-b.smithy")) + .assemble() + .unwrap(); + List events = ModelDiff.compare(modelA, modelB); + + assertThat(TestHelper.findEvents(events, "ChangedMemberTarget").size(), equalTo(1)); + ValidationEvent event = TestHelper.findEvents(events, "ChangedMemberTarget").get(0); + assertThat(event.getSeverity(), equalTo(Severity.WARNING)); + assertThat(event.getMessage(), + equalTo(String.format("The shape targeted by the member `smithy.example#A$member` changed from " + + "`smithy.example#B1` (map) to `smithy.example#B2` (map). This was determined backward " + + "compatible. This will result in the following effective differences:%n%n" + + "- [WARNING] The shape targeted by the member `smithy.example#B1$value` changed from " + + "`smithy.example#MyString` (string) to `smithy.example#MyString2` (string). This was " + + "determined backward compatible."))); } @Test - public void detectsInvalidMapValueTargetChange() { + public void detectsNestedMapValueChangedToIncompatibleTarget() { Model modelA = Model.assembler() - .addImport(getClass().getResource("changed-member-target-invalid-nested-mapvalue2-a.smithy")) + .addImport(getClass() + .getResource("changed-member-target/nested-map-value-incompatible-changed-target-a.smithy")) .assemble() .unwrap(); Model modelB = Model.assembler() - .addImport(getClass().getResource("changed-member-target-invalid-nested-mapvalue2-b.smithy")) + .addImport(getClass() + .getResource("changed-member-target/nested-map-value-incompatible-changed-target-b.smithy")) .assemble() .unwrap(); List events = ModelDiff.compare(modelA, modelB); @@ -359,9 +535,40 @@ public void detectsInvalidMapValueTargetChange() { ValidationEvent event = TestHelper.findEvents(events, "ChangedMemberTarget").get(0); assertThat(event.getSeverity(), equalTo(Severity.ERROR)); assertThat(event.getMessage(), - equalTo("The shape targeted by the member `smithy.example#A$member` changed from " - + "`smithy.example#B1` (map) to `smithy.example#B2` (map). Both the old and new " - + "shapes are a map, but the old shape value targeted `smithy.example#MyString` while " - + "the new shape targets `smithy.example#MyString2`.")); + equalTo(String.format("The shape targeted by the member `smithy.example#A$member` changed from " + + "`smithy.example#B1` (map) to `smithy.example#B2` (map). This will result in the following " + + "effective differences:%n%n" + + "- [ERROR] The shape targeted by the member `smithy.example#B1$value` changed from " + + "`smithy.example#MyString` (string) to `smithy.example#MyInteger` (integer). The type of the " + + "targeted shape changed from string to integer."))); + } + + @Test + public void handlesDeeplyNestedDiffs() { + Model modelA = Model.assembler() + .addImport(getClass() + .getResource("changed-member-target/double-nesting-a.smithy")) + .assemble() + .unwrap(); + Model modelB = Model.assembler() + .addImport(getClass() + .getResource("changed-member-target/double-nesting-b.smithy")) + .assemble() + .unwrap(); + List events = ModelDiff.compare(modelA, modelB); + + assertThat(TestHelper.findEvents(events, "ChangedMemberTarget").size(), equalTo(1)); + ValidationEvent event = TestHelper.findEvents(events, "ChangedMemberTarget").get(0); + assertThat(event.getSeverity(), equalTo(Severity.WARNING)); + assertThat(event.getMessage(), + equalTo(String.format("The shape targeted by the member `smithy.example#A$member` changed from " + + "`smithy.example#B` (list) to `smithy.example#NewB` (list). This was determined backward " + + "compatible. This will result in the following effective differences:%n%n" + + "- [WARNING] The shape targeted by the member `smithy.example#B$member` changed from " + + "`smithy.example#C` (list) to `smithy.example#NewC` (list). This was determined backward " + + "compatible. This will result in the following effective differences:%n" + + " %n" + + " - [WARNING] Added trait `smithy.api#pattern` with value `foo:.*`; The @pattern trait " + + "should only be added if the string already had adhered to the pattern."))); } } diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/double-nesting-a.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/double-nesting-a.smithy new file mode 100644 index 00000000000..1b5e71f4236 --- /dev/null +++ b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/double-nesting-a.smithy @@ -0,0 +1,16 @@ +$version: "2.0" + +namespace smithy.example + + +list A { + member: B +} + +list B { + member: C +} + +list C { + member: String +} diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/double-nesting-b.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/double-nesting-b.smithy new file mode 100644 index 00000000000..f3a816b53e3 --- /dev/null +++ b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/double-nesting-b.smithy @@ -0,0 +1,17 @@ +$version: "2.0" + +namespace smithy.example + + +list A { + member: NewB +} + +list NewB { + member: NewC +} + +list NewC { + @pattern("foo:.*") + member: String +} diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-invalid-nested1-a.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-list-added-compatible-member-trait-a.smithy similarity index 100% rename from smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-invalid-nested1-a.smithy rename to smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-list-added-compatible-member-trait-a.smithy diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-invalid-nested1-b.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-list-added-compatible-member-trait-b.smithy similarity index 100% rename from smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-invalid-nested1-b.smithy rename to smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-list-added-compatible-member-trait-b.smithy diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-invalid-nested2-a.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-list-added-incompatible-member-trait-a.smithy similarity index 100% rename from smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-invalid-nested2-a.smithy rename to smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-list-added-incompatible-member-trait-a.smithy diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-list-added-incompatible-member-trait-b.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-list-added-incompatible-member-trait-b.smithy new file mode 100644 index 00000000000..bc8d6295dfb --- /dev/null +++ b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-list-added-incompatible-member-trait-b.smithy @@ -0,0 +1,23 @@ +// See ChangedMemberTargetTest +$version: "2.0" + +namespace smithy.example + +list A { + member: B2 +} + +list B2 { + @noAddingTrait + member: MyString +} + +string MyString + +@trait( + selector: "member", + breakingChanges: [ + {change: "add"} + ] +) +structure noAddingTrait {} diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-valid-nested1-a.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-list-compatible-changed-member-a.smithy similarity index 100% rename from smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-valid-nested1-a.smithy rename to smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-list-compatible-changed-member-a.smithy diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-invalid-nested2-b.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-list-compatible-changed-member-b.smithy similarity index 100% rename from smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-invalid-nested2-b.smithy rename to smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-list-compatible-changed-member-b.smithy diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-list-incompatible-changed-member-a.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-list-incompatible-changed-member-a.smithy new file mode 100644 index 00000000000..a024b2e5079 --- /dev/null +++ b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-list-incompatible-changed-member-a.smithy @@ -0,0 +1,14 @@ +// See ChangedMemberTargetTest +$version: "2.0" + +namespace smithy.example + +list A { + member: B1 +} + +list B1 { + member: MyString +} + +string MyString diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-list-incompatible-changed-member-b.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-list-incompatible-changed-member-b.smithy new file mode 100644 index 00000000000..1d34eb5b99c --- /dev/null +++ b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-list-incompatible-changed-member-b.smithy @@ -0,0 +1,14 @@ +// See ChangedMemberTargetTest +$version: "2.0" + +namespace smithy.example + +list A { + member: B2 +} + +list B2 { + member: MyInteger +} + +integer MyInteger diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-list-unchanged-member-a.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-list-unchanged-member-a.smithy new file mode 100644 index 00000000000..a024b2e5079 --- /dev/null +++ b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-list-unchanged-member-a.smithy @@ -0,0 +1,14 @@ +// See ChangedMemberTargetTest +$version: "2.0" + +namespace smithy.example + +list A { + member: B1 +} + +list B1 { + member: MyString +} + +string MyString diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-valid-nested1-b.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-list-unchanged-member-b.smithy similarity index 100% rename from smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-valid-nested1-b.smithy rename to smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-list-unchanged-member-b.smithy diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-invalid-nested-mapkey1-a.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-key-added-compatible-trait-a.smithy similarity index 100% rename from smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-invalid-nested-mapkey1-a.smithy rename to smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-key-added-compatible-trait-a.smithy diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-invalid-nested-mapkey1-b.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-key-added-compatible-trait-b.smithy similarity index 100% rename from smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-invalid-nested-mapkey1-b.smithy rename to smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-key-added-compatible-trait-b.smithy diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-invalid-nested-mapkey2-a.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-key-added-incompatible-trait-a.smithy similarity index 100% rename from smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-invalid-nested-mapkey2-a.smithy rename to smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-key-added-incompatible-trait-a.smithy diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-key-added-incompatible-trait-b.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-key-added-incompatible-trait-b.smithy new file mode 100644 index 00000000000..0d365f1be53 --- /dev/null +++ b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-key-added-incompatible-trait-b.smithy @@ -0,0 +1,25 @@ +// See ChangedMemberTargetTest +$version: "2.0" + +namespace smithy.example + +structure A { + member: B2 +} + +map B2 { + @noAddingTrait + key: MyString + + value: MyString +} + +string MyString + +@trait( + selector: "member", + breakingChanges: [ + {change: "add"} + ] +) +structure noAddingTrait {} diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-invalid-nested-mapvalue1-a.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-key-compatible-changed-target-a.smithy similarity index 100% rename from smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-invalid-nested-mapvalue1-a.smithy rename to smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-key-compatible-changed-target-a.smithy diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-invalid-nested-mapkey2-b.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-key-compatible-changed-target-b.smithy similarity index 100% rename from smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-invalid-nested-mapkey2-b.smithy rename to smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-key-compatible-changed-target-b.smithy diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-invalid-nested-mapvalue2-a.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-key-incompatible-changed-target-a.smithy similarity index 100% rename from smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-invalid-nested-mapvalue2-a.smithy rename to smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-key-incompatible-changed-target-a.smithy diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-key-incompatible-changed-target-b.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-key-incompatible-changed-target-b.smithy new file mode 100644 index 00000000000..4bf76a7cf16 --- /dev/null +++ b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-key-incompatible-changed-target-b.smithy @@ -0,0 +1,19 @@ +// See ChangedMemberTargetTest +$version: "2.0" + +namespace smithy.example + +structure A { + member: B2 +} + +map B2 { + key: MyEnum + value: MyString +} + +string MyString + +enum MyEnum { + FOO = "foo" +} diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-valid-nested2-a.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-unchanged-member-a.smithy similarity index 100% rename from smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-valid-nested2-a.smithy rename to smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-unchanged-member-a.smithy diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-valid-nested2-b.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-unchanged-member-b.smithy similarity index 100% rename from smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-valid-nested2-b.smithy rename to smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-unchanged-member-b.smithy diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-value-added-compatible-trait-a.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-value-added-compatible-trait-a.smithy new file mode 100644 index 00000000000..66fb1a03a37 --- /dev/null +++ b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-value-added-compatible-trait-a.smithy @@ -0,0 +1,15 @@ +// See ChangedMemberTargetTest +$version: "2.0" + +namespace smithy.example + +structure A { + member: B1 +} + +map B1 { + key: MyString + value: MyString +} + +string MyString diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-invalid-nested-mapvalue1-b.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-value-added-compatible-trait-b.smithy similarity index 100% rename from smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-invalid-nested-mapvalue1-b.smithy rename to smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-value-added-compatible-trait-b.smithy diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-value-added-incompatible-trait-a.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-value-added-incompatible-trait-a.smithy new file mode 100644 index 00000000000..66fb1a03a37 --- /dev/null +++ b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-value-added-incompatible-trait-a.smithy @@ -0,0 +1,15 @@ +// See ChangedMemberTargetTest +$version: "2.0" + +namespace smithy.example + +structure A { + member: B1 +} + +map B1 { + key: MyString + value: MyString +} + +string MyString diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-value-added-incompatible-trait-b.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-value-added-incompatible-trait-b.smithy new file mode 100644 index 00000000000..a87db7c8f49 --- /dev/null +++ b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-value-added-incompatible-trait-b.smithy @@ -0,0 +1,25 @@ +// See ChangedMemberTargetTest +$version: "2.0" + +namespace smithy.example + +structure A { + member: B2 +} + +map B2 { + key: MyString + + @noAddingTrait + value: MyString +} + +string MyString + +@trait( + selector: "member", + breakingChanges: [ + {change: "add"} + ] +) +structure noAddingTrait {} diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-value-compatible-changed-target-a.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-value-compatible-changed-target-a.smithy new file mode 100644 index 00000000000..66fb1a03a37 --- /dev/null +++ b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-value-compatible-changed-target-a.smithy @@ -0,0 +1,15 @@ +// See ChangedMemberTargetTest +$version: "2.0" + +namespace smithy.example + +structure A { + member: B1 +} + +map B1 { + key: MyString + value: MyString +} + +string MyString diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-invalid-nested-mapvalue2-b.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-value-compatible-changed-target-b.smithy similarity index 100% rename from smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target-invalid-nested-mapvalue2-b.smithy rename to smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-value-compatible-changed-target-b.smithy diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-value-incompatible-changed-target-a.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-value-incompatible-changed-target-a.smithy new file mode 100644 index 00000000000..66fb1a03a37 --- /dev/null +++ b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-value-incompatible-changed-target-a.smithy @@ -0,0 +1,15 @@ +// See ChangedMemberTargetTest +$version: "2.0" + +namespace smithy.example + +structure A { + member: B1 +} + +map B1 { + key: MyString + value: MyString +} + +string MyString diff --git a/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-value-incompatible-changed-target-b.smithy b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-value-incompatible-changed-target-b.smithy new file mode 100644 index 00000000000..dc565e4d382 --- /dev/null +++ b/smithy-diff/src/test/resources/software/amazon/smithy/diff/evaluators/changed-member-target/nested-map-value-incompatible-changed-target-b.smithy @@ -0,0 +1,17 @@ +// See ChangedMemberTargetTest +$version: "2.0" + +namespace smithy.example + +structure A { + member: B2 +} + +map B2 { + key: MyString + value: MyInteger +} + +string MyString + +integer MyInteger