master a3db503351e: Move lexical-binding warning from checkdoc to byte-compiler

[Mattias Engdegård]

branch: master
commit a3db503351e9a8720cdea2f1ca42d55d2de684e2
Author: Mattias Engdegård <mattiase@acm.org>
Commit: Mattias Engdegård <mattiase@acm.org>

    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                                           | 32 +++++++++++++-----
 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, 64 insertions(+), 52 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 3d4cdd876b3..609c3fa1596 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -854,14 +854,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
 
 +++
@@ -1179,6 +1171,30 @@ 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" section in the manual.
+
+If you are unable to convert the code to lexical binding, you can insert
+the line
+
+    ;;; -*- lexical-binding: nil -*-
+
+first in the file to declare that it uses the old dialect.
+
 ---
 *** 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 @@ See also `emacs-lisp-byte-compile-and-load'."
               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 @@ See Info node `(elisp) Documentation Tips' for background."
   :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 @@ Code:, and others referenced in the style guide."
              (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 @@ byte-compiled.  Run with dynamic binding."
        (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 @@ literals (Bug#20852)."
         (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 @@ literals (Bug#20852)."
                                     "`?\\]' 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 @@ literals (Bug#20852)."
 (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 @@ writable (Bug#44631)."
            (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 @@ mountpoint (Bug#44631)."
     (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