(set-auto-mode): Streamline to fix bug#67795

The old code tested if the mode function is `fboundp` but in an
inconsistent way and without paying attention to
`major-mode-remap`.

* lisp/files.el (set-auto-mode-0): Return `:keep` rather than nil if
the mode was already set.  And emit a warning when the mode function
doesn't exist.
(set-auto-mode): Remove checks that the mode function exists now that
`set-auto-mode-0` does it for us.  Adjust to the new return values of
that function, and simplify the code using a big `or` instead of
a sequence of steps each setting&testing `done`.
(hack-local-variables--find-variables): Use `major-mode-remap`
when skipping the "mode:" entries that specify modes we don't have.
Also, when (eq handle-mode t), don't bother building a list of results
only to return a single element in the end.

1 files changed, 118 insertions, 113 deletions

diff --git a/lisp/files.el b/lisp/files.el
index 3ca4f047144..766ed573392 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -3425,7 +3425,7 @@ set the major mode only if that would change it.  In other words
 we don't actually set it to the same mode the buffer already has."
   ;; Look for -*-MODENAME-*- or -*- ... mode: MODENAME; ... -*-
   (let ((try-locals (not (inhibit-local-variables-p)))
-        end done mode modes)
+        end modes)
     ;; Once we drop the deprecated feature where mode: is also allowed to
     ;; specify minor-modes (ie, there can be more than one "mode:"), we can
     ;; remove this section and just let (hack-local-variables t) handle it.
@@ -3456,100 +3456,96 @@ we don't actually set it to the same mode the buffer already has."
              (push (intern (concat (downcase (buffer-substring (point) end))
                                    "-mode"))
                    modes))))
-    ;; If we found modes to use, invoke them now, outside the save-excursion.
-    (if modes
-        (catch 'nop
-          (dolist (mode (nreverse modes))
-            (if (not (functionp mode))
-                (message "Ignoring unknown mode `%s'" mode)
-              (setq done t)
-              (or (set-auto-mode-0 mode keep-mode-if-same)
-                  ;; continuing would call minor modes again, toggling them off
-                  (throw 'nop nil))))))
-    ;; Check for auto-mode-alist entry in dir-locals.
-    (unless done
-      (with-demoted-errors "Directory-local variables error: %s"
-        ;; Note this is a no-op if enable-local-variables is nil.
-        (let* ((mode-alist (cdr (hack-dir-local--get-variables
-                                 (lambda (key) (eq key 'auto-mode-alist))))))
-          (setq done (set-auto-mode--apply-alist mode-alist
-                                                 keep-mode-if-same t)))))
-    (and (not done)
-         (setq mode (hack-local-variables t (not try-locals)))
-         (not (memq mode modes))        ; already tried and failed
-         (if (not (functionp mode))
-             (message "Ignoring unknown mode `%s'" mode)
-           (setq done t)
-           (set-auto-mode-0 mode keep-mode-if-same)))
-    ;; If we didn't, look for an interpreter specified in the first line.
-    ;; As a special case, allow for things like "#!/bin/env perl", which
-    ;; finds the interpreter anywhere in $PATH.
-    (and (not done)
-         (setq mode (save-excursion
-                      (goto-char (point-min))
-                      (if (looking-at auto-mode-interpreter-regexp)
-                          (match-string 2))))
-         ;; Map interpreter name to a mode, signaling we're done at the
-         ;; same time.
-         (setq done (assoc-default
-                     (file-name-nondirectory mode)
-                     (mapcar (lambda (e)
-                               (cons
-                                (format "\\`%s\\'" (car e))
-                                (cdr e)))
-                             interpreter-mode-alist)
-                     #'string-match-p))
-         ;; If we found an interpreter mode to use, invoke it now.
-         (set-auto-mode-0 done keep-mode-if-same))
-    ;; Next try matching the buffer beginning against magic-mode-alist.
-    (unless done
-      (if (setq done (save-excursion
-                       (goto-char (point-min))
-                       (save-restriction
-                         (narrow-to-region (point-min)
-                                           (min (point-max)
-                                                (+ (point-min) magic-mode-regexp-match-limit)))
-                         (assoc-default
-                          nil magic-mode-alist
-                          (lambda (re _dummy)
-                            (cond
-                             ((functionp re)
-                              (funcall re))
-                             ((stringp re)
-                              (let ((case-fold-search nil))
-                                (looking-at re)))
-                             (t
-                              (error
-                               "Problem in magic-mode-alist with element %s"
-                               re))))))))
-          (set-auto-mode-0 done keep-mode-if-same)))
-    ;; Next compare the filename against the entries in auto-mode-alist.
-    (unless done
-      (setq done (set-auto-mode--apply-alist auto-mode-alist
-                                             keep-mode-if-same nil)))
-    ;; Next try matching the buffer beginning against magic-fallback-mode-alist.
-    (unless done
-      (if (setq done (save-excursion
-                       (goto-char (point-min))
-                       (save-restriction
-                         (narrow-to-region (point-min)
-                                           (min (point-max)
-                                                (+ (point-min) magic-mode-regexp-match-limit)))
-                         (assoc-default nil magic-fallback-mode-alist
-                                        (lambda (re _dummy)
-                                          (cond
-                                           ((functionp re)
-                                            (funcall re))
-                                           ((stringp re)
-                                            (let ((case-fold-search nil))
-                                              (looking-at re)))
-                                           (t
-                                            (error
-                                             "Problem with magic-fallback-mode-alist element: %s"
-                                             re))))))))
-          (set-auto-mode-0 done keep-mode-if-same)))
-    (unless done
-      (set-buffer-major-mode (current-buffer)))))
+    (or
+     ;; If we found modes to use, invoke them now, outside the save-excursion.
+     ;; Presume `modes' holds a major mode followed by minor modes.
+     (let ((done ()))
+       (dolist (mode (nreverse modes))
+         (if (eq done :keep)
+             ;; `keep-mode-if-same' is set and the (major) mode
+             ;; was already set.  Refrain from calling the following
+             ;; minor modes since they have already been set.
+             ;; It was especially important in the past when calling
+             ;; minor modes without an arg would toggle them, but it's
+             ;; still preferable to avoid re-enabling them,
+             nil
+           (let ((res (set-auto-mode-0 mode keep-mode-if-same)))
+             (setq done (or res done)))))
+       done)
+     ;; Check for auto-mode-alist entry in dir-locals.
+     (with-demoted-errors "Directory-local variables error: %s"
+       ;; Note this is a no-op if enable-local-variables is nil.
+       (let* ((mode-alist (cdr (hack-dir-local--get-variables
+                                (lambda (key) (eq key 'auto-mode-alist))))))
+         (set-auto-mode--apply-alist mode-alist keep-mode-if-same t)))
+     (let ((mode (hack-local-variables t (not try-locals))))
+       (unless (memq mode modes)        ; already tried and failed
+         (set-auto-mode-0 mode keep-mode-if-same)))
+     ;; If we didn't, look for an interpreter specified in the first line.
+     ;; As a special case, allow for things like "#!/bin/env perl", which
+     ;; finds the interpreter anywhere in $PATH.
+     (when-let
+         ((interp (save-excursion
+                    (goto-char (point-min))
+                    (if (looking-at auto-mode-interpreter-regexp)
+                        (match-string 2))))
+          ;; Map interpreter name to a mode, signaling we're done at the
+          ;; same time.
+          (mode (assoc-default
+                 (file-name-nondirectory interp)
+                 (mapcar (lambda (e)
+                           (cons
+                            (format "\\`%s\\'" (car e))
+                            (cdr e)))
+                         interpreter-mode-alist)
+                 #'string-match-p)))
+       ;; If we found an interpreter mode to use, invoke it now.
+       (set-auto-mode-0 mode keep-mode-if-same))
+     ;; Next try matching the buffer beginning against magic-mode-alist.
+     (let ((mode (save-excursion
+                   (goto-char (point-min))
+                   (save-restriction
+                     (narrow-to-region (point-min)
+                                       (min (point-max)
+                                            (+ (point-min) magic-mode-regexp-match-limit)))
+                     (assoc-default
+                      nil magic-mode-alist
+                      (lambda (re _dummy)
+                        (cond
+                         ((functionp re)
+                          (funcall re))
+                         ((stringp re)
+                          (let ((case-fold-search nil))
+                            (looking-at re)))
+                         (t
+                          (error
+                           "Problem in magic-mode-alist with element %s"
+                           re)))))))))
+       (set-auto-mode-0 mode keep-mode-if-same))
+     ;; Next compare the filename against the entries in auto-mode-alist.
+     (set-auto-mode--apply-alist auto-mode-alist
+                                 keep-mode-if-same nil)
+     ;; Next try matching the buffer beginning against magic-fallback-mode-alist.
+     (let ((mode (save-excursion
+                   (goto-char (point-min))
+                   (save-restriction
+                     (narrow-to-region (point-min)
+                                       (min (point-max)
+                                            (+ (point-min) magic-mode-regexp-match-limit)))
+                     (assoc-default nil magic-fallback-mode-alist
+                                    (lambda (re _dummy)
+                                      (cond
+                                       ((functionp re)
+                                        (funcall re))
+                                       ((stringp re)
+                                        (let ((case-fold-search nil))
+                                          (looking-at re)))
+                                       (t
+                                        (error
+                                         "Problem with magic-fallback-mode-alist element: %s"
+                                         re)))))))))
+       (set-auto-mode-0 mode keep-mode-if-same))
+     (set-buffer-major-mode (current-buffer)))))
 
 (defvar-local set-auto-mode--last nil
   "Remember the mode we have set via `set-auto-mode-0'.")
@@ -3583,18 +3579,29 @@ and it is meant to be modified by packages rather than users.")
   "Apply MODE and return it.
 If optional arg KEEP-MODE-IF-SAME is non-nil, MODE is chased of
 any aliases and compared to current major mode.  If they are the
-same, do nothing and return nil."
-  (unless (and keep-mode-if-same
-               (or (eq (indirect-function mode)
-                       (indirect-function major-mode))
-                   (and set-auto-mode--last
-                        (eq mode (car set-auto-mode--last))
-                        (eq major-mode (cdr set-auto-mode--last)))))
-    (when mode
-      (funcall (major-mode-remap mode))
-      (unless (eq mode major-mode)
-        (setq set-auto-mode--last (cons mode major-mode)))
-      mode)))
+same, do nothing and return `:keep'.
+Return nil if MODE could not be applied."
+  (when mode
+    (if (and keep-mode-if-same
+             (or (eq (indirect-function mode)
+                     (indirect-function major-mode))
+                 (and set-auto-mode--last
+                      (eq mode (car set-auto-mode--last))
+                      (eq major-mode (cdr set-auto-mode--last)))))
+        :keep
+      (let ((modefun (major-mode-remap mode)))
+        (if (not (functionp modefun))
+            (progn
+              (message "Ignoring unknown mode `%s'%s" mode
+                       (if (eq mode modefun) ""
+                         (format " (remapped to `%S')" modefun)))
+              nil)
+          (funcall modefun)
+          (unless (or (eq mode major-mode) ;`set-auto-mode--last' is overkill.
+                      ;; `modefun' is something like a minor mode.
+                      (local-variable-p 'set-auto-mode--last))
+            (setq set-auto-mode--last (cons mode major-mode)))
+          mode)))))
 
 (defvar file-auto-mode-skip "^\\(#!\\|'\\\\\"\\)"
   "Regexp of lines to skip when looking for file-local settings.
@@ -4201,8 +4208,9 @@ major-mode."
                            (not (string-match
                                  "-minor\\'"
                                  (setq val2 (downcase (symbol-name val)))))
-                           ;; Allow several mode: elements.
-                           (push (intern (concat val2 "-mode")) result))
+                           (let ((mode (intern (concat val2 "-mode"))))
+                             (when (fboundp (major-mode-remap mode))
+                               (setq result mode))))
                     (cond ((eq var 'coding))
                           ((eq var 'lexical-binding)
                            (unless hack-local-variables--warned-lexical
@@ -4233,10 +4241,7 @@ major-mode."
                                          val)
                                    result))))))
                 (forward-line 1)))))))
-    (if (eq handle-mode t)
-        ;; Return the final mode: setting that's defined.
-        (car (seq-filter #'fboundp result))
-      result)))
+    result))
 
 (defun hack-local-variables-apply ()
   "Apply the elements of `file-local-variables-alist'.