From d803a17d30ba81a4324fe459d7a0f694034b1eb3 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Fri, 12 Dec 2014 12:41:13 -0800 Subject: [PATCH] Enhance wrong-arity linter to catch wrong number of args in new case Currently tools.analyzer.jvm always uses the :arglists metadata to determine whether to mark and AST with a :maybe-wrong-arity key. Thus if someone were to override the :arglists metadata on a function in such a way that a call with a wrong number of args appeared to have a correct number of args, Eastwood would never have warned about it. Now Eastwood ignores the :maybe-wrong-arity key in the ASTs, and does its own independent wrong-arity checking, based upon :arglists provided in its configs if they are there, otherwise the :arglists metadata on the function. Also added a couple more configs to disable unwanted wrong-arity warnings in crucible projects. --- cases/testcases/f01.clj | 34 +++++++++++ resource/config.clj | 37 +++++++++++ src/eastwood/linters/misc.clj | 81 ++++++++++++------------ test/eastwood/test/linters_test.clj | 95 +++++++++++++++++------------ 4 files changed, 169 insertions(+), 78 deletions(-) diff --git a/cases/testcases/f01.clj b/cases/testcases/f01.clj index 3eb5ecc1..98a4feb0 100644 --- a/cases/testcases/f01.clj +++ b/cases/testcases/f01.clj @@ -111,3 +111,37 @@ Tellman is to be credited/blamed for this test case." (catch x Throwable y)) ([x y z] [x y z])) + + +(defn fn-with-arglists-meta + {:arglists '([x y])} + [x y z] + (+ x y z)) + + +;; Eastwood can now detect the call to fn-with-arglists-meta below as +;; having the wrong number of arguments, if you add appropriate +;; configuration to override the :arglists metadata given above, which +;; makes it appear that it is called with the correct number of args +;; below. + +;; This is a toy test example that I don't think anyone would create +;; in real development. It is only intended to verify that Eastwood +;; is actually using the configuration correctly. + +(defn wrong-args1 [x] + (fn-with-arglists-meta x (* 2 x))) + + +(defn wrong-args-threw-exception-before-bugfix1 [x] + ((if x + (fn [coll x] (conj coll x)) + (fn [coll x] (disj coll x))) + #{:a :b} :c :d)) + + +(defn wrong-args-threw-exception-before-bugfix2 [x] + ((do + (println "foo") + (fn [coll x] (disj coll x))) + #{:a :b} :c :d)) diff --git a/resource/config.clj b/resource/config.clj index 8d1e2b35..a9184b57 100644 --- a/resource/config.clj +++ b/resource/config.clj @@ -389,3 +389,40 @@ :if-inside-macroexpansion-of #{'schema.core/fn} :within-depth 7 :reason "schema.core/fn macro expansions sometimes contain a loop with an empty body."}) + +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; Just for Eastwood testing +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(disable-warning + {:linter :wrong-arity + :function-symbol 'testcases.f01/fn-with-arglists-meta + :arglists-for-linting + '([x y z]) + :reason "Only for Eastwood testing"}) + + +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; Configs to disable warnings in Midje, version 3.0-alpha4 +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(disable-warning + {:linter :wrong-arity + :function-symbol 'midje.sweet/has-suffix + :arglists-for-linting + '([expected-suffix] + [expected-suffix looseness?]) + :reason "midje.sweet/has-suffix takes an optional extra argument which is implied by ? after looseness, but its :arglists metadta does not have a 1-arg arity, which this config adds."}) + + +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; Configs to disable warnings in congomongo, version 0.4.1 +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(disable-warning + {:linter :wrong-arity + :function-symbol 'somnium.congomongo/insert! + :arglists-for-linting + '([coll obj & {:keys [from to many write-concern] + :or {from :clojure to :clojure many false}}]) + :reason "somnium.congomongo/insert! uses metadata to override the default value of :arglists for documentation purposes. This configuration tells Eastwood what the actual :arglists is, i.e. would have been without that."}) diff --git a/src/eastwood/linters/misc.clj b/src/eastwood/linters/misc.clj index d5067866..4e8bf9dd 100644 --- a/src/eastwood/linters/misc.clj +++ b/src/eastwood/linters/misc.clj @@ -2,7 +2,7 @@ (:require [clojure.string :as string] [clojure.pprint :as pp] [eastwood.copieddeps.dep1.clojure.tools.analyzer.ast :as ast] - [eastwood.copieddeps.dep1.clojure.tools.analyzer.utils :refer [resolve-var]] + [eastwood.copieddeps.dep1.clojure.tools.analyzer.utils :refer [resolve-var arglist-for-arity]] [eastwood.copieddeps.dep1.clojure.tools.analyzer.env :as env] [eastwood.copieddeps.dep2.clojure.tools.analyzer.jvm :as j] [eastwood.util :as util])) @@ -424,57 +424,62 @@ ;; Wrong arity (defn wrong-arity [{:keys [asts]} opt] - (let [asts (->> asts - (mapcat ast/nodes) - (filter :maybe-arity-mismatch))] - (for [ast asts - :let [loc (-> ast :fn :form meta) - call-args (-> ast :args) - num-args-in-call (count call-args) - fn-kind (-> ast :fn :op) - + (let [invoke-asts (->> asts + (mapcat ast/nodes) + (filter #(and (= :invoke (:op %)) + (-> % :fn :arglists))))] + (for [ast invoke-asts + :let [args (:args ast) + func (:fn ast) + fn-kind (-> func :op) [fn-var fn-sym] - (case fn-kind - :var [(-> ast :fn :var) - (util/var-to-fqsym (-> ast :fn :var))] - :local [nil - (-> ast :fn :form)]) - - fn-arglists (-> ast :fn :arglists) - w (util/add-loc-info + (case fn-kind + :var [(-> func :var) + (util/var-to-fqsym (-> func :var))] + :local [nil (-> func :form)] + [nil 'no-name]) + arglists (:arglists func) + override-arglists (-> opt :warning-enable-config + :wrong-arity fn-sym) + lint-arglists (or (-> override-arglists + :arglists-for-linting) + arglists) + loc (-> func :form meta) +;; _ (println (format "fn=%s (count args)=%d lint-arglists=%s ok=%s arglist-for-arity=%s loc=%s" +;; fn-sym (count args) +;; (seq lint-arglists) +;; (arg-count-compatible-with-arglists +;; (count args) lint-arglists) +;; (arglist-for-arity +;; {:arglists lint-arglists} (count args)) +;; (select-keys loc #{:file :line :column}) +;; )) + ] + :when (not (arg-count-compatible-with-arglists (count args) + lint-arglists)) + :let [w (util/add-loc-info loc {:linter :wrong-arity :wrong-arity {:kind :the-only-kind :fn-var fn-var - :call-args call-args} + :call-args args} :msg (format "Function on %s %s called with %s args, but it is only known to take one of the following args: %s" (name fn-kind) - (case fn-kind - :var fn-var - :local fn-sym) - num-args-in-call - (string/join " " fn-arglists))}) - override-arglists (-> opt :warning-enable-config :wrong-arity - fn-sym) - arglists-for-linting (or (-> override-arglists - :arglists-for-linting) - fn-arglists) - good-num-args? (arg-count-compatible-with-arglists - num-args-in-call arglists-for-linting) - allow? (not good-num-args?)] - :when allow?] + (if (= :var fn-kind) fn-var fn-sym) + (count args) + (string/join " " lint-arglists))})]] (do - (util/debug-warning w ast opt #{} + (util/debug-warning w ast opt #{:enclosing-macros} (fn [] (println (format "was generated because of a function call on '%s' with %d args" - fn-sym num-args-in-call)) + fn-sym (count args))) (println "arglists from metadata on function var:") - (pp/pprint fn-arglists) + (pp/pprint arglists) ;; (println (format " argvec-kinds=")) -;; (pp/pprint (map argvec-kind fn-arglists)) +;; (pp/pprint (map argvec-kind arglists)) (when override-arglists (println "arglists overridden by Eastwood config to the following:") - (pp/pprint arglists-for-linting) + (pp/pprint lint-arglists) (println "Reason:" (-> override-arglists :reason))))) w)))) diff --git a/test/eastwood/test/linters_test.clj b/test/eastwood/test/linters_test.clj index 67be33a0..75e6ed6f 100644 --- a/test/eastwood/test/linters_test.clj +++ b/test/eastwood/test/linters_test.clj @@ -140,14 +140,29 @@ the next." 1, {:linter :wrong-arity, :msg "Function on local f1 called with 0 args, but it is only known to take one of the following args: [x]", - :file "testcases/f01.clj", + :file (fname-from-parts "testcases" "f01.clj"), :line 98, :column 6} 1, {:linter :wrong-arity, :msg "Function on local f1 called with 0 args, but it is only known to take one of the following args: [p1__# p2__#]", - :file "testcases/f01.clj", + :file (fname-from-parts "testcases" "f01.clj"), :line 102, :column 6} 1, + {:linter :wrong-arity, + :msg "Function on var #'testcases.f01/fn-with-arglists-meta called with 2 args, but it is only known to take one of the following args: [x y z]", + :file (fname-from-parts "testcases" "f01.clj"), + :line 133, :column 4} + 1, + {:linter :wrong-arity, + :msg "Function on if no-name called with 3 args, but it is only known to take one of the following args: [coll x]", + :file (fname-from-parts "testcases" "f01.clj"), + :line 137, :column 4} + 1, + {:linter :wrong-arity, + :msg "Function on do no-name called with 3 args, but it is only known to take one of the following args: [coll x]", + :file (fname-from-parts "testcases" "f01.clj"), + :line 144, :column 4} + 1, }) ;; TBD: I do not know why the i-am-inner-defonce-sym warning appears @@ -873,229 +888,229 @@ the next." {:linter :suspicious-expression, :msg "-> called with 1 args. (-> x) always returns x. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 25, :column 1} 1, {:linter :suspicious-expression, :msg "->> called with 1 args. (->> x) always returns x. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 26, :column 1} 1, {:linter :suspicious-expression, :msg "and called with 0 args. (and) always returns true. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 27, :column 1} 1, {:linter :suspicious-expression, :msg "and called with 1 args. (and x) always returns x. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 28, :column 1} 1, {:linter :suspicious-expression, :msg "as-> called with 2 args. (as-> expr name) always returns expr. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 29, :column 1} 1, {:linter :suspicious-expression, :msg "case called with 2 args. (case x y) always returns y. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 30, :column 1} 1, {:linter :suspicious-expression, :msg "cond called with 0 args. (cond) always returns nil. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 31, :column 1} 1, {:linter :suspicious-expression, :msg "cond-> called with 1 args. (cond-> x) always returns x. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 32, :column 12} 1, {:linter :suspicious-expression, :msg "cond->> called with 1 args. (cond->> x) always returns x. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 33, :column 12} 1, {:linter :suspicious-expression, :msg "condp called with 3 args. (condp pred test-expr expr) always returns expr. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 34, :column 1} 1, {:linter :suspicious-expression, :msg "declare called with 0 args. (declare) always returns nil. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 35, :column 1} 1, {:linter :suspicious-expression, :msg "delay called with 0 args. (delay) always returns (delay nil). Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 36, :column 1} 1, {:linter :suspicious-expression, :msg "doseq called with 1 args. (doseq [x coll]) always returns nil. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 37, :column 1} 1, {:linter :suspicious-expression, :msg "dotimes called with 1 args. (dotimes [i n]) always returns nil. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 38, :column 1} 1, {:linter :suspicious-expression, :msg "doto called with 1 args. (doto x) always returns x. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 39, :column 1} 1, {:linter :suspicious-expression, :msg "import called with 0 args. (import) always returns nil. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 40, :column 1} 1, {:linter :suspicious-expression, :msg "lazy-cat called with 0 args. (lazy-cat) always returns (). Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 41, :column 1} 1, {:linter :suspicious-expression, :msg "let called with 1 args. (let bindings) always returns nil. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 42, :column 1} 1, {:linter :suspicious-expression, :msg "letfn called with 1 args. (letfn bindings) always returns nil. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 43, :column 1} 1, {:linter :suspicious-expression, :msg "locking called with 1 args. (locking x) always returns nil. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 44, :column 1} 1, {:linter :suspicious-expression, :msg "loop called with 1 args. (loop bindings) always returns nil. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 45, :column 1} 1, {:linter :suspicious-expression, :msg "or called with 0 args. (or) always returns nil. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 46, :column 1} 1, {:linter :suspicious-expression, :msg "or called with 1 args. (or x) always returns x. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 47, :column 1} 1, {:linter :suspicious-expression, :msg "pvalues called with 0 args. (pvalues) always returns (). Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 48, :column 1} 1, {:linter :suspicious-expression, :msg "some-> called with 1 args. (some-> expr) always returns expr. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 49, :column 1} 1, {:linter :suspicious-expression, :msg "some->> called with 1 args. (some->> expr) always returns expr. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 50, :column 1} 1, {:linter :suspicious-expression, :msg "when called with 1 args. (when test) always returns nil. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 51, :column 1} 1, {:linter :suspicious-expression, :msg "when-first called with 1 args. (when-first [x y]) always returns nil. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 52, :column 1} 1, {:linter :suspicious-expression, :msg "when-let called with 1 args. (when-let [x y]) always returns nil. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 53, :column 1} 1, {:linter :suspicious-expression, :msg "when-not called with 1 args. (when-not test) always returns nil. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 54, :column 1} 1, {:linter :suspicious-expression, :msg "when-some called with 1 args. (when-some [x y]) always returns nil. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 55, :column 1} 1, {:linter :suspicious-expression, :msg "with-bindings called with 1 args. (with-bindings map) always returns nil. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 56, :column 1} 1, {:linter :suspicious-expression, :msg "with-in-str called with 1 args. (with-in-str s) always returns nil. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 57, :column 1} 1, {:linter :suspicious-expression, :msg "with-local-vars called with 1 args. (with-local-vars bindings) always returns nil. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 58, :column 1} 1, {:linter :suspicious-expression, :msg "with-open called with 1 args. (with-open bindings) always returns nil. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 59, :column 1} 2, {:linter :suspicious-expression, :msg "with-out-str called with 0 args. (with-out-str) always returns \"\". Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 60, :column 1} 1, {:linter :suspicious-expression, :msg "with-precision called with 1 args. (with-precision precision) always returns nil. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 61, :column 1} 1, {:linter :suspicious-expression, :msg "with-redefs called with 1 args. (with-redefs bindings) always returns nil. Perhaps there are misplaced parentheses?", - :file "testcases/suspicious.clj", + :file (fname-from-parts "testcases" "suspicious.clj"), :line 62, :column 1} 1, })