Skip to content

Commit

Permalink
Make flycheck-eldev' properly work on files Eldev' and `Eldev-local…
Browse files Browse the repository at this point in the history
…' as long as those are byte-compilable.
  • Loading branch information
doublep committed Feb 28, 2021
1 parent 30ecc61 commit 0b782d3
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 7 deletions.
2 changes: 1 addition & 1 deletion Eldev
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
; -*- mode: emacs-lisp; lexical-binding: t; no-byte-compile: t -*-
; -*- mode: emacs-lisp; lexical-binding: t -*-

(eldev-require-version "0.5")

Expand Down
4 changes: 4 additions & 0 deletions README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ require that.
* Additional test dependencies (see `eldev-add-extra-dependencies`)
are seen from the test files, but not from the main files.

* Also checks files `Eldev` and `Eldev-local` (as long as they are
byte-compilable, but this is a common Flycheck requirement). Detect
wrong `(eldev-...)` function calls as you type!

== Installation

Download and install the package from {uri-melpa-stable}[MELPA Stable]
Expand Down
22 changes: 21 additions & 1 deletion flycheck-eldev.el
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,11 @@ If FROM is nil, search from `default-directory'."
(setf eval-forms `(,(flycheck-emacs-lisp-bytecomp-config-form) ,flycheck-emacs-lisp-check-form)))
;; Explicitly specify various options in case a user has different defaults.
`("--as-is" "--load-newer"
;; Don't load file `Eldev' or `Eldev-local' if we are checking it.
,@(cond ((file-equal-p real-filename "Eldev")
`("--setup-first" ,(flycheck-sexp-to-string '(setf eldev-skip-project-config t))))
((file-equal-p real-filename "Eldev-local")
`("--setup-first" ,(flycheck-sexp-to-string '(setf eldev-skip-local-project-config t)))))
;; Ignore the original file for project initialization purposes. If
;; `eldev-project-main-file' is specified, this does nothing.
"--setup-first"
Expand Down Expand Up @@ -302,6 +307,17 @@ If FROM is nil, search from `default-directory'."
(flycheck-filter-errors errors 'emacs-lisp)))


(defvar flycheck-eldev--original-emacs-lisp-enabled-p nil)

;; Easier to hack than to wait for https://github.com/flycheck/flycheck/pull/1832 to be
;; "reviewed". Disable `checkdoc' on Eldev files, it makes zero sense there.
(defun flycheck-eldev--emacs-lisp-checkdoc-enabled-p ()
"Check whether to enable Emacs Lisp Checkdoc in the current buffer."
(and (funcall flycheck-eldev--original-emacs-lisp-enabled-p)
;; These files are valid Lisp, but don't contain "standard" comments.
(not (member (file-name-base (or (buffer-file-name) "-")) '("Eldev" "Eldev-local")))))


;;;###autoload
(defun flycheck-eldev--initialize ()
(add-to-list 'flycheck-checkers 'elisp-eldev)
Expand Down Expand Up @@ -335,7 +351,11 @@ See Info Node `(elisp)Byte Compilation'."
(process-put process 'flycheck-working-directory (file-name-directory (buffer-file-name))))
process))))
;; I don't think we need a separate package just for this, so let's do it here.
(add-to-list 'auto-mode-alist `(,(rx "/" (or "Eldev" "Eldev-local") eos) . emacs-lisp-mode) t))
(add-to-list 'auto-mode-alist `(,(rx "/" (or "Eldev" "Eldev-local") eos) . emacs-lisp-mode) t)
;; Hack in disabling of `checkdoc' on Eldev files.
(unless flycheck-eldev--original-emacs-lisp-enabled-p
(setf flycheck-eldev--original-emacs-lisp-enabled-p (flycheck-checker-get 'emacs-lisp-checkdoc 'enabled)
(flycheck-checker-get 'emacs-lisp-checkdoc 'enabled) #'flycheck-eldev--emacs-lisp-checkdoc-enabled-p)))


;;;###autoload
Expand Down
47 changes: 43 additions & 4 deletions test/flycheck-eldev-test.el
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,18 @@
(declare (indent 1) (debug (sexp body)))
(let ((flycheck-eldev-whitelist nil)
(flycheck-eldev-blacklist nil)
(flycheck-eldev-unknown-projects 'trust))
(flycheck-eldev-unknown-projects 'trust)
(enabled-checkers '(emacs-lisp elisp-eldev)))
(while (keywordp (car body))
(pcase (pop body)
(:whitelist (setf flycheck-eldev-whitelist (pop body)))
(:blacklist (setf flycheck-eldev-blacklist (pop body)))
(:unknown-projects (setf flycheck-eldev-unknown-projects (pop body)))))
(:unknown-projects (setf flycheck-eldev-unknown-projects (pop body)))
(:enable-checkdoc (if (pop body)
(push 'emacs-lisp-checkdoc enabled-checkers)
(setf enabled-checkers (delq 'emacs-lisp-checkdoc enabled-checkers))))))
;; Don't use `emacs-lisp-checkdoc'.
`(let ((flycheck-checkers (--filter (memq it '(emacs-lisp elisp-eldev)) flycheck-checkers))
`(let ((flycheck-checkers (--filter (memq it ',enabled-checkers) flycheck-checkers))
(flycheck-disabled-checkers nil)
(flycheck-check-syntax-automatically nil)
(flycheck-eldev-whitelist ',flycheck-eldev-whitelist)
Expand Down Expand Up @@ -77,6 +81,14 @@
(flycheck-eldev--test-recheck)
(flycheck-eldev--test-expect-no-errors)))

(ert-deftest flycheck-eldev-basics-2 ()
(flycheck-eldev--test "project-a/Eldev"
;; Enable `checkdoc'. Must be disabled at runtime (currently through our own hack)
;; for the test to pass.
:enable-checkdoc t
(flycheck-eldev--test-recheck)
(flycheck-eldev--test-expect-no-errors)))

(ert-deftest flycheck-eldev-self-1 ()
(flycheck-eldev--test "../flycheck-eldev.el"
(flycheck-eldev--test-recheck)
Expand All @@ -87,6 +99,17 @@
(flycheck-eldev--test-recheck)
(flycheck-eldev--test-expect-no-errors)))

;; This indirectly tests that we see functions in `eldev' namespace from `Eldev' and
;; `Eldev-local'. Unfortunately, we also see them in normal files, but I cannot think of
;; a robust way to avoid this currently.
(ert-deftest flycheck-eldev-self-3 ()
(flycheck-eldev--test "../Eldev"
;; Enable `checkdoc'. Must be disabled at runtime (currently through our own hack)
;; for the test to pass.
:enable-checkdoc t
(flycheck-eldev--test-recheck)
(flycheck-eldev--test-expect-no-errors)))

(ert-deftest flycheck-eldev-test-file-1 ()
(flycheck-eldev--test "project-a/test/project-a.el"
(flycheck-eldev--test-recheck)
Expand Down Expand Up @@ -117,11 +140,27 @@
;; Test that faulty project initialization code is handled fine.
(ert-deftest flycheck-eldev-faulty-eldev-local-1 ()
(flycheck-eldev--test-with-temp-file "project-a/Eldev-local"
(insert "this is not a valid Lisp")
(insert "this-variable-certainly-doesnt-exist")
(flycheck-eldev--test "project-a/project-a.el"
(flycheck-eldev--test-recheck)
(flycheck-eldev--test-expect-errors '(:matches "cannot be initialized")))))

;; When checking the faulty `Eldev-local', we must not use it for initialization.
(ert-deftest flycheck-eldev-faulty-eldev-local-2 ()
(flycheck-eldev--test-with-temp-file "project-a/Eldev-local"
(insert "this-variable-certainly-doesnt-exist")
(flycheck-eldev--test "project-a/Eldev-local"
(flycheck-eldev--test-recheck)
(flycheck-eldev--test-expect-errors '(:matches "this-variable-certainly-doesnt-exist")))))

;; Test that `flycheck-eldev' really works on Eldev files if those are byte-compilable.
(ert-deftest flycheck-eldev-suspicious-eldev-local-1 ()
(flycheck-eldev--test-with-temp-file "project-a/Eldev-local"
(insert "(defun just-for-testing () (function-with-this-name-certainly-doesnt-exist))")
(flycheck-eldev--test "project-a/Eldev-local"
(flycheck-eldev--test-recheck)
(flycheck-eldev--test-expect-errors '(:matches "function-with-this-name-certainly-doesnt-exist")))))


(ert-deftest flycheck-eldev-project-trusted-p-1 ()
(let ((flycheck-eldev-whitelist nil)
Expand Down
2 changes: 1 addition & 1 deletion test/project-a/Eldev
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
; -*- mode: emacs-lisp; lexical-binding: t; no-byte-compile: t -*-
; -*- mode: emacs-lisp; lexical-binding: t -*-

(eldev-use-package-archive `("archive-a" . ,(expand-file-name "../package-archive-a")))

Expand Down

0 comments on commit 0b782d3

Please sign in to comment.