fix: detect unconsumed tokens after parse to catch unbalanced braces (#445)

ANTLR's default error recovery silently absorbed certain malformed queries
(e.g. missing opening braces) without ever calling the syntaxError listener,
producing a structurally different but syntactically valid parse tree and
leaving trailing tokens unconsumed.

Fix by comparing the document tree's stop-token index against the last
non-EOF token after parsing; throw ParseError if any tokens were skipped.

Also fix two test queries that had a genuine extra `}` that was previously
silently ignored: entities-query in federation_tests and
later-fragments-do-not-override-earlier in fragments_tests.

Author: hlship

src/com/walmartlabs/lacinia/parser/antlr.clj

@@ -5,7 +5,7 @@
            (java.util.concurrent ConcurrentHashMap)
            (org.antlr.v4.runtime ANTLRErrorListener
                                  CharStreams CommonTokenStream Lexer Parser
-                                 RecognitionException)
+                                 ParserRuleContext RecognitionException Token)
            (org.antlr.v4.runtime.tree ParseTree Tree)))
 
 (def ^ConcurrentHashMap fast-keyword-cache
@@ -132,7 +132,8 @@
             (.removeErrorListeners)
             (.addErrorListener error-listener))
 
-        parser (parser ap (CommonTokenStream. lexer))
+        token-stream (CommonTokenStream. lexer)
+        parser (parser ap token-stream)
         _ (doto parser
             (.removeErrorListeners)
             (.addErrorListener error-listener))
@@ -142,5 +143,27 @@
     (when-let [errors @error-listener]
       (throw (parse-error errors tree)))
 
+    ;; ANTLR's error recovery can silently produce a valid parse tree even when
+    ;; there are unconsumed tokens (e.g. unbalanced braces). Detect this case by
+    ;; checking that every non-EOF token was consumed by the document rule.
+    (.fill token-stream)
+    (let [tokens (.getTokens token-stream)
+          n (count tokens)
+          ;; tokens is [...real-tokens... EOF]; last-real-idx is the index of the
+          ;; last non-EOF token (n-2), or -1 when the input is empty.
+          last-real-idx (- n 2)
+          stop-token (.getStop ^ParserRuleContext tree)
+          stop-idx (if stop-token (.getTokenIndex ^Token stop-token) -1)]
+      (when (< stop-idx last-real-idx)
+        (let [first-unconsumed (.get token-stream (inc stop-idx))
+              line (.getLine ^Token first-unconsumed)
+              col (inc (.getCharPositionInLine ^Token first-unconsumed))]
+          (throw (parse-error [{:line line
+                                :char col
+                                :message (str "extraneous input '"
+                                              (.getText ^Token first-unconsumed)
+                                              "' expecting EOF")}]
+                               tree)))))
+
     {:tree tree
      :parser parser}))

test/com/walmartlabs/lacinia/federation_tests.clj

@@ -52,7 +52,6 @@ query($reps : [_Any!]!) {
     ... on User { id name }
 
     ... on Account { acct_number name }
-    }
   }
 }")
 

test/com/walmartlabs/lacinia/fragments_tests.clj

@@ -90,8 +90,7 @@
                    }
                 }
               }
-           }
-         }"))))
+           }"))))
 
 (deftest named-fragments
   (is (= {:data {:characters [{:name "R2-D2"

test/com/walmartlabs/lacinia/parser/query_test.clj

@@ -95,7 +95,7 @@
         e (is (thrown? Throwable
                        (parser/parse-query schema "query [hero]")))]
     (when e
-      (is (= "Failed to parse GraphQL query." (.getMessage e)))
+      (is (= "Failed to parse GraphQL query." (ex-message e)))
       ;; TODO: See if we can get a proper column number here!
       (is (= {:errors [{:locations [{:column nil
                                      :line 1}]
@@ -105,6 +105,22 @@
                         :message "mismatched input ']' expecting {'(', '{', '@'}"}]}
              (ex-data e))))))
 
+(deftest unbalanced-braces
+  ;; Regression test for https://github.com/walmartlabs/lacinia/issues/445
+  ;; ANTLR's error recovery can silently produce a valid parse tree for
+  ;; certain malformed queries with unbalanced/missing braces.
+  (testing "missing opening brace after query keyword"
+    (let [e (is (thrown? Throwable
+                         (parse-query "query\n  myQuery {\n    name\n  }\n}")))]
+      (when e
+        (is (= "Failed to parse GraphQL query." (ex-message e))))))
+
+  (testing "missing opening brace for nested field"
+    (let [e (is (thrown? Throwable
+                         (parse-query "query {\n  myQuery\n    name\n  }\n}")))]
+      (when e
+        (is (= "Failed to parse GraphQL query." (ex-message e)))))))
+
 (deftest requires-compiled-schema
   (is (thrown-with-msg? IllegalStateException
                         #"The provided schema has not been compiled"