Skip to content

CLJS-3438: Inference for goog.object/containsKey returns any, not boolean #262

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/cljs/cljs/core.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -12180,7 +12180,7 @@ reduces them without incurring seq initialization"
(let [k (munge (str_ sym))]
;; FIXME: this shouldn't need ^boolean due to GCL library analysis,
;; but not currently working
(when ^boolean (gobject/containsKey obj k)
(when (gobject/containsKey obj k)
(let [var-sym (symbol (str_ name) (str_ sym))
var-meta {:ns this}]
(Var. (ns-lookup obj k) var-sym var-meta)))))
Expand Down
23 changes: 16 additions & 7 deletions src/main/clojure/cljs/analyzer.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -1214,9 +1214,12 @@

(defmethod resolve* :goog-module
[env sym full-ns current-ns]
{:name (symbol (str current-ns) (str (munge-goog-module-lib full-ns) "." (name sym)))
:ns current-ns
:op :var})
(let [sym-ast (gets @env/*compiler* ::namespaces full-ns :defs (symbol (name sym)))]
(merge sym-ast
{:name (symbol (str current-ns) (str (munge-goog-module-lib full-ns) "." (name sym)))
:ns current-ns
:op :var
:unaliased-name (symbol (str full-ns) (name sym))})))

(defmethod resolve* :global
[env sym full-ns current-ns]
Expand Down Expand Up @@ -3887,15 +3890,15 @@
bind-args? (and HO-invoke?
(not (all-values? args)))]
(when ^boolean fn-var?
(let [{^boolean variadic :variadic? :keys [max-fixed-arity method-params name ns macro]} (:info fexpr)]
;; don't warn about invalid arity when when compiling a macros namespace
(let [{^boolean variadic :variadic? :keys [max-fixed-arity method-params name unaliased-name ns macro]} (:info fexpr)]
;; don't warn about invalid arity when compiling a macros namespace
;; that requires itself, as that code is not meant to be executed in the
;; `$macros` ns - António Monteiro
(when (and #?(:cljs (not (and (gstring/endsWith (str cur-ns) "$macros")
(symbol-identical? cur-ns ns)
(true? macro))))
(invalid-arity? argc method-params variadic max-fixed-arity))
(warning :fn-arity env {:name name :argc argc}))))
(warning :fn-arity env {:name (or unaliased-name name) :argc argc}))))
(when (and kw? (not (or (== 1 argc) (== 2 argc))))
(warning :fn-arity env {:name (first form) :argc argc}))
(let [deprecated? (-> fexpr :info :deprecated)
Expand Down Expand Up @@ -3946,14 +3949,20 @@
{:op :host-field
:env (:env expr)
:form (list '. prefix field)
:target (desugar-dotted-expr (-> expr
;; goog.module vars get converted to the form of
;; current.ns/goog$module.theDef, we need to dissoc
;; actual extern var info so we get something well-formed
:target (desugar-dotted-expr (-> (dissoc expr :info)
(assoc :name prefix
:form prefix)
(dissoc :tag)
(assoc-in [:info :name] prefix)
(assoc-in [:env :context] :expr)))
:field field
:tag (:tag expr)
;; in the case of goog.module var if there is :info,
;; we need to adopt it now as this is where :ret-tag info lives
:info (:info expr)
:children [:target]})
expr)
;:var
Expand Down
37 changes: 24 additions & 13 deletions src/main/clojure/cljs/externs.clj
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
[clojure.string :as string])
(:import [com.google.javascript.jscomp
CompilerOptions CompilerOptions$Environment SourceFile CompilerInput CommandLineRunner]
[com.google.javascript.jscomp.parsing Config$JsDocParsing]
[com.google.javascript.jscomp.parsing Config$JsDocParsing JsDocInfoParser$ExtendedTypeInfo]
[com.google.javascript.rhino
Node Token JSTypeExpression JSDocInfo$Visibility]
Node Token JSTypeExpression JSDocInfo JSDocInfo$Visibility]
[java.util.logging Level]
[java.net URL]))

Expand Down Expand Up @@ -88,14 +88,13 @@
(some-> (.getRoot texpr) parse-texpr simplify-texpr))

(defn params->method-params [xs]
(letfn [(not-opt? [x]
(not (string/starts-with? (name x) "opt_")))]
(let [required (into [] (take-while not-opt? xs))
opts (drop-while not-opt? xs)]
(loop [ret [required] opts opts]
(if-let [opt (first opts)]
(recur (conj ret (conj (last ret) opt)) (drop 1 opts))
(seq ret))))))
(let [not-opt? (complement :optional?)
required (into [] (map :name (take-while not-opt? xs)))
opts (map :name (drop-while not-opt? xs))]
(loop [ret [required] opts opts]
(if-let [opt (first opts)]
(recur (conj ret (conj (last ret) opt)) (drop 1 opts))
(seq ret)))))

(defn generic? [t]
(let [s (name t)]
Expand All @@ -108,6 +107,18 @@
(= t 'Array) 'array
:else t)))

(defn get-params
"Return param information in JSDoc appearance order. GCL is relatively
civilized, so this isn't really a problem."
[^JSDocInfo info]
(map
(fn [n]
(let [t (.getParameterType info n)]
{:name (symbol n)
:optional? (.isOptionalArg t)
:var-args? (.isVarArgs t)}))
(.getParameterNames info)))

(defn get-var-info [^Node node]
(when node
(let [info (.getJSDocInfo node)]
Expand All @@ -124,15 +135,15 @@
(if (or (.hasReturnType info)
(as-> (.getParameterCount info) c
(and c (pos? c))))
(let [arglist (into [] (map symbol (.getParameterNames info)))
(let [arglist (get-params info)
arglists (params->method-params arglist)]
{:tag 'Function
:js-fn-var true
:ret-tag (or (some-> (.getReturnType info)
get-tag gtype->cljs-type)
'clj-nil)
:variadic? (boolean (some '#{var_args} arglist))
:max-fixed-arity (count (take-while #(not= 'var_args %) arglist))
:variadic? (boolean (some :var-args? arglist))
:max-fixed-arity (count (take-while (complement :var-args?) arglist))
:method-params arglists
:arglists arglists}))))
{:file *source-file*
Expand Down
10 changes: 10 additions & 0 deletions src/test/clojure/cljs/compiler_tests.clj
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,16 @@
(if (gstring/contains "foobar" "foo") true false)]))]
(is (nil? (re-find #"truth_" code))))))

(deftest test-goog-module-infer-cljs-3438
(testing "goog.object/containKey requires correct handling of vars from
goog.module namespace"
(let [code (env/with-compiler-env (env/default-compiler-env)
(compile-form-seq
'[(ns test.foo
(:require [goog.object :as gobject]))
(if (gobject/containsKey nil nil) true false)]))]
(is (nil? (re-find #"truth_" code))))))

;; CLJS-1225

(comment
Expand Down
5 changes: 1 addition & 4 deletions src/test/clojure/cljs/type_inference_tests.clj
Original file line number Diff line number Diff line change
Expand Up @@ -403,14 +403,11 @@
(:require [goog.string :as gstring]))
(gstring/contains "foobar" "foo")]
{} true)))))
;; FIXME: infers any instead of boolean, nothing wrong w/ the externs parsing
;; but this definitely does not work at the moment
#_(is (= 'boolean
(is (= 'boolean
(:tag
(env/with-compiler-env (env/default-compiler-env)
(ana/analyze-form-seq
'[(ns test.foo
(:require [goog.object :as gobject]))
(gobject/containsKey (js-object) "foo")]
{} true))))))