Skip to content

Commit

Permalink
Fix off-by-one issues in TokenTree and TreeCursor (#1891)
Browse files Browse the repository at this point in the history
Fixes an issue in TokenTree where if the smithy file was missing a
trailing newline, any trees containing the last piece of text in
the file would have an end location of (0, 0). This happened because
the BR TreeType didn't append any tokens for the EOF case, so the
BR tree had no end location.

Fixes an issue in TreeCursor::findAt where if you tried to get the
tree at a location right at the start of a tree, you would get the
previous tree.

Tests were added for both of these cases in TreeCursorTest.
  • Loading branch information
milesziemer authored Jul 31, 2023
1 parent 8db9d56 commit 59578f3
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,18 @@ public IdlToken next() {
if (getToken().getIdlToken() == IdlToken.EOF) {
throw new NoSuchElementException();
}
trees.getFirst().appendChild(TokenTree.of(CapturedToken.from(this, this.stringTable)));
appendCurrentTokenToFirstTree();
return tokens.get(++cursor).getIdlToken();
}

void eof() {
appendCurrentTokenToFirstTree();
}

private void appendCurrentTokenToFirstTree() {
trees.getFirst().appendChild(TokenTree.of(CapturedToken.from(this, this.stringTable)));
}

CapturedToken peekPastSpaces() {
return peekWhile(0, token -> token == IdlToken.SPACE);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ public TreeCursor findAt(int line, int column) {
isMatch = column >= startColumn && column < endColumn;
} else if (line == startLine && column >= startColumn) {
isMatch = true;
} else if (line == endLine && column <= endColumn) {
} else if (line == endLine && column < endColumn) {
isMatch = true;
} else if (line > startLine && line < endLine) {
isMatch = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1012,6 +1012,8 @@ void parse(CapturingTokenizer tokenizer) {
optionalWs(tokenizer);
break;
case EOF:
tokenizer.eof();
break;
default:
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
public class TreeCursorTest {
@Test
public void hasChildren() {
TokenTree tree = createTree();
TokenTree tree = createTree("simple-model.smithy");
TreeCursor cursor = tree.zipper();
List<TreeCursor> children = cursor.getChildren();

Expand All @@ -36,7 +36,7 @@ public void hasChildren() {

@Test
public void hasParentAndSiblings() {
TokenTree tree = createTree();
TokenTree tree = createTree("simple-model.smithy");
TreeCursor cursor = tree.zipper();
List<TreeCursor> children = cursor.getChildren();

Expand All @@ -55,7 +55,7 @@ public void hasParentAndSiblings() {

@Test
public void findsNodeAtPosition() {
TokenTree tree = createTree();
TokenTree tree = createTree("simple-model.smithy");
TreeCursor cursor = tree.zipper();
TreeCursor click = cursor.findAt(3, 17);

Expand All @@ -65,8 +65,32 @@ public void findsNodeAtPosition() {
assertThat(click.getRoot(), equalTo(cursor));
}

private TokenTree createTree() {
String model = IoUtils.readUtf8Url(getClass().getResource("formatter/simple-model.smithy"));
@Test
public void findsNodeAtPositionBetweenTokens() {
TokenTree tree = createTree("incorrect-indentation.smithy");
TreeCursor cursor = tree.zipper();
TreeCursor click = cursor.findAt(10, 5);

assertThat(click, notNullValue());
assertThat(click.getTree().getType(), is(TreeType.TOKEN));
assertThat(click.getTree().tokens().iterator().next().getLexeme().toString(), equalTo("@"));
assertThat(click.getRoot(), equalTo(cursor));
}

@Test
public void findsNodeAtLastLineOfFile() {
TokenTree tree = createTree("missing-trailing-newline.smithy");
TreeCursor cursor = tree.zipper();
TreeCursor click = cursor.findAt(8, 4);

assertThat(click, notNullValue());
assertThat(click.getTree().getType(), is(TreeType.TOKEN));
assertThat(click.getTree().tokens().iterator().next().getLexeme().toString(), equalTo("foo"));
assertThat(click.getRoot(), equalTo(cursor));
}

private TokenTree createTree(String filename) {
String model = IoUtils.readUtf8Url(getClass().getResource("formatter/" + filename));
IdlTokenizer tokenizer = IdlTokenizer.create(model);
return TokenTree.of(tokenizer);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
$version: "2.0"

namespace smithy.example

structure Foo {}

structure Bar {}

// Comment
@input
structure Baz {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
$version: "2.0"

namespace smithy.example

structure Foo {}

structure Bar {}

// Comment
@input
structure Baz {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
$version: "2.0"

namespace smithy.example

@trait
structure foo {}

@foo
structure Bar {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
$version: "2.0"

namespace smithy.example

@trait
structure foo {}

@foo
structure Bar {}

0 comments on commit 59578f3

Please sign in to comment.