Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Code For: clojure.set/join getting chosen over clojure.string/join #24

Merged
merged 2 commits into from

2 participants

@AlexBaranosky

No description provided.

@technomancy technomancy merged commit e9421e5 into from
@technomancy
Owner

Looks good; thanks. Do you think we should cut a point release for this or wait till we have a bit more?

@AlexBaranosky

I think this issue would affect a lot of people negatively, so I would tend to prefer the point release.

@technomancy
Owner

Cool; just pushed 1.3.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 36 additions and 8 deletions.
  1. +18 −8 src/slam/hound/regrow.clj
  2. +18 −0 test/slam/hound_test.clj
View
26 src/slam/hound/regrow.clj
@@ -97,17 +97,27 @@
:require-refer [:require :use] ; could've been require/refer or use/only
[new-type]))
+(defn- referred-to-in-originals-pred [type originals]
+ (if-not (= type :require-refer)
+ (constantly false)
+ (fn [[ns1 _ [alias1]]]
+ (some (fn [[[ns2 _ [alias2]]]]
+ (and (= alias1 alias2) (= ns1 ns2)))
+ originals))))
+
(defn- disambiguate [candidates missing ns-map type]
;; TODO: prefer things in src/classes to jars
(debug :disambiguating missing :in candidates)
- (->> candidates
- (sort-by (juxt (complement (in-originals-pred
- (map #(get (:old ns-map) %)
- (new-type-to-old-types type))))
- ;; TODO: prefer candidates where last segment matches
- (comp count str)))
- (remove #(re-find disambiguator-blacklist (str %)))
- first))
+ (let [orig-clauses (map #(get (:old ns-map) %)
+ (new-type-to-old-types type))]
+ (->> candidates
+ (sort-by (juxt (complement (in-originals-pred orig-clauses))
+ (complement (referred-to-in-originals-pred
+ type orig-clauses))
+ ;; TODO: prefer candidates where last segment matches
+ (comp count str)))
+ (remove #(re-find disambiguator-blacklist (str %)))
+ first)))
(defn- grow-step [missing type ns-map body]
(if-let [addition (disambiguate (candidates type missing body)
View
18 test/slam/hound_test.clj
@@ -62,6 +62,24 @@
[[:name] String]
{:name "Bob"})}}))))))
+(deftest ^:integration test-chooses-referred-vars-preferring-those-referred-to-in-old-ns
+ (testing "since both clojure.string and clojure.set are present in the original namespace
+ we must disambiguate further by preferring namespaces that also referred the var
+ in the old version of the ns form"
+ (is (= "(ns foo.bar
+ (:require [clojure.set :refer [union]]
+ [clojure.string :refer [join]]))
+"
+ (reconstruct (StringReader.
+ (str '(ns foo.bar
+ (:require [clojure.string :only [join]]
+ [clojure.set :refer [union]]))
+ '(do
+ (defn f [xs]
+ (join "," xs))
+ (defn g [a b]
+ (union a b))))))))))
+
(deftest ^:integration test-prefers-requires-as-clauses-from-orig-ns
;; korma.core is on the :dev-dependencies, and was getting erroneously picked
;; for these 2 namespaces
Something went wrong with that request. Please try again.