Skip to content

Commit 60d4694

Browse files
[flake8-logging] Allow closures in except handlers for LOG004 (#24464)
## Summary Fixes #18646. `outside_handlers` walks AST ancestors looking for a `Try` statement whose handler range contains the call offset. It used to bail out at `FunctionDef` nodes to avoid crossing scope boundaries, which meant any `logging.exception()` inside a closure defined in an except block got flagged. The range check applies to where the function is defined, not where it's called (which Ruff can't track). This means functions defined inside an except block but called outside of it won't be flagged. This tradeoff fixes the more common false positive at the cost of a less common false negative. Dropped the `FunctionDef` break. ## Test Plan - Updated fixture to move function-inside-except to no-errors section - Added closure-called-within-except test case - local ecosystem runs showed expected results --------- Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
1 parent d1a23cd commit 60d4694

6 files changed

Lines changed: 56 additions & 99 deletions

File tree

crates/ruff_linter/resources/test/fixtures/flake8_logging/LOG004_0.py

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,17 @@ def _():
2121
exc("")
2222

2323

24+
### No errors
25+
26+
try:
27+
...
28+
except ...:
29+
logging.exception("")
30+
logger.exception("")
31+
exc("")
32+
33+
34+
# Closures defined in except handlers have access to exc_info
2435
try:
2536
...
2637
except ...:
@@ -30,14 +41,12 @@ def _():
3041
exc("")
3142

3243

33-
### No errors
34-
3544
try:
3645
...
3746
except ...:
38-
logging.exception("")
39-
logger.exception("")
40-
exc("")
47+
def on_failure():
48+
logging.exception("closure called within except")
49+
on_failure()
4150

4251

4352
def _():

crates/ruff_linter/src/rules/flake8_logging/helpers.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,6 @@ use ruff_text_size::{Ranged, TextSize};
44

55
pub(super) fn outside_handlers(offset: TextSize, semantic: &SemanticModel) -> bool {
66
for stmt in semantic.current_statements() {
7-
if matches!(stmt, Stmt::FunctionDef(_)) {
8-
break;
9-
}
10-
117
let Stmt::Try(StmtTry { handlers, .. }) = stmt else {
128
continue;
139
};

crates/ruff_linter/src/rules/flake8_logging/rules/exc_info_outside_except_handler.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,27 @@ use crate::{Fix, FixAvailability, Violation};
4141
/// logging.warning("Foobar")
4242
/// ```
4343
///
44+
/// ## Known limitations
45+
/// This rule checks whether a call is _defined_ inside an exception handler, not
46+
/// whether it _executes_ inside one. A function defined in an `except` block but
47+
/// called outside of it will not be flagged, despite the fact that the call may
48+
/// not have access to an active exception at runtime:
49+
///
50+
/// ```python
51+
/// import logging
52+
///
53+
///
54+
/// try:
55+
/// raise ValueError()
56+
/// except Exception:
57+
///
58+
/// def handler():
59+
/// logging.error("Foobar", exc_info=True) # LOG014 not raised (false negative)
60+
///
61+
///
62+
/// handler()
63+
/// ```
64+
///
4465
/// ## Fix safety
4566
/// The fix is always marked as unsafe, as it changes runtime behavior.
4667
///

crates/ruff_linter/src/rules/flake8_logging/rules/log_exception_outside_except_handler.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,27 @@ use crate::{Edit, Fix, FixAvailability, Violation};
3939
/// logging.error("Foobar")
4040
/// ```
4141
///
42+
/// ## Known limitations
43+
/// This rule checks whether a call is _defined_ inside an exception handler, not
44+
/// whether it _executes_ inside one. A function defined in an `except` block but
45+
/// called outside of it will not be flagged, despite the fact that the call may
46+
/// not have access to an active exception at runtime:
47+
///
48+
/// ```python
49+
/// import logging
50+
///
51+
///
52+
/// try:
53+
/// raise ValueError()
54+
/// except Exception:
55+
///
56+
/// def handler():
57+
/// logging.exception("Foobar") # LOG004 not raised (false negative)
58+
///
59+
///
60+
/// handler()
61+
/// ```
62+
///
4263
/// ## Fix safety
4364
/// The fix, if available, will always be marked as unsafe, as it changes runtime behavior.
4465
///

crates/ruff_linter/src/rules/flake8_logging/snapshots/ruff_linter__rules__flake8_logging__tests__LOG004_LOG004_0.py.snap

Lines changed: 0 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -100,54 +100,3 @@ LOG004 `.exception()` call outside exception handlers
100100
| ^^^^^^^
101101
|
102102
help: Replace with `.error()`
103-
104-
LOG004 [*] `.exception()` call outside exception handlers
105-
--> LOG004_0.py:28:9
106-
|
107-
26 | except ...:
108-
27 | def _():
109-
28 | logging.exception("")
110-
| ^^^^^^^^^^^^^^^^^^^^^
111-
29 | logger.exception("")
112-
30 | exc("")
113-
|
114-
help: Replace with `.error()`
115-
25 | ...
116-
26 | except ...:
117-
27 | def _():
118-
- logging.exception("")
119-
28 + logging.error("")
120-
29 | logger.exception("")
121-
30 | exc("")
122-
31 |
123-
note: This is an unsafe fix and may change runtime behavior
124-
125-
LOG004 [*] `.exception()` call outside exception handlers
126-
--> LOG004_0.py:29:9
127-
|
128-
27 | def _():
129-
28 | logging.exception("")
130-
29 | logger.exception("")
131-
| ^^^^^^^^^^^^^^^^^^^^
132-
30 | exc("")
133-
|
134-
help: Replace with `.error()`
135-
26 | except ...:
136-
27 | def _():
137-
28 | logging.exception("")
138-
- logger.exception("")
139-
29 + logger.error("")
140-
30 | exc("")
141-
31 |
142-
32 |
143-
note: This is an unsafe fix and may change runtime behavior
144-
145-
LOG004 `.exception()` call outside exception handlers
146-
--> LOG004_0.py:30:9
147-
|
148-
28 | logging.exception("")
149-
29 | logger.exception("")
150-
30 | exc("")
151-
| ^^^^^^^
152-
|
153-
help: Replace with `.error()`

crates/ruff_linter/src/rules/flake8_logging/snapshots/ruff_linter__rules__flake8_logging__tests__LOG014_LOG014_0.py.snap

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -112,42 +112,3 @@ help: Remove `exc_info=`
112112
24 |
113113
25 | try:
114114
note: This is an unsafe fix and may change runtime behavior
115-
116-
LOG014 [*] `exc_info=` outside exception handlers
117-
--> LOG014_0.py:29:26
118-
|
119-
27 | except ...:
120-
28 | def _():
121-
29 | logging.info("", exc_info=True)
122-
| ^^^^^^^^^^^^^
123-
30 | logger.info("", exc_info=True)
124-
|
125-
help: Remove `exc_info=`
126-
26 | ...
127-
27 | except ...:
128-
28 | def _():
129-
- logging.info("", exc_info=True)
130-
29 + logging.info("")
131-
30 | logger.info("", exc_info=True)
132-
31 |
133-
32 |
134-
note: This is an unsafe fix and may change runtime behavior
135-
136-
LOG014 [*] `exc_info=` outside exception handlers
137-
--> LOG014_0.py:30:25
138-
|
139-
28 | def _():
140-
29 | logging.info("", exc_info=True)
141-
30 | logger.info("", exc_info=True)
142-
| ^^^^^^^^^^^^^
143-
|
144-
help: Remove `exc_info=`
145-
27 | except ...:
146-
28 | def _():
147-
29 | logging.info("", exc_info=True)
148-
- logger.info("", exc_info=True)
149-
30 + logger.info("")
150-
31 |
151-
32 |
152-
33 | ### No errors
153-
note: This is an unsafe fix and may change runtime behavior

0 commit comments

Comments
 (0)