Faulty ns macro desugaring

CLJS-866.clj

(comment
  ;; Bug report for CLJS-721 (support :include-macros true modifier in :require),
  ;; Ref. http://dev.clojure.org/jira/browse/CLJS-721,
  ;;      http://goo.gl/MQ3fWd (GitHub commit de6ee41b3)

  ;; desugar-ns-specs from clojurescript/src/clj/cljs/analyzer.clj
  ;; (`cljs.analyzer` ns)

  (desugar-ns-specs '[(:require [foo.bar :as bar :include-macros true])])
  ;; =>
  ((:require-macros (foo.bar :as bar))
   (:require        (foo.bar :as bar))) ; Correct

  (desugar-ns-specs '[(:require [foo.bar :as bar :include-macros true :refer [baz]])])
  ;; =>
  ((:require-macros (foo.bar :as bar :refer [baz]))
   (:require        (foo.bar :as bar))) ; Seems off, but what's the intended behaviour?

  (desugar-ns-specs '[(:require [foo.bar :as bar :refer [baz] :include-macros true])])
  ;; =>
  ((:require-macros (foo.bar :as bar :refer [baz]))
   (:require        (foo.bar :as bar :refer [baz])))

  ;; Is the position of `:include-macros true` supposed to influence expansion
  ;; like this?

  ;; And is the interaction between `:include-macros true` and `:refer` as
  ;; intended? I would have expected something like this:
  (desugar-ns-specs '[(:require [foo.bar :as bar :include-macros true :refer [baz]])])
  ;; =
  (desugar-ns-specs '[(:require [foo.bar :as bar :refer [baz] :include-macros true])])
  ;; =>
  ((:require-macros (foo.bar :as bar)) ; No :refer
   (:require        (foo.bar :as bar :refer [baz])))

  ;;; --------------------------------------------------------------------------

  ;; There's an additional problem with `:refer` + `:refer-macros`:

  (desugar-ns-specs '[(:require [foo.bar :as bar :refer [baz] :refer-macros [qux]])])
  ;; =>
  ((:require-macros (foo.bar :as bar :refer [baz] :refer [qux])) ; Faulty
   (:require        (foo.bar :as bar :refer [baz])))

  ;; I believe the correct expansion should be:
  ((:require-macros (foo.bar :as bar :refer [qux]))
   (:require        (foo.bar :as bar :refer [baz]))))

CLJS-866.patch

From 5f20005531d8280c2ee4b8367c9b6c08065e06b5 Mon Sep 17 00:00:00 2001
From: Peter Taoussanis <[email protected]>
Date: Fri, 3 Oct 2014 13:31:38 +0700
Subject: [PATCH] CLJS-866: Faulty ns macro desugaring

Fixes:
* `:refer-macros` sugar producing an invalid expansion.
* `:include-macros` sugar behaving unpredictably.
---
 src/clj/cljs/analyzer.clj | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/clj/cljs/analyzer.clj b/src/clj/cljs/analyzer.clj
index 08f2ad0..56f10ec 100644
--- a/src/clj/cljs/analyzer.clj
+++ b/src/clj/cljs/analyzer.clj
@@ -1131,18 +1131,20 @@
           (map (fn [[k & specs]] [k (into [] specs)]))
           (into {}))
         sugar-keys #{:include-macros :refer-macros}
+        remove-from-spec
+        (fn [pred spec]
+          (if-not (and (sequential? spec) (some pred spec))
+            spec
+            (let [[l r] (split-with (complement pred) spec)]
+              (recur pred (concat l (drop 2 r))))))
         to-macro-specs
         (fn [specs]
           (->> specs
             (filter #(and (sequential? %) (some sugar-keys %)))
-            (map #(->> % (remove #{:include-macros true})
+            (map #(->> % (remove-from-spec #{:include-macros})
+                         (remove-from-spec #{:refer})
                          (map (fn [x] (if (= x :refer-macros) :refer x)))))))
-        remove-sugar
-        (fn [spec]
-          (if (and (sequential? spec) (some sugar-keys spec))
-            (let [[l & r] (split-with #(not (contains? sugar-keys %)) spec)]
-              (concat l (drop 2 r)))
-            spec))]
+        remove-sugar (partial remove-from-spec sugar-keys)]
     (if-let [require-specs (seq (to-macro-specs require))]
       (map (fn [[k v]] (cons k (map remove-sugar v)))
         (update-in indexed [:require-macros] (fnil into []) require-specs))
-- 
1.9.3 (Apple Git-50)