]> git.eshelyaron.com Git - emacs.git/commitdiff
Move lexical-binding warning from checkdoc to byte-compiler
authorMattias Engdegård <mattiase@acm.org>
Sun, 15 Oct 2023 20:01:06 +0000 (22:01 +0200)
committerMattias Engdegård <mattiase@acm.org>
Sat, 21 Oct 2023 13:12:55 +0000 (15:12 +0200)
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
lisp/emacs-lisp/bytecomp.el
lisp/emacs-lisp/checkdoc.el
test/lisp/emacs-lisp/bytecomp-resources/no-byte-compile.el
test/lisp/emacs-lisp/bytecomp-tests.el

index 3d4cdd876b3366b03a4f91c5c8bde5ce71febad6..609c3fa1596cf0744e7a97939b3405091c96c7dc 100644 (file)
--- 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',
index 92abe6b46244341f859ccdfd38d5a8f5cfc4a83d..cc68db73c9fe2912f36455f21f4c0742f8acb7f1 100644 (file)
@@ -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)))
index 440e133f44ba801fcf90876595876e2f63d0627e..471a2fbdf48eab9408807f14ddfc3b8bc7239510 100644 (file)
 ;; 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
index 00ad1947507bbf7e7b407f17d2eabc26068a3591..1de5cf66b6660782b619e917b1cffcbce3c5eec9 100644 (file)
@@ -1 +1 @@
-;; -*- no-byte-compile: t; -*-
+;; -*- no-byte-compile: t; lexical-binding: t; -*-
index e644417c3d41c60dcbf33859a73435919009d1f0..4aa555f1e92fe8eadd8ce8ea7c2ce95a1d3b0002 100644 (file)
@@ -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