Semicolon Cleanup

1. Semicolons were required in the wrong spot.
2. Make semicolons "invisible" (from the tree-sitter perspective).

Signed-off-by: Will Hawkins <hawkinsw@obs.cr>

Sources/P4Parser/Expression.swift

@@ -168,14 +168,16 @@ struct Expression {
     ]
 
     for le_parser in localElementsParsers {
-      if case Result.Ok(.some(let parsed)) = le_parser.parse(
+      switch le_parser.parse(
         node: node, inTree: inTree, withScopes: scopes)
       {
-        return .Ok(parsed)
+      case .Ok(.some(let parsed)): return .Ok(parsed)
+      case .Error(let e): return .Error(e)
+      default: continue
       }
     }
 
-    return Result.Error(Error(withMessage: "Could not parse into expression."))
+    return Result.Error(Error(withMessage: "\(node.range): Could not parse into expression"))
   }
 }
 

Sources/P4Parser/Parser.swift

@@ -212,26 +212,27 @@ public struct Parser {
       }
       let body_node = body[0].node
       var kses: [KeysetExpression] = Array()
-      for childidx in 0..<body_node.childCount {
+      var kses_errors: [Error] = Array()
-
-        if !childidx.isMultiple(of: 2) {
-          let maybe_semicolon = body_node.child(at: childidx)!
-          if maybe_semicolon.nodeType != "semicolon" {
-            return .Error(Error(withMessage: "Expected a semicolon but saw \(maybe_semicolon)"))
-          }
-          continue
-        }
 
+      body_node.enumerateNamedChildren { current_node in
         let maybe_parsed_kse = TransitionSelectExpressionCaseStatement.Parse(
-          node: body_node.child(at: childidx)!, inTree: tree, withLexicalScopes: scopes)
+          node: current_node, inTree: tree, withLexicalScopes: scopes)
         if case .Ok((let parsed_kse, _)) = maybe_parsed_kse {
           kses.append(parsed_kse)
         } else {
-          return .Error(
+          kses_errors.append(Error(withMessage: "\(maybe_parsed_kse.error()!)"))
-            Error(withMessage: "Error when parsing select case: \(maybe_parsed_kse.error()!)"))
         }
       }
 
+      if !kses_errors.isEmpty {
+        return .Error(
+          Error(
+            withMessage: "Error(s) parsing select cases: "
+              + (kses_errors.map { error in
+                return "\(error.msg)"
+              }.joined(separator: ";\n"))))
+      }
+
       return .Ok(
         (
           ParserTransitionSelectExpression(withSelector: selector, withKeysetExpressions: kses),
@@ -288,7 +289,7 @@ public struct Parser {
         let parser_state_query = try? SwiftTreeSitter.Query(
           language: p4lang,
           data: String(
-            "(parserState (state) (identifier) @state-name (parserLocalElements ((parserLocalElement) @state-local-element (semicolon))*)* (parserStatements ((parserStatement) @state-statement (semicolon))*)* (parserTransitionStatement) @transition)"
+            "(parserState (state) (identifier) @state-name (parserLocalElements ((parserLocalElement) @state-local-element)*)? (parserStatements ((parserStatement) @state-statement)*)? (parserTransitionStatement) @transition)"
           ).data(using: String.Encoding.utf8)!)
       else {
         return Result.Error(Error(withMessage: "Could not compile the tree sitter query"))
@@ -312,7 +313,7 @@ public struct Parser {
       }
 
       for state_le in state_le_capture {
-        state_le.node.enumerateChildren { node in
+        state_le.node.enumerateNamedChildren { node in
           switch LocalElements.Parse(
             node: node, inTree: tree, withScope: current_scopes)
           {
@@ -333,7 +334,7 @@ public struct Parser {
 
       if !statements_capture.isEmpty {
         for statement in statements_capture {
-          statement.node.enumerateChildren { node in
+          statement.node.enumerateNamedChildren { node in
             switch Statements.Parse(
               node: node, inTree: tree, withScope: current_scopes)
             {

Sources/P4Parser/Statement.swift

@@ -32,7 +32,7 @@ extension BlockStatement: ParseableStatement {
       let block_statement_query = try? SwiftTreeSitter.Query(
         language: p4lang,
         data: String(
-          "(statement . (blockStatement . (statements . ((statement) @astatement (semicolon))*) @statements) @block-statement) @statement"
+          "(statement . (blockStatement . (statements . ((statement) @astatement)*) @statements) @block-statement) @statement"
         ).data(using: String.Encoding.utf8)!)
     else {
       return Result.Ok((.none, scopes))
@@ -55,6 +55,7 @@ extension BlockStatement: ParseableStatement {
     let statement_node = statement_capture[0].node
     let blockstatement_capture_node = blockstatement_capture[0].node
     let statements_capture_node = statements_capture[0].node
+
     /*
         if statement_node.parent != node.parent
         {

Sources/TreeSitterExtensions/Extensions.swift

@@ -53,3 +53,15 @@ extension MutableTree {
     return false
   }
 }
+
+extension Node {
+  public func enumerateNamedChildren(block: (Node) -> Void) {
+    for childIdx in 0..<self.childCount {
+      let child = self.child(at: childIdx)!
+      if !child.isNamed {
+        continue
+      }
+      block(child)
+    }
+  }
+}

Tests/p4rseTests/ParserTests.swift

@@ -75,3 +75,21 @@ import TreeSitterP4
   let program = try #UseOkResult(Program.Parse(simple_parser_declaration))
   #expect(#RequireOkResult(program.find_parser(withName: Identifier(name: "main_parser"))))
 }
+
+@Test func test_invalid_transition_expression_keyset_expressions() async throws {
+  let simple_parser_declaration = """
+    parser main_parser() {
+       state start {
+           transition select (false) {
+              asdf: reject;
+              asde: reject;
+           };
+       }
+    };
+    """
+
+  let compilation_error = try #UseErrorResult(Program.Parse(simple_parser_declaration))
+
+  #expect(compilation_error.msg.contains("asde"))
+  #expect(compilation_error.msg.contains("asdf"))
+}

Tests/p4rseTests/RuntimeTests.swift

@@ -132,7 +132,7 @@ import TreeSitterP4
           if (x) {
             x = false;
             check = "valid";
-          };
+          }
           transition select (x) {
             false: reject;
             true: accept;
@@ -167,7 +167,7 @@ import TreeSitterP4
           } else {
             x = true;
             check = "b";
-          };
+          }
           transition select (x) {
             false: reject;
             true: accept;

tree-sitter-p4/grammar.js

@@ -22,7 +22,7 @@ export default grammar({
     name: 'p4',
     rules: {
         // Start symbol
-        p4program: $ => optional(repeat(seq(choice($.declaration, $.instantiation), $.semicolon))),
+        p4program: $ => optional(repeat(seq(choice($.declaration, $.instantiation), $._semicolon))),
 
         // Common
 
@@ -43,14 +43,14 @@ export default grammar({
 
         // Mark with higher precedence so that the local states are preferred when parsing!
         // TODO: Test!
-        parserLocalElements: $ => prec(2, repeat1(seq($.parserLocalElement, $.semicolon))),
+        parserLocalElements: $ => prec(2, repeat1($.parserLocalElement)),
 
         parserStates: $ => repeat1($.parserState),
-        parserState: $ => seq(optional($.annotations), $.state, $.identifier, '{', optional($.parserLocalElements), optional($.parserStatements), $.parserTransitionStatement, $.semicolon, '}'),
+        parserState: $ => seq(optional($.annotations), $.state, $.identifier, '{', optional($.parserLocalElements), optional($.parserStatements), $.parserTransitionStatement, '}'),
 
         parserLocalElement: $ => choice($.variableDeclaration, $.todo),
 
-        selectBody: $ => repeat1(seq($.selectCase, $.semicolon)),
+        selectBody: $ => repeat1(seq($.selectCase, $._semicolon)),
         selectCase: $ => seq($.keysetExpression, $.colon, $.identifier),
 
         annotations: $ => repeat1($.annotation),
@@ -69,23 +69,23 @@ export default grammar({
         parserTypeDeclaration: $ => seq(optional($.annotations), $.parser, field('parser_name', $.identifier), optional($.typeParameters), '(', optional($.parameterList), ')'),
         parserDeclaration: $ => seq($.parserType, optional($.constructorParameters), '{', optional($.parserLocalElements), $.parserStates, '}'),
 
-        variableDeclaration: $ => seq(optional($.annotations), $.typeRef, field('variable_name', $.identifier), optional(seq($.assignment, $.expression))),
+        variableDeclaration: $ => seq(optional($.annotations), $.typeRef, field('variable_name', $.identifier), optional(seq($.assignment, $.expression)), $._semicolon),
 
         // Statements
 
         // General statements
-        statements: $ => repeat1(seq($.statement, $.semicolon)),
+        statements: $ => repeat1($.statement),
         statement: $ => choice($.conditionalStatement, $.blockStatement, $.expressionStatement, $.assignmentStatement),// Limited, so far.
         blockStatement: $ => seq(optional($.annotations), '{', optional($.statements), '}'),
         conditionalStatement: $ => choice(prec(1, seq($.if, '(', $.expression, ')', $.statement)), prec(2, seq($.if, '(', $.expression, ')', $.statement, $.else, $.statement))),
-        expressionStatement: $=> $.expression,
+        expressionStatement: $=> seq($.expression, $._semicolon),
-        assignmentStatement: $=> seq($.expression, $.assignment, $.expression),
+        assignmentStatement: $=> seq($.expression, $.assignment, $.expression, $._semicolon),
 
         // Parser statements
-        parserStatements: $ => repeat1(seq($.parserStatement, $.semicolon)),
+        parserStatements: $ => repeat1($.parserStatement),
         parserStatement: $ => choice($.conditionalStatement, $.parserBlockStatement, $.expressionStatement, $.assignmentStatement), // Limited, so far.
         parserBlockStatement: $ => seq(optional($.annotations), '{', $.parserStatements, '}'),
-        parserTransitionStatement: $ => seq($.transition, $.transitionSelectionExpression),
+        parserTransitionStatement: $ => seq($.transition, $.transitionSelectionExpression, $._semicolon),
 
         // Expressions
         expression: $ => choice($.identifier, $.integer, $.true, $.false, $.string_literal), // Very limited.
@@ -94,7 +94,7 @@ export default grammar({
         keysetExpression: $ => $.expression,
 
         // Tokens
-        semicolon: $ => ";",
+        _semicolon: $ => ";",
         colon: $ => ":",
         assignment: $ => "=",
         todo: $ => "todo",

tree-sitter-p4/test.txt

@@ -1,16 +1,7 @@
       parser main_parser() {
-        state next_state {
-          transition reject;
-        }
-        state not_next_state {
-          transition reject;
-        }
         state start {
-          transition starts;
+          bool where_to = true;
-        }
+          string where_from = where_to;
-        state starts {
+          transition reject;
-          transition select (se) {
-            true: next_state;
-          };
         }
       };

tree-sitter-p4/test/corpus/declarations.txt

@@ -31,7 +31,6 @@ parser simple() {
         (identifier)
        )
       )
-      (semicolon)
      )
      (parserTransitionStatement
       (transition)
@@ -39,12 +38,10 @@ parser simple() {
        (identifier)
       )
      )
-     (semicolon)
     )
    )
   )
  )
- (semicolon)
 )
 
 =========================
@@ -80,7 +77,6 @@ parser simple() {
         (identifier)
        )
       )
-      (semicolon)
      )
      (parserTransitionStatement
       (transition)
@@ -88,12 +84,10 @@ parser simple() {
        (identifier)
       )
      )
-     (semicolon)
     )
    )
   )
  )
- (semicolon)
 )
 
 =========================
@@ -129,7 +123,6 @@ parser simple() {
         (identifier)
        )
       )
-      (semicolon)
      )
      (parserTransitionStatement
       (transition)
@@ -137,12 +130,10 @@ parser simple() {
        (identifier)
       )
      )
-     (semicolon)
     )
    )
   )
  )
- (semicolon)
 )
 
 =========================
@@ -182,7 +173,6 @@ parser simple() {
         )
        )
       )
-      (semicolon)
      )
      (parserTransitionStatement
       (transition)
@@ -190,11 +180,9 @@ parser simple() {
        (identifier)
       )
      )
-     (semicolon)
     )
    )
   )
  )
- (semicolon)
 )
 

tree-sitter-p4/test/corpus/instantiations.txt

@@ -12,6 +12,5 @@ bool() main;
   )
   (identifier)
  ) 
- (semicolon)
 )
 

tree-sitter-p4/test/corpus/parsers.txt

@@ -25,12 +25,10 @@ parser simple() {
        (identifier)
       )
      )
-     (semicolon)
     )
    )
   )
  )
- (semicolon)
 )
 
 
@@ -71,10 +69,8 @@ parser imple(bool pname) {
        (identifier)
       )
      )
-     (semicolon)
     )
    )
   )
  )
- (semicolon)
 )

tree-sitter-p4/test/corpus/statements.txt

@@ -4,7 +4,7 @@ Simple If Statement (No Else)
 parser simple() {
   state start {
     if (true) {
-    };
+    }
     transition accept;
   }
 };
@@ -33,7 +33,6 @@ parser simple() {
         )
        )
       )
-      (semicolon)
      )
      (parserTransitionStatement
       (transition)
@@ -41,12 +40,10 @@ parser simple() {
        (identifier)
       )
      )
-     (semicolon)
     )
    )
   )
  )
- (semicolon)
 )
 
 =========================
@@ -56,7 +53,7 @@ parser simple() {
   state start {
     if (true) {
     } else {
-    };
+    }
     transition accept;
   }
 };
@@ -89,7 +86,6 @@ parser simple() {
         )
        )
       )
-      (semicolon)
      )
      (parserTransitionStatement
       (transition)
@@ -97,12 +93,10 @@ parser simple() {
        (identifier)
       )
      )
-     (semicolon)
     )
    )
   )
  )
- (semicolon)
 )
 
 =========================
@@ -143,7 +137,6 @@ parser simple() {
         )
        )
       )
-      (semicolon)
      )
      (parserStatements
       (parserStatement
@@ -157,7 +150,6 @@ parser simple() {
         )
        )
       )
-      (semicolon)
      )
      (parserTransitionStatement
       (transition)
@@ -165,12 +157,10 @@ parser simple() {
        (identifier)
       )
      )
-     (semicolon)
     )
    )
   )
  )
- (semicolon)
 )
  
 =========================
@@ -181,7 +171,7 @@ parser simple() {
     bool x = true;
     if (x) {
       x = false;
-    };
+    }
     transition accept;
   }
 };
@@ -213,7 +203,6 @@ parser simple() {
         )
        )
       )
-      (semicolon)
      )
      (parserStatements
       (parserStatement
@@ -236,13 +225,11 @@ parser simple() {
              )
             )
            )
-           (semicolon)
           )
          )
         )
        )
       )
-      (semicolon)
      )
      (parserTransitionStatement
       (transition)
@@ -250,12 +237,10 @@ parser simple() {
        (identifier)
       )
      )
-     (semicolon)
     )
    )
   )
  )
- (semicolon)
 )
  
 =========================
@@ -269,7 +254,7 @@ parser simple() {
       y = 1;
     } else {
       y = 2;
-    };
+    }
     transition accept;
   }
 };
@@ -301,7 +286,6 @@ parser simple() {
         )
        )
       )
-      (semicolon)
       (parserLocalElement
        (variableDeclaration
         (typeRef
@@ -316,7 +300,6 @@ parser simple() {
         )
        )
       )
-      (semicolon)
      )
      (parserStatements
       (parserStatement
@@ -339,7 +322,6 @@ parser simple() {
              )
             )
            )
-           (semicolon)
           )
          )
         )
@@ -358,13 +340,11 @@ parser simple() {
              )
             )
            )
-           (semicolon)
           )
          )
         )
        )
       )
-      (semicolon)
      )
      (parserTransitionStatement
       (transition)
@@ -372,11 +352,9 @@ parser simple() {
        (identifier)
       )
      )
-     (semicolon)
     )
    )
   )
  )
- (semicolon)
 )
  

tree-sitter-p4/test/corpus/transitions.txt

@@ -25,12 +25,10 @@ parser simple() {
        (identifier)
       )
      )
-     (semicolon)
     )
    )
   )
  )
- (semicolon)
 )
 
 =========================
@@ -74,15 +72,12 @@ parser simple() {
           (colon)
           (identifier)
          )
-         (semicolon)
         )
        )
       )
      )
-     (semicolon)
     )
    )
   )
  )
- (semicolon)
 )