Fix clj-kondo#2288: fix static method analysis and suppressing :java-…

…static-field-call locally (clj-kondo#2290)
tomdl89 · Feb 23, 2024 · ace53da · ace53da
1 parent c1780ac
commit ace53da
Show file tree

Hide file tree

Showing 11 changed files with 170 additions and 10 deletions.
diff --git a/.clj-kondo/borkdude/deflet/borkdude/deflet.clj_kondo b/.clj-kondo/borkdude/deflet/borkdude/deflet.clj_kondo
@@ -0,0 +1,26 @@
+(ns borkdude.deflet
+  (:require [clj-kondo.hooks-api :as hooks-api]))
+
+(defn deflet* [children]
+  (let [f (first children)
+        r (next children)]
+    (if (and (hooks-api/list-node? f)
+             (#{'def 'defp} (hooks-api/sexpr (first (:children f)))))
+      (let [def-children (:children f)]
+        (with-meta (hooks-api/list-node
+                    [(hooks-api/coerce 'clojure.core/let)
+                     (hooks-api/vector-node [(second def-children)
+                                             (nth def-children 2)])
+                     (deflet* r)])
+          (meta f)))
+      (if-not r (or f (hooks-api/coerce nil))
+              (with-meta
+                (hooks-api/list-node (list (hooks-api/coerce 'do)
+                                           f
+                                           (deflet* r)))
+                (meta f))))))
+
+(defn deflet [{:keys [node]}]
+  (let [children (:children node)
+        new-node (deflet* children)]
+    {:node new-node}))
diff --git a/.clj-kondo/borkdude/deflet/config.edn b/.clj-kondo/borkdude/deflet/config.edn
@@ -0,0 +1,3 @@
+{:lint-as {borkdude.deflet/defp clojure.core/def}
+ :hooks {:analyze-call {borkdude.deflet/deflet borkdude.deflet/deflet
+                        borkdude.deflet/defletp borkdude.deflet/deflet}}}
diff --git a/.clj-kondo/config.edn b/.clj-kondo/config.edn
@@ -41,4 +41,5 @@
  :ns-groups [{:pattern ".*-test"
               :name test-namespaces}]
  :config-in-ns {test-namespaces {:linters {:warn-on-reflection {:level :off}}}}
+ :config-paths ["borkdude/deflet"]
  }
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -21,6 +21,7 @@ For a list of breaking changes, check [here](#breaking-changes).
 
 - [#2276](https://github.com/clj-kondo/clj-kondo/issues/2276): New Clojure 1.12 array notation (`String*`) may occur outside of metadata
 - [#2278](https://github.com/clj-kondo/clj-kondo/issues/2278): `bigint` in CLJS is a known symbol in `extend-type`
+- [#2288](https://github.com/clj-kondo/clj-kondo/issues/2288): fix static method analysis and suppressing `:java-static-field-call` locally
 
 ## 2024.02.12
 

diff --git a/deps.edn b/deps.edn
@@ -33,7 +33,8 @@
              borkdude/missing.test.assertions {:mvn/version "0.0.2"}
              babashka/process {:mvn/version "0.5.21"}
              org.clojure/tools.deps.alpha {:mvn/version "0.12.1048"}
-             nubank/matcher-combinators {:mvn/version "3.5.1" :exclusions [midje/midje]}}
+             nubank/matcher-combinators {:mvn/version "3.5.1" :exclusions [midje/midje]}
+             io.github.borkdude/deflet {:mvn/version "0.1.0"}}
             :extra-paths ["test" "extract"]
             :main-opts ["-m" "cognitect.test-runner"]}
            :clojure-1.9.0 {:extra-deps {org.clojure/clojure {:mvn/version "1.9.0"}}}

diff --git a/project.clj b/project.clj
@@ -28,6 +28,7 @@
                                    [org.clojure/tools.deps.alpha "0.12.1048"]
                                    [jonase/eastwood "1.4.0"]
                                    [babashka/process "0.5.21"]
+                                   [io.github.borkdude/deflet "0.1.0"]
                                    [borkdude/missing.test.assertions "0.0.2"]]
                     :source-paths ["src" "parser" "inlined" "extract"]}
              :uberjar {:dependencies [[com.github.clj-easy/graal-build-time "0.1.0"]]

diff --git a/src/clj_kondo/core.clj b/src/clj_kondo/core.clj
@@ -210,7 +210,7 @@
           ;;_ (prn (some-> analysis deref :java-class-usages))
           ;; _ (prn :used-nss @used-nss)
           _ (when analyze-java-class-usages?
-              (swap! analysis assoc :java-class-usages @java-class-usages))
+              (swap! analysis assoc :java-class-usages (mapv #(dissoc % :ctx) @java-class-usages)))
           idacs (when (or dependencies (not skip-lint) analysis)
                   (-> (core-impl/index-defs-and-calls ctx)
                       (overrides)

diff --git a/src/clj_kondo/impl/analysis/java.clj b/src/clj_kondo/impl/analysis/java.clj
@@ -38,12 +38,116 @@
     (io/copy xin xout)
     (.toByteArray xout)))
 
-(defn ^:private opcode->flags []
+#_(defn ^:private opcode->flags []
   {Opcodes/ACC_PUBLIC #{:public}
    Opcodes/ALOAD #{:public :field :static}
    Opcodes/SIPUSH #{:public :field :final}
    Opcodes/LCONST_0 #{:public :method :static}})
 
+(defn- opcode->flags
+  "Thanks @hiredman for https://downey.family/p/2024-02-22/modifiers.clj.html. Generated with:"
+  #_(clojure.pprint/pprint `(fn [~'x] (cond-> #{} ~@(->> (.getFields clojure.asm.Opcodes) (filter #(.startsWith (.getName %) "ACC_")) (map (fn [field] `[(= (bit-and ~'x ~(symbol "clojure.asm.Opcodes" (.getName field))) ~(symbol "clojure.asm.Opcodes" (.getName field))) (conj ~(keyword (-> (.getName field) (.replaceAll "^ACC_" "") (.toLowerCase))))])) (apply concat)))))
+
+  [x]
+  (clojure.core/cond->
+      #{}
+    (clojure.core/=
+     (clojure.core/bit-and x Opcodes/ACC_PUBLIC)
+     Opcodes/ACC_PUBLIC)
+    (clojure.core/conj :public)
+    #_#_(clojure.core/=
+     (clojure.core/bit-and x Opcodes/ACC_PRIVATE)
+     Opcodes/ACC_PRIVATE)
+    (clojure.core/conj :private)
+    #_#_(clojure.core/=
+     (clojure.core/bit-and x Opcodes/ACC_PROTECTED)
+     Opcodes/ACC_PROTECTED)
+    (clojure.core/conj :protected)
+    (clojure.core/=
+     (clojure.core/bit-and x Opcodes/ACC_STATIC)
+     Opcodes/ACC_STATIC)
+    (clojure.core/conj :static)
+    (clojure.core/=
+     (clojure.core/bit-and x Opcodes/ACC_FINAL)
+     Opcodes/ACC_FINAL)
+    (clojure.core/conj :final)
+    #_(clojure.core/=
+     (clojure.core/bit-and x Opcodes/ACC_SUPER)
+     Opcodes/ACC_SUPER)
+    #_(clojure.core/conj :super)
+    #_(clojure.core/=
+     (clojure.core/bit-and x Opcodes/ACC_SYNCHRONIZED)
+     Opcodes/ACC_SYNCHRONIZED)
+    #_(clojure.core/conj :synchronized)
+    #_(clojure.core/=
+     (clojure.core/bit-and x Opcodes/ACC_OPEN)
+     Opcodes/ACC_OPEN)
+    #_(clojure.core/conj :open)
+    #_(clojure.core/=
+     (clojure.core/bit-and x Opcodes/ACC_TRANSITIVE)
+     Opcodes/ACC_TRANSITIVE)
+    #_(clojure.core/conj :transitive)
+    #_(clojure.core/=
+     (clojure.core/bit-and x Opcodes/ACC_VOLATILE)
+     Opcodes/ACC_VOLATILE)
+    #_(clojure.core/conj :volatile)
+    #_#_(clojure.core/=
+     (clojure.core/bit-and x Opcodes/ACC_BRIDGE)
+     Opcodes/ACC_BRIDGE)
+    (clojure.core/conj :bridge)
+    #_#_(clojure.core/=
+     (clojure.core/bit-and x Opcodes/ACC_STATIC_PHASE)
+     Opcodes/ACC_STATIC_PHASE)
+    (clojure.core/conj :static_phase)
+    #_#_(clojure.core/=
+     (clojure.core/bit-and x Opcodes/ACC_VARARGS)
+     Opcodes/ACC_VARARGS)
+    (clojure.core/conj :varargs)
+    #_#_(clojure.core/=
+     (clojure.core/bit-and x Opcodes/ACC_TRANSIENT)
+     Opcodes/ACC_TRANSIENT)
+    (clojure.core/conj :transient)
+    #_#_(clojure.core/=
+     (clojure.core/bit-and x Opcodes/ACC_NATIVE)
+     Opcodes/ACC_NATIVE)
+    (clojure.core/conj :native)
+    #_#_(clojure.core/=
+     (clojure.core/bit-and x Opcodes/ACC_INTERFACE)
+     Opcodes/ACC_INTERFACE)
+    (clojure.core/conj :interface)
+    #_#_(clojure.core/=
+     (clojure.core/bit-and x Opcodes/ACC_ABSTRACT)
+     Opcodes/ACC_ABSTRACT)
+    (clojure.core/conj :abstract)
+    #_#_(clojure.core/=
+     (clojure.core/bit-and x Opcodes/ACC_STRICT)
+     Opcodes/ACC_STRICT)
+    (clojure.core/conj :strict)
+    #_#_(clojure.core/=
+     (clojure.core/bit-and x Opcodes/ACC_SYNTHETIC)
+     Opcodes/ACC_SYNTHETIC)
+    (clojure.core/conj :synthetic)
+    #_#_(clojure.core/=
+     (clojure.core/bit-and x Opcodes/ACC_ANNOTATION)
+     Opcodes/ACC_ANNOTATION)
+    (clojure.core/conj :annotation)
+    #_#_(clojure.core/=
+     (clojure.core/bit-and x Opcodes/ACC_ENUM)
+     Opcodes/ACC_ENUM)
+    (clojure.core/conj :enum)
+    #_#_(clojure.core/=
+     (clojure.core/bit-and x Opcodes/ACC_MANDATED)
+     Opcodes/ACC_MANDATED)
+    (clojure.core/conj :mandated)
+    #_#_(clojure.core/=
+     (clojure.core/bit-and x Opcodes/ACC_MODULE)
+     Opcodes/ACC_MODULE)
+    (clojure.core/conj :module)
+    #_#_(clojure.core/=
+     (clojure.core/bit-and x Opcodes/ACC_DEPRECATED)
+     Opcodes/ACC_DEPRECATED)
+    (clojure.core/conj :deprecated)))
+
 (defn ^:private modifier-keyword->flag []
   (reduce #(assoc %1 %2 (keyword (str/lower-case (.asString ^Modifier$Keyword %2))))
           {}
@@ -54,8 +158,7 @@
   [^InputStream class-is]
   (let [class-reader (ClassReader. (input-stream->bytes class-is))
         class-name (str/replace (.getClassName class-reader) "/" ".")
-        result* (atom {class-name {:members []}})
-        opcode->flags (opcode->flags)]
+        result* (atom {class-name {:members []}})]
     (.accept
      class-reader
      (proxy [ClassVisitor] [Opcodes/ASM9]
@@ -182,7 +285,8 @@
               (merge {:class class-name
                       :uri (:uri ctx)
                       :filename (:filename ctx)
-                      :call (:call opts)}
+                      :call (:call opts)
+                      :ctx ctx}
                      loc+data
                      (when method-name
                        {:method-name method-name})

diff --git a/src/clj_kondo/impl/linters.clj b/src/clj_kondo/impl/linters.clj
@@ -838,15 +838,16 @@
 
             :when (:call usage)]
       (let [method (:method-name usage)
-            clazz (:class usage)]
+            clazz (:class usage)
+            ctx (or (:ctx usage) ctx)]
         (when-let [info (get jm clazz)]
           ;; (prn :info info)
           (when-let [meth-info (get (:members info) method)]
             (when (and (contains? (:flags meth-info) :field)
                        (:call usage))
               (findings/reg-finding!
                ctx
-               (assoc usage
+               (assoc (dissoc usage :ctx)
                       :type :java-static-field-call
                       :message "Static fields should be referenced without parens unless they are intended as function calls")))))))))
 

diff --git a/test/clj_kondo/analysis/java_test.clj b/test/clj_kondo/analysis/java_test.clj
@@ -1,6 +1,7 @@
 (ns clj-kondo.analysis.java-test
   (:require
    [babashka.process :as p]
+   [borkdude.deflet :as deflet]
    [clj-kondo.core :as clj-kondo]
    [clj-kondo.impl.utils :refer [err]]
    [clj-kondo.test-utils :as tu :refer [assert-submap2 assert-submaps2]]
@@ -87,7 +88,7 @@
        {:class "foo.bar.AwesomeClass",
         :uri #"file:.*/corpus/java/classes/foo/bar/AwesomeClass.class"
         :name "bar3"
-        :flags #{:public :field :static}
+        :flags #{:public :field :static :final}
         :type "java.lang.Double"}
        {:class "foo.bar.AwesomeClass",
         :uri #"file:.*/corpus/java/classes/foo/bar/AwesomeClass.class"
@@ -244,6 +245,22 @@
        :row 16}]
      java-class-usages)))
 
+(deftest issue-2288-test
+  (deflet/deflet
+    (def deps '{:deps {com.google.cloud/google-cloud-vision {:mvn/version "3.32.0"}}
+                :mvn/repos {"central" {:url "https://repo1.maven.org/maven2/"}
+                            "clojars" {:url "https://repo.clojars.org/"}}})
+    (def jar (-> (deps/resolve-deps deps nil)
+                 (get-in ['com.google.cloud/google-cloud-vision :paths 0])))
+
+    (def ana (analyze [jar]))
+    (def create-meth (some #(when (and (= (:class %) "com.google.cloud.vision.v1.ImageAnnotatorClient")
+                                       (= "create" (:name %)))
+                         %) (:java-member-definitions ana)))
+    (assert-submaps2
+     #{:method :public :static :final}
+     (:flags create-meth))))
+
 (comment
 
   #_(assert-submap {:filename #"\.class"} {:filename "/Users/borkdude/.m2/repository/org/clojure/clojure/1.10.3/clojure-1.10.3.jar:clojure/lang/RT.class"})

diff --git a/test/clj_kondo/main_test.clj b/test/clj_kondo/main_test.clj
@@ -3496,7 +3496,12 @@ foo/")))
        '({:file "corpus/static_field_call.clj", :row 4, :col 1, :level :error, :message "Static fields should be referenced without parens unless they are intended as function calls"}
          {:file "corpus/static_field_call.clj", :row 7, :col 1, :level :error, :message "Static fields should be referenced without parens unless they are intended as function calls"}
          {:file "corpus/static_field_call.clj", :row 8, :col 1, :level :error, :message "Static fields should be referenced without parens unless they are intended as function calls"})
-       (lint! (io/file "corpus" "static_field_call.clj")))))
+       (lint! (io/file "corpus" "static_field_call.clj"))))
+  (is (empty? (lint! "#_:clj-kondo/ignore (System/err)")))
+  (is (empty? (lint! "(ns foo) (System/err)"
+                     '{:config-in-ns {foo {:linters {:java-static-field-call {:level :off}}}}})))
+  (is (empty? (lint! "(ns foo) (defmacro foo [x] x) (foo (System/err))"
+                     '{:config-in-call {foo/foo {:linters {:java-static-field-call {:level :off}}}}}))))
 
 (deftest lint-without-kondo-dir
   (let [user-dir (System/getProperty "user.dir")]