]> git.eshelyaron.com Git - emacs.git/commitdiff
Add empty-body warning for when, unless etc
authorMattias Engdegård <mattiase@acm.org>
Thu, 29 Dec 2022 12:01:47 +0000 (13:01 +0100)
committerMattias Engdegård <mattiase@acm.org>
Thu, 29 Dec 2022 12:01:47 +0000 (13:01 +0100)
Warn about code like (when SOME-CONDITION) because these may indicate
bugs.  Warnings currently apply to `when`, `unless`, `ignore-error`,
`with-suppressed-warnings` and (as before) `let` and `let*`.

* lisp/emacs-lisp/byte-run.el (with-suppressed-warnings):
Update doc string.
* lisp/emacs-lisp/bytecomp.el: (byte-compile-warning-types)
(byte-compile-warnings): Add empty-body.
(byte-compile-initial-macro-environment):
Add empty-body warning for with-suppressed-warnings.
* lisp/emacs-lisp/macroexp.el (macroexp--expand-all):
Use the empty-body category for let and let*.
* lisp/subr.el (when, unless, ignore-error): Add empty-body warning.
* test/lisp/emacs-lisp/bytecomp-tests.el
(bytecomp-test--with-suppressed-warnings): Add test cases.

lisp/emacs-lisp/byte-run.el
lisp/emacs-lisp/bytecomp.el
lisp/emacs-lisp/macroexp.el
lisp/subr.el
test/lisp/emacs-lisp/bytecomp-tests.el

index b5e887db83697d6545cf2ea312a49bb6ae5aac20..d909395e973cdb19e2e23579309959444f7a2c5b 100644 (file)
@@ -649,8 +649,8 @@ in `byte-compile-warning-types'; see the variable
 `byte-compile-warnings' for a fuller explanation of the warning
 types.  The types that can be suppressed with this macro are
 `free-vars', `callargs', `redefine', `obsolete',
-`interactive-only', `lexical', `mapcar', `constants' and
-`suspicious'.
+`interactive-only', `lexical', `mapcar', `constants',
+`suspicious' and `empty-body'.
 
 For the `mapcar' case, only the `mapcar' function can be used in
 the symbol list.  For `suspicious', only `set-buffer', `lsh' and `eq'
index 1a488977390cbdf66f467c831051e0cd9b5c3313..a41e076f9b013f064f297a8d1caa8b3ad1b561ba 100644 (file)
@@ -295,7 +295,8 @@ The information is logged to `byte-compile-log-buffer'."
   '(redefine callargs free-vars unresolved
              obsolete noruntime interactive-only
              make-local mapcar constants suspicious lexical lexical-dynamic
-             docstrings docstrings-non-ascii-quotes not-unused)
+             docstrings docstrings-non-ascii-quotes not-unused
+             empty-body)
   "The list of warning types used when `byte-compile-warnings' is t.")
 (defcustom byte-compile-warnings t
   "List of warnings that the byte-compiler should issue (t for almost all).
@@ -326,6 +327,7 @@ Elements of the list may be:
   docstrings-non-ascii-quotes docstrings that have non-ASCII quotes.
                               This depends on the `docstrings' warning type.
   suspicious  constructs that usually don't do what the coder wanted.
+  empty-body  body argument to a special form or macro is empty.
 
 If the list begins with `not', then the remaining elements specify warnings to
 suppress.  For example, (not mapcar) will suppress warnings about mapcar.
@@ -541,15 +543,19 @@ Return the compile-time value of FORM."
              ;; Later `internal--with-suppressed-warnings' binds it again, this
              ;; time in order to affect warnings emitted during the
              ;; compilation itself.
-             (let ((byte-compile--suppressed-warnings
-                    (append warnings byte-compile--suppressed-warnings)))
-               ;; This function doesn't exist, but is just a placeholder
-               ;; symbol to hook up with the
-               ;; `byte-hunk-handler'/`byte-defop-compiler-1' machinery.
-               `(internal--with-suppressed-warnings
-                 ',warnings
-                 ,(macroexpand-all `(progn ,@body)
-                                   macroexpand-all-environment))))))
+             (if body
+                 (let ((byte-compile--suppressed-warnings
+                        (append warnings byte-compile--suppressed-warnings)))
+                   ;; This function doesn't exist, but is just a placeholder
+                   ;; symbol to hook up with the
+                   ;; `byte-hunk-handler'/`byte-defop-compiler-1' machinery.
+                   `(internal--with-suppressed-warnings
+                     ',warnings
+                     ,(macroexpand-all `(progn ,@body)
+                                       macroexpand-all-environment)))
+               (macroexp-warn-and-return
+                "`with-suppressed-warnings' with empty body"
+                nil '(empty-body with-suppressed-warnings) t warnings)))))
   "The default macro-environment passed to macroexpand by the compiler.
 Placing a macro here will cause a macro to have different semantics when
 expanded by the compiler as when expanded by the interpreter.")
index 8953e5fd01952a80c2e6fc87a692ef0de782184e..8aa9cb860c49a7571cc4f44b426bb6f1790a3a43 100644 (file)
@@ -368,7 +368,7 @@ Assumes the caller has bound `macroexpand-all-environment'."
                      (macroexp-unprogn
                       (macroexp-warn-and-return
                        (format "Empty %s body" fun)
-                       nil nil 'compile-only fun))
+                       nil (list 'empty-body fun) 'compile-only fun))
                    (macroexp--all-forms body))
                  (cdr form))
                 form)))
index 5e8f3c82a2acfe45058b170f01cbec35721e17d7..69e6198e1bd3d59ad9301a91ecd91f44455e0e7a 100644 (file)
@@ -280,14 +280,20 @@ change the list."
 When COND yields non-nil, eval BODY forms sequentially and return
 value of last one, or nil if there are none."
   (declare (indent 1) (debug t))
-  (list 'if cond (cons 'progn body)))
+  (if body
+      (list 'if cond (cons 'progn body))
+    (macroexp-warn-and-return "`when' with empty body"
+                              cond '(empty-body when) t)))
 
 (defmacro unless (cond &rest body)
   "If COND yields nil, do BODY, else return nil.
 When COND yields nil, eval BODY forms sequentially and return
 value of last one, or nil if there are none."
   (declare (indent 1) (debug t))
-  (cons 'if (cons cond (cons nil body))))
+  (if body
+      (cons 'if (cons cond (cons nil body)))
+    (macroexp-warn-and-return "`unless' with empty body"
+                              cond '(empty-body unless) t)))
 
 (defsubst subr-primitive-p (object)
   "Return t if OBJECT is a built-in primitive function."
@@ -383,14 +389,19 @@ Otherwise, return result of last form in BODY.
 CONDITION can also be a list of error conditions.
 The CONDITION argument is not evaluated.  Do not quote it."
   (declare (debug t) (indent 1))
-  (if (and (eq (car-safe condition) 'quote)
-           (cdr condition) (null (cddr condition)))
-      (macroexp-warn-and-return
-       (format "`ignore-error' condition argument should not be quoted: %S"
-               condition)
-       `(condition-case nil (progn ,@body) (,(cadr condition) nil))
-       nil t condition)
-    `(condition-case nil (progn ,@body) (,condition nil))))
+  (cond
+   ((and (eq (car-safe condition) 'quote)
+         (cdr condition) (null (cddr condition)))
+    (macroexp-warn-and-return
+     (format "`ignore-error' condition argument should not be quoted: %S"
+             condition)
+     `(condition-case nil (progn ,@body) (,(cadr condition) nil))
+     nil t condition))
+   (body
+    `(condition-case nil (progn ,@body) (,condition nil)))
+   (t
+    (macroexp-warn-and-return "`ignore-error' with empty body"
+                              nil '(empty-body ignore-error) t condition))))
 
 \f
 ;;;; Basic Lisp functions.
index 61f4998f6ba7b5ca84e5beae585e2b74a60fa3e3..eec66c9585a4c5c6f0e3d85012486603ac75a96a 100644 (file)
@@ -1433,7 +1433,50 @@ literals (Bug#20852)."
         (set-buffer (get-buffer-create "foo"))
         nil))
    '((suspicious set-buffer))
-   "Warning: Use .with-current-buffer. rather than"))
+   "Warning: Use .with-current-buffer. rather than")
+
+  (test-suppression
+   '(defun zot ()
+      (let ((_ 1))
+        ))
+   '((empty-body let))
+   "Warning: Empty let body")
+
+  (test-suppression
+   '(defun zot ()
+      (let* ((_ 1))
+        ))
+   '((empty-body let*))
+   "Warning: Empty let\\* body")
+
+  (test-suppression
+   '(defun zot (x)
+      (when x
+        ))
+   '((empty-body when))
+   "Warning: `when' with empty body")
+
+  (test-suppression
+   '(defun zot (x)
+      (unless x
+        ))
+   '((empty-body unless))
+   "Warning: `unless' with empty body")
+
+  (test-suppression
+   '(defun zot (x)
+      (ignore-error arith-error
+        ))
+   '((empty-body ignore-error))
+   "Warning: `ignore-error' with empty body")
+
+  (test-suppression
+   '(defun zot (x)
+      (with-suppressed-warnings ((suspicious eq))
+        ))
+   '((empty-body with-suppressed-warnings))
+   "Warning: `with-suppressed-warnings' with empty body")
+  )
 
 (ert-deftest bytecomp-tests--not-writable-directory ()
   "Test that byte compilation works if the output directory isn't