Skip to content

Commit

Permalink
Load other shapes when a mixin can't be resolved
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
milesziemer committed Mar 26, 2024
1 parent 7c9e134 commit 79e306f
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ private List<ShapeId> sort() {
emitUnresolved(shape, e.getUnresolved(), e.getResolved());
}
}
return Collections.emptyList();
return e.getResolved();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Model> result = Model.assembler()
.addUnparsedModel("foo.smithy", modelText)
.assemble();

assertThat(result.isBroken(), is(true));
assertThat(result.getResult().isPresent(), is(true));
Set<ShapeId> 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")
));
}
}

0 comments on commit 79e306f

Please sign in to comment.