Fork me on GitHub
#clj-kondo
<
2021-11-19
>
borkdude20:11:29

I'm considering to automatically opt in to configs that are found in:

.clj-kondo/<org>/</lib>/config.edn
From a usability standpoint this would be much better. But there might be some dangers to this. E.g. a library could accidentally export a config which will make clj-kondo crash or get stuck in a loop while expanding a macro or so. Any thoughts?

đź‘Ť 2
dominicm20:12:00

I would love this, is there an issue I can upvote?

borkdude20:12:40

@U09LZR36F I don't think there is one yet. I'll create one now.

dominicm20:12:29

Fwiw, my opinion on the security of this is that you're already loading my code I can be as malicious as I want 🙂

dominicm20:12:40

I could modify your .clj-kondo file and add the infinite loop

borkdude20:12:46

from which code?

dominicm20:12:15

If you've got my library on your cp, you're likely doing a require on it anyway.

borkdude20:12:00

clj-kondo only executes hook code

borkdude20:12:07

and hook code doesn't have access to the file system

borkdude20:12:28

until now hook code was always opt-in

borkdude20:12:34

but 1483 might automate that a bit

borkdude20:11:12

Perhaps this should be an option: :auto-include-configs true which can be set to false in case of difficulties?

lread20:11:18

Yeah, that seems good to me. So the only way exports get imported is if --copy-configs and --dependencies option are specified. So it seems like a deliberate action in itself. Then again, I think clojure-lsp calls with these options so maybe not so obvious in this case.

borkdude20:11:22

We might also deprecate --copy-configs and do this automatically now

lread20:11:50

Right… yeah.

borkdude20:11:58

Since lsp already does this all the time, there are lots of users that have already opted in without knowing

lread20:11:47

Yeah, so long as there is a way to opt out, auto-opt-in seems like a usability plus.

lread20:11:16

Potemkin-import-vars-like-ish?

borkdude20:11:02

-ish yeah, you write (sci/copy-ns <ns-sym> <sci-object> <opts>) but since it's a macro you don't quote <ns-sym> and <opts>

borkdude20:11:11

and you would get red squirlies for those in some cases

borkdude20:11:47

this is for copying an entire Clojure ns to the SCI context

borkdude20:11:10

which in some cases even works for macros in CLJS

lread20:11:50

That’ll be handy!

borkdude20:11:52

let's discuss in #sci for follow up on the specifics if you want to know more

lread20:11:30

I suppose with auto-opt-in for imported configs… the only risk of a support headache would be an errant hook. But I suppose you could blacklist a bad hook if it comes to that. Which it probably won’t.

lread21:11:04

Another option would be to emit clj-kondo warnings for hooks that are present but not activated.

lread21:11:14

If you want to be more cautious.

borkdude21:11:45

I'm thinking about the LSP use case. Do users see those warnings? I usually don't in emacs, unless I go look into some dark corner of some logs

lread21:11:03

Ah, as opposed to squigglies in place? I often show flycheck error list. But there’s a chance I might be odd.

borkdude21:11:39

lsp doesn't work via flycheck

borkdude21:11:06

clj-kondo could make info warnings on the first line or so: you have this or that config not activated

borkdude21:11:11

but I'm not sure if that's the way to go

lread21:11:25

Yeah, certainly not as easy for the user.

borkdude21:11:03

maybe the automatic feature should be opt-in then

borkdude21:11:04

for a while

borkdude21:11:17

and after that while, we can re-consider

lread21:11:45

Not sure why but I do see lsp warnings. I use spacemacs.

borkdude21:11:53

Perhaps @UKFSJSM38 could enlighten us.

lread21:11:00

He often does! simple_smile

ericdallo21:11:12

yeah, that looks lsp-mode is working indeed with clojure-lsp

ericdallo21:11:31

actually, lsp-mode use flycheck by default, you can use flymake if you want

ericdallo21:11:17

there is a custom code on lsp-mode that register a lsp-flycheck, all LSP diagnostics use that: ref: https://github.com/emacs-lsp/lsp-mode/blob/master/lsp-diagnostics.el#L36

borkdude22:11:38

I'm also using flycheck.. I think

borkdude22:11:50

at least for clj-kondo

borkdude22:11:10

but I was thinking about errors here

borkdude22:11:29

runtime errors... lsp logs them to a file but doesn't show them as diagnostics

ericdallo22:11:43

what kind of runtime errors?

borkdude22:11:02

null pointer exceptions for example

ericdallo22:11:36

yeah, we only show on log indeed, not sure it makes sense to use diagnostics feature for NPE that could happen anywhere on the code

ericdallo22:11:56

for example a NPE during the process of lsp-lens

borkdude22:11:59

I'm also not sure

ericdallo22:11:56

Usually, LSP diagnostics are related to user's code, a NPE means a clojure-lsp or third error, user don't care about that, but if something stops working and he wants to understand he should seek the logs I think

ericdallo22:11:27

or at least we catch the excpetion, log and show a pretty message like "Some error ocurred"

ericdallo22:11:45

we already support that custom friendly message, we just show for specific actions

ericdallo22:11:21

like user tries to rename something, and we can't for some reason, we return that message explanining why we couldn't rename

borkdude22:11:13

clj-kondo does show some errors as lint messages

borkdude22:11:19

like: parse error such and so

ericdallo22:11:44

yeah, but they are related with a error in user's code right

borkdude22:11:06

or with the config

ericdallo22:11:17

I got your point, I just think LSP showMessage would be something that should fit better

ericdallo21:11:47

sorry for the delay: > Since lsp already does this all the time, there are lots of users that have already opted in without knowing Yes, having that enabled by default would be really good for users IMO, the corner case probably could be solved with a option to disable a hook or something like you mentioned > I'm thinking about the LSP use case. Do users see those warnings? I usually don't in emacs, unless I go look into some dark corner of some logs Yeah, they will only see the warnings if they search on the logs, is not something we show activelly for the users.

ericdallo21:11:13

@borkdude at Nubank we created a common lib that has all necessary clj-kondo config-paths and all services use that, with that we avoid the need to add that config for each service, it works really well, so IMHO I'd go with a default true flag which could be opt-out