Upgrade Eastwood (#188)

Author: vemv

.circleci/config.yml

@@ -9,7 +9,7 @@ commands:
             - v2-dependencies-{{ checksum "project.clj" }}
             # fallback to using the latest cache if no exact match is found
             - v2-dependencies-
-      - run: lein with-profile -dev,+ci,+test,+refactor-nrepl deps
+      - run: lein with-profile -user,-dev,+ci,+test,+refactor-nrepl deps
       - save_cache:
           paths:
             - ~/.m2
@@ -70,6 +70,8 @@ jobs:
           command: << parameters.lein_test_command >>
       - run:
           command: .circleci/e2e.sh
+      - run:
+          command: lein with-profile -user,+test eastwood
   deploy:
     executor: openjdk8
     steps:
@@ -79,13 +81,13 @@ jobs:
           command: echo -e "$GPG_KEY_V2" | gpg --import
       - run:
           name: Perform pre-release sanity check
-          command: lein with-profile -dev,+ci,+ncrw run -m nedap.ci.release-workflow.api sanity-check
+          command: lein with-profile -user,-dev,+ci,+ncrw run -m nedap.ci.release-workflow.api sanity-check
       - run:
           name: release to JFrog
-          command: lein deploy
+          command: lein with-profile -user deploy
       - run:
           name: release to Clojars
-          command: lein deploy clojars
+          command: lein with-profile -user deploy clojars
 
 test_code_filters: &test_code_filters
   filters:
@@ -101,57 +103,45 @@ workflows:
       - test_code:
           name: "JDK 8 including refactor-nrepl"
           jdk_version: openjdk8
-          lein_test_command: lein with-profile -dev,+ci,+refactor-nrepl do clean, test
-          <<: *test_code_filters
-      - test_code:
-          name: "JDK 8 including refactor-nrepl, and parallel Eastwood linters"
-          jdk_version: openjdk8-parallel
-          lein_test_command: lein with-profile -dev,+ci,+refactor-nrepl,+parallel-eastwood do clean, test
+          lein_test_command: lein with-profile -user,-dev,+ci,+refactor-nrepl do clean, test
           <<: *test_code_filters
       - test_code:
           name: "JDK 8 including refactor-nrepl (at its latest version)"
           jdk_version: openjdk8
-          lein_test_command: lein with-profile -dev,+ci,+refactor-nrepl-latest do clean, test
+          lein_test_command: lein with-profile -user,-dev,+ci,+refactor-nrepl-latest do clean, test
           <<: *test_code_filters
       - test_code:
           name: "JDK 8 excluding refactor-nrepl"
           jdk_version: openjdk8
-          lein_test_command: lein with-profile -dev,+ci do clean, test
+          lein_test_command: lein with-profile -user,-dev,+ci do clean, test
           <<: *test_code_filters
       - test_code:
           name: "JDK 8, with an old clojurescript dependency on the classpath"
           jdk_version: openjdk8
-          lein_test_command: lein with-profile -dev,-provided,+ci,+cljs-old do clean, test
+          lein_test_command: lein with-profile -user,-dev,-provided,+ci,+cljs-old do clean, test
           <<: *test_code_filters
       - test_code:
           name: "JDK 11 including refactor-nrepl"
           jdk_version: openjdk11
-          lein_test_command: lein with-profile -dev,+ci,+refactor-nrepl do clean, test
-          <<: *test_code_filters
-      - test_code:
-          name: "JDK 11 including refactor-nrepl, and parallel Eastwood linters"
-          jdk_version: openjdk11-parallel
-          lein_test_command: lein with-profile -dev,+ci,+refactor-nrepl,+parallel-eastwood do clean, test
+          lein_test_command: lein with-profile -user,-dev,+ci,+refactor-nrepl do clean, test
           <<: *test_code_filters
       - test_code:
           name: "JDK 11 including refactor-nrepl (at its latest version)"
           jdk_version: openjdk11
-          lein_test_command: lein with-profile -dev,+ci,+refactor-nrepl-latest do clean, test
+          lein_test_command: lein with-profile -user,-dev,+ci,+refactor-nrepl-latest do clean, test
           <<: *test_code_filters
       - test_code:
           name: "JDK 11 excluding refactor-nrepl"
           jdk_version: openjdk11
-          lein_test_command: lein with-profile -dev,+ci do clean, test
+          lein_test_command: lein with-profile -user,-dev,+ci do clean, test
           <<: *test_code_filters
       - deploy:
           context: JFrog
           requires:
             - "JDK 8, with an old clojurescript dependency on the classpath"
-            - "JDK 8 including refactor-nrepl, and parallel Eastwood linters"
             - "JDK 8 including refactor-nrepl"
             - "JDK 8 excluding refactor-nrepl"
             - "JDK 11 including refactor-nrepl"
-            - "JDK 11 including refactor-nrepl, and parallel Eastwood linters"
             - "JDK 11 excluding refactor-nrepl"
           filters:
             branches:

project.clj

@@ -1,3 +1,5 @@
+(def eastwood-version "0.9.9")
+
 ;; Please don't bump the library version by hand - use ci.release-workflow instead.
 (defproject formatting-stack "4.4.0-alpha1"
   ;; Please keep the dependencies sorted a-z.
@@ -10,7 +12,7 @@
                  [com.nedap.staffing-solutions/utils.modular "2.2.0-alpha3"]
                  [com.nedap.staffing-solutions/utils.spec.predicates "1.1.0"]
                  [io.reflectoring.diffparser/diffparser "1.4"]
-                 [jonase/eastwood "0.3.14"]
+                 [jonase/eastwood ~eastwood-version]
                  [medley "1.2.0"]
                  [org.clojure/clojure "1.10.1"]
                  [org.clojure/java.classpath "1.0.0"]
@@ -55,7 +57,8 @@
 
   :monkeypatch-clojure-test false
 
-  :plugins [[lein-pprint "1.1.2"]]
+  :plugins [[lein-pprint "1.1.2"]
+            [jonase/eastwood ~eastwood-version]]
 
   ;; A variety of common dependencies are bundled with `nedap/lein-template`.
   ;; They are divided into two categories:
@@ -102,9 +105,9 @@
                                                     [integrant "0.8.0"]
                                                     [org.clojure/clojurescript "1.7.228"]]}
 
-             :provided              {:dependencies [[org.clojure/clojurescript "1.10.597"]
-                                                    [com.stuartsierra/component "0.4.0"]
-                                                    [integrant "0.8.0"]]
+             :provided              {:dependencies         [[org.clojure/clojurescript "1.10.597"]
+                                                            [com.stuartsierra/component "0.4.0"]
+                                                            [integrant "0.8.0"]]
                                      :managed-dependencies [[com.cognitect/transit-clj "1.0.324"]
                                                             [com.google.code.findbugs/jsr305 "3.0.2"]
                                                             [com.google.errorprone/error_prone_annotations "2.1.3"]
@@ -131,14 +134,12 @@
                                      ;; (and shipped with this refactor-nrepl):
                                      :plugins      [[cider/cider-nrepl "0.24.0"]]}
 
-             :parallel-eastwood     {:jvm-opts ["-Dformatting-stack.eastwood.parallelize-linters=true"]}
-
-             :ncrw                  {:global-vars  {*assert* true} ;; `ci.release-workflow` relies on runtime assertions
+             :ncrw                  {:global-vars    {*assert* true} ;; `ci.release-workflow` relies on runtime assertions
                                      :source-paths   ^:replace []
                                      :test-paths     ^:replace []
                                      :resource-paths ^:replace []
                                      :plugins        ^:replace []
                                      :dependencies   ^:replace [[com.nedap.staffing-solutions/ci.release-workflow "1.12.0"]]}
 
-             :ci                    {:pedantic?    :abort
-                                     :jvm-opts     ["-Dclojure.main.report=stderr"]}})
+             :ci                    {:pedantic? :abort
+                                     :jvm-opts  ["-Dclojure.main.report=stderr"]}})

resources/eastwood/config/formatting_stack.clj

@@ -5,43 +5,30 @@
    [eastwood.lint]
    [formatting-stack.linters.eastwood.impl :as impl]
    [formatting-stack.protocols.linter :as linter]
-   [formatting-stack.util :refer [ns-name-from-filename]]
+   [formatting-stack.util :refer [ns-name-from-filename silence]]
    [medley.core :refer [assoc-some deep-merge]]
    [nedap.utils.modular.api :refer [implement]])
   (:import
    (java.io File)))
 
 (def default-eastwood-options
-  ;; Avoid false positives or undesired checks:
-  (let [linters (remove #{:suspicious-test :unused-ret-vals :constant-test :wrong-tag}
-                        eastwood.lint/default-linters)]
-    (-> eastwood.lint/default-opts
-        (assoc :linters linters
-               :rethrow-exceptions? true))))
-
-(def parallelize-linters? (System/getProperty "formatting-stack.eastwood.parallelize-linters"))
-
-(def config-filename "formatting_stack.clj")
-
-(assert (io/resource (str (io/file "eastwood" "config" config-filename)))
-        "The formatting-stack config file must exist and be prefixed by `eastwood/config`
-(note that this prefix must not be passed to Eastwood itself).")
+  (-> eastwood.lint/default-opts
+      (assoc :rethrow-exceptions? true)))
 
 (defn lint! [{:keys [options]} filenames]
   (let [namespaces (->> filenames
                         (remove #(str/ends-with? % ".edn"))
                         (keep ns-name-from-filename))
         reports    (atom nil)
-        exceptions (atom nil)
-        output     (with-out-str
-                     (binding [*warn-on-reflection* true]
-                       (try
-                         (cond-> options
-                           true                 (assoc :namespaces namespaces)
-                           parallelize-linters? (update :builtin-config-files conj config-filename)
-                           true                 (eastwood.lint/eastwood (impl/->TrackingReporter reports)))
-                         (catch Exception e
-                           (swap! exceptions conj e)))))]
+        exceptions (atom nil)]
+    
+    (silence
+      (try
+        (-> options
+            (assoc :namespaces namespaces)
+            (eastwood.lint/eastwood (impl/->TrackingReporter reports)))
+        (catch Exception e
+          (swap! exceptions conj e))))
     (->> @reports
          :warnings
          (map :warn-data)
@@ -53,8 +40,7 @@
                             :filename            (if (string? uri-or-file-name)
                                                    uri-or-file-name
                                                    (-> ^File uri-or-file-name .getCanonicalPath)))))
-         (concat (impl/warnings->reports output)
-                 (impl/exceptions->reports @exceptions)))))
+         (into (impl/exceptions->reports @exceptions)))))
 
 (defn new [{:keys [eastwood-options]
             :or   {eastwood-options {}}}]

src/formatting_stack/linters/eastwood.clj

@@ -5,45 +5,6 @@
    [formatting-stack.protocols.spec :as protocols.spec]
    [nedap.speced.def :as speced]))
 
-(speced/defn ^boolean? contains-dynamic-assertions?
-  "Does this linting result refer to a :pre/:post containing dynamic assertions?
-
-See https://git.io/fhQTx"
-  [{{{ast-form :form} :ast} :wrong-pre-post
-    msg                     :msg}]
-  (let [[_fn* fn-tails] (if (coll? ast-form)
-                          ast-form
-                          [])
-        [_arglist & body] (if (coll? fn-tails)
-                            fn-tails
-                            [])]
-    (->> body
-         (some (fn [form]
-                 (when (and (coll? form)
-                            (= 'clojure.core/assert
-                               (first form)))
-                   (let [v      (second form)
-                         v-name (when (symbol? v)
-                                  (name v))]
-                     (and v-name
-                          (string/includes? msg v-name) ;; make sure it's the same symbol as in msg
-                          (= \*
-                             (first v-name)
-                             (last v-name)))))))
-         (boolean))))
-
-(speced/defn ^::protocols.spec/reports warnings->reports
-  [^string? warnings]
-  (->> warnings
-       (re-seq #"(.*):(\d+):(\d+): Reflection warning - (.+)\.")
-       (map (fn [[_, filename, line, column, msg]]
-              {:filename filename
-               :line     (Long/parseLong line)
-               :column   (Long/parseLong column)
-               :msg      msg
-               :level    :warning
-               :source   :eastwood/warn-on-reflection}))))
-
 (speced/defn ^::protocols.spec/reports exceptions->reports
   [exceptions]
   (->> exceptions

src/formatting_stack/linters/eastwood/impl.clj

@@ -13,7 +13,7 @@
   A strategy may not return nil."
   (:require
    [clojure.string :as string]
-   [clojure.tools.namespace.repl :refer [refresh-dirs]]
+   [clojure.tools.namespace.repl :as tools.namespace.repl]
    [formatting-stack.protocols.spec :as protocols.spec]
    [formatting-stack.strategies.impl :as impl]
    [formatting-stack.strategies.impl.git-status :as git-status]
@@ -163,7 +163,9 @@
   and then ensuring that code-evaluating tools such as refactor-nrepl or Eastwood also respect that exclusion.
 
   That can avoid some code-reloading issues related to duplicate `defprotocol` definitions, etc."
-  [& {:keys [^::protocols.spec/filenames files]}]
+  [& {:keys [^::protocols.spec/filenames files
+             refresh-dirs]
+      :or {refresh-dirs tools.namespace.repl/refresh-dirs}}]
   {:pre [(check! seq                            refresh-dirs
                  (partial every? (speced/fn [^string? refresh-dir]
                                    (let [file (-> refresh-dir File.)]

src/formatting_stack/strategies.clj

@@ -27,11 +27,11 @@
 
       "test-resources/eastwood_warning.clj"
       (matchers/in-any-order
-       [{:source              :eastwood/warn-on-reflection
-         :msg                 "reference to field getPath can't be resolved"
+       [{:source              :eastwood/reflection
+         :msg                 "reference to field getPath can't be resolved."
          :line                pos-int?
          :column              pos-int?
-         :warning-details-url matchers/absent
+         :warning-details-url "https://github.com/jonase/eastwood#reflection"
          :filename            "test-resources/eastwood_warning.clj"}
         {:source              :eastwood/def-in-def
          :line                pos-int?

test/functional/formatting_stack/linters/eastwood.clj

@@ -5,6 +5,7 @@
    [clojure.set :as set]
    [clojure.string :as string]
    [clojure.test :refer [deftest is testing use-fixtures]]
+   [clojure.tools.namespace.repl :as tools.namespace.repl]
    [formatting-stack.strategies :as sut]
    [formatting-stack.test-helpers :as test-helpers :refer [git-integration-dir]]
    [formatting-stack.util :refer [rcomp]]
@@ -233,7 +234,11 @@
 (deftest namespaces-within-refresh-dirs-only
   (speced/let [^{::speced/spec (rcomp count (partial < 100))}
                all-files (sut/all-files :files [])
-               result (sut/namespaces-within-refresh-dirs-only :files all-files)]
+               refresh-dirs (->> tools.namespace.repl/refresh-dirs
+                                 (mapv (rcomp (partial io/file git-integration-dir)
+                                              str)))
+               result (sut/namespaces-within-refresh-dirs-only :files all-files
+                                                               :refresh-dirs refresh-dirs)]
     (is (seq result)
         "Returns non-empty results (since f-s itself has namespaces within `src`, `test`, etc)")