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

Middleware throws exceptions on unkown reader macros #25

Closed Malabarba closed 9 years ago

Malabarba commented 9 years ago

Starting with version 0.10.0, CIDER will support some debugging reader macros. So if the user writes, for instance,

(defn some-function [x]
  #break (reduce + (range x)))

that indicates that cider should wrap a breakpoint around that sexp.

The problem is, as soon as I write that, I start getting these messages about exceptions from flycheck:

Error from syntax checker clojure-cider-typed: Form #[257 \300\301"\207 [format (do (require 'squiggly-clojure.core) (squiggly-clojure.core/check-tc '%s))] 4 

(fn NS)] of checker clojure-cider-typed failed: class clojure.lang.ExceptionInfo

These messages keep happening repetitively, until I delete that #break from my code. It's made a little worse by the fact that it's multiline, so it's constantly expanding the echo area.

One way I could think of handling this would be for the clojure side to dynamically bind *default-data-reader-fn* to identity before reading the code. Another option would be for the checker definition (on the Emacs side) to better handle this situation.

pnf commented 9 years ago

I'm having trouble getting the reader macro to work even without squiggly. Maybe you can help me, so I can reproduce your issue. Per the HEAD on github now, I've upgraded to clojure 1.7, cider 2.10, typed 0.3.7 and kibit 0.1.2; not all of these may be necessary. With flycheck-clojure disabled, I get:

RuntimeException No reader function for tag break clojure.lang.LispReader$CtorReader.readTagged (LispReader.java:1245) RuntimeException Unmatched delimiter: ) clojure.lang.Util.runtimeException (Util.java:221)user>

Malabarba commented 9 years ago

To use the #break reader macro you'll have to add cider-nrepl 0.10.0 to your project's dependencies. But you don't need cider to test reader macros in general. For instance, try evaluating the following:

(binding [*data-readers* {'add-1 #'inc}]
  (read-string "#add-1 10"))

That's roughly what cider does with the #brake and #dbg readers. When one of these macros doesn't have a corresponding entry, the reader will throw an exception, and I think squiggly clojure isn't properly reporting that exception (probably because it happens at read time, not load time).

pnf commented 9 years ago

I (think I) understand readers. What I'm unable to figure out is how these readers are supposed to get bound when you cider-load-file. I'm using cider-nrepl 0.10.0-20150718.212352-15.

Malabarba commented 9 years ago

They don't. They are only bound for evaluation commands. Like cider-eval-last-sexp, or cider-eval-defun-at-point The way this happens is as part of the cider-nrepl middleware. The wrap-debug function inside debug.clj will add these readers to the messages session map, which is then used by nREPL.

Malabarba commented 9 years ago

Still, I think it's not so much about the readers cider uses. It's more about the fact that, when squiggly-clojure runs into an exception for an unknown reader macro, instead of being properly reported to flycheck this causes an error that's repeatedly presented in the echo area.

pnf commented 9 years ago

The error is definitely being reported properly to flycheck, which is what generates the "Error from syntax checker..." message. What is supposed to happen (and then does happen for me) is that flycheck shuts off in the buffer and displays "FlyC!" in the mode-line. The problem is in the checks themselves.

If, roughly following your suggestion, I wrap the checking in (binding [*default-data-reader-fn* (fn [_ v] v)] ...) and I turn off clojure-cider-typed, then the flycheck completes without evaluation errors. Sadly (1) typed will still throw an exception if you don't disable it, and (2) eastwood completes successfully but returns an empty warnings list. The is reproducible in the repl, without any flycheck apparatus.

We may need to raise this with the linter projects themselves.

Malabarba commented 9 years ago

The error is definitely being reported properly to flycheck, which is what generates the "Error from syntax checker..." message. What is supposed to happen (and then does happen for me) is that flycheck shuts off in the buffer and displays "FlyC!" in the mode-line. The problem is in the checks themselves.

I see "FlyC!" in the mode-line, but the message keeps popping up whenever I save the file.

If, roughly following your suggestion, I wrap the ...

Wait, I'm really sorry for the confusion, but it seems I was too quick to blame. It's not about reader macros. If I just follow these steps I get the problem too:

  1. Using this profiles.clj:

    {:user {:plugins [[refactor-nrepl "1.2.0-SNAPSHOT"]
                     [cider/cider-nrepl "0.10.0-SNAPSHOT"]]
           :dependencies [[acyclic/squiggly-clojure "0.1.3-SNAPSHOT"]
                          ^:replace [org.clojure/tools.nrepl "0.2.10"]]}}
  2. create a new project with lein new app bugged;
  3. cider-jack-in inside src/bugged/core.clj;
  4. Write undefined-var at the bottom of the file and save it.

The error I get from flycheck is the following and, like I said, it happens everytime I save the file.

Error from syntax checker clojure-cider-eastwood: Form #[257 \300\301"\207 [format (do (require 'squiggly-clojure.core) (squiggly-clojure.core/check-ew '%s))] 4 

(fn NS)] of checker clojure-cider-eastwood failed: class clojure.lang.Compiler$CompilerException

We may need to raise this with the linter projects themselves.

Yes, the above error message makes it look like the problem is in eastwood, so you think we should file this with them? I also noticed you said flycheck should disable itself. Does that mean we should file a bug with flycheck as well (since it does not seem to disable itself for me).

defclass commented 9 years ago

I get this error also when save everytime I save file

Error from syntax checker clojure-cider-eastwood: Form #[257 \300\301"\207 [format (do (require 'squiggly-clojure.core) (squiggly-clojure.core/check-ew '%s))] 4 

(fn NS)] of checker clojure-cider-eastwood failed: class clojure.lang.Compiler$CompilerException

I dont know how to solve this.

pnf commented 9 years ago

What happens when you lint the file with Eastwood outside of emacs?


Sent from my VAX-11/780. Please excuse RADIX-50 encoding.

On Jul 27, 2015, at 23:06, Micheal Wong notifications@github.com wrote:

I get this error also when save everytime I save the file

Error from syntax checker clojure-cider-eastwood: Form #[257 \300\301�"\207 [format (do (require 'squiggly-clojure.core) (squiggly-clojure.core/check-ew '%s))] 4

(fn NS)] of checker clojure-cider-eastwood failed: class clojure.lang.Compiler$CompilerException I dont know how to solve this.

— Reply to this email directly or view it on GitHub.

Malabarba commented 9 years ago

@pnf On a brand new project, with undefined-var added to the end of core.clj:

% ~/Git/bugtest 0 lein eastwood
== Eastwood 0.2.1 Clojure 1.6.0 JVM 1.8.0_45
Directories scanned for source files:
  src test
== Linting bugtest.core ==
Exception thrown during phase :analyze+eval of linting namespace bugtest.core
Got exception with extra ex-data:
    msg='Could not resolve var: undefined-var'
    (keys dat)=(:end-line :line :column :end-column :file :var)
ExceptionInfo Could not resolve var: undefined-var
    clojure.core/ex-info (core.clj:4403)
    eastwood.copieddeps.dep2.clojure.tools.analyzer.passes.jvm.validate/eval1996/fn--1998 (validate.clj:29)
    clojure.lang.MultiFn.invoke (MultiFn.java:227)
    eastwood.copieddeps.dep2.clojure.tools.analyzer.passes.jvm.validate/validate (validate.clj:264)
    clojure.lang.Var.invoke (Var.java:379)
    eastwood.copieddeps.dep1.clojure.tools.analyzer.passes/compile-passes/fn--574/fn--579 (passes.clj:171)
    eastwood.copieddeps.dep1.clojure.tools.analyzer.passes/compile-passes/fn--574/fn--581 (passes.clj:173)
    eastwood.copieddeps.dep1.clojure.tools.analyzer.passes/compile-passes/fn--574/fn--581 (passes.clj:173)
    eastwood.copieddeps.dep1.clojure.tools.analyzer.passes/compile-passes/fn--574/fn--581 (passes.clj:173)
    clojure.core/apply (core.clj:628)
    clojure.core/partial/fn--4230 (core.clj:2470)
    eastwood.copieddeps.dep1.clojure.tools.analyzer.ast/walk/walk--191 (ast.clj:102)
    eastwood.copieddeps.dep1.clojure.tools.analyzer.ast/walk (ast.clj:95)
    eastwood.copieddeps.dep1.clojure.tools.analyzer.ast/postwalk (ast.clj:115)
    eastwood.copieddeps.dep1.clojure.tools.analyzer.ast/postwalk (ast.clj:113)
    eastwood.copieddeps.dep1.clojure.tools.analyzer.passes/compile-passes/analyze--586 (passes.clj:175)
    clojure.core/comp/fn--4192 (core.clj:2403)
    clojure.core/comp/fn--4192 (core.clj:2403)
    eastwood.analyze-ns/run-passes (analyze_ns.clj:157)
    eastwood.copieddeps.dep2.clojure.tools.analyzer.jvm/analyze/fn--3551 (jvm.clj:474)
    clojure.core/apply (core.clj:624)
    clojure.core/with-bindings* (core.clj:1862)
    eastwood.copieddeps.dep2.clojure.tools.analyzer.jvm/analyze (jvm.clj:470)
    eastwood.copieddeps.dep2.clojure.tools.analyzer.jvm/analyze+eval (jvm.clj:518)
    eastwood.analyze-ns/analyze-file/fn--4171/fn--4173 (analyze_ns.clj:279)
    eastwood.analyze-ns/analyze-file/fn--4171 (analyze_ns.clj:276)
    eastwood.analyze-ns/analyze-file (analyze_ns.clj:274)
    eastwood.analyze-ns/analyze-ns (analyze_ns.clj:327)
    eastwood.lint/lint-ns (lint.clj:569)
    eastwood.lint/eastwood-core/fn--6334 (lint.clj:1041)
    eastwood.lint/eastwood-core (lint.clj:1040)
    eastwood.lint/eastwood (lint.clj:1154)
    eastwood.lint/eastwood-from-cmdline (lint.clj:1167)
    clojure.lang.Var.invoke (Var.java:379)
    eastwood.versioncheck/run-eastwood (versioncheck.clj:15)
    user/eval21 (form-init1103805185781903147.clj:1)
    clojure.lang.Compiler.eval (Compiler.java:6703)
    clojure.lang.Compiler.eval (Compiler.java:6693)
    clojure.lang.Compiler.load (Compiler.java:7130)
    clojure.lang.Compiler.loadFile (Compiler.java:7086)
    clojure.main/load-script (main.clj:274)
    clojure.main/init-opt (main.clj:279)
    clojure.main/initialize (main.clj:307)
    clojure.main/null-opt (main.clj:342)
    clojure.main/main (main.clj:420)
    clojure.lang.Var.invoke (Var.java:383)
    clojure.lang.Var.applyTo (Var.java:700)
    clojure.main.main (main.java:37)

The following form was being processed during the exception:
undefined-var

Shown again with metadata for debugging (some metadata elided for brevity):
^{:line 9} undefined-var

An exception was thrown while analyzing namespace bugtest.core 
Lint results may be incomplete.  If there are compilation errors in
your code, try fixing those.  If not, check above for info on the
exception.

Stopped analyzing namespaces after bugtest.core
due to exception thrown.  1 namespaces left unanalyzed.

If you wish to force continuation of linting after an exception in one
namespace, make the option map key :continue-on-exception have the
value true.

WARNING: This can cause exceptions to be thrown while analyzing later
namespaces that would not otherwise occur.  For example, if a function
is defined in the namespace where the first exception occurs, after
the exception, it will never be evaluated.  If the function is then
used in namespaces analyzed later, it will be undefined, causing
error.

== Warnings: 0 (not including reflection warnings)  Exceptions thrown: 1
Subprocess failed
pnf commented 9 years ago

Sounds like something to report to Eastwood then. They'll probably ask for a self-contained repro.


Sent from my VAX-11/780. Please excuse RADIX-50 encoding.

On Jul 28, 2015, at 09:11, Artur Malabarba notifications@github.com wrote:

@pnf On a brand new project, with undefined-var added to the end of core.clj:

% ~/Git/bugtest 0 lein eastwood == Eastwood 0.2.1 Clojure 1.6.0 JVM 1.8.0_45 Directories scanned for source files: src test == Linting bugtest.core == Exception thrown during phase :analyze+eval of linting namespace bugtest.core Got exception with extra ex-data: msg='Could not resolve var: undefined-var' (keys dat)=(:end-line :line :column :end-column :file :var) ExceptionInfo Could not resolve var: undefined-var clojure.core/ex-info (core.clj:4403) eastwood.copieddeps.dep2.clojure.tools.analyzer.passes.jvm.validate/eval1996/fn--1998 (validate.clj:29) clojure.lang.MultiFn.invoke (MultiFn.java:227) eastwood.copieddeps.dep2.clojure.tools.analyzer.passes.jvm.validate/validate (validate.clj:264) clojure.lang.Var.invoke (Var.java:379) eastwood.copieddeps.dep1.clojure.tools.analyzer.passes/compile-passes/fn--574/fn--579 (passes.clj:171) eastwood.copieddeps.dep1.clojure.tools.analyzer.passes/compile-passes/fn--574/fn--581 (passes.clj:173) eastwood.copieddeps.dep1.clojure.tools.analyzer.passes/compile-passes/fn--574/fn--581 (passes.clj:173) eastwood.copieddeps.dep1.clojure.tools.analyzer.passes/compile-passes/fn--574/fn--581 (passes.clj:173) clojure.core/apply (core.clj:628) clojure.core/partial/fn--4230 (core.clj:2470) eastwood.copieddeps.dep1.clojure.tools.analyzer.ast/walk/walk--191 (ast.clj:102) eastwood.copieddeps.dep1.clojure.tools.analyzer.ast/walk (ast.clj:95) eastwood.copieddeps.dep1.clojure.tools.analyzer.ast/postwalk (ast.clj:115) eastwood.copieddeps.dep1.clojure.tools.analyzer.ast/postwalk (ast.clj:113) eastwood.copieddeps.dep1.clojure.tools.analyzer.passes/compile-passes/analyze--586 (passes.clj:175) clojure.core/comp/fn--4192 (core.clj:2403) clojure.core/comp/fn--4192 (core.clj:2403) eastwood.analyze-ns/run-passes (analyze_ns.clj:157) eastwood.copieddeps.dep2.clojure.tools.analyzer.jvm/analyze/fn--3551 (jvm.clj:474) clojure.core/apply (core.clj:624) clojure.core/with-bindings* (core.clj:1862) eastwood.copieddeps.dep2.clojure.tools.analyzer.jvm/analyze (jvm.clj:470) eastwood.copieddeps.dep2.clojure.tools.analyzer.jvm/analyze+eval (jvm.clj:518) eastwood.analyze-ns/analyze-file/fn--4171/fn--4173 (analyze_ns.clj:279) eastwood.analyze-ns/analyze-file/fn--4171 (analyze_ns.clj:276) eastwood.analyze-ns/analyze-file (analyze_ns.clj:274) eastwood.analyze-ns/analyze-ns (analyze_ns.clj:327) eastwood.lint/lint-ns (lint.clj:569) eastwood.lint/eastwood-core/fn--6334 (lint.clj:1041) eastwood.lint/eastwood-core (lint.clj:1040) eastwood.lint/eastwood (lint.clj:1154) eastwood.lint/eastwood-from-cmdline (lint.clj:1167) clojure.lang.Var.invoke (Var.java:379) eastwood.versioncheck/run-eastwood (versioncheck.clj:15) user/eval21 (form-init1103805185781903147.clj:1) clojure.lang.Compiler.eval (Compiler.java:6703) clojure.lang.Compiler.eval (Compiler.java:6693) clojure.lang.Compiler.load (Compiler.java:7130) clojure.lang.Compiler.loadFile (Compiler.java:7086) clojure.main/load-script (main.clj:274) clojure.main/init-opt (main.clj:279) clojure.main/initialize (main.clj:307) clojure.main/null-opt (main.clj:342) clojure.main/main (main.clj:420) clojure.lang.Var.invoke (Var.java:383) clojure.lang.Var.applyTo (Var.java:700) clojure.main.main (main.java:37)

The following form was being processed during the exception: undefined-var

Shown again with metadata for debugging (some metadata elided for brevity): ^{:line 9} undefined-var

An exception was thrown while analyzing namespace bugtest.core Lint results may be incomplete. If there are compilation errors in your code, try fixing those. If not, check above for info on the exception.

Stopped analyzing namespaces after bugtest.core due to exception thrown. 1 namespaces left unanalyzed.

If you wish to force continuation of linting after an exception in one namespace, make the option map key :continue-on-exception have the value true.

WARNING: This can cause exceptions to be thrown while analyzing later namespaces that would not otherwise occur. For example, if a function is defined in the namespace where the first exception occurs, after the exception, it will never be evaluated. If the function is then used in namespaces analyzed later, it will be undefined, causing error.

== Warnings: 0 (not including reflection warnings) Exceptions thrown: 1 Subprocess failed — Reply to this email directly or view it on GitHub.

Malabarba commented 9 years ago

I'm sorry if this is a silly question (I'm not terribly familiar with eastwood), but isn't that the intended behavior? The reported exception looks correct, and it finishes with a return value of 1 to indicate there was an error.

pnf commented 9 years ago

In the case of Eastwood, I can set the flag suggested in the error output, but what would happen next is an exception from typer. In general, some errors get reported in a reasonable fashion, while others end up throwing. I'm going to do the somewhat obvious thing and wrap the linting calls in try, reporting anything I catch as normal flycheck error on the first line of the file. So stand by...

Malabarba commented 9 years ago

I'm going to do the somewhat obvious thing and wrap the linting calls in try, reporting anything Icatch as normal flycheck error on the first line of the file.

Sounds good. It's a shame that the line information is given as a message instead of being part of the exception. Maybe that is somethign worth reporting to eastwood

pnf commented 9 years ago

Actually, Eastwood is ok here. The :continue-on-exception does what we want, subject to caveats they list. The typer, however, bombs irretrievably.
Anyway, I just committed and pushed but haven't deployed anything. The story with reader macros is still not great: if you add one, you'll see exactly one exclamation point error at the top of the file, and all the real errors will disappear.

pnf commented 9 years ago

Another 1.3 snapshot deployed to clojars.

Malabarba commented 9 years ago

Ok, it works now. I no longer get messages at every save, and flycheck indeed places a ! at the start of the buffer indicating the error: clojure.lang.ExceptionInfo: Could not resolve var: hello {:var hello, :file "squid/core.clj", :end-column 6, :column 1, :line 8, :end-line 8} The same happens with reader tags.

Thanks for your work @pnf !

Malabarba commented 9 years ago

By the way, since the error seems to have all necessary line/col info, would it be possible to have flycheck display it on the right position? (unless it's too much work)

pnf commented 9 years ago

I'd still like to figure out a better way to deal with reader tags. What happens now is almost equivalent to not using flycheck at all.

Sent from my VAX-11/780. Please excuse RADIX-50 encoding.

On Jul 31, 2015, at 06:02, Artur Malabarba notifications@github.com wrote:

Ok, it works now. I no longer get messages at every save, and flycheck indeed places a ! at the start of the buffer indicating the error: clojure.lang.ExceptionInfo: Could not resolve var: hello {:var hello, :file "squid/core.clj", :end-column 6, :column 1, :line 8, :end-line 8} The same happens with reader tags.

Thanks for your work @pnf !

— Reply to this email directly or view it on GitHub.

pnf commented 9 years ago

Good idea.


Sent from my VAX-11/780. Please excuse RADIX-50 encoding.

On Jul 31, 2015, at 06:04, Artur Malabarba notifications@github.com wrote:

By the way, since the error seems to have all necessary line/col info, would it be possible to have flycheck display it on the right position? (unless it's too much work)

— Reply to this email directly or view it on GitHub.

pnf commented 9 years ago

Ok, try now.

Malabarba commented 9 years ago

Works great! =D

Malabarba commented 9 years ago

By the way, nice work on the package. :+1:

jethrokuan commented 9 years ago

Sorry to bring this up again, but i've been trying to get this to work with no avail. I've followed @Malabarba 's config pretty closely.

Here's my profiles.clj:

{:user {:plugins [[cider/cider-nrepl "0.10.0-SNAPSHOT"]
              [refactor-nrepl "1.2.0-SNAPSHOT"]]
    :dependencies [[acyclic/squiggly-clojure "0.1.3-SNAPSHOT"]
                   ^:replace [org.clojure/tools.nrepl "0.2.10"]]}}
  1. create a new lein project: lein new app bugged
  2. cider jack-in in core.clj
  3. write undefined-var at bottom of file and save it

I get the same error:

Error from syntax checker clojure-cider-eastwood: 
Form (closure (t) (ns) (format (do (require 'squiggly-clojure.core) (squiggly-clojure.core/check-ew '%s)) ns)) of checker clojure-cider-eastwood failed: 
class clojure.lang.Compiler$CompilerException

The difference is now lein eastwood is not even a valid command (outside of emacs):

 ~/Documents/Code/Clojure/bugged $ lein eastwood
 'eastwood' is not a task. See 'lein help'.

lein eastwood only becomes valid when i explicitly pull the eastwood dependency in profiles.clj with

{:user {:plugins [[jonase/eastwood "0.2.1"]] }}

Would love to get this over and done with. Thanks for the help in advance!

pnf commented 9 years ago

First, could you upgrade to [acyclic/squiggly-clojure "0.1.4"]. It's not actually different from the last 0.1.3-SNAPSHOT, but, with these non-RT snapshots, there's limited forensic value to lein deps :tree and lein deps :implicits, output for your project from both of which would be helpful.

I believe that if you wish to use the lein plugin for eastwood (in addition to using it via flycheck), you will need to use its plugin directly. See https://github.com/jonase/eastwood.

jethrokuan commented 9 years ago

Hello!

thanks for the quick reply! I upgraded squiggly to 0.1.4, and did a lein deps :tree and saw some confusion between the requirements for org.clojure/clojure itself. I upgraded project.clj to require the newer 1.7.0 and everything worked smoothly.

pnf commented 9 years ago

Great! Let me know if anything else comes up.


Sent from my HAL 9000. Please excuse lies and gibberish.

On Aug 22, 2015, at 13:41, Jethro Kuan Sheng Yuan notifications@github.com wrote:

Hello!

thanks for the quick reply! I upgraded squiggly to 0.1.4, and did a lein deps :tree and saw some confusion between the requirements for org.clojure/clojure itself. I upgraded project.clj to require the newer 1.7.0 and everything worked smoothly.

— Reply to this email directly or view it on GitHub.