Skip to content

Commit

Permalink
Fix file patterns for windows
Browse files Browse the repository at this point in the history
The file patterns we were using for telling the client which files to
watch, and to match file events in `didChangeWatchedFiles` to projects,
were not working properly on windows because they didn't use the correct
file separator.
  • Loading branch information
milesziemer committed Aug 9, 2024
1 parent c4845cb commit b68eda0
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

package software.amazon.smithy.lsp.project;

import java.io.File;
import java.nio.file.FileSystems;
import java.nio.file.Path;
import java.nio.file.PathMatcher;
Expand Down Expand Up @@ -48,13 +49,14 @@ public static PathMatcher getSmithyFilesPathMatcher(Project project) {
*/
public static String getBuildFilesWatchPattern(Project project) {
Path root = project.root();
String smithyBuildJsonPattern = root.resolve(ProjectConfigLoader.SMITHY_BUILD).toString();
String smithyProjectJsonPattern = root.resolve(ProjectConfigLoader.SMITHY_PROJECT).toString();
String buildJsonPattern = escapeBackslashes(root.resolve(ProjectConfigLoader.SMITHY_BUILD).toString());
String projectJsonPattern = escapeBackslashes(root.resolve(ProjectConfigLoader.SMITHY_PROJECT).toString());

List<String> patterns = new ArrayList<>();
patterns.add(smithyBuildJsonPattern);
patterns.add(smithyProjectJsonPattern);
patterns.add(buildJsonPattern);
patterns.add(projectJsonPattern);
for (String buildExt : ProjectConfigLoader.SMITHY_BUILD_EXTS) {
patterns.add(root.resolve(buildExt).toString());
patterns.add(escapeBackslashes(root.resolve(buildExt).toString()));
}

return "{" + String.join(",", patterns) + "}";
Expand All @@ -76,20 +78,24 @@ public static PathMatcher getBuildFilesPathMatcher(Project project) {
private static String getSmithyFilePattern(Path path, boolean isWatcherPattern) {
String glob = path.toString();
if (glob.endsWith(".smithy") || glob.endsWith(".json")) {
return glob;
return escapeBackslashes(glob);
}

// we have a directory
if (glob.endsWith("/")) {
glob = glob + "**";
} else {
glob = glob + "/**";
if (!glob.endsWith(File.separator)) {
glob += File.separator;
}
glob += "**";

if (isWatcherPattern) {
glob += ".{smithy,json}";
}

return glob;
return escapeBackslashes(glob);
}

// In glob patterns, '\' is an escape character, so it needs to escaped
// itself to work as a separator (i.e. for windows)
private static String escapeBackslashes(String pattern) {
return pattern.replace("\\", "\\\\");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1807,7 +1807,11 @@ public void multiRootAddingWatchedFile() throws Exception {

server.didChange(RequestBuilders.didChange()
.uri(fooUri)
.text("$version: \"2\"\nnamespace com.foo\nstructure Foo {}\n")
.text("""
$version: "2"
namespace com.foo
structure Foo {}
""")
.range(LspAdapter.origin())
.build());

Expand All @@ -1817,7 +1821,9 @@ public void multiRootAddingWatchedFile() throws Exception {

server.didChange(RequestBuilders.didChange()
.uri(fooUri)
.text("\nstructure Bar {}")
.text("""
structure Bar {}""")
.range(LspAdapter.point(3, 0))
.build());

Expand All @@ -1826,9 +1832,9 @@ public void multiRootAddingWatchedFile() throws Exception {
Project projectFoo = server.getProjects().getProjectByName("foo");
Project projectBar = server.getProjects().getProjectByName("bar");

assertThat(projectFoo.smithyFiles(), hasKey(endsWith("model/main.smithy")));
assertThat(projectBar.smithyFiles(), hasKey(endsWith("model/main.smithy")));
assertThat(projectBar.smithyFiles(), hasKey(endsWith("model/other.smithy")));
assertThat(projectFoo.smithyFiles(), hasKey(endsWith("main.smithy")));
assertThat(projectBar.smithyFiles(), hasKey(endsWith("main.smithy")));
assertThat(projectBar.smithyFiles(), hasKey(endsWith("other.smithy")));

assertThat(projectFoo.modelResult(), SmithyMatchers.hasValue(SmithyMatchers.hasShapeWithId("com.foo#Foo")));
assertThat(projectFoo.modelResult(), SmithyMatchers.hasValue(SmithyMatchers.hasShapeWithId("com.foo#Bar")));
Expand Down Expand Up @@ -1913,8 +1919,8 @@ public void multiRootChangingBuildFile() throws Exception {
Project projectFoo = server.getProjects().getProjectByName("foo");
Project projectBar = server.getProjects().getProjectByName("bar");

assertThat(projectFoo.smithyFiles(), hasKey(endsWith("model/main.smithy")));
assertThat(projectBar.smithyFiles(), hasKey(endsWith("model/main.smithy")));
assertThat(projectFoo.smithyFiles(), hasKey(endsWith("main.smithy")));
assertThat(projectBar.smithyFiles(), hasKey(endsWith("main.smithy")));
assertThat(projectBar.smithyFiles(), hasKey(endsWith("other.smithy")));

assertThat(projectFoo.modelResult(), SmithyMatchers.hasValue(SmithyMatchers.hasShapeWithId("com.foo#Foo")));
Expand Down
20 changes: 20 additions & 0 deletions src/test/java/software/amazon/smithy/lsp/UtilMatchers.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

package software.amazon.smithy.lsp;

import java.nio.file.Path;
import java.nio.file.PathMatcher;
import java.util.Optional;
import org.hamcrest.CustomTypeSafeMatcher;
import org.hamcrest.Description;
Expand Down Expand Up @@ -33,4 +35,22 @@ public void describeMismatchSafely(Optional<T> item, Description description) {
}
};
}

public static Matcher<PathMatcher> canMatchPath(Path path) {
// PathMatcher implementations don't seem to have a nice toString, so this Matcher
// doesn't print out the PathMatcher that couldn't match, but we could wrap the
// system default PathMatcher in one that stores the original pattern, if this
// Matcher becomes too hard to diagnose failures for.
return new CustomTypeSafeMatcher<PathMatcher>("A matcher that matches " + path) {
@Override
protected boolean matchesSafely(PathMatcher item) {
return item.matches(path);
}

@Override
protected void describeMismatchSafely(PathMatcher item, Description mismatchDescription) {
mismatchDescription.appendText("did not match");
}
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,18 @@
package software.amazon.smithy.lsp.handler;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.endsWith;
import static org.hamcrest.Matchers.hasItem;

import java.nio.file.FileSystems;
import java.nio.file.PathMatcher;
import java.util.HashSet;
import java.util.List;
import java.util.stream.Collectors;
import org.eclipse.lsp4j.DidChangeWatchedFilesRegistrationOptions;
import org.eclipse.lsp4j.Registration;
import org.junit.jupiter.api.Test;
import software.amazon.smithy.build.model.SmithyBuildConfig;
import software.amazon.smithy.lsp.TestWorkspace;
import software.amazon.smithy.lsp.UtilMatchers;
import software.amazon.smithy.lsp.project.Project;
import software.amazon.smithy.lsp.project.ProjectLoader;
import software.amazon.smithy.lsp.project.ProjectManager;
Expand Down Expand Up @@ -44,17 +45,21 @@ public void createsCorrectRegistrations() {
.build();

Project project = ProjectLoader.load(workspace.getRoot(), new ProjectManager(), new HashSet<>()).unwrap();
List<String> watcherPatterns = FileWatcherRegistrationHandler.getSmithyFileWatcherRegistrations(List.of(project))
List<PathMatcher> matchers = FileWatcherRegistrationHandler.getSmithyFileWatcherRegistrations(List.of(project))
.stream()
.map(Registration::getRegisterOptions)
.map(o -> (DidChangeWatchedFilesRegistrationOptions) o)
.flatMap(options -> options.getWatchers().stream())
.map(watcher -> watcher.getGlobPattern().getLeft())
.collect(Collectors.toList());
// The watcher glob patterns will look different between windows/unix, so turning
// them into path matchers lets us do platform-agnostic assertions.
.map(pattern -> FileSystems.getDefault().getPathMatcher("glob:" + pattern))
.toList();

assertThat(watcherPatterns, containsInAnyOrder(
endsWith("foo/**.{smithy,json}"),
endsWith("other/**.{smithy,json}"),
endsWith("abc.smithy")));
assertThat(matchers, hasItem(UtilMatchers.canMatchPath(workspace.getRoot().resolve("foo/abc.smithy"))));
assertThat(matchers, hasItem(UtilMatchers.canMatchPath(workspace.getRoot().resolve("foo/foo/abc/def.smithy"))));
assertThat(matchers, hasItem(UtilMatchers.canMatchPath(workspace.getRoot().resolve("other/abc.smithy"))));
assertThat(matchers, hasItem(UtilMatchers.canMatchPath(workspace.getRoot().resolve("other/foo/abc.smithy"))));
assertThat(matchers, hasItem(UtilMatchers.canMatchPath(workspace.getRoot().resolve("abc.smithy"))));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@
package software.amazon.smithy.lsp.project;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;

import java.nio.file.Path;
import java.nio.file.PathMatcher;
import java.util.HashSet;
import org.junit.jupiter.api.Test;
import software.amazon.smithy.build.model.SmithyBuildConfig;
import software.amazon.smithy.lsp.TestWorkspace;
import software.amazon.smithy.lsp.UtilMatchers;
import software.amazon.smithy.utils.ListUtils;

public class ProjectFilePatternsTest {
Expand Down Expand Up @@ -42,10 +42,10 @@ public void createsPathMatchers() {
PathMatcher buildMatcher = ProjectFilePatterns.getBuildFilesPathMatcher(project);

Path root = project.root();
assertThat(smithyMatcher.matches(root.resolve("abc.smithy")), is(true));
assertThat(smithyMatcher.matches(root.resolve("foo/bar/baz.smithy")), is(true));
assertThat(smithyMatcher.matches(root.resolve("other/bar.smithy")), is(true));
assertThat(buildMatcher.matches(root.resolve("smithy-build.json")), is(true));
assertThat(buildMatcher.matches(root.resolve(".smithy-project.json")), is(true));
assertThat(smithyMatcher, UtilMatchers.canMatchPath(root.resolve("abc.smithy")));
assertThat(smithyMatcher, UtilMatchers.canMatchPath(root.resolve("foo/bar/baz.smithy")));
assertThat(smithyMatcher, UtilMatchers.canMatchPath(root.resolve("other/bar.smithy")));
assertThat(buildMatcher, UtilMatchers.canMatchPath(root.resolve("smithy-build.json")));
assertThat(buildMatcher, UtilMatchers.canMatchPath(root.resolve(".smithy-project.json")));
}
}

0 comments on commit b68eda0

Please sign in to comment.