Skip to content

Commit

Permalink
Fix parse errors for empty build files
Browse files Browse the repository at this point in the history
Document::rangeBetween would return `null` if the document was empty,
but I wasn't checking for that in ToSmithyNode (which creates parse
events).

The reason the range is `null` is because Document.lineOfIndex returns
oob for an index of 0 into an empty document. Makes sense, as an empty
document has no lines. I updated a DocumentTest to clarify this
behavior.
  • Loading branch information
milesziemer committed Feb 13, 2025
1 parent b124b3d commit 7d59a04
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package software.amazon.smithy.lsp.project;

import java.util.List;
import org.eclipse.lsp4j.Range;
import software.amazon.smithy.lsp.document.Document;
import software.amazon.smithy.lsp.protocol.LspAdapter;
import software.amazon.smithy.lsp.syntax.Syntax;
Expand Down Expand Up @@ -104,6 +105,10 @@ yield switch (value) {
}

private SourceLocation nodeStartSourceLocation(Syntax.Node node) {
return LspAdapter.toSourceLocation(path, document.rangeBetween(node.start(), node.end()));
Range range = document.rangeBetween(node.start(), node.end());
if (range == null) {
range = LspAdapter.origin();
}
return LspAdapter.toSourceLocation(path, range);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ public void getsLineOfIndex() {
Document leadingAndTrailingWs = makeDocument("\nabc\n");
Document threeLine = makeDocument("abc\ndef\nhij\n");

assertThat(empty.lineOfIndex(0), is(-1)); // empty has no lines, so oob
assertThat(empty.lineOfIndex(1), is(-1)); // oob
assertThat(single.lineOfIndex(0), is(0)); // start
assertThat(single.lineOfIndex(2), is(0)); // end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.List;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import org.eclipse.lsp4j.Position;
import org.junit.jupiter.api.Test;
import software.amazon.smithy.build.model.MavenRepository;
import software.amazon.smithy.cli.dependencies.DependencyResolver;
Expand All @@ -32,6 +33,7 @@
import software.amazon.smithy.lsp.ServerState;
import software.amazon.smithy.lsp.TextWithPositions;
import software.amazon.smithy.lsp.document.Document;
import software.amazon.smithy.lsp.protocol.LspAdapter;

public class ProjectConfigTest {
@Test
Expand Down Expand Up @@ -78,6 +80,21 @@ public void mergesBuildExts() {
assertThat(config.maven().getDependencies(), containsInAnyOrder("foo"));
}

@Test
public void handlesEmptyFiles() {
var root = Path.of("foo");
var buildFiles = createBuildFiles(root, BuildFileType.SMITHY_BUILD, "");
var result = load(root, buildFiles);

var smithyBuild = buildFiles.getByType(BuildFileType.SMITHY_BUILD);
assertThat(smithyBuild, notNullValue());
assertThat(result.events(), containsInAnyOrder(allOf(
eventWithId(equalTo("Model")),
eventWithMessage(containsString("Error parsing JSON")),
eventWithSourceLocation(equalTo(LspAdapter.toSourceLocation(smithyBuild.path(), new Position(0, 0))))
)));
}

@Test
public void validatesSmithyBuildJson() {
var text = TextWithPositions.from("""
Expand Down

0 comments on commit 7d59a04

Please sign in to comment.