Skip to content

Commit aa53807

Browse files
committed
CLJS-712 & CLJS-2957: resolve-var for dot still wrong, externs inference regression
The AST alignment changes reveal that our handling of the dot form was still incorrect. When resolve-var returns a dotted :var like foo.bar/baz.woz it gets desugared. In this case of GCL import this would lead to malformed :host-field AST nodes. Now in resolve var if we have a dotted symbol first we check that the prefix is *not* something that can be resolved. If can't be resolved then we don't do anything other than convert the form from foo.bar.baz to foo.bar/baz. Add tests cases for type inference, externs inference, and code gen.
1 parent abe3160 commit aa53807

File tree

5 files changed

+60
-29
lines changed

5 files changed

+60
-29
lines changed

src/main/clojure/cljs/analyzer.cljc

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,8 +1156,11 @@
11561156
(defn resolve-var
11571157
"Resolve a var. Accepts a side-effecting confirm fn for producing
11581158
warnings about unresolved vars."
1159-
([env sym] (resolve-var env sym nil))
1159+
([env sym]
1160+
(resolve-var env sym nil))
11601161
([env sym confirm]
1162+
(resolve-var env sym confirm true))
1163+
([env sym confirm default?]
11611164
(let [locals (:locals env)]
11621165
(if #?(:clj (= "js" (namespace sym))
11631166
:cljs (identical? "js" (namespace sym)))
@@ -1209,21 +1212,15 @@
12091212
(let [idx (.indexOf s ".")
12101213
prefix (symbol (subs s 0 idx))
12111214
suffix (subs s (inc idx))]
1212-
(if-some [lb (handle-symbol-local prefix (get locals prefix))]
1213-
{:op :local
1214-
:name (symbol (str (:name lb) "." suffix))}
1215-
(if-some [full-ns (gets @env/*compiler* ::namespaces current-ns :imports prefix)]
1216-
{:op :js-var
1217-
:name (symbol (str full-ns) suffix)}
1218-
(if-some [info (gets @env/*compiler* ::namespaces current-ns :defs prefix)]
1219-
(merge info
1220-
{:name (symbol (str current-ns) (str sym))
1221-
:op :var
1222-
:ns current-ns})
1223-
(merge (gets @env/*compiler* ::namespaces prefix :defs (symbol suffix))
1224-
{:name (if (= "" prefix) (symbol suffix) (symbol (str prefix) suffix))
1225-
:op :var
1226-
:ns prefix})))))
1215+
;; check if prefix is some existing def
1216+
(if-let [resolved (resolve-var env prefix nil false)]
1217+
(update resolved :name #(symbol (str % "." suffix)))
1218+
(let [idx (.lastIndexOf s ".")
1219+
pre (subs s 0 idx)
1220+
suf (subs s (inc idx))]
1221+
{:op :var
1222+
:name (symbol pre suf)
1223+
:ns (symbol pre)})))
12271224

12281225
(some? (gets @env/*compiler* ::namespaces current-ns :uses sym))
12291226
(let [full-ns (gets @env/*compiler* ::namespaces current-ns :uses sym)]
@@ -1236,7 +1233,7 @@
12361233
(resolve* env sym full-ns current-ns))
12371234

12381235
(some? (gets @env/*compiler* ::namespaces current-ns :imports sym))
1239-
(recur env (gets @env/*compiler* ::namespaces current-ns :imports sym) confirm)
1236+
(recur env (gets @env/*compiler* ::namespaces current-ns :imports sym) confirm default?)
12401237

12411238
(some? (gets @env/*compiler* ::namespaces current-ns :defs sym))
12421239
(do
@@ -1260,7 +1257,7 @@
12601257
(resolve-invokeable-ns s current-ns env)
12611258

12621259
:else
1263-
(do
1260+
(when default?
12641261
(when (some? confirm)
12651262
(confirm env current-ns sym))
12661263
(merge (gets @env/*compiler* ::namespaces current-ns :defs sym)

src/test/clojure/cljs/analyzer_tests.clj

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
[cljs.analyzer.api :as ana-api]
1313
[cljs.compiler :as comp]
1414
[cljs.env :as env]
15-
[cljs.js-deps :as deps]
1615
[cljs.test-util :refer [unsplit-lines]]
1716
[cljs.util :as util]
1817
[clojure.java.io :as io]

src/test/clojure/cljs/compiler_tests.clj

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,17 @@
3232
(def aenv (assoc-in (ana/empty-env) [:ns :name] 'cljs.user))
3333
(def cenv (env/default-compiler-env))
3434

35+
(defn compile-form-seq
36+
([forms]
37+
(compile-form-seq forms
38+
(when env/*compiler*
39+
(:options @env/*compiler*))))
40+
([forms opts]
41+
(with-out-str
42+
(binding [ana/*cljs-ns* 'cljs.user]
43+
(doseq [form forms]
44+
(comp/emit (ana/analyze (ana/empty-env) form)))))))
45+
3546
#_(deftest should-recompile
3647
(let [src (File. "test/hello.cljs")
3748
dst (File/createTempFile "compilertest" ".cljs")
@@ -331,6 +342,22 @@
331342
(is (not (str/includes? snippet3 "(function (x){")))
332343
))
333344

345+
(deftest test-goog-ctor-import-gen
346+
(is (true? (str/includes?
347+
(env/with-compiler-env (env/default-compiler-env)
348+
(compile-form-seq
349+
'[(ns test.foo
350+
(:import [goog.history Html5History]))
351+
(defn bar [] Html5History)]))
352+
"return goog.history.Html5History;")))
353+
(is (true? (str/includes?
354+
(env/with-compiler-env (env/default-compiler-env)
355+
(compile-form-seq
356+
'[(ns test.foo
357+
(:import [goog.history Html5History]))
358+
(def hist (Html5History.))]))
359+
"(new goog.history.Html5History());"))))
360+
334361
;; CLJS-1225
335362

336363
(comment

src/test/clojure/cljs/externs_infer_tests.clj

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -383,10 +383,7 @@
383383
:warnings ws
384384
:warn true
385385
:with-core? true}))]
386-
(is (empty? @ws))
387-
;; TODO:
388-
;; test that the ctor returns the expected type in a let
389-
))
386+
(is (empty? @ws))))
390387

391388
(comment
392389
(binding [ana/*cljs-ns* ana/*cljs-ns*]

src/test/clojure/cljs/type_inference_tests.clj

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -357,9 +357,20 @@
357357
)
358358

359359
(deftest test-ctor-infer
360-
(is (= (env/with-compiler-env test-cenv
361-
(ana/no-warn
362-
(:tag (analyze test-cenv
363-
'(let [g (Foo.)]
364-
q)))))
365-
'cljs.core/Foo)))
360+
(is (= 'cljs.core/Foo
361+
(:tag
362+
(env/with-compiler-env test-cenv
363+
(ana/no-warn
364+
(analyze test-env
365+
'(let [g (Foo.)]
366+
g))))))))
367+
368+
(deftest test-goog-import-ctor-infer
369+
(is (= 'goog.history/Html5History
370+
(:tag
371+
(env/with-compiler-env (env/default-compiler-env)
372+
(ana/analyze-form-seq
373+
'[(ns test.foo
374+
(:import [goog.history Html5History]))
375+
(Html5History.)]
376+
{} true))))))

0 commit comments

Comments
 (0)