Skip to content

Commit

Permalink
Enhance wrong-arity linter to catch wrong number of args in new case
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@jafingerhut
jafingerhut committed Dec 12, 2014
1 parent 07b9475 commit d803a17
Show file tree
Hide file tree
Showing 4 changed files with 169 additions and 78 deletions.
34 changes: 34 additions & 0 deletions cases/testcases/f01.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
37 changes: 37 additions & 0 deletions resource/config.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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."})
81 changes: 43 additions & 38 deletions src/eastwood/linters/misc.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]))
Expand Down Expand Up @@ -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))))

Expand Down
Loading

0 comments on commit d803a17

Please sign in to comment.