From 6887fadd1c262954303d706d0d556bef4b5148c5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Sun, 15 Oct 2023 22:01:06 +0200
Subject: [PATCH] Move lexical-binding warning from checkdoc to byte-compiler

This warning is much more appropriate for the compiler, since lexical
binding affects what it can reason and warn about, than for checkdoc
as the warning has no bearing to documentation at all.
The move also improves the reach of the warning.

* etc/NEWS: Update.
* lisp/emacs-lisp/checkdoc.el (checkdoc-lexical-binding-flag)
(checkdoc-file-comments-engine): Move warning from here....
* lisp/emacs-lisp/bytecomp.el (byte-compile-file): ...to here.
* test/lisp/emacs-lisp/bytecomp-resources/no-byte-compile.el:
* test/lisp/emacs-lisp/bytecomp-tests.el
(bytecomp-tests--unescaped-char-literals)
(bytecomp-tests-function-put, bytecomp-tests--not-writable-directory)
(bytecomp-tests--target-file-no-directory):
Update tests.
(bytecomp-tests--log-from-compilation)
(bytecomp-tests--lexical-binding-cookie): New test.
---
 etc/NEWS                                      | 25 ++++++++----
 lisp/emacs-lisp/bytecomp.el                   |  4 ++
 lisp/emacs-lisp/checkdoc.el                   | 39 -------------------
 .../bytecomp-resources/no-byte-compile.el     |  2 +-
 test/lisp/emacs-lisp/bytecomp-tests.el        | 39 +++++++++++++++++--
 5 files changed, 57 insertions(+), 52 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 02b794a2964..e7f51e1c6c4 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -845,14 +845,6 @@ This can help avoid some awkward skip conditions.  For example
 '(skip-unless (not noninteractive))' can be changed to the easier
 to read '(skip-when noninteractive)'.
 
-** Checkdoc
-
----
-*** New checkdock warning if not using lexical-binding.
-Checkdoc now warns if the first line of an Emacs Lisp file does not
-end with a "-*- lexical-binding: t -*-" cookie.  Customize the user
-option 'checkdoc-lexical-binding-flag' to nil to disable this warning.
-
 ** URL
 
 +++
@@ -1153,6 +1145,23 @@ sexp-related motion commands.
 
 ** New or changed byte-compilation warnings
 
+---
+*** Warn about missing 'lexical-binding' directive.
+The compiler now warns if an Elisp file lacks the standard
+'-*- lexical-binding: ... -*-' cookie on the first line.
+This line typically looks something like
+
+    ;;; My little pony mode  -*- lexical-binding: t -*-
+
+It is needed to inform the compiler about which dialect of ELisp
+your code is using: the modern dialect with lexical binding or
+the old dialect with only dynamic binding.
+
+Lexical binding avoids some name conflicts and allows the compiler
+to detect more mistakes and generate more efficient code.  To adapt
+your code to lexical binding, see the "(elisp) Converting to Lexical
+Binding" node in the manual.
+
 ---
 *** Warn about empty bodies for more special forms and macros.
 The compiler now warns about an empty body argument to 'when',
diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 92abe6b4624..cc68db73c9f 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -2201,6 +2201,10 @@ byte-compile-file
               filename buffer-file-name))
       ;; Don't inherit lexical-binding from caller (bug#12938).
       (unless (local-variable-p 'lexical-binding)
+        (let ((byte-compile-current-buffer (current-buffer)))
+          (byte-compile-warn-x
+           (position-symbol 'a (point-min))
+           "file has no `lexical-binding' directive on its first line"))
         (setq-local lexical-binding nil))
       ;; Set the default directory, in case an eval-when-compile uses it.
       (setq default-directory (file-name-directory filename)))
diff --git a/lisp/emacs-lisp/checkdoc.el b/lisp/emacs-lisp/checkdoc.el
index 440e133f44b..471a2fbdf48 100644
--- a/lisp/emacs-lisp/checkdoc.el
+++ b/lisp/emacs-lisp/checkdoc.el
@@ -128,14 +128,6 @@
 ;; simple style rules to follow which checkdoc will auto-fix for you.
 ;; `y-or-n-p' and `yes-or-no-p' should also end in "?".
 ;;
-;; Lexical binding:
-;;
-;;   We recommend always using lexical binding in new code, and
-;; converting old code to use it.  Checkdoc warns if you don't have
-;; the recommended string "-*- lexical-binding: t -*-" at the top of
-;; the file.  You can disable this check with the user option
-;; `checkdoc-lexical-binding-flag'.
-;;
 ;; Adding your own checks:
 ;;
 ;;   You can experiment with adding your own checks by setting the
@@ -347,12 +339,6 @@ checkdoc-column-zero-backslash-before-paren
   :type 'boolean
   :version "28.1")
 
-(defcustom checkdoc-lexical-binding-flag t
-  "Non-nil means generate warnings if file is not using lexical binding.
-See Info node `(elisp) Converting to Lexical Binding' for more."
-  :type 'boolean
-  :version "30.1")
-
 ;; This is how you can use checkdoc to make mass fixes on the Emacs
 ;; source tree:
 ;;
@@ -2391,31 +2377,6 @@ checkdoc-file-comments-engine
 	      (point-min) (save-excursion (goto-char (point-min))
 					  (line-end-position))))
 	 nil))
-      (when checkdoc-lexical-binding-flag
-        (setq
-         err
-         ;; Lexical binding cookie.
-         (if (not (save-excursion
-                    (save-restriction
-                      (goto-char (point-min))
-                      (narrow-to-region (point) (pos-eol))
-                      (re-search-forward
-                       (rx "-*-" (* (* nonl) ";")
-                           (* space) "lexical-binding:" (* space) "t" (* space)
-                           (* ";" (* nonl))
-                           "-*-")
-                       nil t))))
-             (let ((pos (save-excursion (goto-char (point-min))
-                                        (goto-char (pos-eol))
-                                        (point))))
-               (if (checkdoc-y-or-n-p "There is no lexical-binding cookie!  Add one?")
-                   (progn
-                     (goto-char pos)
-                     (insert "  -*- lexical-binding: t -*-"))
-                 (checkdoc-create-error
-                  "The first line should end with \"-*- lexical-binding: t -*-\""
-                  pos (1+ pos) t)))
-           nil)))
       (setq
        err
        (or
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/no-byte-compile.el b/test/lisp/emacs-lisp/bytecomp-resources/no-byte-compile.el
index 00ad1947507..1de5cf66b66 100644
--- a/test/lisp/emacs-lisp/bytecomp-resources/no-byte-compile.el
+++ b/test/lisp/emacs-lisp/bytecomp-resources/no-byte-compile.el
@@ -1 +1 @@
-;; -*- no-byte-compile: t; -*-
+;; -*- no-byte-compile: t; lexical-binding: t; -*-
diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el
index e644417c3d4..4aa555f1e92 100644
--- a/test/lisp/emacs-lisp/bytecomp-tests.el
+++ b/test/lisp/emacs-lisp/bytecomp-tests.el
@@ -1302,6 +1302,30 @@ bytecomp-tests--with-temp-file
        (let ((elc (concat ,file-name-var ".elc")))
          (if (file-exists-p elc) (delete-file elc))))))
 
+(defun bytecomp-tests--log-from-compilation (source)
+  "Compile the string SOURCE and return the compilation log output."
+  (let ((text-quoting-style 'grave)
+        (byte-compile-log-buffer (generate-new-buffer " *Compile-Log*")))
+    (with-current-buffer byte-compile-log-buffer
+      (let ((inhibit-read-only t)) (erase-buffer)))
+    (bytecomp-tests--with-temp-file el-file
+      (write-region source nil el-file)
+      (byte-compile-file el-file))
+    (with-current-buffer byte-compile-log-buffer
+      (buffer-string))))
+
+(ert-deftest bytecomp-tests--lexical-binding-cookie ()
+  (cl-flet ((cookie-warning (source)
+              (string-search
+               "file has no `lexical-binding' directive on its first line"
+               (bytecomp-tests--log-from-compilation source))))
+    (let ((some-code "(defun my-fun () 12)\n"))
+      (should-not (cookie-warning
+                   (concat ";;; -*-lexical-binding:t-*-\n" some-code)))
+      (should-not (cookie-warning
+                   (concat ";;; -*-lexical-binding:nil-*-\n" some-code)))
+      (should (cookie-warning some-code)))))
+
 (ert-deftest bytecomp-tests--unescaped-char-literals ()
   "Check that byte compiling warns about unescaped character
 literals (Bug#20852)."
@@ -1310,7 +1334,9 @@ bytecomp-tests--unescaped-char-literals
         (byte-compile-debug t)
         (text-quoting-style 'grave))
     (bytecomp-tests--with-temp-file source
-      (write-region "(list ?) ?( ?; ?\" ?[ ?])" nil source)
+      (write-region (concat ";;; -*-lexical-binding:t-*-\n"
+                            "(list ?) ?( ?; ?\" ?[ ?])")
+                    nil source)
       (bytecomp-tests--with-temp-file destination
         (let* ((byte-compile-dest-file-function (lambda (_) destination))
                (err (should-error (byte-compile-file source))))
@@ -1322,7 +1348,9 @@ bytecomp-tests--unescaped-char-literals
                                     "`?\\]' expected!")))))))
     ;; But don't warn in subsequent compilations (Bug#36068).
     (bytecomp-tests--with-temp-file source
-      (write-region "(list 1 2 3)" nil source)
+      (write-region (concat ";;; -*-lexical-binding:t-*-\n"
+                            "(list 1 2 3)")
+                    nil source)
       (bytecomp-tests--with-temp-file destination
         (let ((byte-compile-dest-file-function (lambda (_) destination)))
           (should (byte-compile-file source)))))))
@@ -1330,6 +1358,7 @@ bytecomp-tests--unescaped-char-literals
 (ert-deftest bytecomp-tests-function-put ()
   "Check `function-put' operates during compilation."
   (bytecomp-tests--with-temp-file source
+    (insert  ";;; -*-lexical-binding:t-*-\n")
     (dolist (form '((function-put 'bytecomp-tests--foo 'foo 1)
                     (function-put 'bytecomp-tests--foo 'bar 2)
                     (defmacro bytecomp-tests--foobar ()
@@ -1636,7 +1665,8 @@ bytecomp-tests--not-writable-directory
            (byte-compile-error-on-warn t))
       (unwind-protect
           (progn
-            (write-region "" nil input-file nil nil nil 'excl)
+            (write-region ";;; -*-lexical-binding:t-*-\n"
+                          nil input-file nil nil nil 'excl)
             (write-region "" nil output-file nil nil nil 'excl)
             (set-file-modes input-file #o400)
             (set-file-modes output-file #o200)
@@ -1700,7 +1730,8 @@ bytecomp-tests--target-file-no-directory
     (let* ((default-directory directory)
            (byte-compile-dest-file-function (lambda (_) "test.elc"))
            (byte-compile-error-on-warn t))
-      (write-region "" nil "test.el" nil nil nil 'excl)
+      (write-region  ";;; -*-lexical-binding:t-*-\n"
+                     nil "test.el" nil nil nil 'excl)
       (should (byte-compile-file "test.el"))
       (should (file-regular-p "test.elc"))
       (should (cl-plusp (file-attribute-size
-- 
2.32.0 (Apple Git-132)

