Fork me on GitHub
#clj-kondo
<
2021-03-18
>
leoiacovini14:03:08

Hey folks 👋 , @ericdallo and I have been experimenting with hooks recently, and I found some behaviors that were not completely clear to me, and for several folks at work, and wanted to discuss a bit with you if this makes sense: • Apparently, hooks are only copied from the libs classpath when running cl-kondo with the --no-warnings flag, this make editors that uses the clojure-lsp not install the custom hooks when starting up, which is very misleading in a lot of cases • The interaction between project specific .clj-kondodirection and the home one is not very clear, I found that having some custom config.edn per project, makes the hooks declared in the home folder to not work correctly • Download hooks are versionated, and it’s not clear for me if they are re-copied when I bump my lib version. This could be a problem when working with projects that have different hooks definitions depending on the lib version I see that mostly of those problems comes from the strategy to copy the hooks into somewhere else, using a manual process, Ideally I think that would be cool if clj-kondo would be able to fetch the hooks definition from the classpath at runtime (does that make sense?), but probably tis has several complexities that I am not aware

borkdude14:03:43

@leoiacovini - afaik clojure-lsp already uses the --no-warnings flag when initially linting the classpath, so this should work exactly the same. - hooks are supported in the home directory, but hook code is always resolved relative to the config file it was declared it - when hook code is fetched from the classpath it will overwrite current hook code, so updating is automatic

leoiacovini15:03:28

1. :thinking_face: I am pretty sure it isn’t working then, because we got several people at work (including myself) not having it copied when starting the clojure-lsp, cc/ @ericdallo 2. I have observed that If I have .clj-kondo in my project (to override/add project specific configs) the hooks ended up being download inside it (I need to redo some tests about how the configs were, but I remember being a bit lost at the time) 3. Do you think that having it versionated (like ~/.m2) by the library version that exported it makes sense? Or the always override approach should be enough?

borkdude15:03:48

1. clj-kondo only repro welcome 2. same 3. I don't see why this is needed, since the hook code comes from your project dependencies. if you would have different versions of hook code, then your clj-kondo.edn would also have to change.

ericdallo15:03:58

1. We don't use any no-warning flag @U04V15CAJ: https://github.com/clojure-lsp/clojure-lsp/blob/master/src/clojure_lsp/config.clj#L16-L33, should we pass a :no-warnings ? Is this will return the findings and analysis correctly ?

borkdude15:03:39

@ericdallo clj-kondo assumes you are going to lint your dependencies once, to get arity info for those. and in this phase you are not interested in findings, but you are possibly interested in hook code from the deps. this is why :no-warnings has this effect

ericdallo15:03:29

it makes sense, so the first scan when project start make sense to pass the :no-warnings , right?

borkdude15:03:53

yes, but you won't get any findings then

borkdude15:03:26

so ideally you would split it into two phases: 1) deps only + no-warnings 2) project sources only (if you need findings for those)

borkdude15:03:54

we can also make an explicit flag for this maybe, --copy-hooks

ericdallo15:03:11

We just need the findings when analyzing a single file, so I think passing the :no-warnings https://github.com/clojure-lsp/clojure-lsp/blob/master/src/clojure_lsp/config.clj#L16 would be enough, I'll make some local tests though

ericdallo15:03:27

yeah, a copy-hooks would be util if that doesn't work, I'll let you know, thanks!

ericdallo15:03:15

Oh, the :no-warnings don't print the analysis as well, so that will not work for clojure-lsp, @U04V15CAJ do you think it makes sense to change the check to copy hooks to check for a --copy-hooks instead checking no-warnings ?

borkdude15:03:57

Looks ok. > The --copy-hooks flag is used to indicate that clj-kondo is used to populate the cache. This doc needs to change to: indicate that hook code should be copied from dependencies. Also, undocument the no-warnings flag, we will keep supporting it as it is now, but I want to get rid of it altogether from the public API

borkdude15:03:17

since the name is confusing

👍 3
ericdallo18:03:54

@leoiacovini 1. fixed on clojure-lsp, next releases should fix that issue 2. Maybe we can improve docs to make that easier to understand? still think a repro would help find if there is a bug

borkdude18:03:46

@ericdallo I'm working on renaming the option to --copy-configs

borkdude18:03:09

it doesn't only copy hooks

ericdallo18:03:00

it makes sense