From afa154db27fc5ff710e8c76061c7579eee2cd918 Mon Sep 17 00:00:00 2001 From: Henrik Lissner Date: Sat, 10 Sep 2022 15:33:10 +0200 Subject: [PATCH] refactor!(emacs-lisp): flycheck config in non-packages BREAKING CHANGE: This performs the following backwards-incompatible changes: - Replaces `+emacs-lisp-reduce-flycheck-errors-in-emacs-config-h` with a `+emacs-lisp-non-package-mode` minor-mode. - Removed the `+emacs-lisp-disable-flycheck-in-dirs` variable, as this mechanism no longer checks a directory list to detect a "non-package". If you've referenced either of these symbols, you'll need to update/remove them from your config. No extra config is needed otherwise. Why: Doom has always tried to reduce the verbosity of Flycheck when viewing elisp config files or scripts (i.e. non-packages). These are so stateful that the byte-compiler, package-lint, and checkdoc inundate users with false positives that are more overwhelming than helpful. The heuristic for this has always been a simple "is this file in $DOOMDIR or $EMACSDIR", but this wasn't robust enough, especially in cases where symlinking was involved, so I've employed a new, more general heuristic for detecting non-package files: - The file isn't a theme in `custom-theme-load-path`, - The file doesn't have a (provide ...) or (provide-theme ...) statement whose first argument matches the file name, - The file lives in a project with a .doommodule file (doom modules never have convention package files in them), - Or the file is a dotfile (like .dir-locals.el or .doomrc). I've also tweaked byte-compile-warnings to yield a little more output, but not by much. Whether this is too permissive or not will require further testing to determine. What's more, I've updated this to reflect recent changes to Doom's startup process (in c05e615). Ref: c05e61536ed9 --- modules/lang/emacs-lisp/autoload.el | 89 ++++++++++++++++++++--------- modules/lang/emacs-lisp/config.el | 25 ++++---- 2 files changed, 78 insertions(+), 36 deletions(-) diff --git a/modules/lang/emacs-lisp/autoload.el b/modules/lang/emacs-lisp/autoload.el index c44643ac9..49e8827d9 100644 --- a/modules/lang/emacs-lisp/autoload.el +++ b/modules/lang/emacs-lisp/autoload.el @@ -244,33 +244,70 @@ https://emacs.stackexchange.com/questions/10230/how-to-indent-keywords-aligned" ("Variables" "^\\s-*(\\(def\\(?:c\\(?:onst\\(?:ant\\)?\\|ustom\\)\\|ine-symbol-macro\\|parameter\\|var\\(?:-local\\)?\\)\\)\\s-+\\(\\(?:\\sw\\|\\s_\\|\\\\.\\)+\\)" 2) ("Types" "^\\s-*(\\(cl-def\\(?:struct\\|type\\)\\|def\\(?:class\\|face\\|group\\|ine-\\(?:condition\\|error\\|widget\\)\\|package\\|struct\\|t\\(?:\\(?:hem\\|yp\\)e\\)\\)\\)\\s-+'?\\(\\(?:\\sw\\|\\s_\\|\\\\.\\)+\\)" 2)))) +(defun +emacs-lisp--in-package-buffer-p () + (let* ((file-path (buffer-file-name (buffer-base-buffer))) + (file-base (if file-path (file-name-base file-path)))) + (and (derived-mode-p 'emacs-lisp-mode) + (or (null file-base) + (locate-file file-base (custom-theme--load-path) '(".elc" ".el")) + (save-excursion + (save-restriction + (widen) + (goto-char (point-max)) + (when (re-search-backward "^ *\\((provide\\)\\(?:-theme\\)? +'" + (max (point-min) (- (point-max) 512)) + t) + (goto-char (match-beginning 1)) + (ignore-errors + (and (stringp file-base) + (equal (symbol-name (doom-unquote (nth 1 (read (current-buffer))))) + file-base))))))) + (not (locate-dominating-file default-directory ".doommodule"))))) + ;;;###autoload -(defun +emacs-lisp-reduce-flycheck-errors-in-emacs-config-h () - "Remove `emacs-lisp-checkdoc' checker and reduce `emacs-lisp' checker -verbosity when editing a file in `doom-user-dir' or `doom-emacs-dir'." - (when (and (bound-and-true-p flycheck-mode) - (eq major-mode 'emacs-lisp-mode) - (or (not default-directory) - (null (buffer-file-name (buffer-base-buffer))) - (cl-find-if (doom-partial #'file-in-directory-p default-directory) - +emacs-lisp-disable-flycheck-in-dirs))) - (add-to-list 'flycheck-disabled-checkers 'emacs-lisp-checkdoc) - (set (make-local-variable 'flycheck-emacs-lisp-check-form) - (concat "(progn " - (prin1-to-string - `(ignore-errors - (setq doom-modules ',doom-modules - doom-disabled-packages ',doom-disabled-packages) - (require 'doom) - (ignore-errors (load ,user-init-file t t)) - (setq byte-compile-warnings - '(obsolete cl-functions - interactive-only make-local mapcar - suspicious constants)) - (defmacro map! (&rest _)))) - " " - (default-value 'flycheck-emacs-lisp-check-form) - ")")))) +(define-minor-mode +emacs-lisp-non-package-mode + "Reduce flycheck verbosity where it is appropriate. + +Essentially, this means in any elisp file that either: +- Is not a theme in `custom-theme-load-path', +- Lacks a `provide' statement, +- Lives in a project with a .doommodule file, +- Is a dotfile (like .dir-locals.el or .doomrc). + +This generally applies to your private config (`doom-user-dir') or Doom's source +\(`doom-emacs-dir')." + :since "3.0.0" + (unless (and (bound-and-true-p flycheck-mode) + (not (+emacs-lisp--in-package-buffer-p))) + (setq +emacs-lisp-non-package-mode nil)) + (when (derived-mode-p 'emacs-lisp-mode) + (add-hook 'after-save-hook #'+emacs-lisp-non-package-mode nil t)) + (if (not +emacs-lisp-non-package-mode) + (when (get 'flycheck-disabled-checkers 'initial-value) + (setq-local flycheck-disabled-checkers (get 'flycheck-disabled-checkers 'initial-value)) + (kill-local-variable 'flycheck-emacs-lisp-check-form)) + (with-memoization (get 'flycheck-disabled-checkers 'initial-value) + flycheck-disabled-checkers) + (setq-local flycheck-emacs-lisp-check-form + (prin1-to-string + `(progn + (setq doom-modules ',doom-modules + doom-disabled-packages ',doom-disabled-packages + byte-compile-warnings ',+emacs-lisp-linter-warnings) + (condition-case e + (progn + (require 'doom) + (require 'doom-cli) + (require 'doom-start) + (defmacro map! (&rest _))) + (error + (princ + (format "%s:%d:%d:Error:Failed to load Doom: %s\n" + ,(file-name-nondirectory (buffer-file-name (buffer-base-buffer))) + 0 0 (error-message-string e))))) + ,(read (default-toplevel-value 'flycheck-emacs-lisp-check-form)))) + flycheck-disabled-checkers (cons 'emacs-lisp-checkdoc + flycheck-disabled-checkers)))) ;; diff --git a/modules/lang/emacs-lisp/config.el b/modules/lang/emacs-lisp/config.el index 21286680b..d17f3a9f6 100644 --- a/modules/lang/emacs-lisp/config.el +++ b/modules/lang/emacs-lisp/config.el @@ -7,13 +7,17 @@ "Regexp to use for `outline-regexp' in `emacs-lisp-mode'. This marks a foldable marker for `outline-minor-mode' in elisp buffers.") -(defvar +emacs-lisp-disable-flycheck-in-dirs - (list doom-emacs-dir doom-user-dir) - "List of directories to disable `emacs-lisp-checkdoc' in. +(defconst +emacs-lisp-linter-warnings + '(not free-vars ; don't complain about unknown variables + noruntime ; don't complain about unknown function calls + unresolved) ; don't complain about undefined functions + "The value for `byte-compile-warnings' in non-packages. -This checker tends to produce a lot of false positives in your .emacs.d and -private config, so it is mostly useless there. However, special hacks are -employed so that flycheck still does *some* helpful linting.") +This reduces the verbosity of flycheck in Emacs configs and scripts, which are +so stateful that the deluge of false positives (from the byte-compiler, +package-lint, and checkdoc) can be more overwhelming than helpful. + +See `+emacs-lisp-non-package-mode' for details.") ;; `elisp-mode' is loaded at startup. In order to lazy load its config we need @@ -76,10 +80,11 @@ employed so that flycheck still does *some* helpful linting.") ;; Ensure straight sees modifications to installed packages #'+emacs-lisp-init-straight-maybe-h) - ;; Flycheck's two emacs-lisp checkers produce a *lot* of false positives in - ;; emacs configs, so we disable `emacs-lisp-checkdoc' and reduce the - ;; `emacs-lisp' checker's verbosity. - (add-hook 'flycheck-mode-hook #'+emacs-lisp-reduce-flycheck-errors-in-emacs-config-h) + ;; UX: Flycheck's two emacs-lisp checkers produce a *lot* of false positives + ;; in non-packages (like Emacs configs or elisp scripts), so I disable + ;; `emacs-lisp-checkdoc' and set `byte-compile-warnings' to a subset of the + ;; original in the flycheck instance (see `+emacs-lisp-linter-warnings'). + (add-hook 'flycheck-mode-hook #'+emacs-lisp-non-package-mode) ;; Enhance elisp syntax highlighting, by highlighting Doom-specific ;; constructs, defined symbols, and truncating :pin's in `package!' calls.