Fork me on GitHub
#clj-kondo
<
2019-07-29
>
Jeevaraj MV07:07:58

It’s working fine in Ubuntu machine

borkdude07:07:24

if it's about joker, you might want to ask in #joker

Jeevaraj MV07:07:24

But I need more info on windows machine

lread20:07:10

my pleasure!

borkdude16:07:07

awesome work. some remarks:

:not-empty? false
should be
:not-empty? {:level :off}

borkdude16:07:22

it seems you are linting in two steps, one to populate the cache and one for results you want to see. this is unnecessary, you can use the config to show only results of files you're interested in

kszabo16:07:25

How does one do that? I didn’t find anything relevant under :output

kszabo16:07:00

:include-files only applies to the current dir, not for dependencies

borkdude16:07:40

can you make a repro of this? it might be a bug

borkdude16:07:46

clj-kondo has no notion of "current dir", it just lints whatever you give it, but it does handle relative paths

kszabo16:07:21

In our Jenkins build we run clj-kondo twice

kszabo16:07:50

for the cache building part we ignore errors, so dependency source issues don’t fail the build

kszabo16:07:02

then we run it again with our own lintPaths

borkdude16:07:30

you shouldn't have to, feel free to post an issue if it doesn't work like expected

kszabo16:07:06

ah, I see now. We should just set :include-files

kszabo16:07:10

the comment threw me off

lread17:07:46

@U08E8UGF7, I just tried it and it works over here. If you only want your sources included, one tip is to prefix your dirs with ./ for example:

:output {:include-files ["./src" "./test" "./modules"]}

borkdude17:07:32

you can use ^ and $, these are regexes

lread17:07:27

hmmm… ya something is off with my example. I’ll try regex.

lread17:07:43

thanks @borkdude, the following gave me expected results:

lread17:07:47

:output {:include-files [“^src” “^test” “^modules”]}

lread17:07:20

oh right… I took a shortcut, cljdocs contains a few tiny sub-projects under modules. So I build my cache with the main deps.edn and use it against the sub-projects. So… I might stick with my 2 step shenanigans for now.

lread17:07:13

worthy of a comment in my shell script anyway.

borkdude17:07:43

but why not lint all of that in one go?

borkdude17:07:26

I mean, you can just do $(clojure -Spath) src test modules?

lread17:07:37

oh gee. yeah why not? I think I might have been fixated on building that cache separately as prep step. thanks.

borkdude17:07:00

src is probably already in the path the clojur provides, so just add test and modules

borkdude17:07:58

those modules also already seem to be on the path: https://github.com/cljdoc/cljdoc/blob/master/deps.edn#L58

borkdude17:07:20

so you can just do $(clojure -C:test -Spath) maybe

lread17:07:08

hmmm… good points… oh… deps.edn does not include all modules… you’ve given me great guidance… thanks for your patience!

borkdude17:07:07

actually you need both -R and -C test, R to get the test deps and C to include the test dir itself. I don't know if there is anything in tools.deps that combines those

lread18:07:06

ok must go study -R and -C, thanks @borkdude!

borkdude18:07:40

see #tools-deps

borkdude18:07:40

one small thing: why are you creating a cache? are you re-using it somewhere later?

borkdude18:07:43

not that it matters, in CI everything is mostly a one off thing

lread12:07:33

sorry, missed this one! good question. the script is meant for developers and ci. ci won’t make use of the cache.

lread16:07:14

thanks for the tips!

borkdude16:07:19

the reason for not empty: just look in the docstring of empty?

lread16:07:31

yeah I saw that… and thought it was a bit preachy… I probably don’t understand the rationale… if that is the idiom clojure authors are recommending I should follow suit.

borkdude16:07:22

the reason is that empty? calls seq again, so (not (empty? ...)) is really (not (not (seq ...)), but then you could just write (seq ...)

lread16:07:09

that’s just an implementation detail though, no? The (not (empty? x)) reads pretty clearly to me. That said, I’m going to go with the flow and use (seq x). I’m getting more used to it as we chat about it. simple_smile

borkdude16:07:05

as for destructuring: feel free to post an issue in clj-kondo. it seems a thing people want to be doing in function arguments without getting warnings. so maybe something like {:unused-bindings {:arguments false}} but that would turn off the warning for all function arguments

lread16:07:19

I will raise a git issue and reference your question to the community on the topic.

lread22:07:22

there is a nuance I noticed: people sometimes leave in an unused :as x to describe the overall type.

lread22:07:34

I think your config is good, but might not be extensible should you want to, in the future, exclude other types of things?

borkdude22:07:42

it's extensible by just adding a new key for that scenario, but right now I haven't come across many other situations where I would want to suppress these messages. maybe the :as is a good one, but we can just fit that under this option I guess

borkdude22:07:34

joker has some options for this, but they are a bit too coarse: like, turn off everything always in function arguments

lread22:07:41

I don’t know if you need to have separate options for excluding keys and :as … do you?

borkdude22:07:22

you can always do :as _foo

borkdude22:07:43

I do that a lot actually, :as _ctx

lread22:07:17

that would work for me.

borkdude22:07:23

There is a new rule :unused-keys (defaults to true) which applies to :keys, :strs, and :syms bindings.
This is what joker calls it

borkdude22:07:41

but I would like to enable this only within function arguments

lread22:07:55

yes indeed

borkdude22:07:00

maybe just:

{:linters {:unused-binding {:destructured-fn-args false}}} ;; defaults to true

borkdude22:07:12

@lee I would do that like this:

(defmulti render (fn [page-type _route-params _cache-bundle] page-type))

(defmethod render :default
  [page-type _ _]
  (format "%s not implemented, sorry" page-type))

lread22:07:15

I’m just looking are your default clj-kondo config. You do use the the key :exclude for other things.

borkdude22:07:34

since it's very clear that the latter two args aren't used to dispatch

lread22:07:57

ya when I clj-kondofied, I prefixed with underscores. Makes sense to me, I think others would agree?

lread22:07:35

ah I see from chat above, you agree

borkdude22:07:05

I'm usually using the key :exclude for things like vars and namespaces, but :exclude [:destructured-fn-args] would be a bit weird

lread22:07:18

But in this case you might consider :exclude-checks [:destructured-fn-args]

lread22:07:39

right, agreed, if used would have to qualify

borkdude22:07:02

you can also disable the entire linter in just one namespace, using a namespace local config

borkdude22:07:40

it doesn't work for every linter yet, but if needed, I could make it work

lread22:07:02

was not aware of that, cool.

borkdude22:07:37

I didn't document it until the previous release, but it's been there for a while, mostly for my own debugging

borkdude22:07:45

afk now, zzz

lread22:07:03

later! have a good zzz

lread22:07:25

ya know what? I don’t really see anything wrong with your:

{:linters {:unused-binding {:destructured-fn-args false}}} ;; defaults to true
You might consider something a bit more descriptive:
{:linters {:unused-binding {:exclude-destructured-fn-args false}}} ;; defaults to true

lread22:07:34

while… you sleep… also: this is just for destructured key fn args though right? So maybe:

{:linters {:unused-binding {:exclude-destructured-key-fn-args false}}}