Merge pull request #408 from quil/linting-and-formatting

Linting and formatting

.clj-kondo/config.edn

@@ -0,0 +1 @@
+{}

.github/workflows/test.yaml

@@ -50,6 +50,10 @@ jobs:
       - name: AOT Compile
         run: clojure -T:build aot
 
+      # Only needs to run in one matrix step
+      - name: Linting & Formatting
+        run: bin/lint
+
       - name: Test
         uses: coactions/setup-xvfb@v1
         with:

.gitignore

@@ -22,3 +22,4 @@ dev-resources/snippet-snapshots/**/*-actual.png
 dev-resources/snippet-snapshots/**/*-difference.png
 /libraries/
 /.cpcache/
+/.clj-kondo/.cache/

RELEASE-NOTES.md

@@ -1,5 +1,6 @@
 ## Unreleased
 
+* Automatic format check and clj-kondo linting for tests [#408](https://github.com/quil/quil/pull/408)
 * Improve testing documentation [#407](https://github.com/quil/quil/pull/407)
 * Exclude JOGL deps from POM for uberjar release [#406](https://github.com/quil/quil/pull/406)
 * Automate CLJS snippet test server to run on CI [#405](https://github.com/quil/quil/pull/405)
@@ -13,6 +14,7 @@
 __8th November 2023__
 
 * New automated test harness for assertions of Processing and P5js behaviors [#384](https://github.com/quil/quil/pull/384), [#388](https://github.com/quil/quil/pull/388), [#390](https://github.com/quil/quil/pull/390)
+* P5js 1.7 [#383](https://github.com/quil/quil/pull/383)
 * Basic Github Actions test harness for existing `lein test` on Linux [#381](https://github.com/quil/quil/pull/381)
 * Fix for `#cljs.core/abs` compile warning [#380](https://github.com/quil/quil/pull/380)
 * Support for Processing 4.3, M1 hardware, JVM > 1.8

bin/lint

@@ -0,0 +1,10 @@
+#!/usr/bin/env bash
+
+# Runs clj-kondo linting & clj formatting check
+# Usage: bin/lint
+
+set -exuo pipefail
+
+# for now *only* lint test, there are too many warnings in src/{clj,cljs,cljc}
+clojure -M:clj-kondo --lint test
+clojure -M:format check

deps.edn

@@ -87,4 +87,14 @@
 
            ;; clj -M:dev:fig:server -b dev -s
            :server
-           {:main-opts ["-m" "figwheel.main" "-co" "dev.cljs.edn"]}}}
+           {:main-opts ["-m" "figwheel.main" "-co" "dev.cljs.edn"]}
+
+           ;; clj -M:clj-kondo --lint src test
+           :clj-kondo
+           {:deps {clj-kondo/clj-kondo {:mvn/version "2023.12.15"}}
+            :main-opts ["-m" "clj-kondo.main"]}
+
+           ;; clj -M:format check
+           :format
+           {:deps {cljfmt/cljfmt {:mvn/version "0.9.2"}}
+            :main-opts ["-m" "cljfmt.main"]}}}

project.clj

@@ -67,8 +67,7 @@
                                             [org.clojure/clojurescript "1.10.520"]]
 
                              :cljsbuild
-                             {:builds [
-                                        ; Compiles all tests in advanced mode. Used for release testing.
+                             {:builds [; Compiles all tests in advanced mode. Used for release testing.
                                        {:id "tests"
                                         :source-paths ["test/cljs" "src/cljc" "src/cljs"]
                                         :compiler

src/clj/quil/applet.clj

@@ -31,14 +31,14 @@
   the JVM is closed as well which is not the case for clojure. So here we're
   performing renderer-specific cleanups."
   [applet]
-  (if-let [native (-> applet .getSurface .getNative)]
+  (when-let [native (-> applet .getSurface .getNative)]
     (condp = (.getClass native)
 
       com.jogamp.newt.opengl.GLWindow
-      ; Cannot destroy GLWindow right away because there is some callbacks going on
-      ; and NPE is thrown when they execute if window is destroyed. It doesn't seem
-      ; to affect anything, but annoying to have NPE in logs. Delaying destroying
-      ; window for 1 sec. Ugly hack, but couldn't think of better way. Suggestions welcome.
+      ;; Cannot destroy GLWindow right away because there is some callbacks going on
+      ;; and NPE is thrown when they execute if window is destroyed. It doesn't seem
+      ;; to affect anything, but annoying to have NPE in logs. Delaying destroying
+      ;; window for 1 sec. Ugly hack, but couldn't think of better way. Suggestions welcome.
       (.schedule executor #(.destroy native) 1 java.util.concurrent.TimeUnit/SECONDS)
 
       processing.awt.PSurfaceAWT$SmoothCanvas
@@ -67,17 +67,18 @@
         keep-on-top?   (:keep-on-top m)
         surface        (.getSurface applet)
         resizable?     (:resizable m)]
-    ; TODO: check if resizable and alwaysOnTop work correctly.
+    ;; TODO: check if resizable and alwaysOnTop work correctly.
     (javax.swing.SwingUtilities/invokeLater
      (fn []
        (.setTitle surface title)
        (.setResizable surface resizable?)
        (.setAlwaysOnTop surface keep-on-top?)))
     applet))
 
+;; FIXME: why is renderer ignored here?
 (defn- applet-run
   "Launches the applet to the specified target."
-  [applet title renderer]
+  [applet title _renderer]
   (PApplet/runSketch
    (into-array String
                (vec (filter string?
@@ -109,7 +110,7 @@
   [renderer]
   (cond (keyword? renderer) (u/resolve-constant-key renderer renderer-modes)
         (string? renderer) renderer
-        :default (throw (RuntimeException. ":renderer should be keyword or string"))))
+        :else (throw (RuntimeException. ":renderer should be keyword or string"))))
 
 (defn- validate-size
   "Checks that the `size` vector is exactly two elements. If not, throws
@@ -177,7 +178,7 @@
 (defn -exitActual
   "Overriding `PApplet.exitActual` because we don't want it to call
   `System.exit()`."
-  [this])
+  [_this])
 
 (defn -sketchFullScreen [this] (:present (meta this)))
 
@@ -293,7 +294,8 @@
         options           (merge (dissoc options :features)
                                  features)
 
-        display           (or (:display options) :default)
+        ;; FIXME: display appears unused?
+        ;; display           (or (:display options) :default)
         size              (validate-size (:size options))
 
         title             (or (:title options) (str "Quil " (swap! untitled-applet-id* inc)))

src/clj/quil/helpers/docs.clj

@@ -1,7 +1,6 @@
 (ns ^{:doc "Functions for filtering/displaying docs in REPL and generating wiki pages."}
  quil.helpers.docs
-  (:require [clojure.string :as cstr]
-            [clojure.java.io :as io]))
+  (:require [clojure.string :as cstr]))
 
 (defn link-to-processing-reference
   "Returns a link to the Processing page of the function.

src/clj/quil/helpers/drawing.clj

@@ -18,7 +18,7 @@
   ([interleaved-points]
    (lazy-seq
     (let [head (take 2 interleaved-points)]
-      (if (= 2 (count head))
+      (when (= 2 (count head))
         (cons (apply concat head) (line-join-points (drop 1 interleaved-points)))))))
   ([xs ys]
    (lazy-seq

src/clj/quil/helpers/seqs.clj

@@ -113,7 +113,7 @@
   [s]
   (let [state (atom (seq s))]
     (fn []
-      (let [[old new] (swap-returning-prev! state rest)]
+      (let [[old _new] (swap-returning-prev! state rest)]
         (first old)))))
 
 (defn tally

src/clj/quil/snippets/macro.clj

@@ -1,5 +1,4 @@
-(ns quil.snippets.macro
-  (:require [quil.util :as u]))
+(ns quil.snippets.macro)
 
 (defmacro defsnippet
   "Defines a snippet. A snippet is a small example showing how to use a specific

src/cljs/quil/sketch.cljs

@@ -1,9 +1,7 @@
 (ns quil.sketch
   (:require [quil.util :as u :include-macros true]
             [quil.middlewares.deprecated-options :as do]
-            [goog.dom :as dom]
             [goog.events :as events]
-            [goog.style :as style]
             [goog.object :as object]
             [goog.events.EventType :as EventType])
   (:require-macros [quil.sketch]))
@@ -98,7 +96,8 @@
 
         sketch-size     (:size opts)
         renderer        (:renderer opts)
-        features        (set (:features opts))
+        ;; features is unused in cljs
+        ;; features        (set (:features opts))
 
         setup (fn []
                 (->> (if renderer [renderer] [])
@@ -107,8 +106,8 @@
                 (when (:settings opts) ((:settings opts)))
                 (when (:setup opts) ((:setup opts))))
         mouse-wheel (when-let [wheel-handler (:mouse-wheel opts)]
-                      ; using (get "delta") because in advanced mode
-                      ; it will be renamed otherwise.
+                      ;; using (get "delta") because in advanced mode
+                      ;; it will be renamed otherwise.
                       (fn [evt] (wheel-handler (object/get evt "delta"))))
 
         opts (assoc opts

test/cljs/quil/snippet.cljs

@@ -1,6 +1,5 @@
 (ns quil.snippet
   (:require [quil.core :as q :include-macros true]
-            [dommy.utils :as utils]
             [dommy.core :as d :include-macros true]
             [goog.events :as events]
             [goog.events.EventType :as EventType]
@@ -65,8 +64,8 @@
   (reset! total 0))
 
 (defn remove-canvases []
-   (doseq [c (d/sel [:canvas])]
-     (d/remove! c)))
+  (doseq [c (d/sel [:canvas])]
+    (d/remove! c)))
 
 (defn run-single-test []
   (if (< @test-indx (count test-functions))

test/cljs/test_server.clj

@@ -14,12 +14,12 @@
 
 (def root-page
   (h/html5
-      (header "Quil tests")
-    [:body {:data-page "root"}
-     [:div.centerLayer {:align "center"}
-      [:p [:a {:href "/test.html"} "Common Quil API tests"]]
-      [:p [:a {:href "/manual"} "Manual Quil API tests"]]
-      [:p [:a {:href "/fullscreen"} "Fullscreen Manual test"]]]]))
+   (header "Quil tests")
+   [:body {:data-page "root"}
+    [:div.centerLayer {:align "center"}
+     [:p [:a {:href "/test.html"} "Common Quil API tests"]]
+     [:p [:a {:href "/manual"} "Manual Quil API tests"]]
+     [:p [:a {:href "/fullscreen"} "Fullscreen Manual test"]]]]))
 
 (defn gen-test-container
   ([id doc]
@@ -92,9 +92,9 @@
   (str "dev-resources/snippet-snapshots/" platform "/normal/"))
 
 (defroutes app-routes
-  (GET "/" req root-page)
-  (GET "/manual" req manual-page)
-  (GET "/fullscreen" req fullscreen-page)
+  (GET "/" _req root-page)
+  (GET "/manual" _req manual-page)
+  (GET "/fullscreen" _req fullscreen-page)
   (route/files "/snapshots" {:root (path-to-snippet-snapshots "cljs")})
   ;; Figwheel runs with deps.edn so clojure.basis will be specified
   (route/files "/js" {:root (if (System/getProperty "clojure.basis")

test/quil/manual.clj

@@ -236,14 +236,14 @@
     @lock))
 
 (deftest ^:manual run-all
-  (doseq [fn [resizable-and-keep-on-top
-             fullscreen present-and-bgcolor
-             no-loop-with-start-loop
-             redraw-on-key
-             fun-mode
-             resize-sketch
-             with-stroke
-             with-fill
-             on-close-and-exit-on-close]]
-    (fn)))
+  (doseq [tfn [resizable-and-keep-on-top
+               fullscreen
+               present-and-bgcolor
+               no-loop-with-start-loop
+               redraw-on-key
+               fun-mode
+               resize-sketch with-stroke
+               with-fill
+               on-close-and-exit-on-close]]
+    (tfn)))
 

test/quil/snippets/snapshot_test.clj

@@ -32,8 +32,8 @@
               (when (or sth/manual? sth/github-actions?)
                 (q/frame-rate 4)))
      :mouse-clicked (:mouse-clicked snippet)
-     :settings (fn [] (when-let [settings (:settings opts)]
-                       (settings)))
+     :settings
+     (fn [] (when-let [settings (:settings opts)] (settings)))
      :draw
      (fn []
        (q/background 255)

test/quil/snippets/test_helper.clj

@@ -7,7 +7,7 @@
 (defn- display-density []
   (try
     (.. GraphicsEnvironment (getLocalGraphicsEnvironment) (getDefaultScreenDevice) (getScaleFactor))
-    (catch IllegalArgumentException e
+    (catch IllegalArgumentException _e
       ;; getScaleFactor() method is present only on osx.
       ;; For other systems return 1 for now.
       1)))

test/quil/test_runner.cljs

@@ -8,8 +8,7 @@
    [figwheel.main.testing :refer-macros [run-tests-async]]
 
    ;; require namespaces to tests
-   quil.calculation-test
-   ))
+   quil.calculation-test))
 
 ;; print out browser version information
 (deftest verify-browser