diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderTraitMap.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderTraitMap.java index 1e15e276c92..639cebc889f 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderTraitMap.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderTraitMap.java @@ -21,9 +21,11 @@ import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.logging.Logger; import software.amazon.smithy.model.SourceException; import software.amazon.smithy.model.SourceLocation; @@ -47,6 +49,7 @@ final class LoaderTraitMap { private final List events; private final boolean allowUnknownTraits; private final Map> unclaimed = new HashMap<>(); + private final Set claimed = new HashSet<>(); LoaderTraitMap(TraitFactory traitFactory, List events, boolean allowUnknownTraits) { this.traitFactory = traitFactory; @@ -147,12 +150,19 @@ private void applyTraitsToShape(AbstractShapeBuilder shape, Trait trait) { // Traits can be applied to synthesized members inherited from mixins. Applying these traits is deferred until // the point in which mixin members are synthesized into shapes. Map claimTraitsForShape(ShapeId id) { - return unclaimed.containsKey(id) ? unclaimed.remove(id) : Collections.emptyMap(); + if (!unclaimed.containsKey(id)) { + return Collections.emptyMap(); + } + claimed.add(id); + return unclaimed.get(id); } // Emit events if any traits were applied to shapes that weren't found in the model. void emitUnclaimedTraits() { for (Map.Entry> entry : unclaimed.entrySet()) { + if (claimed.contains(entry.getKey())) { + continue; + } for (Map.Entry traitEntry : entry.getValue().entrySet()) { events.add(ValidationEvent.builder() .id(Validator.MODEL_ERROR) diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelAssemblerTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelAssemblerTest.java index 6f3950fc00c..890f4e7f053 100644 --- a/smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelAssemblerTest.java +++ b/smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelAssemblerTest.java @@ -1341,4 +1341,31 @@ public void exceptionsThrownWhenCreatingTraitsDontCrashSmithy() { assertThat(result.getValidationEvents(Severity.ERROR).get(0).getMessage(), equalTo("Error creating trait `smithy.foo#baz`: Oops!")); } + + @Test + public void resolvesDuplicateTraitApplicationsToDuplicateMixedInMembers() throws Exception { + String model = IoUtils.readUtf8File(Paths.get(getClass().getResource("mixins/apply-to-mixed-member.json").toURI())); + // Should be able to de-conflict the apply statements when the same model is loaded multiple times. + // See https://github.com/smithy-lang/smithy/issues/2004 + Model.assembler() + .addUnparsedModel("test.json", model) + .addUnparsedModel("test2.json", model) + .addUnparsedModel("test3.json", model) + .assemble() + .unwrap(); + } + + @Test + public void resolvesDuplicateTraitApplicationsToSameMixedInMember() throws Exception { + String modelToApplyTo = IoUtils.readUtf8File(Paths.get(getClass().getResource("mixins/mixed-member.smithy").toURI())); + String modelWithApply = IoUtils.readUtf8File(Paths.get(getClass().getResource("mixins/member-apply-other-namespace.smithy").toURI())); + // Should be able to load when you have multiple identical apply statements to the same mixed in member. + // See https://github.com/smithy-lang/smithy/issues/2004 + Model.assembler() + .addUnparsedModel("mixed-member.smithy", modelToApplyTo) + .addUnparsedModel("member-apply-1.smithy", modelWithApply) + .addUnparsedModel("member-apply-2.smithy", modelWithApply) + .assemble() + .unwrap(); + } } diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/apply-to-mixed-member.json b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/apply-to-mixed-member.json new file mode 100644 index 00000000000..544456e8be3 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/apply-to-mixed-member.json @@ -0,0 +1,34 @@ +{ + "smithy": "2.0", + "shapes": { + "com.example#Common": { + "type": "structure", + "members": { + "description": { + "target": "smithy.api#String", + "traits": { + "smithy.api#required": {} + } + } + }, + "traits": { + "smithy.api#mixin": {} + } + }, + "com.example#Thing": { + "type": "structure", + "mixins": [ + { + "target": "com.example#Common" + } + ], + "members": {} + }, + "com.example#Thing$description": { + "type": "apply", + "traits": { + "smithy.api#default": "test" + } + } + } +} diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/member-apply-other-namespace.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/member-apply-other-namespace.smithy new file mode 100644 index 00000000000..faad3a6ecc6 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/member-apply-other-namespace.smithy @@ -0,0 +1,7 @@ +$version: "2.0" + +namespace com.example.other + +use com.example#Thing + +apply Thing$description @default("test") diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/mixed-member.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/mixed-member.smithy new file mode 100644 index 00000000000..864ee0b064f --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/mixins/mixed-member.smithy @@ -0,0 +1,11 @@ +$version: "2.0" + +namespace com.example + +@mixin +structure Common { + @required + description: String +} + +structure Thing with [Common] {}