Skip to content

Commit 2813db5

Browse files
committed
Fix shape search resolution
Fixes an issue with ShapeSearch::findShape where the local namespace was checked before imports. It was inconsistent with the way the model loader resolves shapes, so you could end up jumping to the wrong definition/references, or renaming the wrong shape.
1 parent eb05178 commit 2813db5

File tree

1 file changed

+55
-43
lines changed

1 file changed

+55
-43
lines changed

src/main/java/software/amazon/smithy/lsp/language/ShapeSearch.java

Lines changed: 55 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import java.util.List;
1010
import java.util.Optional;
1111
import software.amazon.smithy.lsp.document.DocumentId;
12+
import software.amazon.smithy.lsp.document.DocumentImports;
1213
import software.amazon.smithy.lsp.project.SmithyFile;
1314
import software.amazon.smithy.lsp.syntax.NodeCursor;
1415
import software.amazon.smithy.lsp.syntax.StatementView;
@@ -34,9 +35,9 @@ private ShapeSearch() {
3435
* Attempts to find a shape using a token, {@code nameOrId}.
3536
*
3637
* <p>When {@code nameOrId} does not contain a '#', this searches for shapes
37-
* either in {@code idlParse}'s namespace, in {@code idlParse}'s
38-
* imports, or the prelude, in that order. When {@code nameOrId} does contain
39-
* a '#', it is assumed to be a full shape id and is searched for directly.
38+
* either in {@code idlParse}'s imports, in {@code idlParse}'s namespace, or
39+
* the prelude, in that order. When {@code nameOrId} does contain a '#', it
40+
* is assumed to be a full shape id and is searched for directly.
4041
*
4142
* @param parseResult The parse result of the file {@code nameOrId} is within.
4243
* @param nameOrId The name or shape id of the shape to find.
@@ -45,58 +46,69 @@ private ShapeSearch() {
4546
*/
4647
static Optional<Shape> findShape(Syntax.IdlParseResult parseResult, String nameOrId, Model model) {
4748
return switch (nameOrId) {
49+
case null -> Optional.empty();
50+
4851
case String s when s.isEmpty() -> Optional.empty();
49-
case String s when s.contains("#") -> tryFrom(s).flatMap(model::getShape);
50-
case String s -> {
51-
Optional<Shape> fromCurrent = tryFromParts(parseResult.namespace().namespace(), s)
52-
.flatMap(model::getShape);
53-
if (fromCurrent.isPresent()) {
54-
yield fromCurrent;
55-
}
5652

57-
if (nameOrId.contains("$")) {
58-
// Relative member id, so it could be a member of an imported shape
59-
String[] split = nameOrId.split("\\$");
60-
String containerName = split[0];
61-
String memberName = split[1];
62-
for (String fileImport : parseResult.imports().imports()) {
63-
var imported = tryFrom(fileImport)
64-
.filter(importId -> importId.getName().equals(containerName))
65-
.map(importId -> importId.withMember(memberName))
66-
.flatMap(model::getShape);
67-
if (imported.isPresent()) {
68-
yield imported;
69-
}
70-
}
71-
} else {
72-
for (String fileImport : parseResult.imports().imports()) {
73-
Optional<Shape> imported = tryFrom(fileImport)
74-
.filter(importId -> importId.getName().equals(s))
75-
.flatMap(model::getShape);
76-
if (imported.isPresent()) {
77-
yield imported;
78-
}
79-
}
80-
}
53+
case String s when s.contains("#") -> tryFrom(s, model);
8154

82-
yield tryFromParts(Prelude.NAMESPACE, s).flatMap(model::getShape);
83-
}
84-
case null -> Optional.empty();
55+
default -> fromImports(parseResult.imports(), nameOrId, model)
56+
.or(() -> tryFromRelative(parseResult.namespace().namespace(), nameOrId, model))
57+
.or(() -> tryFromRelative(Prelude.NAMESPACE, nameOrId, model));
8558
};
8659
}
8760

88-
private static Optional<ShapeId> tryFrom(String id) {
61+
private static Optional<Shape> fromImports(DocumentImports imports, String nameOrId, Model model) {
62+
if (imports.imports().isEmpty()) {
63+
return Optional.empty();
64+
}
65+
66+
if (nameOrId.contains("$")) {
67+
// Relative member id, so it could be a member of an imported shape
68+
String[] split = nameOrId.split("\\$");
69+
String containerName = split[0];
70+
String memberName = split[1];
71+
String matchString = "#" + containerName;
72+
for (String fileImport : imports.imports()) {
73+
if (fileImport.endsWith(matchString)) {
74+
return tryWithMember(fileImport, memberName, model);
75+
}
76+
}
77+
} else {
78+
String matchString = "#" + nameOrId;
79+
for (String fileImport : imports.imports()) {
80+
if (fileImport.endsWith(matchString)) {
81+
return tryFrom(fileImport, model);
82+
}
83+
}
84+
}
85+
86+
return Optional.empty();
87+
}
88+
89+
private static Optional<Shape> tryFrom(String id, Model model) {
90+
try {
91+
ShapeId shapeId = ShapeId.from(id);
92+
return model.getShape(shapeId);
93+
} catch (ShapeIdSyntaxException e) {
94+
return Optional.empty();
95+
}
96+
}
97+
98+
private static Optional<Shape> tryWithMember(String rootId, String memberName, Model model) {
8999
try {
90-
return Optional.of(ShapeId.from(id));
91-
} catch (ShapeIdSyntaxException ignored) {
100+
ShapeId shapeId = ShapeId.from(rootId).withMember(memberName);
101+
return model.getShape(shapeId);
102+
} catch (ShapeIdSyntaxException e) {
92103
return Optional.empty();
93104
}
94105
}
95106

96-
private static Optional<ShapeId> tryFromParts(String namespace, String name) {
107+
private static Optional<Shape> tryFromRelative(String namespace, String name, Model model) {
97108
try {
98-
return Optional.of(ShapeId.fromRelative(namespace, name));
99-
} catch (ShapeIdSyntaxException ignored) {
109+
ShapeId shapeId = ShapeId.fromRelative(namespace, name);
110+
return model.getShape(shapeId);
111+
} catch (ShapeIdSyntaxException e) {
100112
return Optional.empty();
101113
}
102114
}

0 commit comments

Comments
 (0)