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.

Sign up for GitHub

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

Jump to bottom

Implement flake8-simplify SIM107 #1650

Merged
merged 2 commits into from Jan 5, 2023
Merged

Implement flake8-simplify SIM107 #1650

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a06982d
Implement flake8-simplify SIM107
messense Jan 5, 2023
4b584ec
Minor style tweaks
charliermarsh Jan 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Expand Up @@ -968,6 +968,7 @@ For more, see [flake8-simplify](https://pypi.org/project/flake8-simplify/0.19.3/
| SIM117 | MultipleWithStatements | Use a single `with` statement with multiple contexts instead of nested `with` statements | |
| SIM102 | NestedIfStatements | Use a single `if` statement instead of nested `if` statements | |
| SIM105 | UseContextlibSuppress | Use `contextlib.suppress(...)` instead of try-except-pass | |
| SIM107 | ReturnInTryExceptFinally | Don't use `return` in `try`/`except` and `finally` | |
| SIM118 | KeyInDict | Use `key in dict` instead of `key in dict.keys()` | 🛠 |
| SIM220 | AAndNotA | Use `False` instead of `... and not ...` | 🛠 |
| SIM221 | AOrNotA | Use `True` instead of `... or not ...` | 🛠 |
Expand Down
22 changes: 22 additions & 0 deletions resources/test/fixtures/flake8_simplify/SIM107.py
View file
Open in desktop
@@ -0,0 +1,22 @@
# Bad: This example returns 3!
def foo():
try:
1 / 0
return "1"
except:
return "2"
finally:
return "3"


# Good
def foo():
return_value = None
try:
1 / 0
return_value = "1"
except:
return_value = "2"
finally:
return_value = "3" # you might also want to check if "except" happened
return return_value
1 change: 1 addition & 0 deletions ruff.schema.json
View file
Open in desktop
Expand Up @@ -932,6 +932,7 @@
"SIM10",
"SIM102",
"SIM105",
"SIM107",
"SIM11",
"SIM117",
"SIM118",
Expand Down
6 changes: 6 additions & 0 deletions src/checkers/ast.rs
View file
Open in desktop
Expand Up @@ -1259,6 +1259,7 @@ where
}
}
StmtKind::Try {
body,
handlers,
orelse,
finalbody,
Expand Down Expand Up @@ -1293,6 +1294,11 @@ where
self, stmt, handlers, orelse, finalbody,
);
}
if self.settings.enabled.contains(&CheckCode::SIM107) {
flake8_simplify::plugins::return_in_try_except_finally(
self, body, handlers, finalbody,
);
}
}
StmtKind::Assign { targets, value, .. } => {
if self.settings.enabled.contains(&CheckCode::E731) {
Expand Down
1 change: 1 addition & 0 deletions src/flake8_simplify/mod.rs
View file
Open in desktop
Expand Up @@ -14,6 +14,7 @@ mod tests {

#[test_case(CheckCode::SIM102, Path::new("SIM102.py"); "SIM102")]
#[test_case(CheckCode::SIM105, Path::new("SIM105.py"); "SIM105")]
#[test_case(CheckCode::SIM107, Path::new("SIM107.py"); "SIM107")]
#[test_case(CheckCode::SIM117, Path::new("SIM117.py"); "SIM117")]
#[test_case(CheckCode::SIM118, Path::new("SIM118.py"); "SIM118")]
#[test_case(CheckCode::SIM220, Path::new("SIM220.py"); "SIM220")]
Expand Down
3 changes: 3 additions & 0 deletions src/flake8_simplify/plugins/mod.rs
View file
Open in desktop
Expand Up @@ -2,11 +2,14 @@ pub use ast_bool_op::{a_and_not_a, a_or_not_a, and_false, or_true};
pub use ast_if::nested_if_statements;
pub use ast_with::multiple_with_statements;
pub use key_in_dict::{key_in_dict_compare, key_in_dict_for};
pub use return_in_try_except_finally::return_in_try_except_finally;
pub use use_contextlib_suppress::use_contextlib_suppress;
pub use yoda_conditions::yoda_conditions;

mod ast_bool_op;
mod ast_if;
mod ast_with;
mod key_in_dict;
mod return_in_try_except_finally;
mod use_contextlib_suppress;
mod yoda_conditions;
34 changes: 34 additions & 0 deletions src/flake8_simplify/plugins/return_in_try_except_finally.rs
View file
Open in desktop
@@ -0,0 +1,34 @@
use rustpython_ast::{Excepthandler, ExcepthandlerKind, Stmt, StmtKind};

use crate::ast::types::Range;
use crate::checkers::ast::Checker;
use crate::registry::{Check, CheckKind};

fn find_return(stmts: &[Stmt]) -> Option<&Stmt> {
stmts
.iter()
.find(|stmt| matches!(stmt.node, StmtKind::Return { .. }))
}

/// SIM107
pub fn return_in_try_except_finally(
checker: &mut Checker,
body: &[Stmt],
handlers: &[Excepthandler],
finalbody: &[Stmt],
) {
let try_has_return = find_return(body).is_some();
let except_has_return = handlers.iter().any(|handler| {
let ExcepthandlerKind::ExceptHandler { body, .. } = &handler.node;
find_return(body).is_some()
});

if let Some(finally_return) = find_return(finalbody) {
if try_has_return || except_has_return {
checker.add_check(Check::new(
CheckKind::ReturnInTryExceptFinally,
Range::from_located(finally_return),
));
}
}
}
14 changes: 14 additions & 0 deletions src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM107_SIM107.py.snap
View file
Open in desktop
@@ -0,0 +1,14 @@
---
source: src/flake8_simplify/mod.rs
expression: checks
---
- kind: ReturnInTryExceptFinally
location:
row: 9
column: 8
end_location:
row: 9
column: 18
fix: ~
parent: ~

26 changes: 17 additions & 9 deletions src/registry.rs
View file
Open in desktop
Expand Up @@ -220,6 +220,7 @@ pub enum CheckCode {
SIM117,
SIM102,
SIM105,
SIM107,
SIM118,
SIM220,
SIM221,
Expand Down Expand Up @@ -882,7 +883,6 @@ pub enum CheckKind {
UnaryPrefixIncrement,
UnreliableCallableCheck,
UnusedLoopControlVariable(String),
UseContextlibSuppress(String),
UselessComparison,
UselessContextlibSuppress,
UselessExpression,
Expand Down Expand Up @@ -956,6 +956,8 @@ pub enum CheckKind {
// flake8-simplify
MultipleWithStatements,
NestedIfStatements,
UseContextlibSuppress(String),
ReturnInTryExceptFinally,
AAndNotA(String),
AOrNotA(String),
KeyInDict(String, String),
Expand Down Expand Up @@ -1391,6 +1393,7 @@ impl CheckCode {
// flake8-simplify
CheckCode::SIM102 => CheckKind::NestedIfStatements,
CheckCode::SIM105 => CheckKind::UseContextlibSuppress("...".to_string()),
CheckCode::SIM107 => CheckKind::ReturnInTryExceptFinally,
CheckCode::SIM117 => CheckKind::MultipleWithStatements,
CheckCode::SIM118 => CheckKind::KeyInDict("key".to_string(), "dict".to_string()),
CheckCode::SIM220 => CheckKind::AAndNotA("...".to_string()),
Expand Down Expand Up @@ -1926,6 +1929,7 @@ impl CheckCode {
// flake8-simplify
CheckCode::SIM102 => CheckCategory::Flake8Simplify,
CheckCode::SIM105 => CheckCategory::Flake8Simplify,
CheckCode::SIM107 => CheckCategory::Flake8Simplify,
CheckCode::SIM117 => CheckCategory::Flake8Simplify,
CheckCode::SIM118 => CheckCategory::Flake8Simplify,
CheckCode::SIM220 => CheckCategory::Flake8Simplify,
Expand Down Expand Up @@ -2179,14 +2183,15 @@ impl CheckKind {
CheckKind::SysVersionCmpStr10 => &CheckCode::YTT302,
CheckKind::SysVersionSlice1Referenced => &CheckCode::YTT303,
// flake8-simplify
CheckKind::NestedIfStatements => &CheckCode::SIM102,
CheckKind::UseContextlibSuppress(..) => &CheckCode::SIM105,
CheckKind::ReturnInTryExceptFinally => &CheckCode::SIM107,
CheckKind::MultipleWithStatements => &CheckCode::SIM117,
CheckKind::KeyInDict(..) => &CheckCode::SIM118,
CheckKind::AAndNotA(..) => &CheckCode::SIM220,
CheckKind::AOrNotA(..) => &CheckCode::SIM221,
CheckKind::AndFalse => &CheckCode::SIM223,
CheckKind::KeyInDict(..) => &CheckCode::SIM118,
CheckKind::MultipleWithStatements => &CheckCode::SIM117,
CheckKind::NestedIfStatements => &CheckCode::SIM102,
CheckKind::OrTrue => &CheckCode::SIM222,
CheckKind::UseContextlibSuppress(..) => &CheckCode::SIM105,
CheckKind::AndFalse => &CheckCode::SIM223,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Desperately wish we could get these to autosort.

CheckKind::YodaConditions(..) => &CheckCode::SIM300,
// pyupgrade
CheckKind::ConvertNamedTupleFunctionalToClass(..) => &CheckCode::UP014,
Expand Down Expand Up @@ -2698,9 +2703,6 @@ impl CheckKind {
CheckKind::FStringDocstring => "f-string used as docstring. This will be interpreted \
by python as a joined string rather than a docstring."
.to_string(),
CheckKind::UseContextlibSuppress(exception) => {
format!("Use `contextlib.suppress({exception})` instead of try-except-pass")
}
CheckKind::UselessContextlibSuppress => {
"No arguments passed to `contextlib.suppress`. No exceptions will be suppressed \
and therefore this context manager is redundant"
Expand Down Expand Up @@ -2945,6 +2947,12 @@ impl CheckKind {
"`sys.version[:1]` referenced (python10), use `sys.version_info`".to_string()
}
// flake8-simplify
CheckKind::UseContextlibSuppress(exception) => {
format!("Use `contextlib.suppress({exception})` instead of try-except-pass")
}
CheckKind::ReturnInTryExceptFinally => {
"Don't use `return` in `try`/`except` and `finally`".to_string()
}
CheckKind::AAndNotA(name) => format!("Use `False` instead of `{name} and not {name}`"),
CheckKind::AOrNotA(name) => format!("Use `True` instead of `{name} or not {name}`"),
CheckKind::AndFalse => "Use `False` instead of `... and False`".to_string(),
Expand Down
8 changes: 7 additions & 1 deletion src/registry_gen.rs
View file
Open in desktop
Expand Up @@ -520,6 +520,7 @@ pub enum CheckCodePrefix {
SIM10,
SIM102,
SIM105,
SIM107,
SIM11,
SIM117,
SIM118,
Expand Down Expand Up @@ -811,6 +812,7 @@ impl CheckCodePrefix {
CheckCode::SIM117,
CheckCode::SIM102,
CheckCode::SIM105,
CheckCode::SIM107,
CheckCode::SIM118,
CheckCode::SIM220,
CheckCode::SIM221,
Expand Down Expand Up @@ -2636,6 +2638,7 @@ impl CheckCodePrefix {
CheckCode::SIM117,
CheckCode::SIM102,
CheckCode::SIM105,
CheckCode::SIM107,
CheckCode::SIM118,
CheckCode::SIM220,
CheckCode::SIM221,
Expand All @@ -2647,11 +2650,13 @@ impl CheckCodePrefix {
CheckCode::SIM117,
CheckCode::SIM102,
CheckCode::SIM105,
CheckCode::SIM107,
CheckCode::SIM118,
],
CheckCodePrefix::SIM10 => vec![CheckCode::SIM102, CheckCode::SIM105],
CheckCodePrefix::SIM10 => vec![CheckCode::SIM102, CheckCode::SIM105, CheckCode::SIM107],
CheckCodePrefix::SIM102 => vec![CheckCode::SIM102],
CheckCodePrefix::SIM105 => vec![CheckCode::SIM105],
CheckCodePrefix::SIM107 => vec![CheckCode::SIM107],
CheckCodePrefix::SIM11 => vec![CheckCode::SIM117, CheckCode::SIM118],
CheckCodePrefix::SIM117 => vec![CheckCode::SIM117],
CheckCodePrefix::SIM118 => vec![CheckCode::SIM118],
Expand Down Expand Up @@ -3627,6 +3632,7 @@ impl CheckCodePrefix {
CheckCodePrefix::SIM10 => SuffixLength::Two,
CheckCodePrefix::SIM102 => SuffixLength::Three,
CheckCodePrefix::SIM105 => SuffixLength::Three,
CheckCodePrefix::SIM107 => SuffixLength::Three,
CheckCodePrefix::SIM11 => SuffixLength::Two,
CheckCodePrefix::SIM117 => SuffixLength::Three,
CheckCodePrefix::SIM118 => SuffixLength::Three,
Expand Down