Skip to content

Commit

Permalink
Fix Document, UriAdapter, and tests for windows
Browse files Browse the repository at this point in the history
Previously, Document used System.lineSeparator() for figuring out
where line starts would be (index of linesep + 1). But if the file
was created (and, say, packaged in a jar) on another OS, it would
have different lineseps. This change makes use of a simple fact I
overlooked in the initial implementation, which was that '\n' is
still the last character on each line, so we don't need to break
on System.lineSeparator(), just on newline (unless there's still
some OS using '\r' only line breaks).

UriAdapter was updated to handle windows URIs, which would be made
into invalid paths with a leading '/' after removing 'file://'.

A bunch of test cases were also updated, which essentially all had
one or both of the above problems.
  • Loading branch information
milesziemer committed Jun 13, 2024
1 parent cfe22db commit c9c751d
Show file tree
Hide file tree
Showing 12 changed files with 350 additions and 290 deletions.
1 change: 1 addition & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ jobs:
runs-on: ${{ matrix.os }}
name: Java ${{ matrix.java }} ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
java: [8, 11, 17]
os: [ubuntu-latest, windows-latest, macos-latest]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,9 @@ private static int[] computeLineIndicies(StringBuilder buffer) {
// Have to box sadly, unless there's some IntArray I'm not aware of. Maybe IntBuffer
List<Integer> indicies = new ArrayList<>();
indicies.add(0);
while ((next = buffer.indexOf(System.lineSeparator(), off)) != -1) {
// This works with \r\n line breaks by basically forgetting about the \r, since we don't actually
// care about the content of the line
while ((next = buffer.indexOf("\n", off)) != -1) {
indicies.add(next + 1);
off = next + 1;
++matchCount;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ private static void addTextEdits(CompletionItem completionItem, ShapeId shapeId,
}

private static TextEdit getImportTextEdit(SmithyFile smithyFile, String importId) {
String insertText = "\n" + "use " + importId;
String insertText = System.lineSeparator() + "use " + importId;
// We can only know where to put the import if there's already use statements, or a namespace
if (smithyFile.getDocumentImports().isPresent()) {
Range importsRange = smithyFile.getDocumentImports().get().importsRange();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ private ProjectLoader() {
* @return The loaded project
*/
public static Project loadDetached(String uri, String text) {
LOGGER.info("Loading detached project at " + uri);
String asPath = UriAdapter.toPath(uri);
ValidatedResult<Model> modelResult = Model.assembler()
.addUnparsedModel(asPath, text)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import java.net.URL;
import java.net.URLDecoder;
import java.nio.charset.StandardCharsets;
import java.nio.file.Paths;
import java.util.logging.Logger;

/**
Expand All @@ -28,7 +29,7 @@ private UriAdapter() {
*/
public static String toPath(String uri) {
if (uri.startsWith("file://")) {
return uri.replaceFirst("file://", "");
return Paths.get(URI.create(uri)).toString();
} else if (isSmithyJarFile(uri)) {
String decoded = decode(uri);
return fixJarScheme(decoded);
Expand All @@ -39,15 +40,15 @@ public static String toPath(String uri) {
/**
* @param path Path to convert to LSP URI
* @return A URI representation of the given {@code path}, modified to have the
* correct scheme for jars
* correct scheme for our jars
*/
public static String toUri(String path) {
if (path.startsWith("/")) {
return "file://" + path;
} else if (path.startsWith("jar:file")) {
if (path.startsWith("jar:file")) {
return path.replaceFirst("jar:file", "smithyjar");
} else {
} else if (path.startsWith("smithyjar:")) {
return path;
} else {
return Paths.get(path).toUri().toString();
}
}

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 @@ -33,7 +33,7 @@ public void describeMismatchSafely(CompletionItem item, Description description)
}

public static Matcher<TextEdit> makesEditedDocument(Document document, String expected) {
return new CustomTypeSafeMatcher<TextEdit>("the right edit") {
return new CustomTypeSafeMatcher<TextEdit>("makes an edited document " + expected) {
@Override
protected boolean matchesSafely(TextEdit item) {
Document copy = document.copy();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package software.amazon.smithy.lsp;

import java.net.URI;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -176,7 +177,7 @@ public DidOpen text(String text) {

public DidOpenTextDocumentParams build() {
if (text == null) {
text = IoUtils.readUtf8File(URI.create(uri).getPath());
text = IoUtils.readUtf8File(Paths.get(URI.create(uri)));
}
return new DidOpenTextDocumentParams(new TextDocumentItem(uri, languageId, version, text));
}
Expand Down
Loading

0 comments on commit c9c751d

Please sign in to comment.