Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[tryceratops] Add fix for error-instead-of-exception (TRY400) #9520

Merged
merged 6 commits into from
Jan 16, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions crates/ruff_linter/src/rules/tryceratops/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ mod tests {
use test_case::test_case;

use crate::registry::Rule;
use crate::settings::types::PreviewMode;
use crate::settings::LinterSettings;
use crate::test::test_path;
use crate::{assert_messages, settings};

Expand All @@ -33,4 +35,22 @@ mod tests {
assert_messages!(snapshot, diagnostics);
Ok(())
}

#[test_case(Rule::ErrorInsteadOfException, Path::new("TRY400.py"))]
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
"preview__{}_{}",
rule_code.noqa_code(),
path.to_string_lossy()
);
let diagnostics = test_path(
Path::new("tryceratops").join(path).as_path(),
&LinterSettings {
preview: PreviewMode::Enabled,
..LinterSettings::for_rule(rule_code)
},
)?;
assert_messages!(snapshot, diagnostics);
Ok(())
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ast::{Expr, ExprAttribute};
use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::visitor::Visitor;
use ruff_python_ast::{self as ast, ExceptHandler};
Expand Down Expand Up @@ -48,10 +49,16 @@ use crate::rules::tryceratops::helpers::LoggerCandidateVisitor;
pub struct ErrorInsteadOfException;

impl Violation for ErrorInsteadOfException {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;

#[derive_message_formats]
fn message(&self) -> String {
format!("Use `logging.exception` instead of `logging.error`")
}

fn fix_title(&self) -> Option<String> {
Some(format!("Replace with `exception`"))
}
}

/// TRY400
Expand All @@ -67,9 +74,20 @@ pub(crate) fn error_instead_of_exception(checker: &mut Checker, handlers: &[Exce
for (expr, logging_level) in calls {
if matches!(logging_level, LoggingLevel::Error) {
if exc_info(&expr.arguments, checker.semantic()).is_none() {
checker
.diagnostics
.push(Diagnostic::new(ErrorInsteadOfException, expr.range()));
let mut diagnostic = Diagnostic::new(ErrorInsteadOfException, expr.range());

if checker.settings.preview.is_enabled() {
let Expr::Attribute(ExprAttribute { attr, .. }) = expr.func.as_ref() else {
return;
};

diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
"exception".to_string(),
attr.range(),
)));
}

checker.diagnostics.push(diagnostic);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@ TRY400.py:16:9: TRY400 Use `logging.exception` instead of `logging.error`
17 |
18 | if True:
|
= help: Replace with `exception`

TRY400.py:19:13: TRY400 Use `logging.exception` instead of `logging.error`
|
18 | if True:
19 | logging.error("Context message here")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY400
|
= help: Replace with `exception`

TRY400.py:26:9: TRY400 Use `logging.exception` instead of `logging.error`
|
Expand All @@ -27,13 +29,15 @@ TRY400.py:26:9: TRY400 Use `logging.exception` instead of `logging.error`
27 |
28 | if True:
|
= help: Replace with `exception`

TRY400.py:29:13: TRY400 Use `logging.exception` instead of `logging.error`
|
28 | if True:
29 | logger.error("Context message here")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY400
|
= help: Replace with `exception`

TRY400.py:36:9: TRY400 Use `logging.exception` instead of `logging.error`
|
Expand All @@ -44,13 +48,15 @@ TRY400.py:36:9: TRY400 Use `logging.exception` instead of `logging.error`
37 |
38 | if True:
|
= help: Replace with `exception`

TRY400.py:39:13: TRY400 Use `logging.exception` instead of `logging.error`
|
38 | if True:
39 | log.error("Context message here")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY400
|
= help: Replace with `exception`

TRY400.py:46:9: TRY400 Use `logging.exception` instead of `logging.error`
|
Expand All @@ -61,13 +67,15 @@ TRY400.py:46:9: TRY400 Use `logging.exception` instead of `logging.error`
47 |
48 | if True:
|
= help: Replace with `exception`

TRY400.py:49:13: TRY400 Use `logging.exception` instead of `logging.error`
|
48 | if True:
49 | self.logger.error("Context message here")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY400
|
= help: Replace with `exception`

TRY400.py:87:9: TRY400 Use `logging.exception` instead of `logging.error`
|
Expand All @@ -78,13 +86,15 @@ TRY400.py:87:9: TRY400 Use `logging.exception` instead of `logging.error`
88 |
89 | if True:
|
= help: Replace with `exception`

TRY400.py:90:13: TRY400 Use `logging.exception` instead of `logging.error`
|
89 | if True:
90 | error("Context message here")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY400
|
= help: Replace with `exception`

TRY400.py:121:13: TRY400 Use `logging.exception` instead of `logging.error`
|
Expand All @@ -93,5 +103,6 @@ TRY400.py:121:13: TRY400 Use `logging.exception` instead of `logging.error`
121 | error("Context message here")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY400
|
= help: Replace with `exception`


Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
---
source: crates/ruff_linter/src/rules/tryceratops/mod.rs
---
TRY400.py:16:9: TRY400 [*] Use `logging.exception` instead of `logging.error`
|
14 | a = 1
15 | except Exception:
16 | logging.error("Context message here")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY400
17 |
18 | if True:
|
= help: Replace with `exception`

ℹ Safe fix
13 13 | try:
14 14 | a = 1
15 15 | except Exception:
16 |- logging.error("Context message here")
16 |+ logging.exception("Context message here")
17 17 |
18 18 | if True:
19 19 | logging.error("Context message here")

TRY400.py:19:13: TRY400 [*] Use `logging.exception` instead of `logging.error`
|
18 | if True:
19 | logging.error("Context message here")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY400
|
= help: Replace with `exception`

ℹ Safe fix
16 16 | logging.error("Context message here")
17 17 |
18 18 | if True:
19 |- logging.error("Context message here")
19 |+ logging.exception("Context message here")
20 20 |
21 21 |
22 22 | def bad():

TRY400.py:26:9: TRY400 [*] Use `logging.exception` instead of `logging.error`
|
24 | a = 1
25 | except Exception:
26 | logger.error("Context message here")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY400
27 |
28 | if True:
|
= help: Replace with `exception`

ℹ Safe fix
23 23 | try:
24 24 | a = 1
25 25 | except Exception:
26 |- logger.error("Context message here")
26 |+ logger.exception("Context message here")
27 27 |
28 28 | if True:
29 29 | logger.error("Context message here")

TRY400.py:29:13: TRY400 [*] Use `logging.exception` instead of `logging.error`
|
28 | if True:
29 | logger.error("Context message here")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY400
|
= help: Replace with `exception`

ℹ Safe fix
26 26 | logger.error("Context message here")
27 27 |
28 28 | if True:
29 |- logger.error("Context message here")
29 |+ logger.exception("Context message here")
30 30 |
31 31 |
32 32 | def bad():

TRY400.py:36:9: TRY400 [*] Use `logging.exception` instead of `logging.error`
|
34 | a = 1
35 | except Exception:
36 | log.error("Context message here")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY400
37 |
38 | if True:
|
= help: Replace with `exception`

ℹ Safe fix
33 33 | try:
34 34 | a = 1
35 35 | except Exception:
36 |- log.error("Context message here")
36 |+ log.exception("Context message here")
37 37 |
38 38 | if True:
39 39 | log.error("Context message here")

TRY400.py:39:13: TRY400 [*] Use `logging.exception` instead of `logging.error`
|
38 | if True:
39 | log.error("Context message here")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY400
|
= help: Replace with `exception`

ℹ Safe fix
36 36 | log.error("Context message here")
37 37 |
38 38 | if True:
39 |- log.error("Context message here")
39 |+ log.exception("Context message here")
40 40 |
41 41 |
42 42 | def bad():

TRY400.py:46:9: TRY400 [*] Use `logging.exception` instead of `logging.error`
|
44 | a = 1
45 | except Exception:
46 | self.logger.error("Context message here")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY400
47 |
48 | if True:
|
= help: Replace with `exception`

ℹ Safe fix
43 43 | try:
44 44 | a = 1
45 45 | except Exception:
46 |- self.logger.error("Context message here")
46 |+ self.logger.exception("Context message here")
47 47 |
48 48 | if True:
49 49 | self.logger.error("Context message here")

TRY400.py:49:13: TRY400 [*] Use `logging.exception` instead of `logging.error`
|
48 | if True:
49 | self.logger.error("Context message here")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY400
|
= help: Replace with `exception`

ℹ Safe fix
46 46 | self.logger.error("Context message here")
47 47 |
48 48 | if True:
49 |- self.logger.error("Context message here")
49 |+ self.logger.exception("Context message here")
50 50 |
51 51 |
52 52 | def good():


Loading