Skip to content

Commit f677b10

Browse files
jonaseswannodette
authored andcommitted
Use 'error' function instead of 'assert' for better analyzer error messages
1 parent eccf2fc commit f677b10

File tree

1 file changed

+63
-49
lines changed

1 file changed

+63
-49
lines changed

src/clj/cljs/analyzer.clj

Lines changed: 63 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,8 @@
273273

274274
(defmethod parse 'if
275275
[op env [_ test then else :as form] name]
276-
(assert (>= (count form) 3) "Too few arguments to if")
276+
(when (< (count form) 3)
277+
(throw (error env "Too few arguments to if")))
277278
(let [test-expr (disallowing-recur (analyze (assoc env :context :expr) test))
278279
then-expr (analyze env then)
279280
else-expr (analyze env else)]
@@ -315,7 +316,8 @@
315316
(analyze (assoc catchenv :locals locals) `(do ~@(rest cblock))))
316317
body (if name (pop body) body)
317318
try (analyze (if (or name finally) catchenv env) `(do ~@body))]
318-
(when name (assert (not (namespace name)) "Can't qualify symbol in catch"))
319+
(when (and name (namespace name))
320+
(throw (error env "Can't qualify symbol in catch")))
319321
{:env env :op :try* :form form
320322
:try try
321323
:finally finally
@@ -336,7 +338,8 @@
336338
protocol (-> sym meta :protocol)
337339
dynamic (-> sym meta :dynamic)
338340
ns-name (-> env :ns :name)]
339-
(assert (not (namespace sym)) "Can't def ns-qualified name")
341+
(when (namespace sym)
342+
(throw (error env "Can't def ns-qualified name")))
340343
(let [env (if (or (and (not= ns-name 'cljs.core)
341344
(core-name? env sym))
342345
(get-in @namespaces [ns-name :uses sym]))
@@ -486,7 +489,8 @@
486489

487490
(defmethod parse 'letfn*
488491
[op env [_ bindings & exprs :as form] name]
489-
(assert (and (vector? bindings) (even? (count bindings))) "bindings must be vector of even number of elements")
492+
(when-not (and (vector? bindings) (even? (count bindings)))
493+
(throw (error env "bindings must be vector of even number of elements")))
490494
(let [n->fexpr (into {} (map (juxt first second) (partition 2 bindings)))
491495
names (keys n->fexpr)
492496
context (:context env)
@@ -523,7 +527,8 @@
523527

524528
(defn analyze-let
525529
[encl-env [_ bindings & exprs :as form] is-loop]
526-
(assert (and (vector? bindings) (even? (count bindings))) "bindings must be vector of even number of elements")
530+
(when-not (and (vector? bindings) (even? (count bindings)))
531+
(throw (error encl-env "bindings must be vector of even number of elements")))
527532
(let [context (:context encl-env)
528533
[bes env]
529534
(disallowing-recur
@@ -532,7 +537,8 @@
532537
bindings (seq (partition 2 bindings))]
533538
(if-let [[name init] (first bindings)]
534539
(do
535-
(assert (not (or (namespace name) (.contains (str name) "."))) (str "Invalid local name: " name))
540+
(when (or (namespace name) (.contains (str name) "."))
541+
(throw (error encl-env (str "Invalid local name: " name))))
536542
(let [init-expr (binding [*loop-lets* (cons {:params bes} (or *loop-lets* ()))]
537543
(analyze env init))
538544
be {:name name
@@ -587,8 +593,10 @@
587593
(let [context (:context env)
588594
frame (first *recur-frames*)
589595
exprs (disallowing-recur (vec (map #(analyze (assoc env :context :expr) %) exprs)))]
590-
(assert frame "Can't recur here")
591-
(assert (= (count exprs) (count (:params frame))) "recur argument count mismatch")
596+
(when-not frame
597+
(throw (error env "Can't recur here")))
598+
(when-not (= (count exprs) (count (:params frame)))
599+
(throw (error env "recur argument count mismatch")))
592600
(reset! (:flag frame) true)
593601
(assoc {:env env :op :recur :form form}
594602
:frame frame
@@ -601,7 +609,8 @@
601609

602610
(defmethod parse 'new
603611
[_ env [_ ctor & args :as form] _]
604-
(assert (symbol? ctor) "First arg to new must be a symbol")
612+
(when-not (symbol? ctor)
613+
(throw (error env "First arg to new must be a symbol")))
605614
(disallowing-recur
606615
(let [enve (assoc env :context :expr)
607616
ctorexpr (analyze enve ctor)
@@ -633,12 +642,12 @@
633642
(symbol? target)
634643
(do
635644
(let [local (-> env :locals target)]
636-
(assert (or (nil? local)
637-
(and (:field local)
638-
(or (:mutable local)
639-
(:unsynchronized-mutable local)
640-
(:volatile-mutable local))))
641-
"Can't set! local var or non-mutable field"))
645+
(when-not (or (nil? local)
646+
(and (:field local)
647+
(or (:mutable local)
648+
(:unsynchronized-mutable local)
649+
(:volatile-mutable local))))
650+
(throw (error env "Can't set! local var or non-mutable field"))))
642651
(analyze-symbol enve target))
643652

644653
:else
@@ -647,7 +656,8 @@
647656
(when (:field targetexpr)
648657
targetexpr))))
649658
valexpr (analyze enve val)]
650-
(assert targetexpr "set! target must be a field or a symbol naming a var")
659+
(when-not targetexpr
660+
(throw (error env "set! target must be a field or a symbol naming a var")))
651661
(cond
652662
(= targetexpr ::set-unchecked-if) {:env env :op :no-op}
653663
:else {:env env :op :set! :form form :target targetexpr :val valexpr
@@ -685,7 +695,8 @@
685695

686696
(defmethod parse 'ns
687697
[_ env [_ name & args :as form] _]
688-
(assert (symbol? name) "Namespaces must be named by a symbol.")
698+
(when-not (symbol? name)
699+
(throw (error env "Namespaces must be named by a symbol.")))
689700
(let [docstring (if (string? (first args)) (first args))
690701
args (if docstring (next args) args)
691702
metadata (if (map? (first args)) (first args))
@@ -694,8 +705,10 @@
694705
(reduce (fn [s [k exclude xs]]
695706
(if (= k :refer-clojure)
696707
(do
697-
(assert (= exclude :exclude) "Only [:refer-clojure :exclude (names)] form supported")
698-
(assert (not (seq s)) "Only one :refer-clojure form is allowed per namespace definition")
708+
(when-not (= exclude :exclude)
709+
(throw (error env "Only [:refer-clojure :exclude (names)] form supported")))
710+
(when (seq s)
711+
(throw (error env "Only one :refer-clojure form is allowed per namespace definition")))
699712
(into s xs))
700713
s))
701714
#{} args)
@@ -704,19 +717,19 @@
704717
valid-forms (atom #{:use :use-macros :require :require-macros :import})
705718
error-msg (fn [spec msg] (str msg "; offending spec: " (pr-str spec)))
706719
parse-require-spec (fn parse-require-spec [macros? spec]
707-
(assert (or (symbol? spec) (vector? spec))
708-
(error-msg spec "Only [lib.ns & options] and lib.ns specs supported in :require / :require-macros"))
720+
(when-not (or (symbol? spec) (vector? spec))
721+
(throw (error env (error-msg spec "Only [lib.ns & options] and lib.ns specs supported in :require / :require-macros"))))
709722
(when (vector? spec)
710-
(assert (symbol? (first spec))
711-
(error-msg spec "Library name must be specified as a symbol in :require / :require-macros"))
712-
(assert (odd? (count spec))
713-
(error-msg spec "Only :as alias and :refer (names) options supported in :require"))
714-
(assert (every? #{:as :refer} (map first (partition 2 (next spec))))
715-
(error-msg spec "Only :as and :refer options supported in :require / :require-macros"))
716-
(assert (let [fs (frequencies (next spec))]
717-
(and (<= (fs :as 0) 1)
718-
(<= (fs :refer 0) 1)))
719-
(error-msg spec "Each of :as and :refer options may only be specified once in :require / :require-macros")))
723+
(when-not (symbol? (first spec))
724+
(throw (error env (error-msg spec "Library name must be specified as a symbol in :require / :require-macros"))))
725+
(when-not (odd? (count spec))
726+
(throw (error env (error-msg spec "Only :as alias and :refer (names) options supported in :require"))))
727+
(when-not (every? #{:as :refer} (map first (partition 2 (next spec))))
728+
(throw (error env (error-msg spec "Only :as and :refer options supported in :require / :require-macros"))))
729+
(when-not (let [fs (frequencies (next spec))]
730+
(and (<= (fs :as 0) 1)
731+
(<= (fs :refer 0) 1)))
732+
(throw (error env (error-msg spec "Each of :as and :refer options may only be specified once in :require / :require-macros")))))
720733
(if (symbol? spec)
721734
(recur macros? [spec])
722735
(let [[lib & opts] spec
@@ -730,30 +743,29 @@
730743
(ns-unalias *ns* alias)
731744
(clojure.core/alias alias (.name ns))))
732745
(let [alias-type (if macros? :macros :fns)]
733-
(assert (not (contains? (alias-type @aliases)
734-
alias))
735-
(error-msg spec ":as alias must be unique"))
746+
(when (contains? (alias-type @aliases) alias)
747+
(throw (error env (error-msg spec ":as alias must be unique"))))
736748
(swap! aliases
737749
update-in [alias-type]
738750
conj alias)))
739-
(assert (or (symbol? alias) (nil? alias))
740-
(error-msg spec ":as must be followed by a symbol in :require / :require-macros"))
741-
(assert (or (and (sequential? referred) (every? symbol? referred))
742-
(nil? referred))
743-
(error-msg spec ":refer must be followed by a sequence of symbols in :require / :require-macros"))
751+
(when-not (or (symbol? alias) (nil? alias))
752+
(throw (error env (error-msg spec ":as must be followed by a symbol in :require / :require-macros"))))
753+
(when-not (or (and (sequential? referred) (every? symbol? referred))
754+
(nil? referred))
755+
(throw (error env (error-msg spec ":refer must be followed by a sequence of symbols in :require / :require-macros"))))
744756
(when-not macros?
745757
(swap! deps conj lib))
746758
(merge
747759
(when alias
748760
{rk (merge {alias lib} {lib lib})})
749761
(when referred {uk (apply hash-map (interleave referred (repeat lib)))})))))
750762
use->require (fn use->require [[lib kw referred :as spec]]
751-
(assert (and (symbol? lib) (= :only kw) (sequential? referred) (every? symbol? referred))
752-
(error-msg spec "Only [lib.ns :only (names)] specs supported in :use / :use-macros"))
763+
(when-not (and (symbol? lib) (= :only kw) (sequential? referred) (every? symbol? referred))
764+
(throw (error env (error-msg spec "Only [lib.ns :only (names)] specs supported in :use / :use-macros"))))
753765
[lib :refer referred])
754766
parse-import-spec (fn parse-import-spec [spec]
755-
(assert (and (symbol? spec) (nil? (namespace spec)))
756-
(error-msg spec "Only lib.Ctor specs supported in :import"))
767+
(when-not (and (symbol? spec) (nil? (namespace spec)))
768+
(throw (error env (error-msg spec "Only lib.Ctor specs supported in :import"))))
757769
(swap! deps conj spec)
758770
(let [ctor-sym (symbol (last (string/split (str spec) #"\.")))]
759771
{:import {ctor-sym spec}
@@ -765,10 +777,10 @@
765777
:import parse-import-spec}
766778
{uses :use requires :require uses-macros :use-macros requires-macros :require-macros imports :import :as params}
767779
(reduce (fn [m [k & libs]]
768-
(assert (#{:use :use-macros :require :require-macros :import} k)
769-
"Only :refer-clojure, :require, :require-macros, :use and :use-macros libspecs supported")
770-
(assert (@valid-forms k)
771-
(str "Only one " k " form is allowed per namespace definition"))
780+
(when-not (#{:use :use-macros :require :require-macros :import} k)
781+
(throw (error env "Only :refer-clojure, :require, :require-macros, :use and :use-macros libspecs supported")))
782+
(when-not (@valid-forms k)
783+
(throw (error env (str "Only one " k " form is allowed per namespace definition"))))
772784
(swap! valid-forms disj k)
773785
(apply merge-with merge m (map (spec-parsers k) libs)))
774786
{} (remove (fn [[r]] (= r :refer-clojure)) args))]
@@ -900,7 +912,8 @@
900912

901913
(defmethod parse 'js*
902914
[op env [_ jsform & args :as form] _]
903-
(assert (string? jsform))
915+
(when-not (string? jsform)
916+
(throw (error env "Invalid js* form")))
904917
(if args
905918
(disallowing-recur
906919
(let [seg (fn seg [^String s]
@@ -1017,7 +1030,8 @@
10171030
:column (or (-> form meta :column)
10181031
(:column env)))]
10191032
(let [op (first form)]
1020-
(assert (not (nil? op)) "Can't call nil")
1033+
(when (nil? op)
1034+
(throw (error env "Can't call nil")))
10211035
(let [mform (macroexpand-1 env form)]
10221036
(if (identical? form mform)
10231037
(wrapping-errors env

0 commit comments

Comments
 (0)