Fork me on GitHub
Slack Archive

This page is not created by, affiliated with, or supported by Slack Technologies, Inc.

2020-08-29

Channels

#clj-kondo
<
2020-08-29
>
snoe00:08:12

I'd like to start a thread about the possibility of adding the ability to go from a cursor position to an analyzer entry. Continuing https://github.com/snoe/clojure-lsp/pull/176#issuecomment-683123972

snoe00:08:19

I imagine the difficulty comes in with macros, and the reticence is because linting may not need it? I think carve shows the utility of the analyzer itself, so I wonder if we think about other tools it could be a useful exercise. Renaming, I think, is right up there beside dead code for utility. A cli tool that could correct any inconsistent aliases, or rename a fully qualified var without resorting to sed and careful diff checking. Is it fair to want the analyzer to grow beyond kondo with some examples like this?

snoe00:08:56

My thinking is that kondo's analyzer feels like the closest thing we have to a tooling analyzer.

borkdude06:08:37

I think I'm fine with this, with the following caveats: 1) Some things are easier to lint while analyzing, this is why clj-kondo intertwines linting with analysis sometimes. So splitting an analyzer into a library is hard because of this. 2) I don't like clj-kondo linting getting slower because of extra work it has to do for other projects. The in-editor experience should be as fast as possible. So if we can prevent that, that's cool. I think we can accomplish that just using some configuration options.

💯 3
borkdude06:08:05

Btw, carve uses clj-kondo analysis but then rewrites the code using rewrite-cljc, not using kondo. So it only uses the data output.

borkdude08:08:18

Maybe we can use the existing analysis hook or introduce new ones to be able to get other tooling to profit as well, in addition to the analysis info

snoe00:08:55

I noticed that sometimes the (meta (:name %)) gives the position info, but not always... maybe we just need to make that more consistent.

borkdude15:08:43

what is :name in this context?

snoe14:08:12

@U04V15CAJ it's a mix of the analysis types, so var-usages seem to have meta on :name but on var-definitions and namespace-definitions and namespace-usages it is nil. I think if we always fill this in from rewrite-cljc I'd be able to use it for my purposes. Is it worth making an issue or do you foresee performance problems or difficulties with this approach?

practicalli-johnny15:08:49

Congratulations, very much deserved

borkdude15:08:15

Same to you! :)

jaihindhreddy19:08:12

Does clj-kondo compile with the following java -version? 

openjdk version "11.0.8" 2020-07-14
OpenJDK Runtime Environment GraalVM CE 20.2.0 (build 11.0.8+10-jvmci-20.2-b03)
OpenJDK 64-Bit Server VM GraalVM CE 20.2.0 (build 11.0.8+10-jvmci-20.2-b03, mixed mode, sharing)

borkdude19:08:40

it should yes

jaihindhreddy19:08:09

I just tried it, and got this error: 

Error: Already registered: java.util.zip.ZipFile$CleanableResource.get(ZipFile, File, int)
com.oracle.svm.core.util.UserError$UserException: Already registered: java.util.zip.ZipFile$CleanableResource.get(ZipFile, File, int)
...
...
...
Error: Image build request failed with exit status 1
com.oracle.svm.driver.NativeImage$NativeImageError: Image build request failed with exit status 1
	at com.oracle.svm.driver.NativeImage.showError(NativeImage.java:1558)
	at com.oracle.svm.driver.NativeImage.build(NativeImage.java:1308)
	at com.oracle.svm.driver.NativeImage.performBuild(NativeImage.java:1269)
	at com.oracle.svm.driver.NativeImage.main(NativeImage.java:1228)
	at com.oracle.svm.driver.NativeImage$JDK9Plus.main(NativeImage.java:1740)

borkdude19:08:15

Ah 20.2, haven't tried it yet, currently using 20.1. But 20.2 should work

borkdude19:08:26

I know what this is. Just use 20.1 for now

borkdude19:08:34

I'll upgrade soon

💯 3
borkdude19:08:19

Or, if you feel like it, you can do it. The clojure reflector fix should be bumped to 20.2 in project.clj, but also other references in the CircleCI and Github config

👍 3
jaihindhreddy19:08:39

Would you rather me make a PR directly, or make an issue first?

jaihindhreddy19:08:02

Well, https://github.com/borkdude/clj-kondo/pull/979, but added diffs in resources/clj_kondo/impl/cache/built_in/clj/clojure.pprint.transit.json and resources/clj_kondo/impl/cache/built_in/clj/clojure.tools.reader.transit.json. Should I add these to .gitignore?

jaihindhreddy19:08:44

also, wow native-image takes a lot of RAM (to compile)!

borkdude19:08:23

Please don't add resources/* to .gitignore, it's essential to clj-kondo. Also, I wonder how did you end up with a diff in there, it's not relevant to the PR?

borkdude19:08:15

So I would rather have you reset that file to the old one

borkdude20:08:16

Yeah, GraalVM is quite heavy on the RAM, although for clj-kondo it doesn't go above 2 or 3GB I think?

jaihindhreddy20:08:10

> So I would rather have you reset that file to the old one Done. > Please don't add resources/* to .gitignore, it's essential to clj-kondo. After reading docs/dev.md, I get this now. > Also, I wonder how did you end up with a diff in there, it's not relevant to the PR? Yeah, it's quite weird. I'm using clojure version 1.10.1.561. Not sure if that can make a difference. I'll see what the actual difference is.

borkdude20:08:27

Thanks, merged!

🎉 3
jaihindhreddy20:08:06

Courtesy of https://cljdoc.org/d/lambdaisland/deep-diff/0.0-47/doc/readme, I was able to find out the differences in the resources. clojure.pprint's cache had the following differences (Addition of :arities): 

c-write-char {:col 1,
              :fixed-arities #{2},
              :name c-write-char,
              :ns clojure.pprint,
              :private true,
              :row 47,
              :top-ns clojure.pprint,
              +:arities {2 {:args (nil :nilable/int)}}},


p-write-char {:col 1,
              :fixed-arities #{2},
              :name p-write-char,
              :ns clojure.pprint,
              :private true,
              :row 360,
              :top-ns clojure.pprint,
              +:arities {2 {:args (nil :nilable/int)}}},
And clojure.tools.reader had the following: 
read+string {:arities {0 {:ret :vector},
                       +1 {:ret :vector},
                       +2 {:ret :vector},
                       +3 {:ret :vector},
                       -:varargs {:min-arity 1, :ret :vector}}, ;; this got removed the the above 3 arities got added.
             :col 1,
             :fixed-arities #{0 +1 +2 +3},
             :name read+string,
             :ns clojure.tools.reader,
             :row 1008,
             :top-ns clojure.tools.reader,
             -:varargs-min-arity 1}, ;; this got removed as well.
Not sure why this is though. All three functions weren't changed in a long time.

borkdude20:08:43

Have you ran any scripts locally that updated these files?

jaihindhreddy20:08:11

I did run script/built-in. Should have mentioned this beforehand :man-facepalming:

borkdude20:08:48

I haven't ran those in a while. It could be that some logic related to that changed.

👍 3
borkdude20:08:54

if you think the changes are correct, then we could add those as well

jaihindhreddy20:08:14

Yeah, https://github.com/clojure/tools.reader/blob/master/src/main/clojure/clojure/tools/reader.clj#L1009 never had a variadic arity. Seems like the new file is correctly identifying arities 0, 1, 2, 3.

borkdude20:08:36

I have a script, script/dump_types that dumps type information in a diff-able manner

borkdude20:08:03

So when you run that on master and on your branch, it's easier to spot differences

borkdude20:08:44

It could also be that one of the deps in clj-kondo got bumped, and the arities changed?

jaihindhreddy20:08:44

Got it. Although, apart from having to read with transit, I don't think it's un-diffable

jaihindhreddy20:08:36

> It could also be that one of the deps in clj-kondo got bumped, and the arities changed? I git-blamed those three functions, and they didn't change in years. So this is unlikely.

borkdude20:08:57

then clj-kondo must have had an error before

🙂 3
jaihindhreddy20:08:37

:man-shrugging:

borkdude21:08:31

@jaihindhreddy So I ran script/built-in and script/dump-types. Then made a branch: https://github.com/borkdude/clj-kondo/pull/980/files Now the diff is readable in Github, is what I mean

💯 3
jaihindhreddy21:08:06

I get it now. Thanks for clarifying!

borkdude21:08:44

I'll just hook script/dump-types into script/built-ins, so we get it automatically

borkdude21:08:37

Merged. Also updated the script (since now babashka has had transit for a while).

babashka 3
borkdude21:08:25

It's funny, you can also run that script using clojure script/dump_types.clj since all the deps are in deps.edn as well. But Clojure prints namespaced maps in script-mode it seems (https://clojuredocs.org/clojure.core/*print-namespace-maps*)

borkdude21:08:02

This works: $ clojure -e "(set! *print-namespace-maps* false)" script/dump_types.clj