Skip to content

Commit

Permalink
fix(emacs): clear diagnostics properly in Flymake
Browse files Browse the repository at this point in the history
When our Flymake code set diagnostics, it set them for the whole
buffer with :range. However, the :range was incorrect; the begin/end
points given were for the *stdout* temporary buffer, not for the
user's code buffer. This meant that Flymake diagnostics markers were
often not removed because they were not inside the range. (stdout is
often much smaller than the JavaScript source being linted.)

Fix this by using :range with points derived from the correct buffer.
  • Loading branch information
strager committed Oct 24, 2023
1 parent d9ba0a5 commit bf780ec
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 7 deletions.
2 changes: 2 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ Semantic Versioning.
in the head of a `for` loop. For example, quick-lint-js no longer warns about
`let x; for (let x = 0;;);`.
* Emacs: .el files are now installed in the correct place on Arch Linux, btw.
* Emacs: The Flymake plugin now reliably clears out diagnostics after issues are
fixed. Sticky diagnostics are no more.
* TypeScript support (still experimental):
* A newline after `public`, `protected`, `private`, or `readonly` inside a
class is now interpreted correctly.
Expand Down
13 changes: 7 additions & 6 deletions plugin/emacs/flymake-quicklintjs.el
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,13 @@ REPORT-FN is Flymake's callback."
(point-min) (point-max))))
(if (not (string-empty-p stderr-data))
(flymake-log :warning "%S" stderr-data))))
(with-current-buffer stdout-buf
(let ((diags (flymake-quicklintjs--make-diagnostics
src-buf
(car (read-from-string
(buffer-substring-no-properties
(point-min) (point-max)))))))
(let ((diags (flymake-quicklintjs--make-diagnostics
src-buf
(car (read-from-string
(with-current-buffer stdout-buf
(buffer-substring-no-properties
(point-min) (point-max))))))))
(with-current-buffer src-buf
(if (or diags (zerop (process-exit-status p)))
(funcall report-fn diags
:region (cons (point-min) (point-max)))
Expand Down
27 changes: 26 additions & 1 deletion plugin/emacs/test/quicklintjs-test.el
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
(expand-file-name default-directory)
".melpa-cache/"))

(setq quicklintjs-test-dir (file-name-directory (or load-file-name buffer-file-name)))

(defun quicklintjs-install-deps (deps)
(mapcar (lambda (pkg) (unless (package-installed-p pkg)
(if (> emacs-major-version 24)
Expand Down Expand Up @@ -93,7 +95,7 @@ foobar\")((16 . 22) 2 \"E0057\" \"use of undeclared variable: foobar\")(\

(ert-deftest quicklintjs-flymake-check-errors-js ()
(with-temp-buffer
(insert-file-contents-literally "error.js" nil nil nil t)
(insert-file-contents-literally (expand-file-name "error.js" quicklintjs-test-dir) nil nil nil t)
(flymake-mode 1)
(add-hook 'flymake-diagnostic-functions #'flymake-quicklintjs nil t)
(flymake-start)
Expand All @@ -102,6 +104,29 @@ foobar\")((16 . 22) 2 \"E0057\" \"use of undeclared variable: foobar\")(\
(ert-fail "Test timed out waiting for diagnostics."))
;; TODO(strager): Assert specific diagnostics
(while (not (flymake-diagnostics))
(accept-process-output nil 0.01)))))

;; This is a regression test. Buffers were mixed up causing
;; diagnostics after a certain point (usually a few bytes in) to not
;; be cleared.
(ert-deftest quicklintjs-flymake-fixing-error-clears-diagnostics ()
(with-temp-buffer
(insert "/*xxx*/ consol")
(flymake-mode 1)
(add-hook 'flymake-diagnostic-functions #'flymake-quicklintjs nil t)
(flymake-start)

(with-timeout (5
(ert-fail "Test timed out waiting for diagnostics."))
(while (not (flymake-diagnostics))
(accept-process-output nil 0.01)))

(insert "e") ;; Buffer content: /*xxx*/ console
(flymake-start)

(with-timeout (5
(ert-fail "Test timed out waiting for diagnostics to be removed."))
(while (flymake-diagnostics)
(accept-process-output nil 0.01))))))

(defun def-eglot-tests ()
Expand Down

0 comments on commit bf780ec

Please sign in to comment.