Clojurians Log v2

What is the correct way to handle this warning from cl-kondo?

(defmulti rte-match
  (fn [rte _items & {:keys [promise-disjoint
                            hot-spot]}]
    (dispatch rte 'rte-match)))

(defmethod rte-match :sequential
  [pattern items & {:keys [promise-disjoint
                           hot-spot]}]
  (rte-match (rte-compile pattern) items :promise-disjoint true :hot-spot hot-spot))

shouldn't the defmulti declare all the keys which methods are allowed to consume?

no, they don’t have to

"have to"? OK I agree it's not strictly necessary. but it seems to me that the defmulti should serve as documentation to applications which want to add methods. right?

you can write it like this

(defmulti rte-match
  (fn [rte _items & _]
    (dispatch rte 'rte-match)))

(defmethod rte-match :sequential
  [pattern items & {:keys [hot-spot]}]
  (rte-match (rte-compile pattern) items :promise-disjoint true :hot-spot hot-spot))

defmulti is a strange beast (imho) in terms of documentation

since dispatch function can be detached from declaration of defmulti

If I do that, then where is the documentation for the callers of the methods to use, and where is the documentation for implementors of methods to use?

(defmulti foo some-dispatch-fn)

in a docstring after the name of defmulti

(defmulti foo "docstring" some-dispatch-fn)

good idea.

on the other hand, clj-kondo seems to have the feature that normal parameters which are not used can be prefixed with _ to disable the warning.

(defn foo [a b _c d] ...)

right

by convention arguments beginning with _ (including single character _) are “might not be used”

but you still can use them in function’s body

(fn foo [_x _y] (+ _x _y))

I see that clj-kondo fails to warn if I use an _ variable. That's unfortunate.

?

in Common Lisp you can mark them as the three different cases.

I.e., in CL i can declare the variable as either unused or maybe-unused

as I mention this is a convention not a rule in clojure

and thus if I mark something as unused, but still use it i get a welcomed warning.

_ or _x are just normal symbols that can be used as a names of vars

(defn foo [_] (inc _))

indeed, but kondo is applying some conventions to the names to aid the programmer in static analysis, right, clojure doesnt care.

exactly, and you always can switch this warnings on and off

but maybe I’m wrong, better to ask @borkdude if it is possible to turn on warning about “unused vars” beeing used

I’m afk today until the evening. Check config.md for documentation about config.

I will open an issue because there is no option (as I can see in config.md) to turn on such warnings

This has already been suggested in one of the issues. Can’t really Google it on my phone :-)

my google fu failed me) will check one more time

https://github.com/borkdude/clj-kondo/issues/861

found it

ah that was my issue from long ago. I forgot about that. but now it rings a bell.

I looked for a bug all day. The bug occurred when I moved a bunch of functions around, basically refactoring name spaces. The problem was that when I run my tests from the lein command line they fail. But when I run them interactively the succeeded. The problem finally turned out to be I was defining the method in two different files. The defmulti is in a file whose namespace is genus and is abbreviated gns every where which requires it. The intent is that other files can add methods to gns/-disjoint?. Unfortunately two different files added the method

(defmethod gns/-disjoint :rte ...)

on the surface that feels like a nice linter

If you are able to change the code, then use :refer [deftest is testing] instead of :refer :all which is more common approach to requiring vars.

it is perfectly legal to redefine methods. otherwise debugging would be more difficult. But static analysis should warn.

I'll file an issue.

https://github.com/borkdude/clj-kondo/issues/1061

everyone should rush there and give it a thumbs up. 👍:skin-tone-2:

I think it doesn’t really matter where defmethod is redefining same dispatch value right now clj-kondo will not complain even about this

(defmulti foo identity)

(defmethod foo :x [_] :x)

(defmethod foo :x [_] :y)

should it though is the question really

sure it is

?

that code above feels smelly and is more likely than not the cause of a bad copy/paste

don’t mind right hand side of my examples ) the important part is on the left and it is not necessary bad copy/paste but could be some leftovers after refactoring or intensive REPL usage

(defmulti foo identity)
(defmethod foo :x [_] :x)
(defmethod foo :x [_] :y)

yeah artifacts of refactoring/updates/etc

ok, makes sense

makes sense

Agree that it doesn't matter where the method is defined. My comment was simply that it might be defined as a naked name in one place (defmethod foo :x ...) and as a decorated name elsewhere (defmethod mynamespace/foo :x ...) and that still deserved a complaint from kondo. in my opinion.

@borkdude do you want me to create an additional circleci path to create the with-profiles standalone.jar?

@b Just commented. I think it's better if you removed the native-image profile for the uberjar creation, since that needs JDK11 (it pulls in JDK11-specific deps)

But an uberjar created with Java 8 should be compileable with Java 11, so I think then it should work

I must be blind, but there is no mention of java8 in any .circleci/config.yml

it downloads graalvm11 only

circleci/clojure:lein-2.8.1

that's quite unfortunate because I had to use both profiles for it to work with native-image for me

I used the clojure:lein-2.8.1 image on my own system to generate the standalone jar and then transfer it to my real host

weird that I didn't get that error, that's why I wrote about the caching of .m2 as a potential cause

but I also build everything after I already had prepended the graalvm directory to my PATH

@b It might be better if the native-image step on CircleCI just posted the uberjar as an artifact as well. This way you are guaranteed to use the same one

So instead of in the JVM step

so sidestepping the normal jobs and creating an additional single job that does everything needed to create the standalone jar?

could do, but could also do it in the linux job

@b Right now we have these files: https://app.circleci.com/pipelines/github/borkdude/clj-kondo/3030/workflows/1c2aa51c-ca4c-4c2b-a95d-d2e924909f54/jobs/11540/artifacts We could simply add the uberjar to it

And then I will post that in the github releases in the future

oh wow, I just tried to reproduce it and I found why I thought it worked in my setup: I tested the circleci steps in circleci/clojure:lein-2.9.1 instead of lein-2.8.1

and that one has java11 right

lein-2.9.1 comes with java11 yeah

I'm using up all your circleci credits with my fails 😮

@b

"$VERSION" -eq "linux" 

"$CLJ_KONDO_PLATFORM" -eq "linux" 

environment:
      LEIN_ROOT: "true"
      GRAALVM_HOME: /home/circleci/graalvm-ce-java11-20.2.0
      CLJ_KONDO_PLATFORM: linux # used in release script
      CLJ_KONDO_TEST_ENV: native
 

so you were close

fair enough and you're probably also right about the bash thing, -eq is for checking return code results 😉

but I think it might not hurt to copy the uberjar if it's not on linux

Technically -eq is for checking integers, = is for strings. In bash.

do you manually download the artifacts then? I didn't want to overwrite anything in case you automatically merged all the artefacts

yeah, I do it manually

@b

cp: cannot stat 'target/clj-kondo--standalone.jar': No such file or directory
  adding: clj-kondo (deflated 71%)

We might want to add set -e to the script

okay

set -eo pipefail

man I don't know what I'm doing right now, sorry for not paying attention.

laptop on couch is not great for concentration

success!

seems to work now right? https://app.circleci.com/pipelines/github/borkdude/clj-kondo/3034/workflows/f45a9937-a0f6-46a2-ba8d-fa517458e928/jobs/11556/artifacts

as a last step, I think we can now remove the uberjar from the jvm tests

I'll test the resulting standalone jar (but it looks good due to the size) and then remove the jvm uberjar creation and rewrite the commit message

:thumbsup:

looks good, but while we're at it. should we save the earth and also zip the jar? a simple zip invocation reduces the file size by more than 50%

a jar is already a zip file

yeah I know

but I don't know how to tweak the settings to have it user better compression

while zip just reduces the filesize to 45%

I would say, let's leave it at this for now

:thumbsup:

Merged the first batch of checks for deps.edn to master: https://github.com/borkdude/clj-kondo/blob/a9df3965716025e93764d36100314edca30e5cee/test/clj_kondo/deps_edn_test.clj