Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

cider-format-buffer can delete whole buffer's contents on invalid Clojure syntax #2102

Closed
vemv opened this issue Oct 31, 2017 · 13 comments
Closed

Comments

@vemv
Copy link
Member

vemv commented Oct 31, 2017

Given the following clojure buffer with a dangling parens (doesn't matter if it has a ns form):

"Precious content"

(

if I run (cider-format-buffer), the whole buffer's contents will disappear.

Issue is easy to cleanly reproduce; GH template seems excessive in this case.

Using:

cider-20171001.112
clj-refactor-20170720.712
clojure-mode-20170819.2159
paredit-20170405.1149

Cheers - Victor

@xiongtx
Copy link
Member

xiongtx commented Oct 31, 2017

Cannot reproduce. You mean you don't see the following?

image

@vemv
Copy link
Member Author

vemv commented Oct 31, 2017

I do get that buffer! (Hadn't noticed before; under my setup most 'foreign' windows/buffers don't pop up)

 Show: Project-Only All 
  Hide: Clojure Java REPL Tooling Duplicates  (78 frames hidden)

  This is an unexpected CIDER middleware error.
  Please submit a bug report via `M-x cider-report-bug`.

  If these stacktraces are occuring frequently, consider using the
  button(s) below to suppress these types of errors for the duration of
  your current CIDER session. The stacktrace buffer will still be
  generated, but it will "pop under" your current buffer instead of
  "popping over". The button toggles this behavior.

 Suppress format-code-error 

1. Unhandled java.lang.Exception
   Unexpected EOF. [at line 39, column 1]

                reader.clj:   18  cider.inlined-deps.cljfmt.v0v5v6.rewrite-clj.v0v5v2.rewrite-clj.reader/throw-reader
                reader.clj:   11  cider.inlined-deps.cljfmt.v0v5v6.rewrite-clj.v0v5v2.rewrite-clj.reader/throw-reader
               RestFn.java:  425  clojure.lang.RestFn/invoke
                  core.clj:   78  cider.inlined-deps.cljfmt.v0v5v6.rewrite-clj.v0v5v2.rewrite-clj.parser.core/eval106591/fn
              MultiFn.java:  229  clojure.lang.MultiFn/invoke
                reader.clj:  132  cider.inlined-deps.cljfmt.v0v5v6.rewrite-clj.v0v5v2.rewrite-clj.reader/read-with-meta
                reader.clj:  128  cider.inlined-deps.cljfmt.v0v5v6.rewrite-clj.v0v5v2.rewrite-clj.reader/read-with-meta
                  core.clj:   34  cider.inlined-deps.cljfmt.v0v5v6.rewrite-clj.v0v5v2.rewrite-clj.parser.core/parse-next
                  core.clj:   32  cider.inlined-deps.cljfmt.v0v5v6.rewrite-clj.v0v5v2.rewrite-clj.parser.core/parse-next
                  core.clj:   42  cider.inlined-deps.cljfmt.v0v5v6.rewrite-clj.v0v5v2.rewrite-clj.parser.core/parse-delim/fn
                reader.clj:  141  cider.inlined-deps.cljfmt.v0v5v6.rewrite-clj.v0v5v2.rewrite-clj.reader/read-repeated

I just enabled (setq debug-on-error t) for getting an elisp stacktrace as well:

Debugger entered--Lisp error: (wrong-type-argument char-or-string-p nil)
  cider--format-buffer(cider-sync-request:format-code)
  cider-format-buffer()
  eval((cider-format-buffer) nil)
  eval-expression((cider-format-buffer) nil)
  funcall-interactively(eval-expression (cider-format-buffer) nil)
  call-interactively(eval-expression nil nil)
  command-execute(eval-expression)

@xiongtx
Copy link
Member

xiongtx commented Oct 31, 2017

You get the error buffer but the contents of the source buffer is still erased? I don't get that.

Can you try w/ emacs -Q so we can exclude issues w/ your init file?

@vemv
Copy link
Member Author

vemv commented Oct 31, 2017 via email

@vemv
Copy link
Member Author

vemv commented Oct 31, 2017

I know the issue!

(defun cider--format-buffer (formatter)
  "Format the contents of the current buffer.

Uses FORMATTER, a function of one argument, to convert the string contents
of the buffer into a formatted string."
  (let* ((original (substring-no-properties (buffer-string)))
         (formatted (funcall formatter original)))
    (unless (equal original formatted)
      (erase-buffer)
      (insert formatted))))

(insert formatted) with formatted being nil causes the original (wrong-type-argument char-or-string-p nil) error.

Fix:

- unless (equal original formatted)
+ (unless (or (not formatted) (equal original formatted)))
...

@xiongtx
Copy link
Member

xiongtx commented Oct 31, 2017

Still doesn't explain why you're seeing this issue that I'm not. An unmatched paren like that should result in an EOF error thrown by the reader that stops everything in its tracks, not wipe the buffer.

@vemv
Copy link
Member Author

vemv commented Nov 1, 2017 via email

@xiongtx
Copy link
Member

xiongtx commented Nov 1, 2017

If someone else can reproduce (cc @bbatsov), I'll look into it further. Randomly adding null checks is not the way to go.

@bbatsov
Copy link
Member

bbatsov commented Dec 10, 2017

I agree. I'll try to find some time to test this myself.

@bbatsov
Copy link
Member

bbatsov commented Dec 10, 2017

I can't reproduce this myself - I get exactly the same results as @xiongtx.

@bbatsov
Copy link
Member

bbatsov commented Dec 10, 2017

I've also checked the message exchange between CIDER and nREPL and there's no response except the error.

At any rate there's a real problem here that's easy to fix - the middleware should gracefully handle bad code and send back some meaningful message to CIDER.

@bbatsov
Copy link
Member

bbatsov commented Jan 2, 2018

Basically the code here https://github.com/clojure-emacs/cider-nrepl/blob/master/src/cider/nrepl/middleware/format.clj#L13 should feature some exception handling and return some meaningful message to CIDER when this happens.

@bbatsov
Copy link
Member

bbatsov commented Jan 2, 2018

(or we can add check-parens in the CIDER command)

gonewest818 added a commit to gonewest818/cider that referenced this issue Feb 17, 2018
gonewest818 added a commit to gonewest818/cider that referenced this issue Feb 17, 2018
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants