(maint) fix eastwood issues

This fixes and ignores issues found by eastwood.

Author: jonathannewman

project.clj

@@ -43,7 +43,8 @@
                  ;; see https://github.com/puppetlabs/trapperkeeper/pull/306#issuecomment-1467059264
                  [puppetlabs/kitchensink nil :exclusions [cheshire]]
                  [puppetlabs/i18n]
-                 [nrepl/nrepl]]
+                 [nrepl/nrepl]
+                 [io.github.clj-kondo/config-slingshot-slingshot "1.0.0"]]
 
   :deploy-repositories [["releases" {:url "https://clojars.org/repo"
                                      :username :env/clojars_jenkins_username
@@ -72,5 +73,22 @@
             [jonase/eastwood "1.2.2" :exclusions [org.clojure/clojure]]
             [puppetlabs/i18n "0.9.2"]]
 
+  :eastwood {:ignored-faults {:reflection {puppetlabs.trapperkeeper.logging [{:line 92}]
+                                           puppetlabs.trapperkeeper.internal [{:line 128}]
+                                           puppetlabs.trapperkeeper.testutils.logging true
+                                           puppetlabs.trapperkeeper.testutils.logging-test true
+                                           puppetlabs.trapperkeeper.services.nrepl.nrepl-service-test true
+                                           puppetlabs.trapperkeeper.plugins-test true}
+                              :local-shadows-var {puppetlabs.trapperkeeper.config-test true
+                                                  puppetlabs.trapperkeeper.services-test true
+                                                  java-service-example.java-service true
+                                                  puppetlabs.trapperkeeper.optional-deps-test true}
+                              :deprecations {puppetlabs.trapperkeeper.testutils.logging true
+                                             puppetlabs.trapperkeeper.testutils.logging-test true
+                                             puppetlabs.trapperkeeper.logging-test true}
+                              :def-in-def {puppetlabs.trapperkeeper.optional-deps-test true}}
+
+             :continue-on-exception true}
+
   :main puppetlabs.trapperkeeper.main)
 

src/puppetlabs/trapperkeeper/internal.clj

@@ -108,7 +108,7 @@
   compilation / instantiation, and inspects it to see if the error data map
   represents a missing trapperkeeper service or function.  If so, throws a
   more meaningful exception.  If not, re-throws the original exception."
-  [e]
+  [^ExceptionInfo e]
   {:pre [(instance? ExceptionInfo e)]}
   (let [data (ex-data e)]
     (condp = (:error data)

src/puppetlabs/trapperkeeper/logging.clj

@@ -1,5 +1,5 @@
 (ns puppetlabs.trapperkeeper.logging
-  (:import [ch.qos.logback.classic Level PatternLayout]
+  (:import [ch.qos.logback.classic Level LoggerContext PatternLayout]
            (ch.qos.logback.core ConsoleAppender)
            (org.slf4j Logger LoggerFactory)
            (ch.qos.logback.classic.joran JoranConfigurator))
@@ -8,7 +8,9 @@
             [puppetlabs.i18n.core :as i18n]))
 
 (defn logging-context
-  []
+  ^LoggerContext []
+  ;; in practice, this returns ch.qos.logback.classic.LoggerContext
+  ;; which the other functions below assume
   (LoggerFactory/getILoggerFactory))
 
 (defn reset-logging
@@ -18,8 +20,8 @@
 (def root-logger-name Logger/ROOT_LOGGER_NAME)
 
 (defn root-logger
-  []
-  (LoggerFactory/getLogger root-logger-name))
+  ^ch.qos.logback.classic.Logger []
+  (LoggerFactory/getLogger ^String root-logger-name))
 
 (defn catch-all-logger
   "A logging function useful for catch-all purposes, that is, to
@@ -72,7 +74,7 @@
    (let [root (root-logger)]
      (.addAppender root (create-console-appender level))
      (when (> (.toInt (.getLevel root))
-              (.toInt level))
+              (.toInt ^Level level))
        (.setLevel root level)))))
 
 (defn configure-logger!
@@ -84,8 +86,8 @@
   ch.qos.logback.core.classic.joran.JoranConfigurator."
   [logging-conf]
   (let [configurator (JoranConfigurator.)
-        context      (LoggerFactory/getILoggerFactory)]
-    (.setContext configurator (LoggerFactory/getILoggerFactory))
+        context      (logging-context)]
+    (.setContext configurator context)
     (.reset context)
     (.doConfigure configurator logging-conf)))
 

src/puppetlabs/trapperkeeper/plugins.clj

@@ -1,5 +1,5 @@
 (ns puppetlabs.trapperkeeper.plugins
-  (:import (java.util.jar JarFile)
+  (:import (java.util.jar JarEntry JarFile)
            (java.io File))
   (:require [clojure.java.io :refer [file]]
             [clojure.tools.logging :as log]
@@ -9,7 +9,7 @@
 (defn- should-process?
   "Helper for `process-file`.  Answers whether or not the duplicate detection
   code should process a file with the given name."
-  [name]
+  [^String name]
   (and
     ;; ignore directories
    (not (or (.isDirectory (file name))
@@ -25,11 +25,11 @@
   "Helper for `process-file`; handles a found duplicate.  Throws an exception
   if the duplicate is a .class or .clj file.  Otherwise, logs a warning and
   returns the accumulator."
-  [container-filename acc filename]
+  [container-filename acc ^String filename]
   (let [error-msg (i18n/trs "Class or namespace {0} found in both {1} and {2}"
                             filename container-filename (acc filename))]
     (if (or (.endsWith filename ".class") (.endsWith filename ".clj"))
-      (throw (IllegalArgumentException. error-msg))
+      (throw (IllegalArgumentException. ^String error-msg))
 
       ;; It is common to have other conflicts (besides classes and clojure
       ;; namespaces), especially during development (for example,
@@ -57,20 +57,20 @@
   (let [file (file container-filename)]
     (if (.exists file)
       (let [filenames (if (.isDirectory file)
-                        (map #(.getPath %) (file-seq file))
-                        (map #(.getName %) (enumeration-seq (.entries (JarFile. file)))))]
+                        (map #(.getPath ^File %) (file-seq file))
+                        (map #(.getName ^JarEntry %) (enumeration-seq (.entries (JarFile. file)))))]
         (reduce (partial process-file container-filename) acc filenames))
-      acc ; There may be directories on the classpath that do not exist.
-)))
+      acc))) ; There may be directories on the classpath that do not exist.
+
 
 (defn jars-in-dir
   "Given a path to a directory on disk, returns a collection of all of the .jar
   files contained in that directory (not recursive)."
-  [dir]
+  [^File dir]
   {:pre [(instance? File dir)]
    :post [(coll? %)
           (every? (partial instance? File) %)]}
-  (filter #(.endsWith (.getAbsolutePath %) ".jar") (.listFiles dir)))
+  (filter #(.endsWith (.getAbsolutePath ^File %) ".jar") (.listFiles dir)))
 
 (defn verify-no-duplicate-resources
   "Examines all resources on the classpath and contained in the given directory
@@ -97,9 +97,9 @@
       (if (.exists plugins)
         (do
           (verify-no-duplicate-resources plugins)
-          (doseq [jar (jars-in-dir plugins)]
+          (doseq [^File jar (jars-in-dir plugins)]
             (log/info (i18n/trs "Adding plugin {0} to classpath." (.getAbsolutePath jar)))
             (kitchensink/add-classpath jar)
             (kitchensink/add-classpath jar (clojure.lang.RT/baseLoader))))
-        (throw (IllegalArgumentException.
-                 (i18n/trs "Plugins directory {0} does not exist" plugins-path)))))))
+        (let [^String msg (i18n/trs "Plugins directory {0} does not exist" plugins-path)]
+          (throw (IllegalArgumentException. msg)))))))

src/puppetlabs/trapperkeeper/services_internal.clj

@@ -1,5 +1,5 @@
 (ns puppetlabs.trapperkeeper.services-internal
-  (:import (clojure.lang IFn))
+  (:import (clojure.lang IFn Namespace))
   (:require [clojure.set :refer [difference union intersection]]
             [schema.core :as schema]
             [puppetlabs.kitchensink.core :as ks]
@@ -79,8 +79,9 @@
 (schema/defn ^:always-validate var->symbol :- Symbol
   "Returns a symbol for the var, including its namespace"
   [fn-var :- Var]
-  (symbol (str (-> fn-var meta :ns .name))
-          (str (-> fn-var meta :name))))
+  (let [m (meta fn-var)]
+    (symbol (str (.name ^Namespace (:ns m)))
+            (str (:name m)))))
 
 (schema/defn ^:always-validate transform-deps-map :- (schema/pred vector?)
   "Given a map of required and optional dependencies, return a vector that

test/puppetlabs/trapperkeeper/testutils/logging.clj

@@ -337,13 +337,13 @@
           (one-element?)))))
 
 (defmethod clojure.test/assert-expr 'logged? [is-msg form]
-  "Asserts that exactly one event in *test-log-events* has a message
-  that matches msg-or-pred.  The match is performed via = if
-  msg-or-pred is a string, via re-find if msg-or-pred is a pattern, or
-  via (msg-or-pred event-map) if msg-or-pred is a function.  If level
-  is specified, the message's keyword level (:info, :error, etc.) must
-  also match.  For example:
-    (with-test-logging (log/info \"123\") (is (logged? #\"2\")))."
+  ;"Asserts that exactly one event in *test-log-events* has a message
+  ;that matches msg-or-pred.  The match is performed via = if
+  ;msg-or-pred is a string, via re-find if msg-or-pred is a pattern, or
+  ;via (msg-or-pred event-map) if msg-or-pred is a function.  If level
+  ;is specified, the message's keyword level (:info, :error, etc.) must
+  ;also match.  For example:
+  ;  (with-test-logging (log/info \"123\") (is (logged? #\"2\")))."
   (assert (#{2 3} (count form)))
   (let [[_ msg-or-pred level] form]
     `(let [events# @@#'puppetlabs.trapperkeeper.testutils.logging/*test-log-events*]