Locals munged to the same JavaScript name (a-b, a_b) shadow each other instead of overwriting (#18)

CHANGELOG.md

@@ -11,6 +11,7 @@ releases, see its [changelog](https://github.com/clojure/clojurescript/blob/mast
 
 ### Fixed
 
+- Locals whose names munge to the same JavaScript identifier (i.e. `board_id` and `board-id`) are renamed apart instead of one overwriting the other, in `let`, `loop`, `letfn`, fn params, destructuring and `catch` (CLJS-2461, CLJS-3426). `letfn` bindings shadowing a local of the same name are renamed apart too.
 - The REPL reported errors raised while macroexpanding (i.e. a macro's `ex-info`) without their message.
 - The watcher recompiles a batch of changed namespaces in dependency order, rather than in the order the changes were reported: a namespace using a var just added to one it requires, both changed at once (i.e. by a rebase or branch switch), no longer warns of an undeclared var, holding back hot reloads until it changes again.
 

src/main/clojure/cljs/analyzer.cljc

@@ -1375,6 +1375,38 @@ x                          (not (contains? ret :info)))
     {:name sym}
     lb))
 
+(defn- munged-local-name
+  "The JavaScript identifier the compiler munges local sym to, before shadow
+  renaming (cljs.compiler/munge)."
+  [sym]
+  (let [s (munge (str sym))]
+    (if (contains? js-reserved s) (str s "$") s)))
+
+(defn- shadow-chain-length [lb]
+  (loop [d 0 lb lb]
+    (if-some [s (:shadow lb)] (recur (inc d) s) d)))
+
+(defn shadowed-local
+  "The local in locals a new local sym shadows: the one sym names or, when
+  another munges to the same JavaScript identifier (a-b and a_b, CLJS-2461),
+  the deepest-shadowing of those, so that the compiler renames sym apart from
+  all of them."
+  [locals sym]
+  (let [lb (handle-symbol-local sym (get locals sym))
+        m  (munged-local-name sym)]
+    (if (and (neg? (.indexOf ^String m "_")) (neg? (.indexOf ^String m "$")))
+      ;; only names munged to something with _ or $ can collide
+      lb
+      (reduce-kv
+        (fn [best k v]
+          (if (and (symbol? k) (not= k sym) (= m (munged-local-name k)))
+            (let [v (handle-symbol-local k v)]
+              (if (or (nil? best) (> (shadow-chain-length v) (shadow-chain-length best)))
+                v
+                best))
+            best))
+        lb locals))))
+
 (defn qualified->dotted
   [sym]
   (symbol (str (namespace sym) "." (name sym))))
@@ -2309,7 +2341,7 @@ x                          (not (contains? ret :info)))
           nmeta  (meta name)
           tag    (:tag nmeta)
           shadow (when (some? locals)
-                   (handle-symbol-local name (locals name)))
+                   (shadowed-local locals name))
           env    (merge (select-keys env [:context])
                    {:line line :column column})
           param  {:op :binding
@@ -2374,7 +2406,7 @@ x                          (not (contains? ret :info)))
 (defn fn-name-var [env locals name]
   (when (some? name)
     (let [ns       (-> env :ns :name)
-          shadow   (or (handle-symbol-local name (get locals name))
+          shadow   (or (shadowed-local locals name)
                        (get-in env [:js-globals name]))
           fn-scope (:fn-scope env)
           name-var {:op :binding
@@ -2507,6 +2539,7 @@ x                          (not (contains? ret :info)))
         (reduce (fn [[{:keys [locals] :as env} bes] n]
                   (let [ret-tag (-> n meta :tag)
                         fexpr (no-warn (analyze env (n->fexpr n)))
+                        shadow (shadowed-local locals n)
                         be (cond->
                              {:op :binding
                               :name n
@@ -2516,7 +2549,8 @@ x                          (not (contains? ret :info)))
                               :line (get-line n env)
                               :column (get-col n env)
                               :local :letfn
-                              :shadow (handle-symbol-local n (locals n))
+                              :shadow shadow
+                              :info {:name n :shadow shadow}
                               :variadic? (:variadic? fexpr)
                               :max-fixed-arity (:max-fixed-arity fexpr)
                               :method-params (map :params (:methods fexpr))}
@@ -2599,7 +2633,7 @@ x                          (not (contains? ret :info)))
           (let [init-expr (analyze-let-binding-init env init (cons {:params bes} *loop-lets*))
                 line (get-line name env)
                 col (get-col name env)
-                shadow (or (handle-symbol-local name (get-in env [:locals name]))
+                shadow (or (shadowed-local (:locals env) name)
                            (get-in env [:js-globals name]))
                 be {:op :binding
                     :name name

src/test/cljs/cljs/core_test.cljs

@@ -571,6 +571,34 @@
     (let [x (fn [] "overwritten")]
       (is (= "original" (y))))))
 
+(deftest test-letfn-shadowing-let
+  (let [x 1]
+    (letfn [(y [] x)
+            (x [] 2)]
+      (is (= 2 (x)))
+      (is (= 2 ((y)))))))
+
+;; CLJS-2461, CLJS-3426: locals that munge to the same JavaScript name
+(defn- munged-same-let [] (let [a_b 1 a-b 2] [a_b a-b]))
+(defn- munged-same-param [a_b] (let [a-b 2] [a_b a-b]))
+(defn- munged-same-keys [{:keys [board_id]}]
+  (let [board-id (str "parsed " board_id)] [board_id board-id]))
+
+(deftest test-locals-munged-to-the-same-name
+  (is (= [1 2] (munged-same-let)))
+  (is (= [1 2] (munged-same-param 1)))
+  (is (= ["x" "parsed x"] (munged-same-keys {:board_id "x"})))
+  (is (= [1 2] (let [a_b 1] (let [a-b 2] [a_b a-b]))))
+  (is (= [3 2] (let [a_b 1 a-b 2 a_b 3] [a_b a-b])))
+  (is (= [3 4] ((fn [a_b] (let [a-b 2 a_b 3 a-b 4] [a_b a-b])) 0)))
+  (is (= [1 3] (loop [a_b 1 a-b 2]
+                 (if (< a-b 3) (recur a_b (inc a-b)) [a_b a-b]))))
+  (is (= [3 2] (let [a_b 1] (let [a-b 2] (let [a_b 3] ((fn [] [a_b a-b])))))))
+  (is (= 1 (letfn [(a_b [] 1) (a-b [] 2)] (a_b))))
+  (is (= 13 (let [a-b 1] (+ 10 (let [a_b 2] (+ a-b a_b))))))
+  (is (= [1 2] (let [a-b 1] (try (throw 2) (catch :default a_b [a-b a_b])))))
+  (is (= [1 2] (let [let 1 let$ 2] [let let$]))))
+
 (deftest test-459
   (is (= (reduce-kv conj [] (sorted-map :foo 1 :bar 2))
         [:bar 2 :foo 1])))