Skip to content

Commit 7ef7e0d

Browse files
diceroll123diceroll123
authored
[tryceratops] Add fix for error-instead-of-exception (TRY400) (#9520)
## Summary add autofix for `error-instead-of-exception` (`TRY400`) ## Test Plan `cargo test`
1 parent 2bddde2 commit 7ef7e0d

File tree

5 files changed

+316
-6
lines changed

5 files changed

+316
-6
lines changed

‎crates/ruff_linter/src/rules/tryceratops/mod.rs

+20
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ mod tests {
1111
use test_case::test_case;
1212

1313
use crate::registry::Rule;
14+
use crate::settings::types::PreviewMode;
15+
use crate::settings::LinterSettings;
1416
use crate::test::test_path;
1517
use crate::{assert_messages, settings};
1618

@@ -33,4 +35,22 @@ mod tests {
3335
assert_messages!(snapshot, diagnostics);
3436
Ok(())
3537
}
38+
39+
#[test_case(Rule::ErrorInsteadOfException, Path::new("TRY400.py"))]
40+
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
41+
let snapshot = format!(
42+
"preview__{}_{}",
43+
rule_code.noqa_code(),
44+
path.to_string_lossy()
45+
);
46+
let diagnostics = test_path(
47+
Path::new("tryceratops").join(path).as_path(),
48+
&LinterSettings {
49+
preview: PreviewMode::Enabled,
50+
..LinterSettings::for_rule(rule_code)
51+
},
52+
)?;
53+
assert_messages!(snapshot, diagnostics);
54+
Ok(())
55+
}
3656
}

‎crates/ruff_linter/src/rules/tryceratops/rules/error_instead_of_exception.rs

+69-5
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
1-
use ruff_diagnostics::{Diagnostic, Violation};
1+
use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation};
22
use ruff_macros::{derive_message_formats, violation};
33
use ruff_python_ast::visitor::Visitor;
4-
use ruff_python_ast::{self as ast, ExceptHandler};
4+
use ruff_python_ast::{self as ast, ExceptHandler, Expr};
55
use ruff_python_semantic::analyze::logging::exc_info;
66
use ruff_python_stdlib::logging::LoggingLevel;
77
use ruff_text_size::Ranged;
88

99
use crate::checkers::ast::Checker;
10+
use crate::importer::ImportRequest;
1011
use crate::rules::tryceratops::helpers::LoggerCandidateVisitor;
1112

1213
/// ## What it does
@@ -42,16 +43,28 @@ use crate::rules::tryceratops::helpers::LoggerCandidateVisitor;
4243
/// logging.exception("Exception occurred")
4344
/// ```
4445
///
46+
/// ## Fix safety
47+
/// This rule's fix is marked as safe when run against `logging.error` calls,
48+
/// but unsafe when marked against other logger-like calls (e.g.,
49+
/// `logger.error`), since the rule is prone to false positives when detecting
50+
/// logger-like calls outside of the `logging` module.
51+
///
4552
/// ## References
4653
/// - [Python documentation: `logging.exception`](https://docs.python.org/3/library/logging.html#logging.exception)
4754
#[violation]
4855
pub struct ErrorInsteadOfException;
4956

5057
impl Violation for ErrorInsteadOfException {
58+
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
59+
5160
#[derive_message_formats]
5261
fn message(&self) -> String {
5362
format!("Use `logging.exception` instead of `logging.error`")
5463
}
64+
65+
fn fix_title(&self) -> Option<String> {
66+
Some(format!("Replace with `exception`"))
67+
}
5568
}
5669

5770
/// TRY400
@@ -67,9 +80,60 @@ pub(crate) fn error_instead_of_exception(checker: &mut Checker, handlers: &[Exce
6780
for (expr, logging_level) in calls {
6881
if matches!(logging_level, LoggingLevel::Error) {
6982
if exc_info(&expr.arguments, checker.semantic()).is_none() {
70-
checker
71-
.diagnostics
72-
.push(Diagnostic::new(ErrorInsteadOfException, expr.range()));
83+
let mut diagnostic = Diagnostic::new(ErrorInsteadOfException, expr.range());
84+
if checker.settings.preview.is_enabled() {
85+
match expr.func.as_ref() {
86+
Expr::Attribute(ast::ExprAttribute { attr, .. }) => {
87+
diagnostic.set_fix(Fix::applicable_edit(
88+
Edit::range_replacement("exception".to_string(), attr.range()),
89+
// When run against `logging.error`, the fix is safe; otherwise,
90+
// the object _may_ not be a logger.
91+
if checker
92+
.semantic()
93+
.resolve_call_path(expr.func.as_ref())
94+
.is_some_and(|call_path| {
95+
matches!(call_path.as_slice(), ["logging", "error"])
96+
})
97+
{
98+
Applicability::Safe
99+
} else {
100+
Applicability::Unsafe
101+
},
102+
));
103+
}
104+
Expr::Name(_) => {
105+
diagnostic.try_set_fix(|| {
106+
let (import_edit, binding) =
107+
checker.importer().get_or_import_symbol(
108+
&ImportRequest::import("logging", "exception"),
109+
expr.start(),
110+
checker.semantic(),
111+
)?;
112+
let name_edit =
113+
Edit::range_replacement(binding, expr.func.range());
114+
Ok(Fix::applicable_edits(
115+
import_edit,
116+
[name_edit],
117+
// When run against `logging.error`, the fix is safe; otherwise,
118+
// the object _may_ not be a logger.
119+
if checker
120+
.semantic()
121+
.resolve_call_path(expr.func.as_ref())
122+
.is_some_and(|call_path| {
123+
matches!(call_path.as_slice(), ["logging", "error"])
124+
})
125+
{
126+
Applicability::Safe
127+
} else {
128+
Applicability::Unsafe
129+
},
130+
))
131+
});
132+
}
133+
_ => {}
134+
}
135+
}
136+
checker.diagnostics.push(diagnostic);
73137
}
74138
}
75139
}

‎crates/ruff_linter/src/rules/tryceratops/snapshots/ruff_linter__rules__tryceratops__tests__error-instead-of-exception_TRY400.py.snap

+11
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,15 @@ TRY400.py:16:9: TRY400 Use `logging.exception` instead of `logging.error`
1010
17 |
1111
18 | if True:
1212
|
13+
= help: Replace with `exception`
1314

1415
TRY400.py:19:13: TRY400 Use `logging.exception` instead of `logging.error`
1516
|
1617
18 | if True:
1718
19 | logging.error("Context message here")
1819
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY400
1920
|
21+
= help: Replace with `exception`
2022

2123
TRY400.py:26:9: TRY400 Use `logging.exception` instead of `logging.error`
2224
|
@@ -27,13 +29,15 @@ TRY400.py:26:9: TRY400 Use `logging.exception` instead of `logging.error`
2729
27 |
2830
28 | if True:
2931
|
32+
= help: Replace with `exception`
3033

3134
TRY400.py:29:13: TRY400 Use `logging.exception` instead of `logging.error`
3235
|
3336
28 | if True:
3437
29 | logger.error("Context message here")
3538
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY400
3639
|
40+
= help: Replace with `exception`
3741

3842
TRY400.py:36:9: TRY400 Use `logging.exception` instead of `logging.error`
3943
|
@@ -44,13 +48,15 @@ TRY400.py:36:9: TRY400 Use `logging.exception` instead of `logging.error`
4448
37 |
4549
38 | if True:
4650
|
51+
= help: Replace with `exception`
4752

4853
TRY400.py:39:13: TRY400 Use `logging.exception` instead of `logging.error`
4954
|
5055
38 | if True:
5156
39 | log.error("Context message here")
5257
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY400
5358
|
59+
= help: Replace with `exception`
5460

5561
TRY400.py:46:9: TRY400 Use `logging.exception` instead of `logging.error`
5662
|
@@ -61,13 +67,15 @@ TRY400.py:46:9: TRY400 Use `logging.exception` instead of `logging.error`
6167
47 |
6268
48 | if True:
6369
|
70+
= help: Replace with `exception`
6471

6572
TRY400.py:49:13: TRY400 Use `logging.exception` instead of `logging.error`
6673
|
6774
48 | if True:
6875
49 | self.logger.error("Context message here")
6976
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY400
7077
|
78+
= help: Replace with `exception`
7179

7280
TRY400.py:87:9: TRY400 Use `logging.exception` instead of `logging.error`
7381
|
@@ -78,13 +86,15 @@ TRY400.py:87:9: TRY400 Use `logging.exception` instead of `logging.error`
7886
88 |
7987
89 | if True:
8088
|
89+
= help: Replace with `exception`
8190

8291
TRY400.py:90:13: TRY400 Use `logging.exception` instead of `logging.error`
8392
|
8493
89 | if True:
8594
90 | error("Context message here")
8695
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY400
8796
|
97+
= help: Replace with `exception`
8898

8999
TRY400.py:121:13: TRY400 Use `logging.exception` instead of `logging.error`
90100
|
@@ -93,5 +103,6 @@ TRY400.py:121:13: TRY400 Use `logging.exception` instead of `logging.error`
93103
121 | error("Context message here")
94104
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY400
95105
|
106+
= help: Replace with `exception`
96107

97108

0 commit comments

Comments
 (0)