Make the :suspicious-test linter omittable

Supports a specific `clojure.test/are` pattern.

cases/testcases/are_true/green.clj

@@ -0,0 +1,25 @@
+(ns testcases.are-true.green
+  (:require
+   [clojure.test :refer [are deftest is join-fixtures testing use-fixtures]]))
+
+(deftest foo
+  (are [a b expected] (testing [a b]
+                        (is (= expected
+                               (+ a b)))
+                        true)
+    1 2 3))
+
+(deftest bar
+  (are [a b expected] (do
+                        (is (= expected
+                               (+ a b)))
+                        true)
+    1 2 3))
+
+(deftest baz
+  (are [a b expected] (let [x [a b]]
+                        (is (= expected
+                               (+ a b))
+                            (pr-str x))
+                        true)
+    1 2 3))

cases/testcases/are_true/red_one.clj

@@ -0,0 +1,7 @@
+(ns testcases.are-true.red-one
+  (:require
+   [clojure.test :refer [are deftest is join-fixtures testing use-fixtures]]))
+
+(deftest foo
+  (are [a b expected] true
+    1 2 3))

cases/testcases/are_true/red_three.clj

@@ -0,0 +1,26 @@
+(ns testcases.are-true.red-three
+  "Uses `::true` instead of `true` (the `:qualifier` value) so this corpus will fail Eastwood."
+  (:require
+   [clojure.test :refer [are deftest is join-fixtures testing use-fixtures]]))
+
+(deftest foo
+  (are [a b expected] (testing [a b]
+                        (is (= expected
+                               (+ a b)))
+                        ::true)
+    1 2 3))
+
+(deftest bar
+  (are [a b expected] (do
+                        (is (= expected
+                               (+ a b)))
+                        ::true)
+    1 2 3))
+
+(deftest baz
+  (are [a b expected] (let [x [a b]]
+                        (is (= expected
+                               (+ a b))
+                            (pr-str x))
+                        ::true)
+    1 2 3))

cases/testcases/are_true/red_two.clj

@@ -0,0 +1,9 @@
+(ns testcases.are-true.red-two
+  (:require
+   [clojure.test :refer [are deftest is join-fixtures testing use-fixtures]]))
+
+(deftest foo
+  (are [a b expected] (do
+                        (is true)
+                        true)
+    1 2 3))

changes.md

@@ -6,6 +6,8 @@
 
 * Now the `:wrong-tag` linter can also be configured via the [`disable-warning`](https://github.com/jonase/eastwood#eastwood-config-files) mechanism.
   * Related: the `disable-warnings` that Eastwood ships by default now prevent false positives against the [speced.def](https://github.com/nedap/speced.def) lib. 
+* Now the `:suspicious-test` linter can also be configured via the [`disable-warning`](https://github.com/jonase/eastwood#eastwood-config-files) mechanism.
+  * Relatedly, a certain pattern of usage of the `clojure.test/are` macro now does not trigger a linter fault. 
 
 #### Bugfixes
 

resource/eastwood/config/clojure.clj

@@ -68,3 +68,11 @@
    :function-symbol 'clojure.core/eduction
    :arglists-for-linting '([& xform])
    :reason "eduction takes a sequence of transducer with a collection as the last item"})
+
+(disable-warning
+ {:linter :suspicious-test
+  :if-inside-macroexpansion-of #{'clojure.test/are}
+  ;; only omit the warning if :suspicious-test failed due to a `true` value:
+  :qualifier true
+  :within-depth 9
+  :reason "Support a specific pattern that tends to fail better."})

src/eastwood/linters/misc.clj

@@ -257,7 +257,7 @@
 ;;                (println (format "  :op=%s" (:op lca-ast)))
 ;;                (println (format "  :form=%s" (:form lca-ast)))
 ;;                )
-            match (some #(util/meets-suppress-condition lca-ast encl-macros %)
+            match (some #(util/meets-suppress-condition lca-ast encl-macros :eastwood/unset %)
                         suppress-conditions)]
         ;; (if (and match (:debug-suppression opt))
         ;;   ((util/make-msg-cb :debug opt)

src/eastwood/linters/typos.clj

@@ -244,49 +244,55 @@
    ;;(println "Reading var-info.edn for :suspicious-test linter")
     (edn/read-string (slurp (io/resource "var-info.edn")))))
 
-(defn predicate-forms [subexpr-maps form-type]
-  (let [var-info-map @var-info-map-delayed]
-    (apply
-     concat
-     (for [{:keys [subexpr ast]} subexpr-maps
-           :let [f subexpr]]
-       (cond
-         (and (not (list? f))
-              (constant-expr? f))
-         [(let [meta-loc (-> f meta)
-                loc (or (pass/has-code-loc? meta-loc)
-                        (pass/code-loc (pass/nearest-ast-with-loc ast)))]
-            {:loc loc :linter :suspicious-test,
-             :msg (format "Found constant form%s with class %s inside %s.  Did you intend to compare its value to something else inside of an 'is' expresssion?"
-                          (cond (-> meta-loc :line) ""
-                                (string? f) (str " \"" f "\"")
-                                :else (str " " f))
-                          (if f (.getName (class f)) "nil") form-type)})]
-
-         (sequential? f)
-         (let [ff (first f)
-               cc-sym (and ff
-                           (instance? clojure.lang.Named ff)
-                           (symbol "clojure.core" (name ff)))
-               var-info (and cc-sym (var-info-map cc-sym))
+(defn predicate-forms [opt subexpr-maps form-type]
+  (let [var-info-map @var-info-map-delayed
+        result (for [{:keys [subexpr ast]} subexpr-maps
+                     :let [f subexpr]]
+                 (cond
+                   (and (not (list? f))
+                        (constant-expr? f))
+                   (let [meta-loc (-> f meta)
+                         loc (or (pass/has-code-loc? meta-loc)
+                                 (pass/code-loc (pass/nearest-ast-with-loc ast)))
+                         warning {:loc loc
+                                  :linter :suspicious-test
+                                  :suspicious-test {:ast ast}
+                                  :qualifier f
+                                  :msg (format "Found constant form%s with class %s inside %s.  Did you intend to compare its value to something else inside of an 'is' expresssion?"
+                                               (cond (-> meta-loc :line) ""
+                                                     (string? f) (str " \"" f "\"")
+                                                     :else (str " " f))
+                                               (if f (.getName (class f)) "nil") form-type)}]
+                     (when (util/allow-warning warning opt)
+                       [warning]))
+
+                   (sequential? f)
+                   (let [ff (first f)
+                         cc-sym (and ff
+                                     (instance? clojure.lang.Named ff)
+                                     (symbol "clojure.core" (name ff)))
+                         var-info (and cc-sym (var-info-map cc-sym))
 ;;              _ (println (format "dbx: predicate-forms ff=%s cc-sym=%s var-info=%s"
 ;;                                 ff cc-sym var-info))
-               loc (-> ff meta)]
-           (cond
-             (and var-info (get var-info :predicate))
-             [{:loc loc
-               :linter :suspicious-test,
-               :msg (format "Found (%s ...) form inside %s.  Did you forget to wrap it in 'is', e.g. (is (%s ...))?"
-                            ff form-type ff)}]
-
-             (and var-info (get var-info :pure-fn))
-             [{:loc loc
-               :linter :suspicious-test,
-               :msg (format "Found (%s ...) form inside %s.  This is a pure function with no side effects, and its return value is unused.  Did you intend to compare its return value to something else inside of an 'is' expression?"
-                            ff form-type)}]
-
-             :else nil))
-         :else nil)))))
+                         loc (-> ff meta)]
+                     (cond
+                       (and var-info (get var-info :predicate))
+                       [{:loc loc
+                         :linter :suspicious-test,
+                         :msg (format "Found (%s ...) form inside %s.  Did you forget to wrap it in 'is', e.g. (is (%s ...))?"
+                                      ff form-type ff)}]
+
+                       (and var-info (get var-info :pure-fn))
+                       [{:loc loc
+                         :linter :suspicious-test,
+                         :msg (format "Found (%s ...) form inside %s.  This is a pure function with no side effects, and its return value is unused.  Did you intend to compare its return value to something else inside of an 'is' expression?"
+                                      ff form-type)}]
+
+                       :else nil))
+                   :else nil))]
+    (->> result
+         (keep identity)
+         (apply concat))))
 
 ;; suspicious-test used to do its job only examining source forms, but
 ;; now it goes through the forms on the :raw-forms lists of the AST
@@ -346,8 +352,8 @@
 
         pr-is-formasts pr-first-is-formasts]
     (concat (suspicious-is-forms pr-is-formasts)
-            (predicate-forms pr-deftest-subexprs 'deftest)
-            (predicate-forms pr-testing-subexprs 'testing))))
+            (predicate-forms opt pr-deftest-subexprs 'deftest)
+            (predicate-forms opt pr-testing-subexprs 'testing))))
 
 ;; Suspicious macro invocations.  Any macros in clojure.core that can
 ;; have 'trivial' expansions are included here, if it can be

src/eastwood/util.clj

@@ -885,7 +885,7 @@ StringWriter."
   (reduce (fn [configs {:keys [linter] :as m}]
             (case linter
               (:constant-test :redefd-vars :unused-ret-vals
-                              :wrong-tag
+                              :wrong-tag :suspicious-test
                               :unused-ret-vals-in-try)
               (update-in configs [linter]
                          conj (dissoc m :linter))
@@ -920,17 +920,19 @@ StringWriter."
           (pst e nil))))
     (process-configs @warning-enable-config-atom)))
 
-(defn meets-suppress-condition [ast enclosing-macros condition]
-  (let [macro-set (:if-inside-macroexpansion-of condition)
-        depth (:within-depth condition)
-        enclosing-macros (if (number? depth)
-                           (take depth enclosing-macros)
-                           enclosing-macros)]
-    (some (fn [m]
-            (when (macro-set (:macro m))
-              {:matching-condition condition
-               :matching-macro (:macro m)}))
-          enclosing-macros)))
+(defn meets-suppress-condition [ast enclosing-macros qualifier condition]
+  (when (or (= qualifier :eastwood/unset) ;; `nil` can be a qualifier value, so :eastwood/unset conveys an absent value
+            (= qualifier (:qualifier condition)))
+    (let [macro-set (:if-inside-macroexpansion-of condition)
+          depth (:within-depth condition)
+          enclosing-macros (if (number? depth)
+                             (take depth enclosing-macros)
+                             enclosing-macros)]
+      (some (fn [m]
+              (when (macro-set (:macro m))
+                {:matching-condition condition
+                 :matching-macro (:macro m)}))
+            enclosing-macros))))
 
 (defn allow-warning-based-on-enclosing-macros [w linter suppress-desc
                                                suppress-conditions opt]
@@ -942,7 +944,10 @@ StringWriter."
         ;; no suppress-conditions to check, to save time.
         encl-macros (when (seq suppress-conditions)
                       (enclosing-macros ast))
-        match (some #(meets-suppress-condition ast encl-macros %)
+        match (some #(meets-suppress-condition ast
+                                               encl-macros
+                                               (:qualifier w :eastwood/unset)
+                                               %)
                     suppress-conditions)]
     (when (and match (:debug-suppression opt))
       ((make-msg-cb :debug opt)
@@ -979,7 +984,7 @@ StringWriter."
            w linter (format " for invocation of macro '%s'" macro-symbol)
            suppress-conditions opt)))
 
-      (:unused-ret-vals :unused-ret-vals-in-try :constant-test :wrong-tag)
+      (:unused-ret-vals :unused-ret-vals-in-try :constant-test :wrong-tag :suspicious-test)
       (let [suppress-conditions (get-in opt [:warning-enable-config linter])]
         (allow-warning-based-on-enclosing-macros
          w linter "" suppress-conditions opt)))))

test/eastwood/analyze_ns_test.clj

@@ -8,8 +8,9 @@
     (are [f input expected] (let [result (f input)]
                               (is (= result input)
                                   "Applying `f` should return an equivalent object (even after removing metadata)")
-                              (= expected
-                                 (-> result second meta :const)))
+                              (is (= expected
+                                     (-> result second meta :const)))
+                              true)
       identity    '(defn ^:const foo) true
       sut/cleanup '(defn ^:const foo) nil
       sut/cleanup '(defn foo)         nil)))

test/eastwood/lint_test.clj

@@ -84,11 +84,12 @@
   (let [valid-namespaces   (take 1 eastwood-src-namespaces)
         invalid-namespaces '[invalid.syntax]]
     (are [namespaces rethrow-exceptions? ok?] (testing [namespaces rethrow-exceptions?]
-                                                (try
-                                                  (eastwood.lint/eastwood (assoc eastwood.lint/default-opts :namespaces namespaces :rethrow-exceptions? rethrow-exceptions?))
-                                                  ok?
-                                                  (catch Exception _
-                                                    (not ok?))))
+                                                (is (try
+                                                      (eastwood.lint/eastwood (assoc eastwood.lint/default-opts :namespaces namespaces :rethrow-exceptions? rethrow-exceptions?))
+                                                      ok?
+                                                      (catch Exception _
+                                                        (not ok?))))
+                                                true)
       []                 false true
       []                 true  true
       invalid-namespaces false true
@@ -102,12 +103,14 @@
            (eastwood.lint/eastwood (assoc eastwood.lint/default-opts :namespaces #{'testcases.large-defprotocol}))))))
 
 (deftest ignore-fault?-test
-  (are [input expected] (= expected
-                           (sut/ignore-fault? input
-                                              {:warn-data {:namespace-sym 'some-ns
-                                                           :line 1
-                                                           :column 2
-                                                           :linter :some-linter}}))
+  (are [input expected] (testing input
+                          (is (= expected
+                                 (sut/ignore-fault? input
+                                                    {:warn-data {:namespace-sym 'some-ns
+                                                                 :line 1
+                                                                 :column 2
+                                                                 :linter :some-linter}})))
+                          true)
     nil                                                                        false
     {}                                                                         false
     {:some-linter {'some-ns true}}                                             true
@@ -180,3 +183,23 @@ relative to a specific macroexpansion"
       {}                                                          {:some-warnings true}
       {:builtin-config-files ["disable_wrong_tag.clj"]}           {:some-warnings false}
       {:builtin-config-files ["disable_wrong_tag_unrelated.clj"]} {:some-warnings true})))
+
+(deftest are-true-test
+  (are [desc input expected] (testing input
+                               (is (= expected
+                                      (-> eastwood.lint/default-opts
+                                          (assoc :namespaces input)
+                                          (eastwood.lint/eastwood)))
+                                   desc)
+                               true)
+    "Supports the \"true at tail position\" pattern for `are`"
+    #{'testcases.are-true.green}     {:some-warnings false}
+
+    "does not ruin `(is true)` detection"
+    #{'testcases.are-true.red-one}   {:some-warnings true}
+
+    "does not ruin an edge case"
+    #{'testcases.are-true.red-two}   {:some-warnings true}
+
+    "only `true` is deemed an apt :qualifier value"
+    #{'testcases.are-true.red-three} {:some-warnings true}))

test/eastwood/linter_executor_test.clj

@@ -6,15 +6,15 @@
 (def proof (atom []))
 
 (deftest works
-  (are [desc input pred] (testing [desc input]
+  (are [desc input pred] (testing input
                            (reset! proof [])
                            (with-out-str
                              (eastwood.lint/eastwood {:namespaces ['eastwood.test.linter-executor.sample-ns]
                                                       :builtin-config-files input}))
-                           (is (pred @proof))
+                           (is (pred @proof)
+                               desc)
                            ;; Avoid duplicate failure reports, did the tests fail:
-                           #_true ;; temporarily disabled because of a false positive
-                           )
+                           true)
     "The custom `set-linter-executor!` successfully runs"
     ["linter_executor.clj"]
     seq