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

Implement nested with detection (SIM117) #1651

Merged
merged 1 commit into from Jan 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion README.md
Expand Up @@ -965,7 +965,8 @@ For more, see [flake8-simplify](https://pypi.org/project/flake8-simplify/0.19.3/

| Code | Name | Message | Fix |
| ---- | ---- | ------- | --- |
| SIM102 | NestedIfStatements | Use a single if-statement instead of nested if-statements | |
| 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 | |
| SIM118 | KeyInDict | Use `key in dict` instead of `key in dict.keys()` | 🛠 |
| SIM220 | AAndNotA | Use `False` instead of `... and not ...` | 🛠 |
Expand Down
13 changes: 13 additions & 0 deletions resources/test/fixtures/flake8_simplify/SIM117.py
@@ -0,0 +1,13 @@
with A() as a: # SIM117
with B() as b:
print("hello")

with A() as a:
a()
with B() as b:
print("hello")

with A() as a:
with B() as b:
print("hello")
a()
3 changes: 3 additions & 0 deletions src/checkers/ast.rs
Expand Up @@ -1216,6 +1216,9 @@ where
if self.settings.enabled.contains(&CheckCode::PT012) {
flake8_pytest_style::plugins::complex_raises(self, stmt, items, body);
}
if self.settings.enabled.contains(&CheckCode::SIM117) {
flake8_simplify::plugins::multiple_with_statements(self, stmt);
}
}
StmtKind::While { body, orelse, .. } => {
if self.settings.enabled.contains(&CheckCode::B023) {
Expand Down
7 changes: 4 additions & 3 deletions src/flake8_simplify/mod.rs
Expand Up @@ -12,14 +12,15 @@ mod tests {
use crate::registry::CheckCode;
use crate::settings;

#[test_case(CheckCode::SIM102, Path::new("SIM102.py"); "SIM102")]
#[test_case(CheckCode::SIM105, Path::new("SIM105.py"); "SIM105")]
#[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")]
#[test_case(CheckCode::SIM221, Path::new("SIM221.py"); "SIM221")]
#[test_case(CheckCode::SIM222, Path::new("SIM222.py"); "SIM222")]
#[test_case(CheckCode::SIM223, Path::new("SIM223.py"); "SIM223")]
#[test_case(CheckCode::SIM300, Path::new("SIM300.py"); "SIM300")]
#[test_case(CheckCode::SIM221, Path::new("SIM221.py"); "SIM221")]
#[test_case(CheckCode::SIM220, Path::new("SIM220.py"); "SIM220")]
#[test_case(CheckCode::SIM102, Path::new("SIM102.py"); "SIM102")]
fn checks(check_code: CheckCode, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", check_code.as_ref(), path.to_string_lossy());
let checks = test_path(
Expand Down
21 changes: 21 additions & 0 deletions src/flake8_simplify/plugins/ast_with.rs
@@ -0,0 +1,21 @@
use rustpython_ast::{Stmt, StmtKind};

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

/// SIM117
pub fn multiple_with_statements(checker: &mut Checker, stmt: &Stmt) {
let StmtKind::With { body, .. } = &stmt.node else {
return;
};
if body.len() != 1 {
return;
}
if matches!(body[0].node, StmtKind::With { .. }) {
checker.add_check(Check::new(
CheckKind::MultipleWithStatements,
Range::from_located(stmt),
));
}
}
3 changes: 2 additions & 1 deletion src/flake8_simplify/plugins/mod.rs
@@ -1,11 +1,12 @@
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 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 use_contextlib_suppress;
mod yoda_conditions;
@@ -0,0 +1,14 @@
---
source: src/flake8_simplify/mod.rs
expression: checks
---
- kind: MultipleWithStatements
location:
row: 1
column: 0
end_location:
row: 3
column: 22
fix: ~
parent: ~

27 changes: 17 additions & 10 deletions src/registry.rs
Expand Up @@ -217,6 +217,7 @@ pub enum CheckCode {
YTT302,
YTT303,
// flake8-simplify
SIM117,
SIM102,
SIM105,
SIM118,
Expand Down Expand Up @@ -953,6 +954,7 @@ pub enum CheckKind {
SysVersionCmpStr10,
SysVersionSlice1Referenced,
// flake8-simplify
MultipleWithStatements,
NestedIfStatements,
AAndNotA(String),
AOrNotA(String),
Expand Down Expand Up @@ -1389,6 +1391,7 @@ impl CheckCode {
// flake8-simplify
CheckCode::SIM102 => CheckKind::NestedIfStatements,
CheckCode::SIM105 => CheckKind::UseContextlibSuppress("...".to_string()),
CheckCode::SIM117 => CheckKind::MultipleWithStatements,
CheckCode::SIM118 => CheckKind::KeyInDict("key".to_string(), "dict".to_string()),
CheckCode::SIM220 => CheckKind::AAndNotA("...".to_string()),
CheckCode::SIM221 => CheckKind::AOrNotA("...".to_string()),
Expand Down Expand Up @@ -1923,6 +1926,7 @@ impl CheckCode {
// flake8-simplify
CheckCode::SIM102 => CheckCategory::Flake8Simplify,
CheckCode::SIM105 => CheckCategory::Flake8Simplify,
CheckCode::SIM117 => CheckCategory::Flake8Simplify,
CheckCode::SIM118 => CheckCategory::Flake8Simplify,
CheckCode::SIM220 => CheckCategory::Flake8Simplify,
CheckCode::SIM221 => CheckCategory::Flake8Simplify,
Expand Down Expand Up @@ -2175,13 +2179,14 @@ impl CheckKind {
CheckKind::SysVersionCmpStr10 => &CheckCode::YTT302,
CheckKind::SysVersionSlice1Referenced => &CheckCode::YTT303,
// flake8-simplify
CheckKind::NestedIfStatements => &CheckCode::SIM102,
CheckKind::UseContextlibSuppress(..) => &CheckCode::SIM105,
CheckKind::KeyInDict(..) => &CheckCode::SIM118,
CheckKind::AAndNotA(..) => &CheckCode::SIM220,
CheckKind::AOrNotA(..) => &CheckCode::SIM221,
CheckKind::OrTrue => &CheckCode::SIM222,
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::YodaConditions(..) => &CheckCode::SIM300,
// pyupgrade
CheckKind::ConvertNamedTupleFunctionalToClass(..) => &CheckCode::UP014,
Expand Down Expand Up @@ -2940,16 +2945,18 @@ impl CheckKind {
"`sys.version[:1]` referenced (python10), use `sys.version_info`".to_string()
}
// flake8-simplify
CheckKind::NestedIfStatements => {
"Use a single if-statement instead of nested if-statements".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(),
CheckKind::KeyInDict(key, dict) => {
format!("Use `{key} in {dict}` instead of `{key} in {dict}.keys()`")
}
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::MultipleWithStatements => "Use a single `with` statement with multiple contexts instead of nested `with` \
statements".to_string(),
CheckKind::NestedIfStatements => {
"Use a single `if` statement instead of nested `if` statements".to_string()
}
CheckKind::OrTrue => "Use `True` instead of `... or True`".to_string(),
CheckKind::AndFalse => "Use `False` instead of `... and False`".to_string(),
CheckKind::YodaConditions(left, right) => {
format!("Yoda conditions are discouraged, use `{left} == {right}` instead")
}
Expand Down
14 changes: 12 additions & 2 deletions src/registry_gen.rs
Expand Up @@ -521,6 +521,7 @@ pub enum CheckCodePrefix {
SIM102,
SIM105,
SIM11,
SIM117,
SIM118,
SIM2,
SIM22,
Expand Down Expand Up @@ -807,6 +808,7 @@ impl CheckCodePrefix {
CheckCode::YTT301,
CheckCode::YTT302,
CheckCode::YTT303,
CheckCode::SIM117,
CheckCode::SIM102,
CheckCode::SIM105,
CheckCode::SIM118,
Expand Down Expand Up @@ -2631,6 +2633,7 @@ impl CheckCodePrefix {
CheckCodePrefix::S107 => vec![CheckCode::S107],
CheckCodePrefix::S108 => vec![CheckCode::S108],
CheckCodePrefix::SIM => vec![
CheckCode::SIM117,
CheckCode::SIM102,
CheckCode::SIM105,
CheckCode::SIM118,
Expand All @@ -2640,11 +2643,17 @@ impl CheckCodePrefix {
CheckCode::SIM223,
CheckCode::SIM300,
],
CheckCodePrefix::SIM1 => vec![CheckCode::SIM102, CheckCode::SIM105, CheckCode::SIM118],
CheckCodePrefix::SIM1 => vec![
CheckCode::SIM117,
CheckCode::SIM102,
CheckCode::SIM105,
CheckCode::SIM118,
],
CheckCodePrefix::SIM10 => vec![CheckCode::SIM102, CheckCode::SIM105],
CheckCodePrefix::SIM102 => vec![CheckCode::SIM102],
CheckCodePrefix::SIM105 => vec![CheckCode::SIM105],
CheckCodePrefix::SIM11 => vec![CheckCode::SIM118],
CheckCodePrefix::SIM11 => vec![CheckCode::SIM117, CheckCode::SIM118],
CheckCodePrefix::SIM117 => vec![CheckCode::SIM117],
CheckCodePrefix::SIM118 => vec![CheckCode::SIM118],
CheckCodePrefix::SIM2 => vec![
CheckCode::SIM220,
Expand Down Expand Up @@ -3619,6 +3628,7 @@ impl CheckCodePrefix {
CheckCodePrefix::SIM102 => SuffixLength::Three,
CheckCodePrefix::SIM105 => SuffixLength::Three,
CheckCodePrefix::SIM11 => SuffixLength::Two,
CheckCodePrefix::SIM117 => SuffixLength::Three,
CheckCodePrefix::SIM118 => SuffixLength::Three,
CheckCodePrefix::SIM2 => SuffixLength::One,
CheckCodePrefix::SIM22 => SuffixLength::Two,
Expand Down