clj-kondo

https://github.com/clj-kondo/clj-kondo
Jim Newton 2020-11-05T08:39:02.335600Z

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))
It warns that promise-disjoint and hot-spot are not used in the defmulti, and that promise-disjoint is not used in the first defmethod. However, these keys ARE used in other defmethods. I'm not sure whether this is a clojure question or a clj-kondo question actually

Jim Newton 2020-11-05T08:40:07.336100Z

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

2020-11-05T08:40:33.336400Z

no, they don’t have to

Jim Newton 2020-11-05T08:41:04.337Z

"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?

2020-11-05T08:41:25.337400Z

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))

2020-11-05T08:42:13.338300Z

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

2020-11-05T08:42:37.339Z

since dispatch function can be detached from declaration of defmulti

Jim Newton 2020-11-05T08:42:40.339300Z

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?

2020-11-05T08:42:52.339600Z

(defmulti foo some-dispatch-fn)

2020-11-05T08:43:19.339900Z

in a docstring after the name of defmulti

2020-11-05T08:43:32.340300Z

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

Jim Newton 2020-11-05T08:43:55.340500Z

good idea.

Jim Newton 2020-11-05T08:45:04.342100Z

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] ...)
clj-kondo will not warn that _c is unused. Is there something similar for key arguments?

2020-11-05T08:45:13.342300Z

right

2020-11-05T08:46:17.343300Z

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

2020-11-05T08:46:57.344300Z

but you still can use them in function’s body

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

Jim Newton 2020-11-05T08:47:05.344400Z

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

2020-11-05T08:47:23.345Z

?

Jim Newton 2020-11-05T08:47:25.345100Z

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

Jim Newton 2020-11-05T08:47:50.345700Z

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

2020-11-05T08:48:23.346200Z

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

Jim Newton 2020-11-05T08:48:34.346700Z

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

2020-11-05T08:48:46.347Z

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

2020-11-05T08:48:57.347300Z

(defn foo [_] (inc _))

Jim Newton 2020-11-05T08:49:20.348Z

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

2020-11-05T08:50:26.348500Z

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

2020-11-05T08:54:18.349900Z

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

borkdude 2020-11-05T08:55:09.351Z

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

2020-11-05T08:58:45.351800Z

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

borkdude 2020-11-05T09:01:47.352800Z

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

2020-11-05T09:02:38.353200Z

my google fu failed me) will check one more time

2020-11-05T09:03:24.354100Z

found it

Jim Newton 2020-11-05T13:01:52.354500Z

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

Jim Newton 2020-11-05T15:10:56.358400Z

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 ...)
with two different implementations. QUESTION: Shouldn't clj-kondo warn me about this?

Darin Douglass 2020-11-05T15:16:33.358800Z

on the surface that feels like a nice linter

Jim Newton 2020-11-05T15:20:58.361300Z

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

Jim Newton 2020-11-05T15:21:06.361500Z

I'll file an issue.

Jim Newton 2020-11-05T15:27:45.361700Z

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

Jim Newton 2020-11-05T15:28:01.362200Z

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

2020-11-05T15:35:07.363400Z

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)
all in the same namespace

Darin Douglass 2020-11-05T15:35:50.363700Z

should it though is the question really

2020-11-05T15:36:00.364Z

sure it is

borkdude 2020-11-05T15:36:36.364700Z

?

Darin Douglass 2020-11-05T15:36:42.364900Z

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

1
2020-11-05T15:38:22.366300Z

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

1
2020-11-05T15:40:17.366700Z

(defmulti foo identity)
(defmethod foo :x [_] :x)
(defmethod foo :x [_] :y)
this example right now looks ok for clj-kondo but might be considered as perfect place for warning about defining different implementations for the same dispatch value of multi method

Darin Douglass 2020-11-05T15:40:46.367100Z

yeah artifacts of refactoring/updates/etc

1
borkdude 2020-11-05T15:57:28.370200Z

ok, makes sense

borkdude 2020-11-05T15:57:49.370500Z

makes sense

Jim Newton 2020-11-05T16:38:27.371800Z

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.

benny 2020-11-05T17:40:46.372800Z

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

borkdude 2020-11-05T17:42:02.373300Z

@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)

borkdude 2020-11-05T17:42:51.374Z

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

benny 2020-11-05T17:44:04.375Z

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

benny 2020-11-05T17:44:31.375500Z

it downloads graalvm11 only

borkdude 2020-11-05T17:44:57.375900Z

circleci/clojure:lein-2.8.1
has java 8 by default

benny 2020-11-05T17:47:00.377500Z

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

benny 2020-11-05T17:47:31.378200Z

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

benny 2020-11-05T17:48:18.378800Z

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

benny 2020-11-05T17:49:34.380100Z

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

borkdude 2020-11-05T17:49:34.380200Z

@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

borkdude 2020-11-05T17:50:31.380900Z

So instead of in the JVM step

benny 2020-11-05T17:51:31.381800Z

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

borkdude 2020-11-05T17:52:12.382100Z

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

borkdude 2020-11-05T17:53:13.382900Z

@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

borkdude 2020-11-05T17:53:31.383600Z

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

benny 2020-11-05T17:53:32.383700Z

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

borkdude 2020-11-05T17:53:51.384100Z

and that one has java11 right

benny 2020-11-05T17:53:55.384300Z

lein-2.9.1 comes with java11 yeah

benny 2020-11-05T18:15:02.384900Z

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

borkdude 2020-11-05T18:17:13.385700Z

@b

"$VERSION" -eq "linux" 
should probably be:
"$CLJ_KONDO_PLATFORM" -eq "linux" 
(not sure about -eq, I always use = for strings in bash)

borkdude 2020-11-05T18:17:35.386Z

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
 

borkdude 2020-11-05T18:17:49.386600Z

so you were close

benny 2020-11-05T18:18:11.387300Z

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

borkdude 2020-11-05T18:18:14.387400Z

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

Michael W 2020-11-05T18:19:15.388Z

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

benny 2020-11-05T18:19:30.388400Z

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

borkdude 2020-11-05T18:19:39.388600Z

yeah, I do it manually

borkdude 2020-11-05T18:26:13.388800Z

@b

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

borkdude 2020-11-05T18:26:29.389Z

We might want to add set -e to the script

benny 2020-11-05T18:26:42.389200Z

okay

borkdude 2020-11-05T18:26:50.389400Z

set -eo pipefail

benny 2020-11-05T18:28:45.389600Z

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

benny 2020-11-05T18:30:15.389900Z

laptop on couch is not great for concentration

benny 2020-11-05T18:33:58.390200Z

success!

borkdude 2020-11-05T18:36:02.390500Z

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

benny 2020-11-05T18:37:27.390700Z

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

borkdude 2020-11-05T18:38:26.390900Z

:thumbsup:

benny 2020-11-05T18:52:06.391100Z

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%

borkdude 2020-11-05T18:52:36.391300Z

a jar is already a zip file

😉 1
benny 2020-11-05T18:52:50.391500Z

yeah I know

benny 2020-11-05T18:53:10.391700Z

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

benny 2020-11-05T18:53:26.391900Z

while zip just reduces the filesize to 45%

borkdude 2020-11-05T18:54:23.392100Z

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

benny 2020-11-05T18:55:00.392400Z

:thumbsup:

borkdude 2020-11-05T20:20:18.392800Z

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

👍 1