Eastwood: always use absolute filenames, internally

Author: vemv

e2e/monorepo-support/test/monorepo_support/core_test.clj

@@ -22,7 +22,7 @@
 
     (doseq [filename all
             :let [file (File. filename)
-                  absolute (-> file .getAbsolutePath)]]
+                  absolute (-> file .getCanonicalPath)]]
       (is (= filename
              absolute)
           "Returns absolutized filenames, which is important for monorepo support")

src/formatting_stack/linters/eastwood.clj

@@ -1,13 +1,17 @@
 (ns formatting-stack.linters.eastwood
   (:require
    [clojure.java.io :as io]
+   [clojure.set :as set]
    [clojure.string :as str]
    [eastwood.lint]
    [formatting-stack.linters.eastwood.impl :as impl]
    [formatting-stack.protocols.linter :as linter]
+   [formatting-stack.protocols.spec :as protocols.spec]
    [formatting-stack.util :refer [ns-name-from-filename]]
    [medley.core :refer [assoc-some deep-merge]]
-   [nedap.utils.modular.api :refer [implement]])
+   [nedap.speced.def :as speced]
+   [nedap.utils.modular.api :refer [implement]]
+   [nedap.utils.spec.api :refer [check!]])
   (:import
    (java.io File)))
 
@@ -16,7 +20,7 @@
   (let [linters (remove #{:suspicious-test :unused-ret-vals :constant-test :wrong-tag}
                         eastwood.lint/default-linters)]
     (-> eastwood.lint/default-opts
-        (assoc :linters linters
+        (assoc :linters             linters
                :rethrow-exceptions? true))))
 
 (def parallelize-linters? (System/getProperty "formatting-stack.eastwood.parallelize-linters"))
@@ -28,6 +32,14 @@
 (note that this prefix must not be passed to Eastwood itself).")
 
 (defn lint! [{:keys [options]} filenames]
+  {:post [(do
+            (assert (check! (speced/fn [^::protocols.spec/reports xs]
+                              (let [output (->> xs (keep :filename) (set))]
+                                (set/subset? output (set filenames))))
+                            %)
+                    "The `:filename`s returned from Eastwood should be a subset of this function's `filenames`.
+Otherwise, it would mean that our filename absolutization out of Eastwood reports is buggy.")
+            true)]}
   (let [namespaces (->> filenames
                         (remove #(str/ends-with? % ".edn"))
                         (keep ns-name-from-filename))
@@ -50,9 +62,11 @@
                             :level               :warning
                             :source              (keyword "eastwood" (name linter))
                             :warning-details-url warning-details-url
-                            :filename            (if (string? uri-or-file-name)
-                                                   uri-or-file-name
-                                                   (-> ^File uri-or-file-name .getCanonicalPath)))))
+                            :filename            (speced/let [^::speced/nilable ^String s (when (string? uri-or-file-name)
+                                                                                            uri-or-file-name)
+                                                              ^File file (or (some-> s File.)
+                                                                             uri-or-file-name)]
+                                                   (-> file .getCanonicalPath)))))
          (concat (impl/warnings->reports output)
                  (impl/exceptions->reports @exceptions)))))
 

src/formatting_stack/linters/eastwood/impl.clj

@@ -3,7 +3,9 @@
    [clojure.string :as string]
    [eastwood.reporting-callbacks :as reporting-callbacks]
    [formatting-stack.protocols.spec :as protocols.spec]
-   [nedap.speced.def :as speced]))
+   [nedap.speced.def :as speced])
+  (:import
+   (java.io File)))
 
 (speced/defn ^boolean? contains-dynamic-assertions?
   "Does this linting result refer to a :pre/:post containing dynamic assertions?
@@ -36,8 +38,8 @@ See https://git.io/fhQTx"
   [^string? warnings]
   (->> warnings
        (re-seq #"(.*):(\d+):(\d+): Reflection warning - (.+)\.")
-       (map (fn [[_, filename, line, column, msg]]
-              {:filename filename
+       (map (speced/fn [[_, ^String filename, line, column, msg]]
+              {:filename (-> filename File. .getCanonicalPath)
                :line     (Long/parseLong line)
                :column   (Long/parseLong column)
                :msg      msg

test/formatting_stack/test_helpers.clj

@@ -8,7 +8,7 @@
 (speced/defn filename-as-resource [^string? filename]
   (str "file:" (-> filename
                    File.
-                   .getAbsolutePath)))
+                   .getCanonicalPath)))
 
 (defn with-mocked-diff-path
   "Fixture to stub the absolute path in #'util.diff/unified-diff"

test/functional/formatting_stack/formatters/clean_ns/impl.clj

@@ -7,7 +7,7 @@
 (deftest has-duplicate-requires?
   (are [input assertion] (let [file (io/file "test" "functional" "formatting_stack" "formatters" "clean_ns" input)
                                _ (assert (-> file .exists))
-                               filename (-> file .getAbsolutePath)
+                               filename (-> file .getCanonicalPath)
                                result (sut/has-duplicate-requires? filename)]
                            (case assertion
                              :has-duplicates result

test/functional/formatting_stack/linters/eastwood.clj

@@ -4,7 +4,9 @@
    [formatting-stack.linters.eastwood :as sut]
    [formatting-stack.protocols.linter :as linter]
    [matcher-combinators.matchers :as matchers]
-   [matcher-combinators.test :refer [match?]]))
+   [matcher-combinators.test :refer [match?]])
+  (:import
+   (java.io File)))
 
 (use-fixtures :once (fn [tests]
                       ;; prevent humongous AST representations from being printed:
@@ -14,7 +16,7 @@
 (deftest lint!
   (let [linter (sut/new {})]
     (are [filename expected] (match? expected
-                                     (linter/lint! linter [filename]))
+                                     (linter/lint! linter [(-> filename File. .getCanonicalPath)]))
       "test-resources/valid_syntax.clj"
       []
 
@@ -32,14 +34,14 @@
          :line                pos-int?
          :column              pos-int?
          :warning-details-url matchers/absent
-         :filename            "test-resources/eastwood_warning.clj"}
+         :filename            (-> "test-resources/eastwood_warning.clj" File. .getCanonicalPath)}
         {:source              :eastwood/def-in-def
          :line                pos-int?
          :column              pos-int?
          :warning-details-url "https://github.com/jonase/eastwood#def-in-def"
-         :filename            "test-resources/eastwood_warning.clj"}
+         :filename            (-> "test-resources/eastwood_warning.clj" File. .getCanonicalPath)}
         {:source              :eastwood/wrong-pre-post
          :line                pos-int?
          :column              pos-int?
          :warning-details-url "https://github.com/jonase/eastwood#wrong-pre-post"
-         :filename            "test-resources/eastwood_warning.clj"}]))))
+         :filename            (-> "test-resources/eastwood_warning.clj" File. .getCanonicalPath)}]))))

test/integration/formatting_stack/strategies/impl.clj

@@ -11,16 +11,16 @@
                                       (sut/dir-contains? dirname filename))
 
     "."                             (-> "src/formatting_stack/strategies/impl.clj" File.) true
-    (-> "." File. .getAbsolutePath) (File. "project.clj")                                 true
+    (-> "." File. .getCanonicalPath) (File. "project.clj")                                 true
     "."                             (File. "project.clj")                                 true
     "."                             (File. "dev/user.clj")                                true
     "dev"                           (File. "dev/user.clj")                                true
     "."                             (File. "LICENSE")                                     true
-    (-> "." File. .getAbsolutePath) (File. "LICENSE")                                     true
+    (-> "." File. .getCanonicalPath) (File. "LICENSE")                                     true
     "."                             (File. "./LICENSE")                                   true
     "dev"                           (File. "LICENSE")                                     false
     "dev"                           (File. "./LICENSE")                                   false
-    (-> "." File. .getAbsolutePath) (File. "I_dont_exist")                                false
+    (-> "." File. .getCanonicalPath) (File. "I_dont_exist")                                false
     "."                             (File. "I_dont_exist")                                false
     "dev"                           (File. "I_dont_exist")                                false
     "dev"                           (File. "user.clj")                                    false))

test/unit/formatting_stack/formatters/cljfmt/impl/magic_symbols.clj

@@ -17,5 +17,5 @@
     false
 
     (-> (io/file "test-resources" "magic.clj")
-        (.getAbsolutePath))
+        (.getCanonicalPath))
     true))

test/unit/formatting_stack/linters/eastwood/impl.clj

@@ -3,7 +3,9 @@
    [clojure.string :as str]
    [clojure.test :refer [are deftest is testing]]
    [formatting-stack.linters.eastwood.impl :as sut]
-   [matcher-combinators.test :refer [match?]]))
+   [matcher-combinators.test :refer [match?]])
+  (:import
+   (java.io File)))
 
 (deftest contains-dynamic-assertions?
   (testing "matches on false-positive wrong-pre-post reports"
@@ -72,7 +74,7 @@
       :column   40
       :line     6
       :msg      "reference to field getPath can't be resolved"
-      :filename "path.clj"}]
+      :filename (-> "path.clj" File. .getCanonicalPath)}]
 
     (str/join "\n"
               ["path.clj:6:40: Reflection warning - reference to field getPath can't be resolved."
@@ -82,20 +84,20 @@
       :column   40
       :line     6
       :msg      "reference to field getPath can't be resolved"
-      :filename "path.clj"}
+      :filename (-> "path.clj" File. .getCanonicalPath)}
      {:source   :eastwood/warn-on-reflection
       :level    :warning
       :column   12
       :line     13
       :msg      "different message"
-      :filename "other-path.clj"}]
+      :filename (-> "other-path.clj" File. .getCanonicalPath)}]
 
     (str/join "\n"
               ["path.clj:6:40: Reflection warning - reference to field getPath can't be resolved."
                "random garbage"
                "path.clj: Incomplete warning - different message."
                "other-path.clj:13:12: Reflection warning - different message."])
     [{:msg      "reference to field getPath can't be resolved"
-      :filename "path.clj"}
+      :filename (-> "path.clj" File. .getCanonicalPath)}
      {:msg      "different message"
-      :filename "other-path.clj"}]))
+      :filename (-> "other-path.clj" File. .getCanonicalPath)}]))