info: don't use a fallback for ns-qualified symbols

Fixes #133
clojure-emacs · Oct 2, 2021 · 3b69a2c · 3b69a2c
1 parent f2ef9c7
commit 3b69a2c
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 48 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## master (unreleased)
 
+### Changes
+
+* [#133](https://github.com/clojure-emacs/orchard/issues/133): `info:` don't fall back to `clojure.core` for fully-qualified symbols. 
+
 ## 0.7.2 (2021-09-30)
 
 ### Changes

diff --git a/src/orchard/info.clj b/src/orchard/info.clj
@@ -7,9 +7,9 @@
    [orchard.cljs.meta :as cljs-meta]
    [orchard.java :as java]
    [orchard.java.classpath :as cp]
+   [orchard.java.resource :as resource]
    [orchard.meta :as m]
-   [orchard.misc :as misc]
-   [orchard.java.resource :as resource]))
+   [orchard.misc :as misc]))
 
 (defn qualify-sym
   "Qualify a symbol, if any in `sym`, with `ns`.
@@ -65,32 +65,29 @@
   {:added "0.5"}
   [{:keys [dialect ns sym computed-ns unqualified-sym] :as opts}]
   {:pre [(= dialect :clj)]}
-  (let [ns (or ns computed-ns)]
-    (if-not (and ns (find-ns ns))
-      ;; Lookups in files whose namespaces don't exist yet should still be able
-      ;; to resolve built-ins and fully-qualified syms
-      (recur (assoc opts :ns 'clojure.core))
-      (or
-       ;; it's a special (special-symbol?)
-       (m/special-sym-meta sym)
-       ;; it's a var
-       (some-> ns (m/resolve-var sym) (m/var-meta))
-       ;; it's a Java constructor/static member symbol
-       (some-> ns (java/resolve-symbol sym))
-       ;; it's an unqualified sym maybe referred
-       (some-> ns (m/resolve-var unqualified-sym) (m/var-meta))
-       ;; it's a Java class/record type symbol
-       (some-> ns (java/resolve-type unqualified-sym))
-       ;; it's an alias for another ns
-       (some-> ns (m/resolve-aliases) (get sym) (m/ns-meta))
-       ;; We use :unqualified-sym *exclusively* here because because our :ns is
-       ;; too ambiguous.
-       ;;
-       ;; Observe the incorrect behavior (should return nil, there is a test):
-       ;;
-       ;;   (info '{:ns clojure.core :sym non-existing}) ;;=> {:author "Rich Hickey" :ns clojure.core ...}
-       ;;
-       (some-> (find-ns unqualified-sym) (m/ns-meta))))))
+  (let [ns (or ns computed-ns)
+        ns (or (when (some-> ns find-ns)
+                 ns)
+               'clojure.core)]
+    (or
+     ;; it's a special (special-symbol?)
+     (m/special-sym-meta sym)
+     ;; it's a var
+     (some-> ns (m/resolve-var sym) (m/var-meta))
+     ;; it's a Java constructor/static member symbol
+     (some-> ns (java/resolve-symbol sym))
+     ;; it's a Java class/record type symbol
+     (some-> ns (java/resolve-type unqualified-sym))
+     ;; it's an alias for another ns
+     (some-> ns (m/resolve-aliases) (get sym) (m/ns-meta))
+     ;; We use :unqualified-sym *exclusively* here because because our :ns is
+     ;; too ambiguous.
+     ;;
+     ;; Observe the incorrect behavior (should return nil, there is a test):
+     ;;
+     ;;   (info '{:ns clojure.core :sym non-existing}) ;;=> {:author "Rich Hickey" :ns clojure.core ...}
+     ;;
+     (some-> (find-ns unqualified-sym) (m/ns-meta)))))
 
 (defn cljs-meta
   {:added "0.5"}

diff --git a/test/orchard/info_test.clj b/test/orchard/info_test.clj
@@ -1,12 +1,12 @@
 (ns orchard.info-test
   (:require
-   [clojure.test :as test :refer [deftest is testing use-fixtures]]
-   [clojure.string :as str]
+   [clojure.java.io :refer [resource]]
+   [clojure.string :as str :refer [replace-first]]
+   [clojure.test :refer [are deftest is testing use-fixtures]]
+   [orchard.cljs.test-env :as test-env]
    [orchard.info :as info]
    [orchard.java :as java]
    [orchard.misc :as misc]
-   [clojure.java.io :refer [resource]]
-   [orchard.cljs.test-env :as test-env]
    [orchard.test-ns]))
 
 @java/cache-initializer ;; make tests more deterministic
@@ -352,7 +352,8 @@
 
 (deftest info-macros-referred-var-test
   (testing "Macro - referred"
-    (let [params '[{:sym orchard.test-ns/my-add}
+    (let [params '[{:sym orchard.test-ns/my-add},
+
                    {:ns orchard.test-ns
                     :sym my-add}]
           expected '{:name my-add
@@ -368,7 +369,7 @@
                     (map #(select-keys % [:ns :name :arglists :macro :file]))))))
 
       (testing "- :clj"
-        (is (= (take 2 (repeat expected))
+        (is (= [{}, expected]
                (->> params
                     (map #(info/info* %))
                     (map #(select-keys % [:ns :name :arglists :macro :file])))))))))
@@ -467,21 +468,43 @@
                :returns int}))
       (is (re-find #"Returns the greater of two" (:doc i))))))
 
+(def some-var nil)
+
 (deftest info-undefined-namespace-test
-  (testing "Fully qualified sym can still be resolved"
-    (is (= '{:added "1.2"
-             :ns clojure.string
-             :name upper-case
-             :file "clojure/string.clj"}
-           (select-keys (info/info* {:ns 'gibberish :sym 'clojure.string/upper-case})
-                        [:added :ns :name :file]))))
-  (testing "clojure.core syms can still be resolved"
-    (is (= '{:added "1.0"
-             :ns clojure.core
-             :name merge
-             :file "clojure/core.clj"}
-           (select-keys (info/info* {:ns 'gibberish :sym 'merge})
-                        [:added :ns :name :file])))))
+  (let [current-ns (-> ::_ namespace symbol)]
+    (are [input expected] (= expected
+                             (select-keys (info/info* input)
+                                          [:added :ns :name :file]))
+      {:ns current-ns :sym 'some-var}                   '{:ns   orchard.info-test,
+                                                          :name some-var,
+                                                          :file "orchard/info_test.clj"}
+      {:ns current-ns :sym 'replace-first}              '{:added "1.2",
+                                                          :ns    clojure.string,
+                                                          :name  replace-first,
+                                                          :file  "clojure/string.clj"}
+      {:ns current-ns :sym 'merge}                      '{:added "1.0"
+                                                          :ns    clojure.core
+                                                          :name  merge
+                                                          :file  "clojure/core.clj"}
+      {:ns current-ns :sym 'non.existing.ns/merge}      {}
+      {:ns current-ns :sym 'clojure.string/upper-case}  '{:added "1.2"
+                                                          :ns    clojure.string
+                                                          :name  upper-case
+                                                          :file  "clojure/string.clj"}
+      {:ns current-ns :sym 'non.existing.ns/upper-case} {}
+
+      {:ns 'gibberish :sym 'some-var}                   {}
+      {:ns 'gibberish :sym 'replace-first}              {}
+      {:ns 'gibberish :sym 'merge}                      '{:added "1.0"
+                                                          :ns    clojure.core
+                                                          :name  merge
+                                                          :file  "clojure/core.clj"}
+      {:ns 'gibberish :sym 'non.existing.ns/merge}      {}
+      {:ns 'gibberish :sym 'clojure.string/upper-case}  '{:added "1.2"
+                                                          :ns    clojure.string
+                                                          :name  upper-case
+                                                          :file  "clojure/string.clj"}
+      {:ns 'gibberish :sym 'non.existing.ns/upper-case} {})))
 
 (deftest javadoc-info-unit-test
   (testing "Get an HTTP URL for a Sun/Oracle Javadoc"