Clojurians Log v2

I noticed that kondo doesn't warn when the same arity is repeated. Pretty minor as Clojure catches it.

(defn foo
  ([x] x)
  ([x] (inc x)))

issue welcome!

Hiya! Has anyone ever asked to warn on usage of prefix lists in namespaces? https://stuartsierra.com/2016/clojure-how-to-ns.html#prefix-lists

@lee I think this will be part of the next release

Awesome!

(already on master)

oh sorry, you mean, warn on usage of these things at all?

yeah

there are some lint warnings about invalid prefix lists

recently added

but not the one you asked for

like follow Stuart’s advice and avoid using them at all

yeah, feel free to add an issue. I think not using them is preferable

ok, thanks, will do

I'd caution against clj-kondo becoming too much like CheckStyle

Catching mistakes and bugs are good, no one will ever complain. Forcing your opinions on a particular style are bad, and people will start to hold grudges and discontent against you 😛

Yeah but they can be off by default.

For style concerns, focus on automated-styling tools instead. Like a tool that automatically rewrites the ns declaration to your agreed team style

We already have some style linters.

like: missing else branch on if

@didibus I don't agree. Clj-kondo checks many style things and almost all of them are disabled by default if they are controversial. This is one of the core things of clj-kondo and adding more makes sense, even if not all people use them.

I wouldn't call that a style linter. That can actually be a bug, and I've had it be a bug before where that warning helped me catch the fact my parenthesis were wrong so I was missing an else branch.

If they are disabled by default its a lot better. That said, I would be worried of seeing a thing where like the default lein template drops a pre-configured clj-kondo config into a project with some forced def "style" linters turned on, etc.

I kinda just hope in my dreams that Clojure doesn't devolve to Java's shenanigans. And that for styling we'd focus efforts on re-formatting tools.

But hey, I have CheckStyle PTSD, this is just my 2 cents.

Like a linter is defined as: > lint, or a linter, is a static code analysis tool used to flag programming errors, bugs, stylistic errors, and suspicious constructs And I think this is all good. The "stylistic error" part I guess is maybe the more controversial, but ya if those are off and can be customized I'll be happy. > Checkstyle is a static code analysis tool used in software development for checking if Java source code is compliant with specified coding rules This is bad bad in my opinion 😛

I get your point, don't worry. Most if not all of the opinionated things going into clj-kondo since a while has been disabled by default.

Ya, I've been very happy with the choices you've made and have full confidence.

Apology, I think my comment came off as more negative, I wanted to emphasize on the alternative more so than on shutting down the use of clj-kondo for style. Like a team can have their own conventions and I have no issue with that. But what is even more awesome than having something tell you you didn't do things the way the team wants them, is a tool that fixes it automatically for you.

You’ll just be annoyed if submitting PRs to my projects because I’ll have all the linters on. (just teasing)

One tool that fixes it automatically for you is clojure-lsp.

It's using clj-kondo under the hood and will fix it using your editor integrations.

And there are other tools like carve which also do this.

Ya, I think clj-kondo would be a great underlying lib to leverage for that

The focus of clj-kondo is code analysis and linting (including style), but it won't provide rewriting

Just the output should be enough to write tools on top of this

Totally make sense.

The custom analysis seem to be a good start for that.

Thanks for test-driving the :unresolved-var linter.

That said, there is one difference. If the linter is: warn on use of "bad style", it is not letting you choose your preferred style, it is making an opinionated choice on what style is good and bad. Like what if I prefer all my ns to be written the other way? (ns foo (:require [a.b c d e g]))

As I write out the issue, I’m remembering that prefix lists are invalid in cljs. I’ll make a note in the issue.

Ah, now that's a good observation 😛 See now you could almost argue it should be a warn by default, since its now in the category of possible error

I knew I’d win you over. :simple_smile:

😛

Ya, I guess its just nuanced. @borkdude has had great instinct I think to put the line in the right place. CheckStyle on the other hand has not.

Like the missing-docstring linter being applied only to public var. Like that's a pretty good way to handle it. Where as in CheckStyle this is one of my most hated linters, but that's because it forces everything to have a docstring. So you'll see stupid things like:

// The customer id
private String customerId;

Also depends how you use it too. CheckStyle is slow to run, and can't be integrated in your editor (well it can but not in a way you get feedback as you type). So people always set it up as a build step or a PR approval step. Pretty frustrating to have it fail when you're ready to push because your private customerId field doesn't have a doc-string. But clj-kondo gives you instant feedback, so I think that helps a lot with frustration. And you could easily set it up to only fail on error, so warns can be a recommendation, like "Probably have a doc-string here, unless the doc-string is a copy/paste of the var name with The added in front 😛"

https://github.com/clj-kondo/clj-kondo/issues/1137, @didibus I was even so cheeky as to include your require example above as my code snippit.

@didibus I sympathize with your PTSD from silly CheckStyle rules. I vaguely remember them too, but the memory for me is fading, and the healing is almost complete.

Ya, CheckStyle has some stupid rules, especially when combined with Java, makes it really annoying. In Clojure, sometimes "style" enforcement is more like: Clojure is so loose, that you can't even remember what "style" is a bug and which one is not. Like using :else in cond. That linter is nice, not because it forces me to use :else and not :default, but because I actually never remember if I'm supposed to use some specific keyword or not.

And then you do get a lot of weird stylistic errors, like not knowing that I could easily think this makes sense:

(cond
  (pred? a)
  (foo a)
  :else
  (bar a)
  :default
  (baz a))

By the way, hum, is that a linter? Would be a good one to have: Unreachable cond branch. Or something like that

@didibus

$ clj-kondo --lint /tmp/foo.clj
/tmp/foo.clj:2:4: error: Unresolved symbol: pred?
/tmp/foo.clj:2:10: error: Unresolved symbol: a
/tmp/foo.clj:3:4: error: Unresolved symbol: foo
/tmp/foo.clj:5:4: error: Unresolved symbol: bar
/tmp/foo.clj:6:3: warning: unreachable code
/tmp/foo.clj:6:3: warning: use :else as the catch-all test expression in cond
/tmp/foo.clj:7:4: error: Unresolved symbol: baz
linting took 130ms, errors: 5, warnings: 2

> warning: unreachable code Nice!

I'm super excited for the unresolved var linter. Thanks so much for putting the work on it by the way!

A number1 complaint I see from people is always: Refactoring in Clojure is hard cause you can't tell what you broke. And so that linter I think is a huge step to help with that.

A few small items to fix and hopefully a release tomorrow or in the weekend... 🤞

one important thing: it should probably support a config to exclude certain vars or entire namespaces

e.g. namespaces that have a lot of auto-generated vars using macros

Yes, I was also wondering, would it work with hooks? Like if I have a hook for a macro that returns:

(do (defn foo ...)
   (def bar ...))

yes

at least, it should

Cool, ya that's great then. I think most macros that generate vars do return a form close to that. So hopefully as more macros provide a hook, the number of vars/namespaces you need to exclude would get smaller.

there is also lint-as and clj-kondo.lint-as/def-catch-all, so in many cases no hook needed

Oh ya, that's true

Even better

but it still takes effort to configure of course. I was thinking we could also have a runtime clj-kondo plugin that scans your project and emits cache info that can then be used by static analysis

so you would at least not get false positives for unresolved vars

Hum... So like it would learn as the code is running? You could have a nRepl middleware maybe. And a way to set it up in your test-runner.

Though there's a disadvantage to that as well. Which is the same with the REPL, its possible that you defed something that you later deleted from the code.

yeah, I think you would run it only occassionally from the command line as a JVM CLI tool

to update the un-analyzable vars

I think if all it did was like load everything and let macros macro-expand. That would make sense.

yes, it would only load namespaces, then all-ns, ns-publics, and emit

Ya that would be like automatic hooks. And it only wouldn't work if you're doing side effect, but in practice it would also work, cause you'd know that when running that, just like running your tests, you need to make sure you have the right environment setup.

Its a good idea

a bit similar: https://github.com/clj-kondo/inspector

I guess it only leaves this:

(ns foo)

(defn foo []
  (def f (fn [a b] (+ a b))))

(ns other (:require [foo :as foo]))

(foo/f 1 2)

Oh that's nice.

Why does it need to be at runtime? When you call emit-types it walks the spec registry?

yes

Ya, that's neat. I guess then this say runtime-analyze tool could do both: load namespaces, then all-ns, ns-publics, and emit cache and emit types.

Why is that an edge case? This could be seen at runtime as well?

For specs though, in theory (and I get that its more work), can't you figure out what the specs are from the require and walking through the namespaces for calls to s/def and s/fdef ?

Like statically

Well, in theory, every run of the program is not guaranteed to have the same defined vars

Specs are quite difficult to analyze accurately statically as they can have many indirections, although clj-kondo could do a best-effort attempt

Hum, ya that's true. It does sound tricky since they can refer to others with namespace keys and all.

you mean, if you conditionally define vars based on the weather?

It can be done, but a lot of work so that's why I went with the runtime approach, just as an experiment

Malli does a similar thing

Spec still being alpha, I don't want to build this into any tools yet

Hum... actually I think you are correct, I don't think this is even possible, cause the symbol passed to def has to be static.

Like you can't do:

(defn foo [username]
  (def username {}))

you can use intern to do this dynamically

I guess you could with a macro:

(defmacro create-user [username]
  `(def ~(symbol username) {}))

(if (< 20 degrees-fahrenheit) (intern 'foo.bar 'baz 1) (intern 'foo.baz 'bar 2))

even with the macro, the input to the macro is static

Ya makes sense. I feel spec 2 is actively being worked on as well so.

I guess you could also use (eval ...)

so barring intern and eval, most of the var names can be statically known, and surely at runtime

Oh ya, you're right. Ok, so only with intern could someone do this? Or is it just not possible?

Ya, I mean, if you're doing eval or intern, you know what you are doing, and won't mind excluding your namespace I think.

or excluding only certain vars

Well, I mean, if you're doing a dynamic var def like I'm describing, you don't know what the vars will be because they are based on external input.

hugsql is an example that comes to mind. but I know a nice trick for that: Have a dedicated foo.domain.hugsql namespace which is only dedicated to turning sql into vars, without any other fns. This namespace doesn't end up in your cache (because it has no statically analyzable vars) so it won't give you any errors in the unresolved-var linter.

I discovered this today at work ;)

Ok ya, that is a good example

If you had that runtime caching tool, it would be able to learn the vars generated from hugsql right?

yes

I think even a hook for hugsql could work here. If the hook is willing to do a "side-effect". In theory, the hugsql hook could be smart enough to say: "Warn can't find sql files". And if it can find them, to return the defs from it

Unless people often put their SQL definition in a DB or something like that, but I don't think so. They're generally a resource I think

Anyway, ya its a pretty good idea to have a runtime caching system. Seems it could cover a lot of those edge cases.

And still, I feel for all these edge cases, its not that big a deal to exclude the namespace or the vars from the unused-var linter.

yeah, it's kind of a last resort that should justify itself with enough frustration and so far the frustration hasn't been big enough yet ;)