clojure-emacs / squiggly-clojure

Flycheck checker for Clojure, using eastwood and core.typed.
GNU General Public License v3.0
204 stars 25 forks source link

The proper place for core.typed #58

Open bsless opened 5 years ago

bsless commented 5 years ago

Hello, I notice there have been several issues opened due to the same root cause - that core.typed was not being required in the project's dependencies. I stumble on this issue and forget to add it myself at times. Where do you think is the proper place to resolve this issue? As far as I understand it, even with the type checks disabled, squiggly is still broken without requiring core.typed? The possible solutions I see are:

Thank you for the work you do on this project Ben

pnf commented 5 years ago

I think the real problem is that kibit has crypto-dependency (i.e. one that does not show up in lein deps :tree on core.tools/reader. You can see that this is the problem by turning on nrepl-log-messages and observing a FileNotFoundException. That library used to be provided via dependency on it by cider itself and, as a backup, by core.typed. To avoid a conflict with cider, we had a line around line 203 of flycheck-clojure.el to explicitly exclude the dependency during jack-in. It appears that cider no longer depends on thus provides reader, but you can still get it as a side-effect of declaring a dependency on core.typed. That is of course silly.
Check out PR #59, which drops the exclusion from the elisp injection, adds an explicit dependency on reader to the plugin, and comments-out all the typed references from the sample project. If you could give this a try, I'd really appreciate it.

bsless commented 5 years ago

I'll gladly give it a shot and report the results back. Thanks!

bsless commented 5 years ago

Hi, I tested it in Clojure 1.8 and got

Caused by: java.io.FileNotFoundException: Could not locate clojure/tools/reader/impl/ExceptionInfo__init.class or clojure/tools/reader/impl/ExceptionInfo.clj on classpath.

I also tried adding the reader as a dependency during jack-in and to the project. neither worked. Any ideas?

pnf commented 5 years ago

Odd. You’d need to do a local install of the unexclude-tools-reader branch to pick up the new explicit dependency, and of course you need to be using the new flycheck-clojure.el that doesn’t exclude during the injection. Can you verify that you’re picking up the rebuilt local install off the new branch and are using the new .el as well? Then can you paste the full jack-in message?

bsless commented 5 years ago

I'll recreate it, but I verified that it worked for me as intended by the following steps:

pnf commented 5 years ago

And you’re sure it’s getting the snapshot from your local install off of the unexclude-tools-reader branch?

bsless commented 5 years ago

Hi, sorry I disappeared for a while, work got a bit too intensive, I verified the way I tested before, and I tested again and couldn't recreate the behavior. Looks like it works for me now. I tried testing and verifying through and through and it looks like I was either doing something wrong previously or had created some state on my system which I couldn't recreate.