Skip to content

Commit

Permalink
Add clj-kondo config - first pass
Browse files Browse the repository at this point in the history
Highlights:
- found 1 bug - `etaoin.ide.api` `:storeTitle` command was calling
`etaoin.api/get-title` with wrong number of args. Fixed.
- found an invalid symbol `num-.` from https://clojure.org/reference/reader:
"Symbols beginning or ending with '.' are reserved by Clojure"
Left as is, but will likely follow up with a name change.

Of note:
- Etaoin uses, and might encourage, the use of `:refer :all` for its
api. Ignoring for now. We can decide how we want to do at some later
date.

Other
- Missing require for `clojure.set` added
- An `if` without else converted to `when`. I like this convention, to me it
conveys that an `if` did not accidentally omit an else condition.
- Unused bindings/paramers prefixed with underscore, unless part of
public API and the unuse is an implementation detail. I find this
convention helpful when glancing at fn signatures, I can quickly tell
what is being used. Mostly left these in as I think they typically
conveyed an interface of sorts, but another option is to omit unused
bindings.
- Unused requires, imports and private vars turfed
- Taught clj-kondo about slingshot macros by bringing in existing clj-kondo
slingshot export config
- Taught clj-kondo about etaoin macros we are using (there are quite a
few!) via `:macroexpand` `:hook`. This is a good first pass, we'll likely
swing around again and create proper clj-kondo hooks in an clj-kondo export
config for fine grained reporting for our users.

Closes clj-commons#390
  • Loading branch information
lread committed May 15, 2022
1 parent e104cdb commit d6667ff
Show file tree
Hide file tree
Showing 17 changed files with 217 additions and 98 deletions.
44 changes: 44 additions & 0 deletions .clj-kondo/clj-kondo/slingshot/clj_kondo/slingshot/try_plus.clj
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
(ns clj-kondo.slingshot.try-plus
(:require [clj-kondo.hooks-api :as api]))

(defn expand-catch [catch-node]
(let [[catch catchee & exprs] (:children catch-node)
catchee-sexpr (api/sexpr catchee)]
(cond (vector? catchee-sexpr)
(let [[selector & exprs] exprs]
(api/list-node
[catch (api/token-node 'Exception) (api/token-node '_e#)
(api/list-node
(list* (api/token-node 'let)
(api/vector-node [selector (api/token-node nil)])
exprs))]))
:else catch-node)))

(defn try+ [{:keys [node]}]
(let [children (rest (:children node))
[body catches]
(loop [body children
body-exprs []
catches []]
(if (seq body)
(let [f (first body)
f-sexpr (api/sexpr f)]
(if (and (seq? f-sexpr) (= 'catch (first f-sexpr)))
(recur (rest body)
body-exprs
(conj catches (expand-catch f)))
(recur (rest body)
(conj body-exprs f)
catches)))
[body-exprs catches]))
new-node (api/list-node
[(api/token-node 'let)
(api/vector-node
[(api/token-node '&throw-context) (api/token-node nil)])
(api/token-node '&throw-context) ;; use throw-context to avoid warning
(with-meta (api/list-node (list* (api/token-node 'try)
(concat body catches)))
(meta node))])]
;; (prn (api/sexpr new-node))
{:node new-node}))

2 changes: 2 additions & 0 deletions .clj-kondo/clj-kondo/slingshot/config.edn
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{:hooks
{:analyze-call {slingshot.slingshot/try+ clj-kondo.slingshot.try-plus/try+}}}
19 changes: 19 additions & 0 deletions .clj-kondo/config.edn
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{:config-paths ^:replace [] ;; don't adopt any user preferences

:hooks
;; for now we'll use the simple macroexpand, can move to hooks for finer grained errors later
{:macroexpand
{etaoin.util/defmethods etaoin.util/defmethods
etaoin.util/with-tmp-dir etaoin.util/with-tmp-dir
etaoin.util/with-tmp-file etaoin.util/with-tmp-file

etaoin.api/with-key-down etaoin.api/with-key-down
etaoin.api/with-pointer-btn-down etaoin.api/with-pointer-btn-down
etaoin.api/with-pointer-left-btn-down etaoin.api/with-pointer-left-btn-down
etaoin.api/with-driver etaoin.api/with-driver
etaoin.api/with-chrome etaoin.api/with-chrome
etaoin.api/with-firefox etaoin.api/with-firefox}}

:linters
;; etaoin is dsl-ish and does make use of :refer :all, can decide if we like that at some later date
{:refer-all {:level :off}}}
41 changes: 41 additions & 0 deletions .clj-kondo/etaoin/api.clj
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
(ns etaoin.api)

(defmacro with-key-down
[input key & body]
`(-> ~input
(add-key-down ~key)
~@body
(add-key-up ~key)))

(defmacro with-pointer-btn-down
[input button & body]
`(-> ~input
(add-pointer-down ~button)
~@body
(add-pointer-up ~button)))

(defmacro with-pointer-left-btn-down
[input & body]
`(-> ~input
add-pointer-down
~@body
add-pointer-up))

;; simplified to remove with-pool which is of no consequence to linting
(defmacro with-driver
[type opt bind & body]
`(let [~bind (boot-driver ~type ~opt)]
(try
~@body
(finally
(quit ~bind)))))

(defmacro with-firefox
[opt bind & body]
`(with-driver :firefox ~opt ~bind
~@body))

(defmacro with-chrome
[opt bind & body]
`(with-driver :chrome ~opt ~bind
~@body))
18 changes: 18 additions & 0 deletions .clj-kondo/etaoin/util.clj
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
(ns etaoin.util)

(defmacro defmethods
"Declares multimethods in batch. For each dispatch value from
dispatch-vals, creates a new method."
[multifn dispatch-vals & fn-tail]
`(doseq [dispatch-val# ~dispatch-vals]
(defmethod ~multifn dispatch-val# ~@fn-tail)))

;; essence only for linting
(defmacro with-tmp-file [prefix suffix bind & body]
`(let [~bind "somepath"]
~@body))

;; essence only for linting
(defmacro with-tmp-dir [prefix bind & body]
`(let [~bind "somepath"]
~@body))
3 changes: 2 additions & 1 deletion CHANGELOG.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ A release with an intentional breaking changes is marked with [breaking]

* https://github.com/clj-commons/etaoin/issues/383[#383]: Drop testing for Safari on Windows, Apple no longer releases Safari for Windows
* https://github.com/clj-commons/etaoin/issues/388[#388]: Drop testing for PhantomJS, development has long ago stopped for PhantomJS
* https://github.com/clj-commons/etaoin/issues/387[#387]: No longer testing multiple key modifiers for a single webdriver send keys request
* https://github.com/clj-commons/etaoin/issues/384[#384]: Look for `safaridriver` on PATH by default
* https://github.com/clj-commons/etaoin/issues/402[#402]: Only send body for webdriver `POST` requests to appease `safaridriver`
* https://github.com/clj-commons/etaoin/issues/403[#403]: The `select` fn now clicks on the `select` element before clicking the `option` element to appease `safaridriver`
Expand All @@ -16,7 +17,7 @@ A release with an intentional breaking changes is marked with [breaking]
* Internal quality
** https://github.com/clj-commons/etaoin/issues/382[#382]: Fix process fork testing on Windows
** https://github.com/clj-commons/etaoin/issues/391[#391]: Identify browser name on failed ide tests
** https://github.com/clj-commons/etaoin/issues/387[#387]: No longer testing multiple key modifiers for a single webdriver send keys request
** https://github.com/clj-commons/etaoin/issues/390[#390]: Add internal clj-kondo config

== v0.4.6

Expand Down
2 changes: 1 addition & 1 deletion env/dev/demo.clj
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@

#_{:clj-kondo/ignore [:use]}
(use 'etaoin.api)
(require '[etaoin.keys :as k])

Expand Down
21 changes: 10 additions & 11 deletions src/etaoin/api.clj
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@
[slingshot.slingshot :refer [try+ throw+]])

(:import java.util.Date
java.text.SimpleDateFormat
(java.io IOException)))
java.text.SimpleDateFormat))

;;
;; defaults
Expand Down Expand Up @@ -107,7 +106,7 @@
:method :get
:path [:session (:session driver) :element :active])))
"
[{:keys [driver method path data result]}]
[{:keys [driver method path data]}]
(client/call driver method path data))

;;
Expand Down Expand Up @@ -839,7 +838,7 @@

(defmethod perform-actions
:phantom
[driver input & inputs]
[_driver _input & _inputs]
(util/error "Phantom doesn't support w3c actions."))

(defmulti release-actions dispatch-driver)
Expand All @@ -854,7 +853,7 @@

(defmethod release-actions
:phantom
[driver input & inputs]
[_driver _input & _inputs]
(util/error "Phantom doesn't support w3c actions."))

;;
Expand Down Expand Up @@ -1765,14 +1764,14 @@
(defn scroll-top
"Scrolls to top of the page keeping current horizontal position."
[driver]
(let [{:keys [x y]} (get-scroll driver)]
(let [{:keys [x _y]} (get-scroll driver)]
(scroll driver x 0)))

(defn scroll-bottom
"Scrolls to bottom of the page keeping current horizontal position."
[driver]
(let [y-max (js-execute driver "return document.body.scrollHeight;")
{:keys [x y]} (get-scroll driver)]
{:keys [x _y]} (get-scroll driver)]
(scroll driver x y-max)))

(def ^{:doc "Default scroll offset in pixels."}
Expand Down Expand Up @@ -2266,7 +2265,7 @@

(defn wait
"Sleeps for N seconds."
([driver sec]
(#_{:clj-kondo/ignore [:unused-binding]} [driver sec]
(wait sec))
([sec]
(Thread/sleep (* sec 1000))))
Expand Down Expand Up @@ -2822,7 +2821,7 @@
Under the hood, it sends the file's name as a sequence of keys
to the input."
(fn [driver q file]
(fn [_driver _q file]
(type file)))

(defmethod upload-file String
Expand Down Expand Up @@ -2999,7 +2998,7 @@

(defmethod screenshot-element
:default
[driver q file]
[_driver _q _file]
(util/error "This driver doesn't support screening elements."))

(defmethods screenshot-element
Expand Down Expand Up @@ -3355,7 +3354,7 @@

(try (delete-session driver)
(catch Exception e
(if (not (= 404 (:status (ex-data e))))
(when (not (= 404 (:status (ex-data e))))
;; the exception was caused by something other than "session not found"
(throw e))))

Expand Down
3 changes: 0 additions & 3 deletions src/etaoin/client.clj
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,6 @@
(defn- get-url-path [items]
(str/join "/" (map url-item-str items)))

(defn- status-selector [resp]
(-> resp :status integer?))

(defmacro with-pool [opt & body]
`(client/with-connection-pool ~opt
~@body))
Expand Down
8 changes: 4 additions & 4 deletions src/etaoin/dev.clj
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
(defn try-parse-int
[line]
(try (Integer/parseInt line)
(catch Exception e
(catch Exception _e
line)))


Expand Down Expand Up @@ -101,9 +101,9 @@
:headers headers}))

:network/responsereceived
(let [{:keys [response]} params
{:keys [method headers mimeType remoteIPAddress]} response
{:keys [status]} headers]
(let [{:keys [response]} params
{:keys [headers mimeType remoteIPAddress]} response
{:keys [status]} headers]
(assoc acc
:state 2
:response {:status (try-parse-int status)
Expand Down
30 changes: 14 additions & 16 deletions src/etaoin/driver.clj
Original file line number Diff line number Diff line change
Expand Up @@ -110,27 +110,27 @@

(defmethod options-name
:firefox
[driver]
[_driver]
:moz:firefoxOptions)

(defmethod options-name
:chrome
[driver]
[_driver]
:chromeOptions)

(defmethod options-name
:safari
[driver]
[_driver]
:safariOptions)

(defmethod options-name
:edge
[driver]
[_driver]
:edgeOptions)

(defmethod options-name
:opera
[driver]
[_driver]
:operaOptions)

(defn set-options-args
Expand All @@ -148,7 +148,7 @@

(defmethod set-profile
:default
[driver profile]
[driver _profile]
(log/infof "This driver doesn't support setting a profile.")
driver)

Expand Down Expand Up @@ -191,7 +191,7 @@

(defmethod set-window-size
:default
[driver w h]
[driver _w _h]
(log/infof "This driver doesn't support setting window size.")
driver)

Expand All @@ -216,7 +216,7 @@

(defmethod set-url
:default
[driver url]
[driver _url]
(log/infof "This driver doesn't support setting initial URL.")
driver)

Expand Down Expand Up @@ -262,7 +262,7 @@

(defmethod is-headless?
:phantom
[driver]
[_driver]
true)

;;
Expand Down Expand Up @@ -299,7 +299,7 @@

(defmethod set-prefs
:default
[driver prefs]
[driver _prefs]
(log/infof "This driver doesn't support setting preferences.")
driver)

Expand All @@ -314,8 +314,8 @@
;; Download folder
;;

(defn- ^String add-trailing-slash
[^String path]
(defn- add-trailing-slash
^String [^String path]
(let [sep java.io.File/separator]
(if (string/ends-with? path sep)
path
Expand All @@ -327,7 +327,7 @@

(defmethod set-download-dir
:default
[driver path]
[driver _path]
(log/infof "This driver doesn't support setting a download directory.")
driver)

Expand Down Expand Up @@ -497,20 +497,18 @@
{:arglists '([driver user-agent])}
dispatch-driver)


(defmethods set-user-agent
[:chrome :edge]
[driver user-agent]
(set-options-args driver [(str "--user-agent=" user-agent)]))


(defmethods set-user-agent
[:firefox]
[driver user-agent]
(set-prefs driver {:general.useragent.override user-agent}))

(defmethods set-user-agent
[:default]
[driver user-agent]
[driver _user-agent]
(log/infof "This driver doesn't support setting a user-agent.")
driver)
Loading

0 comments on commit d6667ff

Please sign in to comment.