[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
master 443c249d850 3/4: Warn about `condition-case` without handlers
From: |
Mattias Engdegård |
Subject: |
master 443c249d850 3/4: Warn about `condition-case` without handlers |
Date: |
Mon, 27 Feb 2023 09:21:59 -0500 (EST) |
branch: master
commit 443c249d85003639512d8d3b6ace184a9ff53bc2
Author: Mattias Engdegård <mattiase@acm.org>
Commit: Mattias Engdegård <mattiase@acm.org>
Warn about `condition-case` without handlers
Omitting handlers from a `condition-case` form makes it useless
since no errors are caught.
* lisp/emacs-lisp/macroexp.el (macroexp--expand-all): New warning.
* test/lisp/emacs-lisp/bytecomp-tests.el
(bytecomp-test--with-suppressed-warnings): Add test case.
* etc/NEWS: Announce.
---
etc/NEWS | 15 +++++++++++++++
lisp/emacs-lisp/macroexp.el | 21 +++++++++++++--------
test/lisp/emacs-lisp/bytecomp-tests.el | 6 ++++++
3 files changed, 34 insertions(+), 8 deletions(-)
diff --git a/etc/NEWS b/etc/NEWS
index 4b0e4e6bd46..9241598f185 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -321,6 +321,21 @@ compared reliably at all.
This warning can be suppressed using 'with-suppressed-warnings' with
the warning name 'suspicious'.
+---
+*** Warn about 'condition-case' without handlers.
+The compiler now warns when the 'condition-case' form is used without
+any actual handlers, as in
+
+ (condition-case nil (read buffer))
+
+because it has no effect other than the execution of the body form.
+In particular, no errors are caught or suppressed. If the intention
+was to catch all errors, add an explicit handler for 'error', or use
+'ignore-error' or 'ignore-errors'.
+
+This warning can be suppressed using 'with-suppressed-warnings' with
+the warning name 'suspicious'.
+
+++
** New function 'file-user-uid'.
This function is like 'user-uid', but is aware of file name handlers,
diff --git a/lisp/emacs-lisp/macroexp.el b/lisp/emacs-lisp/macroexp.el
index c57a27069d6..8cb67c3b8b5 100644
--- a/lisp/emacs-lisp/macroexp.el
+++ b/lisp/emacs-lisp/macroexp.el
@@ -339,14 +339,19 @@ Assumes the caller has bound
`macroexpand-all-environment'."
(`(cond . ,clauses)
(macroexp--cons fn (macroexp--all-clauses clauses) form))
(`(condition-case . ,(or `(,err ,body . ,handlers)
pcase--dontcare))
- (macroexp--cons
- fn
- (macroexp--cons err
- (macroexp--cons (macroexp--expand-all body)
- (macroexp--all-clauses handlers
1)
- (cddr form))
- (cdr form))
- form))
+ (let ((exp-body (macroexp--expand-all body)))
+ (if handlers
+ (macroexp--cons fn
+ (macroexp--cons
+ err (macroexp--cons
+ exp-body
+ (macroexp--all-clauses handlers 1)
+ (cddr form))
+ (cdr form))
+ form)
+ (macroexp-warn-and-return
+ (format-message "`condition-case' without handlers")
+ exp-body (list 'suspicious 'condition-case) t form))))
(`(,(or 'defvar 'defconst) ,(and name (pred symbolp)) . ,_)
(push name macroexp--dynvars)
(macroexp--all-forms form 2))
diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el
b/test/lisp/emacs-lisp/bytecomp-tests.el
index 185abaf5c22..b6dcfeedb0c 100644
--- a/test/lisp/emacs-lisp/bytecomp-tests.el
+++ b/test/lisp/emacs-lisp/bytecomp-tests.el
@@ -1446,6 +1446,12 @@ literals (Bug#20852)."
'((suspicious set-buffer))
"Warning: Use .with-current-buffer. rather than")
+ (test-suppression
+ '(defun zot (x)
+ (condition-case nil (list x)))
+ '((suspicious condition-case))
+ "Warning: `condition-case' without handlers")
+
(test-suppression
'(defun zot ()
(let ((_ 1))