Fix error message: unexpected NEWLINE sometimes reported as unexpected EOF

Author: metawilm

parser/grammar-aclyacc.lisp

@@ -81,6 +81,7 @@ Therefore need to convert TOKEN-KIND to the corresponding TOKEN-CODE before pass
                   (last-newline-in-source (second (assoc :last-newline-in-source pos)))
                   (token (maybe-unwrap-literal-value (excl.yacc:grammar-parse-error-token c)))
                   (encl-error (excl.yacc::grammar-parse-error-enclosed-error c)))
+
              (cond (encl-error ;; Error in one of our grammar rules
                     (when clpython:*exceptions-loaded*
                       (assert (not (typep encl-error '{SyntaxError}))
@@ -91,10 +92,9 @@ Therefore need to convert TOKEN-KIND to the corresponding TOKEN-CODE before pass
                      (format nil "Parse error at line ~A~@[, at token `~S'~].~%[Internal error: ~A~_(caught due to ~S)]"
                              line token encl-error '*catch-yacc-conditions*)))
 
-                   ((and token
-                         (not (eq token (lexer-eof-token yacc-version)))
-                         (not (and (eq token '[newline]) (not last-newline-in-source))))
-                    (raise-syntax-error (format nil "At line ~A, parser got unexpected token: ~S" line token)))
+                   ((or (eq token (lexer-eof-token yacc-version))
+                        (and (eq token '[newline]) (not last-newline-in-source)))
+                    (raise-unexpected-eof))
 
                    (t
-                    (raise-unexpected-eof))))))))
+                    (raise-syntax-error (format nil "At line ~A, parser got unexpected token: ~S" line token)))))))))

parser/grammar-clyacc.lisp

@@ -60,11 +60,12 @@
                 (value (maybe-unwrap-literal-value (yacc:yacc-parse-error-value c)))
                 (expected-tokens (yacc:yacc-parse-error-expected-terminals c)))
 
-           (cond ((and (not (eq token (lexer-eof-token yacc-version)))
-                       (not (and (eq token '[newline]) (not last-newline-in-source))))
+           (cond ((or (eq token (lexer-eof-token yacc-version))
+                      (and (eq token '[newline]) (not last-newline-in-source)))
+                  (raise-unexpected-eof))
+                 
+                 (t
                   (raise-syntax-error
                    (format nil "At line ~A, parser got unexpected token: ~S. ~
                                 ~_Expected one of: ~{`~A'~^ ~}."
-                           line value expected-tokens)))
-                 (t
-                  (raise-unexpected-eof)))))))
+                           line value expected-tokens))))))))

parser/lexer.lisp

@@ -100,7 +100,7 @@ where TOKEN-KIND is a symbol like '[identifier]"
    (bracket-level :accessor ls-bracket-level :initform 0  :type fixnum)
    (open-deco                                :initform nil)
    (return-count                             :initform 0)
-   (last-newline-in-source :reader ls-last-newline-in-source :initform nil))
+   (last-newline-in-source :reader ls-last-newline-in-source :initform :unknown))
   (:metaclass closer-mop:funcallable-standard-class))
 
 (defmethod print-object ((lexer lexer) stream)
@@ -142,8 +142,8 @@ On EOF returns: eof-token, eof-token."
 
 (defmethod call-lexer (yacc-version (lexer lexer) (op (eql nil)))
   (declare (ignorable yacc-version op))
-  (with-slots (last-read-char-ix curr-line-no yacc-version tokens-todo
-               indent-stack bracket-level open-deco return-count last-newline-in-source) lexer
+  (with-slots (last-read-char-ix curr-line-no yacc-version tokens-todo indent-stack
+               bracket-level open-deco return-count last-newline-in-source eof-returned-already) lexer
     (when (= last-read-char-ix -1)
       ;; Check leading whitespace. This will go unnoticed by the lexer otherwise.
       (destructuring-bind (newline-p new-indent eof-p)
@@ -166,6 +166,8 @@ On EOF returns: eof-token, eof-token."
       (loop 
         (let ((c (lex-read-char :eof-error nil)))
           (cond ((not c)
+                 (when (eq last-newline-in-source :unknown)
+                   (setf last-newline-in-source nil))
                  (when (zerop return-count) ;; grammar does not like empty files, so dummy content
                    (lex-return '[identifier] '{None} nil))
                  (when *lex-fake-eof-after-toplevel-form*
@@ -243,8 +245,8 @@ On EOF returns: eof-token, eof-token."
                  (lex-unread-char c)
                  (destructuring-bind (newline-p new-indent eof-p)
                      (read-kind :whitespace c)
-                   (when (and eof-p newline-p)
-                     (setf last-newline-in-source t))
+                   (when eof-p
+                     (setf last-newline-in-source newline-p))
                    (when (and (not eof-p) newline-p (zerop bracket-level))
                      ;; Queue eof before dedents as todo.
                      (when (and (zerop new-indent)

parser/parser.lisp

@@ -29,7 +29,7 @@ Most important options:
   (:method ((x string) &rest options &key (yacc-version *default-yacc-version*) one-expr record-source-location tab-width)
            (declare (ignore one-expr record-source-location tab-width))
            (let ((lexer (apply #'make-lexer yacc-version x (sans options :one-expr :record-source-location))))
-             (apply #'parse-module-with-yacc yacc-version lexer (sans options :tab-width))))
+             (apply #'parse-module-with-yacc yacc-version lexer (sans options :tab-width :yacc-version))))
 
   (:method ((x pathname) &rest options)
            (apply #'parse (slurp-file x) options))

test/parser-test.lisp

@@ -220,6 +220,12 @@ def f(): pass" t))
       (test :unexp-eof-error (try-parse "'") :fail-info "(EOF in Lexer)")
       (test :unexp-eof-error (try-parse "def")
 	    :fail-info "Unexpected EOF in grammar")
+      (test :syntax-error (try-parse "def f(x):
+  if x > 1
+    return 3
+  else:
+    return 1")) ;; missing colon after "x > 1"
+
       (test :unexp-eof-error (try-parse "def f():") :fail-info "(EOF in grammar)")
       (test :syntax-error    (try-parse "def def") :fail-info "(Incorrect grammar)")
       (test :syntax-error    (try-parse " 42") :fail-info "(Leading whitespace)"))

test/tsetup.lisp

@@ -119,10 +119,17 @@ seems to give implementations some freedom here. (In practice: Allegro=NIL, LisW
     (test-false (seq-member 'a #(b)))
     (test-false (seq-member 'a '(b)))))
 
+(defmacro with-all-parser-versions (&body body)
+  `(dolist (.yacc-version (append (when (asdf:find-system :yacc nil) (list :cl-yacc))
+                                    #+allegro '(:allegro-yacc)))
+     (let ((clpython.parser::*default-yacc-version* .yacc-version))
+       ,@body)))
+
 (defun run-tests ()
   (with-subtest (:name "CLPython")
     (test-comp-testfunc)
-    (run-parser-test)
+    (with-all-parser-versions 
+        (run-parser-test))
     #+(or)(run-lispy-test)
     (run-compiler-test)
     (run-code-walker-test)