From 53b9f2219cf33cef09d8bda446570da97e7ac055 Mon Sep 17 00:00:00 2001 From: Miles Ziemer <45497130+milesziemer@users.noreply.github.com> Date: Mon, 31 Jul 2023 10:15:24 -0400 Subject: [PATCH] Fix off-by-one issues in TokenTree and TreeCursor (#1891) 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. --- .../smithy/syntax/CapturingTokenizer.java | 10 +++++- .../amazon/smithy/syntax/TreeCursor.java | 2 +- .../amazon/smithy/syntax/TreeType.java | 2 ++ .../amazon/smithy/syntax/TreeCursorTest.java | 34 ++++++++++++++++--- .../incorrect-indentation.formatted.smithy | 11 ++++++ .../formatter/incorrect-indentation.smithy | 11 ++++++ .../missing-trailing-newline.formatted.smithy | 9 +++++ .../formatter/missing-trailing-newline.smithy | 9 +++++ 8 files changed, 81 insertions(+), 7 deletions(-) create mode 100644 smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/incorrect-indentation.formatted.smithy create mode 100644 smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/incorrect-indentation.smithy create mode 100644 smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/missing-trailing-newline.formatted.smithy create mode 100644 smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/missing-trailing-newline.smithy diff --git a/smithy-syntax/src/main/java/software/amazon/smithy/syntax/CapturingTokenizer.java b/smithy-syntax/src/main/java/software/amazon/smithy/syntax/CapturingTokenizer.java index 1a35c5f1697..1f41d34a999 100644 --- a/smithy-syntax/src/main/java/software/amazon/smithy/syntax/CapturingTokenizer.java +++ b/smithy-syntax/src/main/java/software/amazon/smithy/syntax/CapturingTokenizer.java @@ -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); } diff --git a/smithy-syntax/src/main/java/software/amazon/smithy/syntax/TreeCursor.java b/smithy-syntax/src/main/java/software/amazon/smithy/syntax/TreeCursor.java index b5a103e5907..1e25361332c 100644 --- a/smithy-syntax/src/main/java/software/amazon/smithy/syntax/TreeCursor.java +++ b/smithy-syntax/src/main/java/software/amazon/smithy/syntax/TreeCursor.java @@ -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; diff --git a/smithy-syntax/src/main/java/software/amazon/smithy/syntax/TreeType.java b/smithy-syntax/src/main/java/software/amazon/smithy/syntax/TreeType.java index c63b4d69339..5c7cfb8e236 100644 --- a/smithy-syntax/src/main/java/software/amazon/smithy/syntax/TreeType.java +++ b/smithy-syntax/src/main/java/software/amazon/smithy/syntax/TreeType.java @@ -1012,6 +1012,8 @@ void parse(CapturingTokenizer tokenizer) { optionalWs(tokenizer); break; case EOF: + tokenizer.eof(); + break; default: break; } diff --git a/smithy-syntax/src/test/java/software/amazon/smithy/syntax/TreeCursorTest.java b/smithy-syntax/src/test/java/software/amazon/smithy/syntax/TreeCursorTest.java index 42ad9a4ba7f..297157408c0 100644 --- a/smithy-syntax/src/test/java/software/amazon/smithy/syntax/TreeCursorTest.java +++ b/smithy-syntax/src/test/java/software/amazon/smithy/syntax/TreeCursorTest.java @@ -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 children = cursor.getChildren(); @@ -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 children = cursor.getChildren(); @@ -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); @@ -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); } diff --git a/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/incorrect-indentation.formatted.smithy b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/incorrect-indentation.formatted.smithy new file mode 100644 index 00000000000..09c6d9a5e7a --- /dev/null +++ b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/incorrect-indentation.formatted.smithy @@ -0,0 +1,11 @@ +$version: "2.0" + +namespace smithy.example + +structure Foo {} + +structure Bar {} + +// Comment +@input +structure Baz {} diff --git a/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/incorrect-indentation.smithy b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/incorrect-indentation.smithy new file mode 100644 index 00000000000..1f967d207c2 --- /dev/null +++ b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/incorrect-indentation.smithy @@ -0,0 +1,11 @@ +$version: "2.0" + +namespace smithy.example + +structure Foo {} + + structure Bar {} + +// Comment + @input +structure Baz {} diff --git a/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/missing-trailing-newline.formatted.smithy b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/missing-trailing-newline.formatted.smithy new file mode 100644 index 00000000000..95b66d5abfd --- /dev/null +++ b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/missing-trailing-newline.formatted.smithy @@ -0,0 +1,9 @@ +$version: "2.0" + +namespace smithy.example + +@trait +structure foo {} + +@foo +structure Bar {} diff --git a/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/missing-trailing-newline.smithy b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/missing-trailing-newline.smithy new file mode 100644 index 00000000000..dd356869343 --- /dev/null +++ b/smithy-syntax/src/test/resources/software/amazon/smithy/syntax/formatter/missing-trailing-newline.smithy @@ -0,0 +1,9 @@ +$version: "2.0" + +namespace smithy.example + +@trait +structure foo {} + +@foo +structure Bar {} \ No newline at end of file