Fork me on GitHub
#clj-kondo
<
2020-03-27
>
seancorfield00:03:13

Both clojure.java.jdbc and next.jdbc have a "with transaction" style macro that has the form (with-transaction [sym expr opts] & body) -- sym is introduced by the macro, for the body (like binding or several other core macros) but if I tell kondo to :lint-as any of those macros, it then complains about opts since a binding form should be an even number of forms. Can anyone think of a core macro that has an optional extra expression in a binding like that?

seancorfield00:03:15

(I know I can turn off :unresolved-symbol inside the macro but that's a bit brutal)

seancorfield00:03:38

Also

(+ (byte 32) 1)
what is the justification for error: Expected: number, received: byte? A byte is a number.

borkdude08:03:38

@seancorfield The type warning is a bug in clj-kondo. You can override it with:

$ clj-kondo --lint - --config '{:linters {:type-mismatch {:namespaces {clojure.core {byte {:arities {1 {:ret #{:byte :number}}}}}}}}}' <<< '(+ (byte 32) 1)'
linting took 15ms, errors: 0, warnings: 0

borkdude08:03:41

I'll make an issue for it

borkdude09:03:32

@mitchell_clojure The strange issue you had was related to populating the m2 cache for the tests. It should now be fixed when you run script/test with master.

Mitch11:03:58

thanks for the followup. confirmed fixed. was it just the version mismatch?

borkdude11:03:42

I upgraded from 1.10.1 to 1.10.2-alpha1, so that was the issue

borkdude11:03:56

this version was not yet in your m2 cache probably

Mitch12:03:47

cool makes sense to me. interesting that lein/clojure didn't populate .m2 when run with the clojure-1.10.2-alpha1 profile/alias. thanks again

seancorfield16:03:48

For now I worked around this in my code, but once that drops in a release, I'll change it back.

borkdude16:03:39

cool. you can also use this config in the meanwhile:

{:linters {:type-mismatch {:namespaces {clojure.core {byte {:arities {1 {:ret #{:byte :number}}}}}}}}}

borkdude16:03:27

without having to change your code that is

seancorfield16:03:17

I already changed the code 🙂

borkdude16:03:55

note that you can also introduce type checking for arbitrary other functions you're interested in, this way. it's not used a lot yet, because it's not "for free" I guess

borkdude16:03:33

but for overriding bugs like this it's also quite handy 🙂

seancorfield16:03:45

Good to know. I will say that I am already super-impressed with clj-kondo -- it's an amazing piece of work!

👍 16
borkdude09:03:24

As for a lint-as config for clojura.java.jdbc with-transaction: clojure.core/let doesn't work unfortunately because of the opts, but I think this would be good to support out of the box in clj-kondo. Made an issue for it: https://github.com/borkdude/clj-kondo/issues/821

seancorfield21:03:52

https://github.com/borkdude/clj-kondo/pull/823 adds support for three with-db-* macros in clojure.java.jdbc and also next.jdbc/with-transaction

borkdude22:03:31

Excellent. Reading now.

borkdude22:03:07

Made a few comments. Overall, very good!

borkdude22:03:11

The way I usually test "findings" is to first call (prn (lint! "...")) and then paste that output back into the test file and test it with (assert-submaps the-output (lint! "..."))

borkdude22:03:55

I'll paste that in the issue.

borkdude22:03:12

Merged. Thanks a lot. Do you need a new binary with this included or did you figure out how to build it yourself?

seancorfield00:03:25

I will just wait until you make a new release.

seancorfield00:03:54

(and, thank you again, for an excellent project and also for taking the time to provide feedback in the PR)

borkdude09:03:08

It can take a while since there are plenty other enhancements to make. Until then you can use the :unresolved-symbol config. If you want to speed up things, you could even try to contribute 🙂

seancorfield16:03:23

I'm off work today (I often take Fridays off, so I can have a 3 day weekend) so maybe I'll take a look as part of my "OSS Friday" routine 🙂

seancorfield16:03:58

Any particular guidance for how you like PRs packaged up? Code style? Tests?

seancorfield17:03:12

Yup. Already reading it and I've forked the repo 🙂

seancorfield15:03:54

@borkdude thank you for the swift responses!

borkdude15:03:56

@seancorfield no problem. fwiw I have this in the config of the app I'm now working on:

:lint-as {clojure.java.jdbc/with-db-transaction clojure.core/let
           schema.core/defschema clojure.core/def
           reagent.core/with-let clojure.core/let}
but we're not using any opts in with-db-transaction

borkdude16:03:26

I guess I can remove the config for schema.core/defschema, that's already supported for a while

seancorfield16:03:55

Yeah, both clojure.java.jdbc/with-db-transaction and next.jdbc/with-transaction are like when-let when you don't provide options (only one binding allowed, so not really like let). I've never been happy with the syntax tho', because of this not-quite-let style.

borkdude16:03:43

with-open also works I guess

seancorfield16:03:45

Good to know. I will say that I am already super-impressed with clj-kondo -- it's an amazing piece of work!

👍 16