Clojurians Log v2
clj-kondo
A common hack I use when using :deprecated metadata, is putting info in the string pointing at the var to use instead, kinda like https://guide.clojure.style/#superseded-by. However, I don't think clj-kondo knows about this piece of metadata though. Is superseded-by something that clj-kondo could look at when generating the message for deprecated vars?
It only looks at the value of :deprecated so you could put "Superseded by .." in that string and it will be displayed in the message
Yeah, that makes sense. I was just thinking it would be cool if the generated message could reference superseded-by automatically for me, since they are so tightly related.
If this is something you think is worth doing, I'm also down to try and add the functionality.
I didn't know it was that common, thanks for pointing that out. https://github.com/search?q=%22%3Asuperseded-by%22+language%3AClojure&type=code&l=Clojure Yes, issue + PR welcome
I do think it might not been super popular, but more support should encourage more usage. Do you mind providing some pointers on where in the code I should start looking to add this?
yeah, just look at how deprecated var and ns is implemented, search for :deprecated-var for example. it should be pretty obvious
and if you have any questions, let me know
Awesome, thanks 🙏
Just stumbled on an interesting bit of code that clj-kondo should probably flag with a warning. After doing a bunch of consolidation of some code into a single namespace and renaming things, etc., etc. (imagine lots of hammering and power tool sounds), I ended up with some cruft in the ns form of one namespace that clj-kondo passed over without flagging anything. Essentially, I had a reference to the namespace I was defining in its own :require form. Simplified:
(ns bar.foo
(:require [bar.foo :as foo]))done
It already warns about this:
maybe not by default:
:self-requiring-namespace {:level :warning}we could make that the default I guess
OK. Yea, I’m using default settings, so it wasn’t saying anything about it. What is the criteria for making something checked by default or not? Or to put it another way, why not just enable almost everything by default, unless something is particularly noisy?
With non-critical things I usually don't enable them right away by default so I can test drive them a bit longer before bothering people with false positives, but this one has been around for a while, we could probably flip it to default
Makes sense.
I'll make a note and do it tomorrow