Skip to content

Commit

Permalink
Implement SIM118 (key in dict) of flake8-simplify
Browse files Browse the repository at this point in the history
  • Loading branch information
squiddy committed Dec 11, 2022
1 parent c86e521 commit 246cac9
Show file tree
Hide file tree
Showing 10 changed files with 253 additions and 1 deletion.
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ of [Conda](https://docs.conda.io/en/latest/):
1. [flake8-print (T20)](#flake8-print-t20)
1. [flake8-quotes (Q)](#flake8-quotes-q)
1. [flake8-return (RET)](#flake8-return-ret)
1. [flake8-simplify (SIM)](#flake8-simplify-sim)
1. [flake8-tidy-imports (TID)](#flake8-tidy-imports-tid)
1. [flake8-unused-arguments (ARG)](#flake8-unused-arguments-arg)
1. [eradicate (ERA)](#eradicate-era)
Expand Down Expand Up @@ -771,6 +772,14 @@ For more, see [flake8-return](https://pypi.org/project/flake8-return/1.2.0/) on
| RET507 | SuperfluousElseContinue | Unnecessary `else` after `continue` statement | |
| RET508 | SuperfluousElseBreak | Unnecessary `else` after `break` statement | |
### flake8-simplify (SIM)
For more, see [flake8-simplify](https://pypi.org/project/flake8-simplify/0.19.3/) on PyPI.
| Code | Name | Message | Fix |
| ---- | ---- | ------- | --- |
| SIM118 | KeyInDict | Use 'key in dict' instead of 'key in dict.keys() | 🛠 |
### flake8-tidy-imports (TID)
For more, see [flake8-tidy-imports](https://pypi.org/project/flake8-tidy-imports/4.8.0/) on PyPI.
Expand Down
12 changes: 12 additions & 0 deletions resources/test/fixtures/flake8_simplify/SIM118.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
key in dict.keys() # SIM118

foo["bar"] in dict.keys() # SIM118

foo() in dict.keys() # SIM118

for key in dict.keys(): # SIM118
pass

for key in list(dict.keys()):
if some_property(key):
del dict[key]
15 changes: 14 additions & 1 deletion src/check_ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ use crate::visibility::{module_visibility, transition_scope, Modifier, Visibilit
use crate::{
docstrings, flake8_2020, flake8_annotations, flake8_bandit, flake8_blind_except,
flake8_boolean_trap, flake8_bugbear, flake8_builtins, flake8_comprehensions, flake8_debugger,
flake8_import_conventions, flake8_print, flake8_return, flake8_tidy_imports,
flake8_import_conventions, flake8_print, flake8_return, flake8_simplify, flake8_tidy_imports,
flake8_unused_arguments, mccabe, pep8_naming, pycodestyle, pydocstyle, pyflakes, pygrep_hooks,
pylint, pyupgrade, visibility,
};
Expand Down Expand Up @@ -1059,6 +1059,9 @@ where
if self.settings.enabled.contains(&CheckCode::PLW0120) {
pylint::plugins::useless_else_on_loop(self, stmt, body, orelse);
}
if self.settings.enabled.contains(&CheckCode::SIM118) {
flake8_simplify::plugins::key_in_dict_for(self, target, iter);
}
}
StmtKind::Try { handlers, .. } => {
if self.settings.enabled.contains(&CheckCode::F707) {
Expand Down Expand Up @@ -2078,6 +2081,16 @@ where
comparators,
);
}

if self.settings.enabled.contains(&CheckCode::SIM118) {
flake8_simplify::plugins::key_in_dict_compare(
self,
expr,
left,
ops,
comparators,
);
}
}
ExprKind::Constant {
value: Constant::Str(value),
Expand Down
21 changes: 21 additions & 0 deletions src/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,8 @@ pub enum CheckCode {
YTT301,
YTT302,
YTT303,
// flake8-simplify
SIM118,
// pyupgrade
UP001,
UP003,
Expand Down Expand Up @@ -337,6 +339,7 @@ pub enum CheckCategory {
Flake8Print,
Flake8Quotes,
Flake8Return,
Flake8Simplify,
Flake8TidyImports,
Flake8UnusedArguments,
Eradicate,
Expand Down Expand Up @@ -377,6 +380,7 @@ impl CheckCategory {
CheckCategory::Flake8Quotes => "flake8-quotes",
CheckCategory::Flake8Return => "flake8-return",
CheckCategory::Flake8TidyImports => "flake8-tidy-imports",
CheckCategory::Flake8Simplify => "flake8-simplify",
CheckCategory::Flake8UnusedArguments => "flake8-unused-arguments",
CheckCategory::Isort => "isort",
CheckCategory::McCabe => "mccabe",
Expand Down Expand Up @@ -406,6 +410,7 @@ impl CheckCategory {
CheckCategory::Flake8Print => vec![CheckCodePrefix::T20],
CheckCategory::Flake8Quotes => vec![CheckCodePrefix::Q],
CheckCategory::Flake8Return => vec![CheckCodePrefix::RET],
CheckCategory::Flake8Simplify => vec![CheckCodePrefix::SIM],
CheckCategory::Flake8TidyImports => vec![CheckCodePrefix::TID],
CheckCategory::Flake8UnusedArguments => vec![CheckCodePrefix::ARG],
CheckCategory::Isort => vec![CheckCodePrefix::I],
Expand Down Expand Up @@ -481,6 +486,10 @@ impl CheckCategory {
"https://pypi.org/project/flake8-return/1.2.0/",
&Platform::PyPI,
)),
CheckCategory::Flake8Simplify => Some((
"https://pypi.org/project/flake8-simplify/0.19.3/",
&Platform::PyPI,
)),
CheckCategory::Flake8TidyImports => Some((
"https://pypi.org/project/flake8-tidy-imports/4.8.0/",
&Platform::PyPI,
Expand Down Expand Up @@ -746,6 +755,8 @@ pub enum CheckKind {
SysVersion0Referenced,
SysVersionCmpStr10,
SysVersionSlice1Referenced,
// flake8-simplify
KeyInDict(String, String),
// pyupgrade
TypeOfPrimitive(Primitive),
UselessMetaclassType,
Expand Down Expand Up @@ -1082,6 +1093,8 @@ impl CheckCode {
CheckCode::YTT303 => CheckKind::SysVersionSlice1Referenced,
// flake8-blind-except
CheckCode::BLE001 => CheckKind::BlindExcept("Exception".to_string()),
// flake8-simplify
CheckCode::SIM118 => CheckKind::KeyInDict("key".to_string(), "dict".to_string()),
// pyupgrade
CheckCode::UP001 => CheckKind::UselessMetaclassType,
CheckCode::UP003 => CheckKind::TypeOfPrimitive(Primitive::Str),
Expand Down Expand Up @@ -1442,6 +1455,7 @@ impl CheckCode {
CheckCode::S105 => CheckCategory::Flake8Bandit,
CheckCode::S106 => CheckCategory::Flake8Bandit,
CheckCode::S107 => CheckCategory::Flake8Bandit,
CheckCode::SIM118 => CheckCategory::Flake8Simplify,
CheckCode::T100 => CheckCategory::Flake8Debugger,
CheckCode::T201 => CheckCategory::Flake8Print,
CheckCode::T203 => CheckCategory::Flake8Print,
Expand Down Expand Up @@ -1650,6 +1664,8 @@ impl CheckKind {
CheckKind::SysVersion0Referenced => &CheckCode::YTT301,
CheckKind::SysVersionCmpStr10 => &CheckCode::YTT302,
CheckKind::SysVersionSlice1Referenced => &CheckCode::YTT303,
// flake8-simplify
CheckKind::KeyInDict(..) => &CheckCode::SIM118,
// pyupgrade
CheckKind::TypeOfPrimitive(_) => &CheckCode::UP003,
CheckKind::UselessMetaclassType => &CheckCode::UP001,
Expand Down Expand Up @@ -2316,6 +2332,10 @@ impl CheckKind {
CheckKind::SysVersionSlice1Referenced => {
"`sys.version[:1]` referenced (python10), use `sys.version_info`".to_string()
}
// flake8-simplify
CheckKind::KeyInDict(key, dict) => {
format!("Use '{key} in {dict}' instead of '{key} in {dict}.keys()")
}
// pyupgrade
CheckKind::TypeOfPrimitive(primitive) => {
format!("Use `{}` instead of `type(...)`", primitive.builtin())
Expand Down Expand Up @@ -2660,6 +2680,7 @@ impl CheckKind {
| CheckKind::ImplicitReturn
| CheckKind::ImplicitReturnValue
| CheckKind::IsLiteral
| CheckKind::KeyInDict(..)
| CheckKind::MisplacedComparisonConstant(..)
| CheckKind::NewLineAfterLastParagraph
| CheckKind::NewLineAfterSectionName(..)
Expand Down
13 changes: 13 additions & 0 deletions src/checks_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,10 @@ pub enum CheckCodePrefix {
S105,
S106,
S107,
SIM,
SIM1,
SIM11,
SIM118,
T,
T1,
T10,
Expand Down Expand Up @@ -1494,6 +1498,10 @@ impl CheckCodePrefix {
CheckCodePrefix::S105 => vec![CheckCode::S105],
CheckCodePrefix::S106 => vec![CheckCode::S106],
CheckCodePrefix::S107 => vec![CheckCode::S107],
CheckCodePrefix::SIM => vec![CheckCode::SIM118],
CheckCodePrefix::SIM1 => vec![CheckCode::SIM118],
CheckCodePrefix::SIM11 => vec![CheckCode::SIM118],
CheckCodePrefix::SIM118 => vec![CheckCode::SIM118],
CheckCodePrefix::T => vec![CheckCode::T100, CheckCode::T201, CheckCode::T203],
CheckCodePrefix::T1 => vec![CheckCode::T100],
CheckCodePrefix::T10 => vec![CheckCode::T100],
Expand Down Expand Up @@ -2203,6 +2211,10 @@ impl CheckCodePrefix {
CheckCodePrefix::S105 => SuffixLength::Three,
CheckCodePrefix::S106 => SuffixLength::Three,
CheckCodePrefix::S107 => SuffixLength::Three,
CheckCodePrefix::SIM => SuffixLength::Zero,
CheckCodePrefix::SIM1 => SuffixLength::One,
CheckCodePrefix::SIM11 => SuffixLength::Two,
CheckCodePrefix::SIM118 => SuffixLength::Three,
CheckCodePrefix::T => SuffixLength::Zero,
CheckCodePrefix::T1 => SuffixLength::One,
CheckCodePrefix::T10 => SuffixLength::Two,
Expand Down Expand Up @@ -2303,6 +2315,7 @@ pub const CATEGORIES: &[CheckCodePrefix] = &[
CheckCodePrefix::RET,
CheckCodePrefix::RUF,
CheckCodePrefix::S,
CheckCodePrefix::SIM,
CheckCodePrefix::T,
CheckCodePrefix::TID,
CheckCodePrefix::UP,
Expand Down
29 changes: 29 additions & 0 deletions src/flake8_simplify/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
pub mod plugins;

#[cfg(test)]
mod tests {
use std::convert::AsRef;
use std::path::Path;

use anyhow::Result;
use test_case::test_case;

use crate::checks::CheckCode;
use crate::linter::test_path;
use crate::settings;

#[test_case(CheckCode::SIM118, Path::new("SIM118.py"); "SIM118")]
fn checks(check_code: CheckCode, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", check_code.as_ref(), path.to_string_lossy());
let mut checks = test_path(
Path::new("./resources/test/fixtures/flake8_simplify")
.join(path)
.as_path(),
&settings::Settings::for_rule(check_code),
true,
)?;
checks.sort_by_key(|check| check.location);
insta::assert_yaml_snapshot!(snapshot, checks);
Ok(())
}
}
74 changes: 74 additions & 0 deletions src/flake8_simplify/plugins/key_in_dict.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
use rustpython_ast::{Cmpop, Expr, ExprKind};

use crate::ast::types::Range;
use crate::autofix::Fix;
use crate::check_ast::Checker;
use crate::checks::{Check, CheckCode, CheckKind};

/// SIM118
fn key_in_dict(checker: &mut Checker, left: &Expr, right: &Expr, range: Range) {
let ExprKind::Call {
func,
args,
keywords,
} = &right.node else {
return;
};
if !(args.is_empty() && keywords.is_empty()) {
return;
}

let ExprKind::Attribute { attr, value, .. } = &func.node else {
return;
};
if attr != "keys" {
return;
}

let mut check = Check::new(
CheckKind::KeyInDict(left.to_string(), value.to_string()),
range,
);
if checker.patch(&CheckCode::SIM118) {
let content = right.to_string().replace(".keys()", "");
check.amend(Fix::replacement(
content,
right.location,
right.end_location.unwrap(),
));
}
checker.add_check(check);
}

/// SIM118 in a for loop
pub fn key_in_dict_for(checker: &mut Checker, target: &Expr, iter: &Expr) {
key_in_dict(
checker,
target,
iter,
Range {
location: target.location,
end_location: iter.end_location.unwrap(),
},
);
}

/// SIM118 in a comparison
pub fn key_in_dict_compare(
checker: &mut Checker,
expr: &Expr,
left: &Expr,
ops: &[Cmpop],
comparators: &[Expr],
) {
if !matches!(ops[..], [Cmpop::In]) {
return;
}

if comparators.len() != 1 {
return;
}
let right = comparators.first().unwrap();

key_in_dict(checker, left, right, Range::from_located(expr));
}
3 changes: 3 additions & 0 deletions src/flake8_simplify/plugins/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
pub use key_in_dict::{key_in_dict_compare, key_in_dict_for};

mod key_in_dict;
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
---
source: src/flake8_simplify/mod.rs
expression: checks
---
- kind:
KeyInDict:
- key
- dict
location:
row: 1
column: 0
end_location:
row: 1
column: 18
fix:
content: dict
location:
row: 1
column: 7
end_location:
row: 1
column: 18
- kind:
KeyInDict:
- "foo['bar']"
- dict
location:
row: 3
column: 0
end_location:
row: 3
column: 25
fix:
content: dict
location:
row: 3
column: 14
end_location:
row: 3
column: 25
- kind:
KeyInDict:
- foo()
- dict
location:
row: 5
column: 0
end_location:
row: 5
column: 20
fix:
content: dict
location:
row: 5
column: 9
end_location:
row: 5
column: 20
- kind:
KeyInDict:
- key
- dict
location:
row: 7
column: 4
end_location:
row: 7
column: 22
fix:
content: dict
location:
row: 7
column: 11
end_location:
row: 7
column: 22

Loading

0 comments on commit 246cac9

Please sign in to comment.