There are better alternatives for :skip-comments now: Try :config-in-comment and then disable anything you would not like to see in comment forms

ok nice i will try that thanks!

here goes a 160,000 line linter rework 😅 oh phew only 32 errors 49 warnings

wow this is a MUCH nicer design, kudos!

ok these are working great:

:unresolved-namespace         {:level :off}
:unresolved-symbol            {:level :off}
:unresolved-var               {:level :off}
:duplicate-require            {:level :off}
:unsorted-required-namespaces {:level :off}

:unused-namespace             {:level :off}
:unused-binding               {:level :off}
:redefined-var                {:level :off}

@U04V15CAJ

redefined var should already be disabled in comment

seems it's enabled. sense-checking what version of kondo i have

2022.09.08 should def have it right

This is unused public var, not redefined var

and unused public var isn't implemented by kondo, but by lsp on top of kondo analysis

damn it sorry

np ;)

oooh ok interesting edge case

you can make it up by blogging about your learnings :P

i have (comment (def xx)) followed by (defn xx) and it's complaining that the second one is a redef

not sure what to do about that

i don't think kondo should do anything, it's easy enough to rectify - use another name in the comment

@U04V15CAJ should i be able to :level :off unused-namespace and :unused-binding? the docs don't explicitly say so, but they do indicate a 'default level of :warning' which suggests that it can be altered. i have done so but i am still getting both warning types

do you mean inside comment?

yes

:config-in-comment {:linters {:unresolved-namespace         {:level :off}
                               :unresolved-symbol            {:level :off}
                               :unresolved-var               {:level :off}
                               :duplicate-require            {:level :off}
                               :unsorted-required-namespaces {:level :off}
                               ;; not working
                               :unused-namespace             {:level :off}
                               :unused-binding               {:level :off}}}

but

Ah I see. you might need to submit issues for those

of course you can suppress with #_:clj-kondo/ignore too

ok cool, i submitted an issue https://github.com/clj-kondo/clj-kondo/issues/1826

thank you again for this wonderful tool. can't imagine coding Clojure without it!

Seems similar to a existing issues where it doesn't work on config-in-ns as well

Does this also happen when not connected to a REPL?

I have a similar feature in emacs but I'm never sure which tool does this, I suspected clj-refactor

We’re not using clj-refactor in Calva.

I know, but you are using parts of CIDER/orchard which may also do this (as I said, I'm never sure which part does what)

could be lsp as well, but this is why I asked about the REPL

We’re not using any refactoring features of cider-nrepl and are not loading the refactor-nrepl middleware.

cool :male-detective:

Maybe it's clojure-lsp autocompletion which will include the require for you, but since the user selected that it was intended to add that. Maybe a gif to confirm that?

the feature is called add-missing-libspec in clojure-lsp

(same name in clj-refactor)

add-missing-libspec is a common/code action, it will only be applied if manually triggered, not automatically

but clients could apply code actions automatically. not sure if calva does, @pez?

Calva is not doing this, but it could be that VS Code does something around that. Thinking about the OP. Maybe VS Code has some smartness around offering to apply code actions ...

I don't think vscode automatically apply code actions, but it's worth check

There actually is a setting editor.codeActionsOnSave with two options, one of which is organizeImports. Maybe that is what you have enabled, @U04V70XH6?

Doesn't sound like organizing imports should add requires, though...

I would expect organizeImports to use clean-ns rather than add-missing-libspec

But maybe if it's set to source.fixAll ? 😃

We don't have any code actions with source.fixAll (yet)

does :quick-fix go in the '`fixAll`' bucket though?

@pez I do not have editor.codeActionsOnSave in my settings JSON. This happens without saving, just by typing code. @U04V15CAJ I always have a REPL running/connected but this feels like Calva/LSP to me.

you can debug lsp side via logs, but my guess would be something on repl side

OK, well, since no one seems to know what is causing this, I'm going to blow away the lsp db and have it recreated and I'll try to work for a bit without a running REPL (oh, the horror!) to see if I can isolate it to something specific -- but I often don't notice the new ns has been inserted until some time later. One data point that might be useful: when these "unexpected" ns requires are added, they appear at the end of the :require clause (and therefore often out of order), but when I explicitly use code fix to add a require to match an alias in a newly-added call, that ns gets added in alpha order. Does that ring any bells? Adding an ns at the end of :require instead of in order?

One more reason to think it's a REPL thing, clojure-lsp by default always sort ns after adding a require.

I would be majorly surprised if this was a REPL thing. 😃

OK. Not REPL related. I deleted the .lsp/.cache folder and restarted VS Code. I'll see if I can get this down to a smaller piece of code but here's what I'm seeing: 1. add a comment at the end of your code 2. add a require of some ns with an alias -- that is not in the main ns at the top of the file 3. start to type a new function call using that alias -- and the same ns is added to the end of the :require in the main ns

Is it some advanced snippet thing, maybe?

I can reproduce it.

The ns insert occurs when I accept an auto-complete suggestion from the newly-aliased ns. Here's the before code:

(ns ws.wsql.example)
(comment
  (require '[ws.newrelic.interface :as newrelic]))

(ns ws.wsql.example)
(comment
  (require '[ws.newrelic.interface :as newrelic])
  (newrelic/))

(ns ws.wsql.example
  (:require [ws.newrelic.interface :as newrelic]))
(comment
  (require '[ws.newrelic.interface :as newrelic])
  (newrelic/ignore-appdex))

Me too, the issue is on clojure-lsp side, it's the completion as I mentioned https://clojurians.slack.com/archives/CPABC1H61/p1664880134031669?thread_ts=1664877429.715259&cid=CPABC1H61

Right, and I think at some point it asked me whether to do that automatically or not -- and I said yes -- but now I can't find out how to reset that automation...?

But it's wrong either way since a) it's in an RCF and b) the alias/require is already present in the RCF 🙂

Looks like so for the record.

And this insertion of the ns does not sort the :require after insertion.

By pure accident, my recording shows that too. 😃

> but now I can't find out how to reset that automation...? At least on lsp side we don't have any automation like that > And this insertion of the ns does not sort the :require after insertion. Yeah, it seems I missed this one, it's the only one not doing that, we should fix that as well

Is it happening in Emacs too, or is it VS Code or Calva doing something funny?

emacs too

So this can't be turned off?

(is this a recently-added refactoring/auto-completion? it wasn't happening until fairly recently)

It can't be turned off ATM, we did touch that code recently IIRC c/c @U0BUV7XSA

its been like that since the beginning. I dont think that completions that trigger the add require have really been expanded within the last year or so, only what they require. I have noticed some changes between how we analyze within comment and ignore forms that could change this instance, but afaik we've tried to be consistent.

Well... this hadn't used to happen... I use RCFs containing require all over the place: I would've have noticed this happening before. So I think it's a recent change, even if it was unintentional.

Now I know it's happening, I can keep an eye out for it. But it's kind of annoying 🙂

Just checking on clojure-lsp side if it's inside a comment would probably be enough, but yeah, we've been doing that all the time and never had any complains until now

yeah, in comments, about half the time i like it and half the time I don't. A setting to either add to ns, add a plain (require...to top of comment, or do nothing might be useful.

I'll try to wind back across earlier versions of LSP maybe this weekend but this definitely was not happening until fairly recently.

I don’t want requires to be automatically added for me, regardless if in RCF or not.

Yeah... and I swear that it used to ask me whether to do it at some point and I got tired of saying no... 🙂

I know why it wasn’t happening before… clojure-lsp’s completions have always contained ns edits. But because of a long-standing https://github.com/clojure-lsp/clojure-lsp/issues/728 in clojure-lsp, Calva ignored those edits. In August we https://github.com/clojure-lsp/clojure-lsp/pull/1069 completions so that ns edits are calculated in completionItem/resolve, and in the process fixed the bug. So, as Calva users, you’re finally seeing something that all the other editors have had for a long time

Good catch!!

I don’t think it resolves anything, but it’s interesting to know. As I see it, ideally we’d have two independent settings: 1. How should a namespace that’s used only in a comment form be required? In the ns’s :require, or in a require at the top of the comment? I think this setting should be independent of how the namespace is auto-required—either via a code action, or via completion. 2. Should completion auto-require namespaces at all?

1. Sounds like a nice new setting 2. Yes, I remember @U0BUV7XSA saying it's something that he uses and I find it useful as well, besides this comment issue, I always want to require the ns if I choose that completion for that alias

In relation to 2, I like the feature too @UKFSJSM38, but I think @U04V70XH6 is saying that he never wants completion to trigger a require, which is why I think it should be a setting

Or rather, @pez is saying that: https://clojurians.slack.com/archives/CPABC1H61/p1664914342638219?thread_ts=1664877429.715259&cid=CPABC1H61

@U07M2C8TT Thanks for tracking that down. OK, so I'm not crazy! Hahaha... So I'd be happy if there was a setting that allowed me to say that missing aliases/requires inside comment should be added as a require inside that comment block. That would be awesome. I'd settle for a setting that just let me turn the ns insertion off, since I can get to it easily with ctrl+. -- and I do find sometimes that LSP picks the wrong ns (in Polylith, it's common to have the same function in both <top-ns>.<component>.interface and <top-ns>.<component>.impl and LSP sometimes picks .impl which is nearly always wrong... but sometimes right).

@U04V70XH6 for polylith case, clojure-lsp at least suggests both options via code action right?

@UKFSJSM38 Oh yes, and .interface is usually the first choice (which surprised me since it's not in alpha order 🙂 ) so ctrl+. enter is nearly always enough for me.

Yeah, I'm not sure we sort it by usages, but sounds like a good idea if not

i think that might be due to getting the x 1 on all the add require code actions. seems like we arent counting usages correctly

I’d love if the autocomplete would add a missing require in the RCF. I can move it to the ns form myself later if I want it there. My ns forms are often untidy with unused requires, so I most often don’t want it auto-cleaned.

Oh I'm merciless about ns forms: removing unused stuff, strictly alpha-ordering everything. We even have a little Clojure script in our repo at work that walks over our entire source tree, ensuring nses have sorted :require clauses and sorted :refer clauses (can't remember whether we also sort :import but I think we do). Comes in very handy after I've done some massive find'n'replace as part of the migration to Polylith!

Then a behaviour that when lsp adds a missing require to the ns form, it inserts it alphabetically, would work for both of us. 😃 Regardless if we choose to do it as part of autocompletion or as a quick-fix action.

@U04V70XH6 curious about that script, do you use clojure-lsp clean-ns for that? We use on all Nubank services to do the same thing but also using clojure-lsp format and clojure-lsp diagnostics as the main lint

Anyway, I see oportunities here of improvement indeed: • setting for auto insert ns on completion • Fix the add-require code actions sort if it's wrong indeed Please create an issue for each one if you have the time

@UKFSJSM38 The script is pure Clojure. My team mate doesn't use LSP.

I see, I suggest taking a look at how run clojure-lsp as CLI for your CI: https://clojure-lsp.io/api/

Two issues filed: • https://github.com/clojure-lsp/clojure-lsp/issues/1306 • https://github.com/clojure-lsp/clojure-lsp/issues/1307

BTW, the TypeScript LSP also automatically imports stuff for me and it drives me nuts. 😃

Do we also need an issue for configuring what to do in an RCF—add to require in comment, or add to :require in ns?

I think so, sounds like a different problem/feature suggestion

Our CI does not make/commit code changes so I wouldn't want CI to sort ns requires.

The --dry flag is exactly for that :) just report issues not touch anything

at Nubank we use it as lint and we also have lint-fix in case anyone wants to just fix existing fixable issues

Not sure I want to irritate my team mate by failing CI just because he didn't sort his requires 🙂 I usually catch it in PR reviews (which was what led to him writing the Clojure script in the first place!) and I have clj-kondo setup so unordered requires are :error level for VS Code 🙂

My point is that you can just replace the clojure script with that :) and avoid re-work

if you want or not to use for failing a CI or fix locally, that's your call

I would have to change his workflow for that.

We already have the Clojure script 😛

@U04V70XH6 I don’t think @UKFSJSM38 is proposing you change anyone’s workflow, or change CI. He’s just saying that perhaps you could simplify your script by making it invoke the shell command clojure-lsp clean-ns, which does the same thing as what you described: > We even have a little Clojure script in our repo at work that walks over our entire source tree, ensuring nses have sorted :require clauses and sorted :refer clauses

Parsing ns was the reason I started LSP. Thinking about all these editors, plugins, and scripts that were trying to figure out how to sort/clean-ns, all of them doing some things well and some things poorly, having a shared community resource that could do that was a huge driving factor for putting this project out.

thank you both for elaborating :)

"making it invoke the shell command clojure-lsp clean-ns" -- that assumes my team mate has clojure-lsp installed. Which I don't know.

We don't place any requirements on devs' setups. He uses a very barebones Emacs setup and a REPL. I don't know whether he even bothers with inferior mode. I have the much fancier VS Code setup. We try hard to make sure any common tooling that we all need is vendored into our repo so "everyone" has access to it, but we also try to keep that common tooling as slim as possible.

You could have a custom deps/lein alias that triggers clojure-lsp API to do the same, so no need to install clojure-lsp on any machine

If our team was bigger, I'd probably try to "encourage" more standardization of editor tooling across the board (we used to do that when we had a bigger team and more junior devs learning Clojure but that was nearly a decade ago).

lol, no worries @U04V70XH6 just trying to give a perspective.

@UKFSJSM38 do you have an example of what that looks like?

The different ways of invoking clojure-lsp—from an editor over the LSP protocol, from a shell script, or as Clojure code—aren’t familiar to everyone

@UKFSJSM38 I've set a reminder in Slack to take a look at the LSP API for deps.edn next week at work and maybe I'll get to retire our custom "ns sort" script in the name of incorporating it into our build.clj script 🙂 (I just retired a few shell scripts this week by doing that). I'm pretty sure I can get everyone using clojure -T:build sort-ns 🙂

yeah @U07M2C8TT, we should improve the docs mentioning more clearly all those possibilities indeed, my attempt for that was https://clojure-lsp.io/api/

Awesome @U04V70XH6, please let us know if any issues, at nubank we are using both https://github.com/clojure-lsp/lein-clojure-lsp since most services are still lein, but already using clojure-lsp with deps.edn, I know other companies are already using with deps, @UMMMKKADU configured that for https://github.com/180seg IIRC

Third issue: https://github.com/clojure-lsp/clojure-lsp/issues/1316