Repository: smithy-lang/smithy
Pull request #1891 (2023-07-31 13:47:40)

Fix off-by-one issues in TokenTree and TreeCursor
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.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.



Patch:
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 1a35c5f16..1f41d34a9 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 @@ final class CapturingTokenizer implements IdlTokenizer {
         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 b5a103e59..1e2536133 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 final class TreeCursor implements FromSourceLocation {
                     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 c63b4d693..5c7cfb8e2 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 @@ public enum TreeType {
                         optionalWs(tokenizer);
                         break;
                     case EOF:
+                        tokenizer.eof();
+                        break;
                     default:
                         break;
                 }


Test patch:
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 42ad9a4ba..297157408 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 @@ import software.amazon.smithy.utils.IoUtils;
 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();
 
@@ -36,7 +36,7 @@ public class TreeCursorTest {
 
     @Test
     public void hasParentAndSiblings() {
-        TokenTree tree = createTree();
+        TokenTree tree = createTree("simple-model.smithy");
         TreeCursor cursor = tree.zipper();
         List<TreeCursor> children = cursor.getChildren();
 
@@ -55,7 +55,7 @@ public class TreeCursorTest {
 
     @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 class TreeCursorTest {
         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 000000000..09c6d9a5e
--- /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 000000000..1f967d207
--- /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 000000000..95b66d5ab
--- /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 000000000..dd3568693
--- /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