Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
error on unmatched closing paren/brace
  • Loading branch information
chaos committed Jun 12, 2020
commit b2346920204322a74bbd9c8b977afa5926d76299
67 changes: 37 additions & 30 deletions parseclj-parser.el
Original file line number Diff line number Diff line change
Expand Up @@ -71,36 +71,43 @@ OPTIONS is an association list. This list is also passed down to the
REDUCE-BRANCH function. See `parseclj-parser' for more information on
available options."
(let ((opening-token-type (parseclj--find-opening-token stack closing-token))
(fail-fast (a-get options :fail-fast t))
(collection nil))

;; unwind the stack until opening-token-type is found, adding to collection
(while (and stack (not (eq (parseclj-lex-token-type (car stack)) opening-token-type)))
(push (pop stack) collection))

;; did we find the right token?
(if (eq (parseclj-lex-token-type (car stack)) opening-token-type)
(progn
(when fail-fast
;; any unreduced tokens left: bail early
(when-let ((token (seq-find #'parseclj-lex-token-p collection)))
(parseclj--error "At position %s, unmatched %S"
(a-get token :pos)
(parseclj-lex-token-type token))))

;; all good, call the reducer so it can return an updated stack with a
;; new node at the top.
(let ((opening-token (pop stack)))
(funcall reduce-branch stack opening-token collection options)))

;; Unwound the stack without finding a matching paren: either bail early
;; or return the original stack and continue parsing
(if fail-fast
(parseclj--error "At position %s, unmatched %S"
(a-get closing-token :pos)
(parseclj-lex-token-type closing-token))

(reverse collection)))))
(fail-fast (a-get options :fail-fast t)))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These let bindings should be aligned as before, if you let Emacs reindent it it should do it correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's due to literal tabs, I'll revert to spaces and set indent-tabs-mode in .dir-locals accordingly

(if (not opening-token-type)
(if fail-fast
(parseclj--error "At position %s, unmatched %S"
(a-get closing-token :pos)
(parseclj-lex-token-type closing-token))

stack)

(let ((collection nil))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to pull this into a separate let? if not I would leave it where it was.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessary, but just because its scope has moved a level down into the else part; I'll restore it and replace with progn instead.

;; unwind the stack until opening-token-type is found, adding to collection
(while (and stack (not (eq (parseclj-lex-token-type (car stack)) opening-token-type)))
(push (pop stack) collection))

;; did we find the right token?
(if (eq (parseclj-lex-token-type (car stack)) opening-token-type)
(progn
(when fail-fast
;; any unreduced tokens left: bail early
(when-let ((token (seq-find #'parseclj-lex-token-p collection)))
(parseclj--error "At position %s, unmatched %S"
(a-get token :pos)
(parseclj-lex-token-type token))))

;; all good, call the reducer so it can return an updated stack with a
;; new node at the top.
(let ((opening-token (pop stack)))
(funcall reduce-branch stack opening-token collection options)))

;; Unwound the stack without finding a matching paren: either bail early
;; or return the original stack and continue parsing
(if fail-fast
(parseclj--error "At position %s, unmatched %S"
(a-get closing-token :pos)
(parseclj-lex-token-type closing-token))

(reverse collection)))))))

(defun parseclj--take-value (stack value-p)
"Scan STACK until a value is found.
Expand Down
30 changes: 29 additions & 1 deletion test/parseclj-test.el
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,20 @@
(condition-case errdata
(parseclj-parse-clojure "(1 [2 {3 ( 4}])")
(parseclj-parser-error (cadr errdata)))
"At position 10, unmatched :lparen")))
"At position 10, unmatched :lparen"))

(should (equal
(condition-case errdata
(parseclj-parse-clojure "{:a 1}}")
(parseclj-parser-error (cadr errdata)))
"At position 7, unmatched :rbrace"))

(should (equal
(condition-case errdata
(parseclj-parse-clojure "'(1))")
(parseclj-parser-error (cadr errdata)))
"At position 5, unmatched :rparen"))
)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding tests!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review


(ert-deftest parseclj-parse-clojure-not-fail-fast-test ()
(should (equal (parseclj-parse-clojure "(1 [2 {3 ( 4}])" :fail-fast nil)
Expand All @@ -117,6 +130,21 @@
((:token-type . :lparen) (:form . "(") (:pos . 10))
((:node-type . :number) (:position . 12) (:form . "4") (:value . 4))))))))))))

(should (equal
(parseclj-parse-clojure "{:a 1}}" :fail-fast nil)
'((:node-type . :root)
(:position . 1)
(:children ((:node-type . :map)
(:position . 1)
(:children ((:node-type . :keyword)
(:position . 2)
(:form . ":a")
(:value . :a))
((:node-type . :number)
(:position . 5)
(:form . "1")
(:value . 1))))))))

;; TODO: uneven map forms
)

Expand Down