From 79e306f11d934383a6915ab7a7c28be33a5ab363 Mon Sep 17 00:00:00 2001 From: Miles Ziemer Date: Tue, 26 Mar 2024 10:55:57 -0400 Subject: [PATCH] Load other shapes when a mixin can't be resolved Found this when working on the language server - if you have a model like: ``` $version: "2" namespace com.foo string Foo @mixin structure Bar {} structure Baz with [Unknown] {} ``` loading it will result in `Foo` and `Bar` also not being loaded. It seems like the reason for this was that if TopologicalShapeSort failed, we wouldn't build _any_ shapes into the model, despite TopologicalShapeSort being able to resolve some shapes. Practically speaking I'm not sure if this means much in general, but for the language server its big because without it, you can't have mixin completions unless you either don't reload the model on changes (which means you can't type a mixin, then get completions to use it), or don't update the server's version of the model if the new version is broken (which reduces the quality of diagnostics, and has the same issue as the other way). --- .../smithy/model/loader/LoaderShapeMap.java | 2 +- .../model/loader/ModelAssemblerTest.java | 25 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderShapeMap.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderShapeMap.java index 669b88f6338..122bd5cd20c 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderShapeMap.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderShapeMap.java @@ -229,7 +229,7 @@ private List sort() { emitUnresolved(shape, e.getUnresolved(), e.getResolved()); } } - return Collections.emptyList(); + return e.getResolved(); } } 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 4067b70662c..3c542b41f9d 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 @@ -49,6 +49,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.function.Supplier; import java.util.stream.Collectors; import org.junit.jupiter.api.AfterEach; @@ -1391,4 +1392,28 @@ public void handlesMultipleInheritanceForMixinMembers() { String actualPattern = shape.expectTrait(PatternTrait.class).getValue(); assertThat(actualPattern, equalTo("baz")); } + + @Test + public void loadsShapesWhenThereAreUnresolvedMixins() { + String modelText = "$version: \"2\"\n" + + "namespace com.foo\n" + + "\n" + + "string Foo\n" + + "@mixin\n" + + "structure Bar {}\n" + + "structure Baz with [Unknown] {}\n"; + ValidatedResult result = Model.assembler() + .addUnparsedModel("foo.smithy", modelText) + .assemble(); + + assertThat(result.isBroken(), is(true)); + assertThat(result.getResult().isPresent(), is(true)); + Set fooShapes = result.getResult().get().getShapeIds().stream() + .filter(id -> id.getNamespace().equals("com.foo")) + .collect(Collectors.toSet()); + assertThat(fooShapes, containsInAnyOrder( + ShapeId.from("com.foo#Foo"), + ShapeId.from("com.foo#Bar") + )); + } }