Assert trees are valid, fill in missing tests

All TreeTypeTests now expect the entire tree to be valid. This
helps us find ambiguity and inconsistencies in the parser for
cases when parsing a child tree is invalid only due to the
context of the parent. Fixes minor bugs that caused trees to be
invalid:
- enumMember test didn't have a BR after VALUE_ASSIGNMENT

Fills in missing tests for:
- Operations
- Block apply statements
- Entity type name
- Aggregate type name

Other minor changes and improvements:
- Adds util methods for parsing optional for-resource, mixins,
and value assignment productions.
- Adds util method for parsing any shape type and name.
- Updates peekPastSpaces to not skip the current token, in case
it is not SP. This simplifies parsing value assignments, and has
more intuitive behavior.
- Removes peekPastWs, which was only used for TRAIT_BODY_VALUE,
inlining the implementation. peekPastWs skipped the current token
like peekPastSpaces, which is necessary for TRAIT_BODY_VALUE.
- Add ENTITY_TYPE_NAME production.
- Updates name of some productions to be consistent with grammar.
- Fixed a few places where TOKEN was parsed as IDENTIFIER.

Author: milesziemer

smithy-syntax/src/main/java/software/amazon/smithy/syntax/CapturingTokenizer.java

@@ -137,15 +137,15 @@ public IdlToken next() {
     }
 
     CapturedToken peekPastSpaces() {
-        return peekWhile(1, token -> token == IdlToken.SPACE);
-    }
-
-    CapturedToken peekPastWs() {
-        return peekWhile(1, token -> token.isWhitespace() || token == IdlToken.DOC_COMMENT);
+        return peekWhile(0, token -> token == IdlToken.SPACE);
     }
 
     CapturedToken peekWhile(int offsetFromPosition, Predicate<IdlToken> predicate) {
         int position = cursor + offsetFromPosition;
+        // If the start position is out of bounds, return the EOF token.
+        if (position >= tokens.size()) {
+            return tokens.get(tokens.size() - 1);
+        }
         CapturedToken token = tokens.get(position);
         while (token.getIdlToken() != IdlToken.EOF && predicate.test(token.getIdlToken())) {
             token = tokens.get(++position);

smithy-syntax/src/main/java/software/amazon/smithy/syntax/TreeType.java

@@ -152,7 +152,7 @@ void parse(CapturingTokenizer tokenizer) {
         @Override
         void parse(CapturingTokenizer tokenizer) {
             tokenizer.withState(this, () -> {
-                tokenizer.next();
+                tokenizer.next(); // Skip "use"
                 SP.parse(tokenizer);
                 ABSOLUTE_ROOT_SHAPE_ID.parse(tokenizer);
                 BR.parse(tokenizer);
@@ -236,22 +236,16 @@ void parse(CapturingTokenizer tokenizer) {
         @Override
         void parse(CapturingTokenizer tokenizer) {
             tokenizer.withState(this, () -> {
-                SIMPLE_TYPE_NAME.parse(tokenizer);
-                optionalSpaces(tokenizer);
-                IDENTIFIER.parse(tokenizer);
-                optionalSpaces(tokenizer);
-
-                if (tokenizer.isCurrentLexeme("with")) {
-                    SHAPE_MIXINS.parse(tokenizer);
-                }
+                parseShapeTypeAndName(tokenizer, SIMPLE_TYPE_NAME);
+                parseOptionalMixins(tokenizer);
             });
         }
     },
 
     SIMPLE_TYPE_NAME {
         @Override
         void parse(CapturingTokenizer tokenizer) {
-            // Assumes that the current token is a valid simple type name validated by SHAPE_BODY.
+            // Assumes that the current token is a valid simple type name validated by SHAPE.
             tokenizer.withState(this, tokenizer::next);
         }
     },
@@ -260,14 +254,8 @@ void parse(CapturingTokenizer tokenizer) {
         @Override
         void parse(CapturingTokenizer tokenizer) {
             tokenizer.withState(this, () -> {
-                ENUM_TYPE_NAME.parse(tokenizer);
-                optionalSpaces(tokenizer);
-                IDENTIFIER.parse(tokenizer);
-                optionalSpaces(tokenizer);
-
-                if (tokenizer.isCurrentLexeme("with")) {
-                    SHAPE_MIXINS.parse(tokenizer);
-                }
+                parseShapeTypeAndName(tokenizer, ENUM_TYPE_NAME);
+                parseOptionalMixins(tokenizer);
 
                 optionalWs(tokenizer);
                 ENUM_SHAPE_MEMBERS.parse(tokenizer);
@@ -278,7 +266,7 @@ void parse(CapturingTokenizer tokenizer) {
     ENUM_TYPE_NAME {
         @Override
         void parse(CapturingTokenizer tokenizer) {
-            // Assumes that the current token is a valid enum type name validated by SHAPE_BODY.
+            // Assumes that the current token is a valid enum type name validated by SHAPE.
             tokenizer.withState(this, tokenizer::next);
         }
     },
@@ -308,10 +296,7 @@ void parse(CapturingTokenizer tokenizer) {
             tokenizer.withState(this, () -> {
                 TRAIT_STATEMENTS.parse(tokenizer);
                 IDENTIFIER.parse(tokenizer);
-                if (tokenizer.hasNext() && (tokenizer.getCurrentToken() == IdlToken.EQUAL
-                                            || tokenizer.peekPastSpaces().getIdlToken() == IdlToken.EQUAL)) {
-                    VALUE_ASSIGNMENT.parse(tokenizer);
-                }
+                parseOptionalValueAssignment(tokenizer);
             });
         }
     },
@@ -320,10 +305,7 @@ void parse(CapturingTokenizer tokenizer) {
         @Override
         void parse(CapturingTokenizer tokenizer) {
             tokenizer.withState(this, () -> {
-                AGGREGATE_TYPE_NAME.parse(tokenizer);
-                optionalSpaces(tokenizer);
-                IDENTIFIER.parse(tokenizer);
-                optionalSpaces(tokenizer);
+                parseShapeTypeAndName(tokenizer, AGGREGATE_TYPE_NAME);
                 parseSharedStructureBodyWithinInline(tokenizer);
             });
         }
@@ -332,16 +314,17 @@ void parse(CapturingTokenizer tokenizer) {
     AGGREGATE_TYPE_NAME {
         @Override
         void parse(CapturingTokenizer tokenizer) {
-            // Assumes that the current token is a valid simple type name validated by SHAPE_BODY.
+            // Assumes that the current token is a valid simple type name validated by SHAPE.
             tokenizer.withState(this, tokenizer::next);
         }
     },
 
-    AGGREGATE_SHAPE_RESOURCE {
+    // Don't use this directly. Instead, use parseOptionalForResource
+    FOR_RESOURCE {
         @Override
         void parse(CapturingTokenizer tokenizer) {
             tokenizer.withState(this, () -> {
-                IDENTIFIER.parse(tokenizer);
+                tokenizer.next(); // Skip "for"
                 SP.parse(tokenizer);
                 SHAPE_ID.parse(tokenizer);
             });
@@ -377,11 +360,7 @@ void parse(CapturingTokenizer tokenizer) {
                 } else {
                     EXPLICIT_SHAPE_MEMBER.parse(tokenizer);
                 }
-
-                optionalSpaces(tokenizer);
-                if (tokenizer.getCurrentToken() == IdlToken.EQUAL) {
-                    VALUE_ASSIGNMENT.parse(tokenizer);
-                }
+                parseOptionalValueAssignment(tokenizer);
             });
         }
     },
@@ -416,11 +395,9 @@ void parse(CapturingTokenizer tokenizer) {
         void parse(CapturingTokenizer tokenizer) {
             // Assumes that the shape type is a valid "service" or "resource".
             tokenizer.withState(this, () -> {
-                parseShapeTypeAndName(tokenizer);
+                parseShapeTypeAndName(tokenizer, ENTITY_TYPE_NAME);
 
-                if (tokenizer.isCurrentLexeme("with")) {
-                    SHAPE_MIXINS.parse(tokenizer);
-                }
+                parseOptionalMixins(tokenizer);
 
                 optionalWs(tokenizer);
                 tokenizer.expect(IdlToken.LBRACE);
@@ -429,15 +406,21 @@ void parse(CapturingTokenizer tokenizer) {
         }
     },
 
+    ENTITY_TYPE_NAME {
+       @Override
+       void parse(CapturingTokenizer tokenizer)  {
+           // Assumes that the current token is a valid entity type name validated by SHAPE.
+           tokenizer.withState(this, tokenizer::next);
+       }
+    },
+
     OPERATION_SHAPE {
         @Override
         void parse(CapturingTokenizer tokenizer) {
             tokenizer.withState(this, () -> {
                 parseShapeTypeAndName(tokenizer);
 
-                if (tokenizer.isCurrentLexeme("with")) {
-                    SHAPE_MIXINS.parse(tokenizer);
-                }
+                parseOptionalMixins(tokenizer);
 
                 optionalWs(tokenizer);
                 OPERATION_BODY.parse(tokenizer);
@@ -486,8 +469,7 @@ void parse(CapturingTokenizer tokenizer) {
         @Override
         void parse(CapturingTokenizer tokenizer) {
             tokenizer.withState(this, () -> {
-                tokenizer.expect(IdlToken.IDENTIFIER);
-                tokenizer.next(); // skip "input"
+                IDENTIFIER.parse(tokenizer); // skip "input"
                 optionalWs(tokenizer);
                 operationInputOutputDefinition(tokenizer);
             });
@@ -498,8 +480,7 @@ void parse(CapturingTokenizer tokenizer) {
         @Override
         void parse(CapturingTokenizer tokenizer) {
             tokenizer.withState(this, () -> {
-                tokenizer.expect(IdlToken.IDENTIFIER);
-                tokenizer.next(); // skip "output"
+                IDENTIFIER.parse(tokenizer); // skip "output"
                 optionalWs(tokenizer);
                 operationInputOutputDefinition(tokenizer);
             });
@@ -523,8 +504,7 @@ void parse(CapturingTokenizer tokenizer) {
         @Override
         void parse(CapturingTokenizer tokenizer) {
             tokenizer.withState(this, () -> {
-                tokenizer.expect(IdlToken.IDENTIFIER);
-                tokenizer.next(); // skip "errors"
+                IDENTIFIER.parse(tokenizer); // skip "errors"
                 optionalWs(tokenizer);
                 tokenizer.expect(IdlToken.COLON);
                 tokenizer.next();
@@ -544,11 +524,12 @@ void parse(CapturingTokenizer tokenizer) {
 
     // Mixins =
     //     [SP] %s"with" [WS] "[" [WS] 1*(ShapeId [WS]) "]"
-    SHAPE_MIXINS {
+    // Don't use this directly. Instead, use parseOptionalMixins
+    MIXINS {
         @Override
         void parse(CapturingTokenizer tokenizer) {
             tokenizer.withState(this, () -> {
-                IDENTIFIER.parse(tokenizer); // 'with'
+                tokenizer.next(); // Skip "with"
                 optionalWs(tokenizer);
 
                 tokenizer.expect(IdlToken.LBRACKET);
@@ -567,6 +548,7 @@ void parse(CapturingTokenizer tokenizer) {
         }
     },
 
+    // Don't use this directly. Instead, use parseOptionalValueAssignment
     VALUE_ASSIGNMENT {
         @Override
         void parse(CapturingTokenizer tokenizer) {
@@ -634,7 +616,9 @@ void parse(CapturingTokenizer tokenizer) {
                         case STRING:
                         case IDENTIFIER:
                         default:
-                            if (tokenizer.peekPastWs().getIdlToken() == IdlToken.COLON) {
+                            CapturedToken nextPastWs = tokenizer.peekWhile(1, token ->
+                                    token.isWhitespace() || token == IdlToken.DOC_COMMENT);
+                            if (nextPastWs.getIdlToken() == IdlToken.COLON) {
                                 TRAIT_STRUCTURE.parse(tokenizer);
                             } else {
                                 TRAIT_NODE.parse(tokenizer);
@@ -677,7 +661,7 @@ void parse(CapturingTokenizer tokenizer) {
             tokenizer.withState(this, () -> {
                 // Try to see if this is a singular or block apply statement.
                 IdlToken peek = tokenizer
-                        .peekWhile(1, t -> t != IdlToken.EOF && t != IdlToken.AT && t != IdlToken.LBRACE)
+                        .peekWhile(1, t -> t != IdlToken.AT && t != IdlToken.LBRACE)
                         .getIdlToken();
                 if (peek == IdlToken.LBRACE) {
                     APPLY_STATEMENT_BLOCK.parse(tokenizer);
@@ -692,7 +676,7 @@ void parse(CapturingTokenizer tokenizer) {
         @Override
         void parse(CapturingTokenizer tokenizer) {
             tokenizer.withState(this, () -> {
-                tokenizer.next();
+                tokenizer.next(); // Skip "apply"
                 SP.parse(tokenizer);
                 SHAPE_ID.parse(tokenizer);
                 WS.parse(tokenizer);
@@ -705,7 +689,7 @@ void parse(CapturingTokenizer tokenizer) {
         @Override
         void parse(CapturingTokenizer tokenizer) {
             tokenizer.withState(this, () -> {
-                tokenizer.next();
+                tokenizer.next(); // Skip "apply"
                 SP.parse(tokenizer);
                 SHAPE_ID.parse(tokenizer);
                 WS.parse(tokenizer);
@@ -1089,27 +1073,46 @@ protected static void optionalSpaces(CapturingTokenizer tokenizer) {
     }
 
     protected static void parseShapeTypeAndName(CapturingTokenizer tokenizer) {
-        tokenizer.expect(IdlToken.IDENTIFIER);
-        tokenizer.next(); // skip the shape type
+        parseShapeTypeAndName(tokenizer, null);
+    }
+
+    protected static void parseShapeTypeAndName(CapturingTokenizer tokenizer, TreeType typeName) {
+        if (typeName == null) {
+            tokenizer.next();
+        } else {
+            typeName.parse(tokenizer); // Skip the shape type
+        }
         optionalSpaces(tokenizer);
         IDENTIFIER.parse(tokenizer); // shape name
         optionalSpaces(tokenizer);
     }
 
     protected static void parseSharedStructureBodyWithinInline(CapturingTokenizer tokenizer) {
-        optionalSpaces(tokenizer);
+        parseOptionalForResource(tokenizer);
+        parseOptionalMixins(tokenizer);
 
+        optionalWs(tokenizer);
+        SHAPE_MEMBERS.parse(tokenizer);
+    }
+
+    protected static void parseOptionalForResource(CapturingTokenizer tokenizer) {
+        optionalSpaces(tokenizer);
         if (tokenizer.isCurrentLexeme("for")) {
-            AGGREGATE_SHAPE_RESOURCE.parse(tokenizer);
-            optionalSpaces(tokenizer);
+            FOR_RESOURCE.parse(tokenizer);
         }
+    }
 
+    protected static void parseOptionalMixins(CapturingTokenizer tokenizer) {
+        optionalSpaces(tokenizer);
         if (tokenizer.isCurrentLexeme("with")) {
-            SHAPE_MIXINS.parse(tokenizer);
+            MIXINS.parse(tokenizer);
         }
+    }
 
-        optionalWs(tokenizer);
-        SHAPE_MEMBERS.parse(tokenizer);
+    protected static void parseOptionalValueAssignment(CapturingTokenizer tokenizer) {
+        if (tokenizer.peekPastSpaces().getIdlToken() == IdlToken.EQUAL) {
+            VALUE_ASSIGNMENT.parse(tokenizer);
+        }
     }
 
     protected static void operationInputOutputDefinition(CapturingTokenizer tokenizer) {