From aec61c18a71981a3fcc821129207522713b16b81 Mon Sep 17 00:00:00 2001 From: davidnolen Date: Sun, 6 Jul 2025 18:10:15 -0400 Subject: [PATCH 1/5] CLJS-3438: Inference for `goog.object/containsKey` returns any, not boolean - fix desugar-dotted-expr, generated malformed AST in the case of goog.module var - compiler test for goog.object/containsKey - uncomment test now passes ... but compiler-tests are failing due to warning about incorrect usage of goog.object/get, but that's because we don't parse options params yet also need to resolve to the original goog.module var name and not the local thing in the namespace --- src/main/clojure/cljs/analyzer.cljc | 16 ++++++++++++---- src/test/clojure/cljs/compiler_tests.clj | 10 ++++++++++ src/test/clojure/cljs/type_inference_tests.clj | 5 +---- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/main/clojure/cljs/analyzer.cljc b/src/main/clojure/cljs/analyzer.cljc index ac521fa78..9ba540079 100644 --- a/src/main/clojure/cljs/analyzer.cljc +++ b/src/main/clojure/cljs/analyzer.cljc @@ -1214,9 +1214,11 @@ (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}))) (defmethod resolve* :global [env sym full-ns current-ns] @@ -3946,7 +3948,10 @@ {: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) @@ -3954,6 +3959,9 @@ (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 diff --git a/src/test/clojure/cljs/compiler_tests.clj b/src/test/clojure/cljs/compiler_tests.clj index 95204e650..f6f7b560b 100644 --- a/src/test/clojure/cljs/compiler_tests.clj +++ b/src/test/clojure/cljs/compiler_tests.clj @@ -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 diff --git a/src/test/clojure/cljs/type_inference_tests.clj b/src/test/clojure/cljs/type_inference_tests.clj index 5435cc90f..2bd0855c3 100644 --- a/src/test/clojure/cljs/type_inference_tests.clj +++ b/src/test/clojure/cljs/type_inference_tests.clj @@ -403,9 +403,7 @@ (: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 @@ -413,4 +411,3 @@ (:require [goog.object :as gobject])) (gobject/containsKey (js-object) "foo")] {} true)))))) - From a4f3ee658b2c4308735eec8b9334264ed2f65fb3 Mon Sep 17 00:00:00 2001 From: davidnolen Date: Mon, 7 Jul 2025 09:13:40 -0400 Subject: [PATCH 2/5] add data oriented helper for getting parameter information --- src/main/clojure/cljs/externs.clj | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/main/clojure/cljs/externs.clj b/src/main/clojure/cljs/externs.clj index d25987cde..fd0bb2d3a 100644 --- a/src/main/clojure/cljs/externs.clj +++ b/src/main/clojure/cljs/externs.clj @@ -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])) @@ -108,6 +108,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)] From b42550d614d447aa19c383539c2443a8115c6356 Mon Sep 17 00:00:00 2001 From: David Nolen Date: Mon, 7 Jul 2025 12:38:10 -0400 Subject: [PATCH 3/5] - fix externs parsing parameter handling --- src/main/clojure/cljs/externs.clj | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/main/clojure/cljs/externs.clj b/src/main/clojure/cljs/externs.clj index fd0bb2d3a..e7bf3014b 100644 --- a/src/main/clojure/cljs/externs.clj +++ b/src/main/clojure/cljs/externs.clj @@ -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)] @@ -136,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* From f94705827667ebc93e0d87eab1149ca55b10cb6d Mon Sep 17 00:00:00 2001 From: David Nolen Date: Mon, 7 Jul 2025 13:42:19 -0400 Subject: [PATCH 4/5] - preserve the :unaliased-name when resolving goog.module vars - check for the unaliased-name when warning about arity in parse-invoke* --- src/main/clojure/cljs/analyzer.cljc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/main/clojure/cljs/analyzer.cljc b/src/main/clojure/cljs/analyzer.cljc index 9ba540079..709531e59 100644 --- a/src/main/clojure/cljs/analyzer.cljc +++ b/src/main/clojure/cljs/analyzer.cljc @@ -1218,7 +1218,8 @@ (merge sym-ast {:name (symbol (str current-ns) (str (munge-goog-module-lib full-ns) "." (name sym))) :ns current-ns - :op :var}))) + :op :var + :unaliased-name (symbol (str full-ns) (name sym))}))) (defmethod resolve* :global [env sym full-ns current-ns] @@ -3889,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) From 053aa5e09aae24c602ee174f233a846a49c193bf Mon Sep 17 00:00:00 2001 From: David Nolen Date: Mon, 7 Jul 2025 13:44:40 -0400 Subject: [PATCH 5/5] - goog.object/containsKey hint no longer needed --- src/main/cljs/cljs/core.cljs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/cljs/cljs/core.cljs b/src/main/cljs/cljs/core.cljs index e1c7b067e..efbde48e3 100644 --- a/src/main/cljs/cljs/core.cljs +++ b/src/main/cljs/cljs/core.cljs @@ -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)))))