Skip to content

Commit

Permalink
Fix matching inline inputs/outputs with operations (#111)
Browse files Browse the repository at this point in the history
Issue #, if available: #110

Description of changes:

This fixes #110 (an IIOBE) by preventing the underlying issue: the FileCachingCollector mixing up operations with the same name in different namespaces.

Unfortunately, the test that reproduced the issue is flaky, as it would only fail about half the time before the fix was added - I think there's some non-deterministic behavior in some part of the collector that affects the order of things being collected, and sometimes prevents exhibiting the incorrect behavior. I've confirmed that it reliably passes now after the fix was applied.
  • Loading branch information
kubukoz committed May 30, 2023
1 parent 708bfc4 commit b04ece8
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -239,9 +239,12 @@ private Optional<OperationShape> getOperationForInlinedInputOrOutput(Shape shape
) {
String suffix = getOperationInputOrOutputSuffix(shape, preamble);
String shapeName = shape.getId().getName();

String matchingOperationName = shapeName.substring(0, shapeName.length() - suffix.length());
ShapeId matchingOperationId = ShapeId.fromParts(shape.getId().getNamespace(), matchingOperationName);

return model.shapes(OperationShape.class)
.filter(operationShape -> operationShape.getId().getName().equals(matchingOperationName))
.filter(operationShape -> operationShape.getId().equals(matchingOperationId))
.findFirst()
.filter(operation -> shapeWasDefinedInline(operation, shape, modelFile));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,23 @@ public void allowsEmptyStructsWithMixins() throws Exception {
}
}

// https://github.com/awslabs/smithy-language-server/issues/110
// Note: This test is flaky, it may succeed even if the code being tested is incorrect.
@Test
public void handlesSameOperationNameBetweenNamespaces() throws Exception {
Path baseDir = Paths.get(SmithyProjectTest.class.getResource("models/operation-name-conflict").toURI());
Path modelA = baseDir.resolve("a.smithy");
Path modelB = baseDir.resolve("b.smithy");
List<Path> modelFiles = ListUtils.of(modelA, modelB);

try (Harness hs = Harness.create(SmithyBuildExtensions.builder().build(), modelFiles)) {
Map<ShapeId, Location> locationMap = hs.getProject().getLocations();

correctLocation(locationMap, "a#HelloWorld", 4, 0, 13, 1);
correctLocation(locationMap, "b#HelloWorld", 6, 0, 15, 1);
}
}

@Test
public void definitionLocationsV1() throws Exception {
Path baseDir = Paths.get(SmithyProjectTest.class.getResource("models/v1").toURI());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
$version: "2"

namespace a

operation HelloWorld {
input := {
@required
name: String
}
output := {
@required
name: String
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
$version: "2"

namespace b

string Ignored

operation HelloWorld {
input := {
@required
name: String
}
output := {
@required
name: String
}
}

0 comments on commit b04ece8

Please sign in to comment.