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 flake8-simplify SIM105 rule #1621

Merged
merged 4 commits into from Jan 4, 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
7 changes: 4 additions & 3 deletions README.md
Expand Up @@ -960,9 +960,10 @@ For more, see [flake8-simplify](https://pypi.org/project/flake8-simplify/0.19.3/
| Code | Name | Message | Fix |
| ---- | ---- | ------- | --- |
| 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 ...` | 🛠 |
| SIM221 | AOrNotA | Use `True` instead of `... or not ...` | 🛠 |
| SIM118 | KeyInDict | Use `key in dict` instead of `key in dict.keys()` | 🛠 |
| SIM222 | OrTrue | Use `True` instead of `... or True` | 🛠 |
| SIM223 | AndFalse | Use `False` instead of `... and False` | 🛠 |
| SIM300 | YodaConditions | Use `left == right` instead of `right == left (Yoda-conditions)` | 🛠 |
Expand Down Expand Up @@ -1354,7 +1355,7 @@ natively, including:
- [`flake8-print`](https://pypi.org/project/flake8-print/)
- [`flake8-quotes`](https://pypi.org/project/flake8-quotes/)
- [`flake8-return`](https://pypi.org/project/flake8-return/)
- [`flake8-simplify`](https://pypi.org/project/flake8-simplify/) (6/30)
- [`flake8-simplify`](https://pypi.org/project/flake8-simplify/) (7/30)
- [`flake8-super`](https://pypi.org/project/flake8-super/)
- [`flake8-tidy-imports`](https://pypi.org/project/flake8-tidy-imports/)
- [`isort`](https://pypi.org/project/isort/)
Expand Down Expand Up @@ -1412,7 +1413,7 @@ Today, Ruff can be used to replace Flake8 when used with any of the following pl
- [`flake8-print`](https://pypi.org/project/flake8-print/)
- [`flake8-quotes`](https://pypi.org/project/flake8-quotes/)
- [`flake8-return`](https://pypi.org/project/flake8-return/)
- [`flake8-simplify`](https://pypi.org/project/flake8-simplify/) (6/30)
- [`flake8-simplify`](https://pypi.org/project/flake8-simplify/) (7/30)
- [`flake8-super`](https://pypi.org/project/flake8-super/)
- [`flake8-tidy-imports`](https://pypi.org/project/flake8-tidy-imports/)
- [`mccabe`](https://pypi.org/project/mccabe/)
Expand Down
31 changes: 31 additions & 0 deletions resources/test/fixtures/flake8_simplify/SIM105.py
@@ -0,0 +1,31 @@
def foo():
pass

try:
foo()
except ValueError: # SIM105
pass

try:
foo()
except (ValueError, OSError): # SIM105
pass

try:
foo()
except: # SIM105
pass

try:
foo()
except ValueError:
print('foo')
except OSError:
pass

try:
foo()
except ValueError:
pass
else:
print('bar')
7 changes: 6 additions & 1 deletion src/checkers/ast.rs
Expand Up @@ -1247,7 +1247,9 @@ where
flake8_simplify::plugins::key_in_dict_for(self, target, iter);
}
}
StmtKind::Try { handlers, .. } => {
StmtKind::Try {
handlers, orelse, ..
} => {
if self.settings.enabled.contains(&CheckCode::F707) {
if let Some(check) =
pyflakes::checks::default_except_not_last(handlers, self.locator)
Expand All @@ -1272,6 +1274,9 @@ where
.into_iter(),
);
}
if self.settings.enabled.contains(&CheckCode::SIM105) {
flake8_simplify::plugins::use_contextlib_suppress(self, stmt, handlers, orelse);
}
}
StmtKind::Assign { targets, value, .. } => {
if self.settings.enabled.contains(&CheckCode::E731) {
Expand Down
1 change: 1 addition & 0 deletions src/flake8_simplify/mod.rs
Expand Up @@ -12,6 +12,7 @@ mod tests {
use crate::registry::CheckCode;
use crate::settings;

#[test_case(CheckCode::SIM105, Path::new("SIM105.py"); "SIM105")]
#[test_case(CheckCode::SIM118, Path::new("SIM118.py"); "SIM118")]
#[test_case(CheckCode::SIM222, Path::new("SIM222.py"); "SIM222")]
#[test_case(CheckCode::SIM223, Path::new("SIM223.py"); "SIM223")]
Expand Down
2 changes: 2 additions & 0 deletions src/flake8_simplify/plugins/mod.rs
@@ -1,7 +1,9 @@
pub use bool_ops::{a_and_not_a, a_or_not_a, and_false, or_true};
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 bool_ops;
mod key_in_dict;
mod use_contextlib_suppress;
mod yoda_conditions;
38 changes: 38 additions & 0 deletions src/flake8_simplify/plugins/use_contextlib_suppress.rs
@@ -0,0 +1,38 @@
use rustpython_ast::{Excepthandler, ExcepthandlerKind, Stmt, StmtKind};

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

/// SIM105
pub fn use_contextlib_suppress(
checker: &mut Checker,
stmt: &Stmt,
handlers: &[Excepthandler],
orelse: &[Stmt],
) {
if handlers.len() != 1 || !orelse.is_empty() {
return;
}
let handler = &handlers[0];
let ExcepthandlerKind::ExceptHandler { body, .. } = &handler.node;
if body.len() == 1 {
if matches!(body[0].node, StmtKind::Pass) {
let handler_names: Vec<_> = helpers::extract_handler_names(handlers)
.into_iter()
.flatten()
.collect();
let exception = if handler_names.is_empty() {
"Exception".to_string()
} else {
handler_names.join(", ")
};
let check = Check::new(
CheckKind::UseContextlibSuppress(exception),
Copy link
Member

Choose a reason for hiding this comment

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

This might be a hard one to autofix right now because it depends on contextlib being imported already.

Range::from_located(stmt),
);
checker.add_check(check);
}
}
}
@@ -0,0 +1,35 @@
---
source: src/flake8_simplify/mod.rs
expression: checks
---
- kind:
UseContextlibSuppress: ValueError
location:
row: 4
column: 0
end_location:
row: 7
column: 8
fix: ~
parent: ~
- kind:
UseContextlibSuppress: "ValueError, OSError"
location:
row: 9
column: 0
end_location:
row: 12
column: 8
fix: ~
parent: ~
- kind:
UseContextlibSuppress: Exception
location:
row: 14
column: 0
end_location:
row: 17
column: 8
fix: ~
parent: ~

10 changes: 9 additions & 1 deletion src/registry.rs
Expand Up @@ -217,9 +217,10 @@ pub enum CheckCode {
YTT302,
YTT303,
// flake8-simplify
SIM105,
SIM118,
SIM220,
SIM221,
SIM118,
SIM222,
SIM223,
SIM300,
Expand Down Expand Up @@ -876,6 +877,7 @@ pub enum CheckKind {
UnaryPrefixIncrement,
UnreliableCallableCheck,
UnusedLoopControlVariable(String),
UseContextlibSuppress(String),
UselessComparison,
UselessContextlibSuppress,
UselessExpression,
Expand Down Expand Up @@ -1377,6 +1379,7 @@ impl CheckCode {
// flake8-blind-except
CheckCode::BLE001 => CheckKind::BlindExcept("Exception".to_string()),
// flake8-simplify
CheckCode::SIM105 => CheckKind::UseContextlibSuppress("..".to_string()),
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 @@ -1903,6 +1906,7 @@ impl CheckCode {
CheckCode::S106 => CheckCategory::Flake8Bandit,
CheckCode::S107 => CheckCategory::Flake8Bandit,
// flake8-simplify
CheckCode::SIM105 => CheckCategory::Flake8Simplify,
CheckCode::SIM118 => CheckCategory::Flake8Simplify,
CheckCode::SIM220 => CheckCategory::Flake8Simplify,
CheckCode::SIM221 => CheckCategory::Flake8Simplify,
Expand Down Expand Up @@ -2154,6 +2158,7 @@ impl CheckKind {
CheckKind::SysVersionCmpStr10 => &CheckCode::YTT302,
CheckKind::SysVersionSlice1Referenced => &CheckCode::YTT303,
// flake8-simplify
CheckKind::UseContextlibSuppress(..) => &CheckCode::SIM105,
CheckKind::KeyInDict(..) => &CheckCode::SIM118,
CheckKind::AAndNotA(..) => &CheckCode::SIM220,
CheckKind::AOrNotA(..) => &CheckCode::SIM221,
Expand Down Expand Up @@ -2667,6 +2672,9 @@ 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
14 changes: 11 additions & 3 deletions src/registry_gen.rs
Expand Up @@ -515,6 +515,8 @@ pub enum CheckCodePrefix {
S107,
SIM,
SIM1,
SIM10,
SIM105,
SIM11,
SIM118,
SIM2,
Expand Down Expand Up @@ -801,9 +803,10 @@ impl CheckCodePrefix {
CheckCode::YTT301,
CheckCode::YTT302,
CheckCode::YTT303,
CheckCode::SIM105,
CheckCode::SIM118,
CheckCode::SIM220,
CheckCode::SIM221,
CheckCode::SIM118,
CheckCode::SIM222,
CheckCode::SIM223,
CheckCode::SIM300,
Expand Down Expand Up @@ -2612,14 +2615,17 @@ impl CheckCodePrefix {
CheckCodePrefix::S106 => vec![CheckCode::S106],
CheckCodePrefix::S107 => vec![CheckCode::S107],
CheckCodePrefix::SIM => vec![
CheckCode::SIM105,
CheckCode::SIM118,
CheckCode::SIM220,
CheckCode::SIM221,
CheckCode::SIM118,
CheckCode::SIM222,
CheckCode::SIM223,
CheckCode::SIM300,
],
CheckCodePrefix::SIM1 => vec![CheckCode::SIM118],
CheckCodePrefix::SIM1 => vec![CheckCode::SIM105, CheckCode::SIM118],
CheckCodePrefix::SIM10 => vec![CheckCode::SIM105],
CheckCodePrefix::SIM105 => vec![CheckCode::SIM105],
CheckCodePrefix::SIM11 => vec![CheckCode::SIM118],
CheckCodePrefix::SIM118 => vec![CheckCode::SIM118],
CheckCodePrefix::SIM2 => vec![
Expand Down Expand Up @@ -3583,6 +3589,8 @@ impl CheckCodePrefix {
CheckCodePrefix::S107 => SuffixLength::Three,
CheckCodePrefix::SIM => SuffixLength::Zero,
CheckCodePrefix::SIM1 => SuffixLength::One,
CheckCodePrefix::SIM10 => SuffixLength::Two,
CheckCodePrefix::SIM105 => SuffixLength::Three,
CheckCodePrefix::SIM11 => SuffixLength::Two,
CheckCodePrefix::SIM118 => SuffixLength::Three,
CheckCodePrefix::SIM2 => SuffixLength::One,
Expand Down