Skip to content

Commit

Permalink
Normalize paths in the project loader
Browse files Browse the repository at this point in the history
Fixes an issue where the server can't find project files when smithy-build.json
has unnormalized paths.
  • Loading branch information
milesziemer committed Apr 26, 2024
1 parent 6fce052 commit 53c688e
Show file tree
Hide file tree
Showing 17 changed files with 152 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ static Result<List<Path>, Exception> resolveDependencies(Path root, ProjectConfi
.collect(Collectors.toCollection(ArrayList::new));
config.getDependencies().forEach((projectDependency) -> {
// TODO: Not sure if this needs to check for existence
Path path = root.resolve(projectDependency.getPath());
Path path = root.resolve(projectDependency.getPath()).normalize();
deps.add(path);
});
return deps;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,14 @@ public static Result<Project, List<Exception>> load(Path root) {

List<Path> dependencies = resolveResult.unwrap();

// TODO: We need some default behavior for when no project files are specified, like running in
// 'detached' mode or something
List<Path> sources = config.getSources().stream().map(root::resolve).collect(Collectors.toList());
List<Path> imports = config.getImports().stream().map(root::resolve).collect(Collectors.toList());
List<Path> sources = config.getSources().stream()
.map(root::resolve)
.map(Path::normalize)
.collect(Collectors.toList());
List<Path> imports = config.getImports().stream()
.map(root::resolve)
.map(Path::normalize)
.collect(Collectors.toList());

// The model assembler factory is used to get assemblers that already have the correct
// dependencies resolved for future loads
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/software/amazon/smithy/lsp/LspMatchers.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public final class LspMatchers {
private LspMatchers() {}

public static Matcher<CompletionItem> hasLabel(String label) {
return new CustomTypeSafeMatcher<CompletionItem>("a completion item with the right label") {
return new CustomTypeSafeMatcher<CompletionItem>("a completion item with the label + `" + label + "`") {
@Override
protected boolean matchesSafely(CompletionItem item) {
return item.getLabel().equals(label);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.google.gson.JsonPrimitive;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.CompletableFuture;
import java.util.stream.Collectors;
Expand All @@ -48,6 +49,7 @@
import org.eclipse.lsp4j.services.LanguageClient;
import org.hamcrest.Matcher;
import org.junit.jupiter.api.Test;
import software.amazon.smithy.build.model.SmithyBuildConfig;
import software.amazon.smithy.lsp.document.Document;
import software.amazon.smithy.lsp.protocol.RangeAdapter;

Expand Down Expand Up @@ -968,6 +970,34 @@ public void removingAttachedFile() {
assertThat(server.getProjects().isDetached(movedUri), is(true));
}

@Test
public void loadsProjectWithUnNormalizedSourcesDirs() {
SmithyBuildConfig config = SmithyBuildConfig.builder()
.version("1")
.sources(Collections.singletonList("./././smithy"))
.build();
String filename = "smithy/main.smithy";
String modelText = "$version: \"2\"\n"
+ "namespace com.foo\n"
+ "\n"
+ "string Foo\n";
TestWorkspace workspace = TestWorkspace.builder()
.withSourceDir(TestWorkspace.dir()
.path("./smithy")
.withSourceFile("main.smithy", modelText))
.withConfig(config)
.build();
SmithyLanguageServer server = initFromWorkspace(workspace);

String uri = workspace.getUri(filename);

server.didOpen(RequestBuilders.didOpen()
.uri(uri)
.text(modelText)
.build());
assertThat(server.getProjects().isDetached(uri), is(false));
}

public static SmithyLanguageServer initFromWorkspace(TestWorkspace workspace) {
return initFromWorkspace(workspace, new StubClient());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public final class SmithyMatchers {
private SmithyMatchers() {}

public static Matcher<Model> hasShapeWithId(String id) {
return new CustomTypeSafeMatcher<Model>("a model with the right shape id") {
return new CustomTypeSafeMatcher<Model>("a model with the shape id `" + id + "`") {
@Override
protected boolean matchesSafely(Model item) {
return item.getShape(ShapeId.from(id)).isPresent();
Expand Down
22 changes: 17 additions & 5 deletions src/test/java/software/amazon/smithy/lsp/TestWorkspace.java
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@ public static Builder builder() {
return new Builder();
}

public static Dir dir() {
return new Dir();
}

public static class Dir {
String path;
Map<String, String> sourceModels = new HashMap<>();
Expand Down Expand Up @@ -163,6 +167,7 @@ private static void writeModels(Path toDir, Map<String, String> models) throws E
}

public static final class Builder extends Dir {
private SmithyBuildConfig config = null;
private Builder() {}

@Override
Expand All @@ -189,6 +194,11 @@ public Builder withImportDir(Dir dir) {
return this;
}

public Builder withConfig(SmithyBuildConfig config) {
this.config = config;
return this;
}

public TestWorkspace build() {
try {
if (path == null) {
Expand All @@ -205,11 +215,13 @@ public TestWorkspace build() {
imports.addAll(importModels.keySet());
imports.addAll(importDirs.stream().map(d -> d.path).collect(Collectors.toList()));

SmithyBuildConfig config = SmithyBuildConfig.builder()
.version("1")
.sources(sources)
.imports(imports)
.build();
if (config == null) {
config = SmithyBuildConfig.builder()
.version("1")
.sources(sources)
.imports(imports)
.build();
}
String configString = Node.prettyPrintJson(MAPPER.serialize(config));
Files.write(root.resolve("smithy-build.json"), configString.getBytes(StandardCharsets.UTF_8));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,18 @@ public void loadsProjectWithSubdir() {
Project project = ProjectLoader.load(root).unwrap();

assertThat(project.getRoot(), equalTo(root));
assertThat(project.getSources(), hasItem(root.resolve("model")));
assertThat(project.getSources(), hasItems(
root.resolve("model"),
root.resolve("model2")));
assertThat(project.getSmithyFiles().keySet(), hasItems(
containsString("model/main.smithy"),
containsString("model/subdir/sub.smithy"),
containsString("model2/subdir2/sub2.smithy"),
containsString("model2/subdir2/subsubdir/subsub.smithy")));
assertThat(project.getModelResult().isBroken(), is(false));
assertThat(project.getModelResult().unwrap(), hasShapeWithId("com.foo#Foo"));
assertThat(project.getModelResult().unwrap(), hasShapeWithId("com.foo#Bar"));
assertThat(project.getModelResult().unwrap(), hasShapeWithId("com.foo#Baz"));
}

@Test
Expand Down Expand Up @@ -222,4 +231,23 @@ public void failsLoadingUnresolvableProjectDependency() {

assertThat(result.isErr(), is(true));
}

@Test
public void loadsProjectWithUnNormalizedDirs() {
Path root = Paths.get(getClass().getResource("unnormalized-dirs").getPath());
Project project = ProjectLoader.load(root).unwrap();

assertThat(project.getRoot(), equalTo(root));
assertThat(project.getSources(), hasItems(
root.resolve("model"),
root.resolve("model2")));
assertThat(project.getImports(), hasItem(root.resolve("model3")));
assertThat(project.getSmithyFiles().keySet(), hasItems(
equalTo(root.resolve("model/test-traits.smithy").toString()),
equalTo(root.resolve("model/one.smithy").toString()),
equalTo(root.resolve("model2/two.smithy").toString()),
equalTo(root.resolve("model3/three.smithy").toString()),
containsString(root.resolve("smithy-test-traits.jar") + "!/META-INF/smithy/smithy.test.json")));
assertThat(project.getDependencies(), hasItem(root.resolve("smithy-test-traits.jar")));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
$version: "2.0"

namespace com.foo

string Baz
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
$version: "2.0"

namespace com.foo

string Boz
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{
"version": "1",
"sources": ["model/"]
"sources": ["model", "model2"]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"dependencies": [
{
"name": "smithy-test-traits",
"path": "./././/smithy-test-traits.jar"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
$version: "2.0"

namespace com.foo

string One
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
$version: "1.0"

namespace ns.test

use smithy.test#test

@test()
service Weather {
version: "2022-05-24",
operations: [GetCurrentTime]
}

@readonly
operation GetCurrentTime {
input: GetCurrentTimeInput,
output: GetCurrentTimeOutput
}

@input
structure GetCurrentTimeInput {}

@output
structure GetCurrentTimeOutput {
@required
time: Timestamp,
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
$version: "2.0"

namespace com.foo

string Two
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
$version: "2.0"

namespace com.foo

string Three
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"version": "1",
"sources": ["./model/", "model2////"],
"imports": ["././././model3//"]
}
Binary file not shown.

0 comments on commit 53c688e

Please sign in to comment.