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 SIM220 and SIM221 #1630

Merged
merged 1 commit into from Jan 4, 2023
Merged

Implement SIM220 and SIM221 #1630

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 6 additions & 4 deletions README.md
View file
Open in desktop
Expand Up @@ -960,6 +960,8 @@ For more, see [flake8-simplify](https://pypi.org/project/flake8-simplify/0.19.3/

| Code | Name | Message | Fix |
| ---- | ---- | ------- | --- |
| 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` | 🛠 |
Expand Down Expand Up @@ -1352,9 +1354,9 @@ 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/) (4/37)
- [`flake8-simplify`](https://pypi.org/project/flake8-simplify/) (6/30)
- [`flake8-super`](https://pypi.org/project/flake8-super/)
- [`flake8-tidy-imports`](https://pypi.org/project/flake8-tidy-imports/) (1/3)
- [`flake8-tidy-imports`](https://pypi.org/project/flake8-tidy-imports/)
- [`isort`](https://pypi.org/project/isort/)
- [`mccabe`](https://pypi.org/project/mccabe/)
- [`pep8-naming`](https://pypi.org/project/pep8-naming/)
Expand Down Expand Up @@ -1410,9 +1412,9 @@ 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/) (4/37)
- [`flake8-simplify`](https://pypi.org/project/flake8-simplify/) (6/30)
- [`flake8-super`](https://pypi.org/project/flake8-super/)
- [`flake8-tidy-imports`](https://pypi.org/project/flake8-tidy-imports/) (1/3)
- [`flake8-tidy-imports`](https://pypi.org/project/flake8-tidy-imports/)
- [`mccabe`](https://pypi.org/project/mccabe/)
- [`pep8-naming`](https://pypi.org/project/pep8-naming/)
- [`pydocstyle`](https://pypi.org/project/pydocstyle/)
Expand Down
17 changes: 17 additions & 0 deletions resources/test/fixtures/flake8_simplify/SIM220.py
View file
Open in desktop
@@ -0,0 +1,17 @@
if a and not a:
pass

if (a and not a) and b:
pass

if (a and not a) or b:
pass

if a:
pass

if not a:
pass

if a and not b:
pass
17 changes: 17 additions & 0 deletions resources/test/fixtures/flake8_simplify/SIM221.py
View file
Open in desktop
@@ -0,0 +1,17 @@
if a or not a:
pass

if (a or not a) or b:
pass

if (a or not a) and b:
pass

if a:
pass

if not a:
pass

if a or not b:
pass
2 changes: 2 additions & 0 deletions ruff.schema.json
View file
Open in desktop
Expand Up @@ -899,6 +899,8 @@
"SIM118",
"SIM2",
"SIM22",
"SIM220",
"SIM221",
"SIM222",
"SIM223",
"SIM3",
Expand Down
6 changes: 6 additions & 0 deletions src/checkers/ast.rs
View file
Open in desktop
Expand Up @@ -2578,6 +2578,12 @@ where
if self.settings.enabled.contains(&CheckCode::PLR1701) {
pylint::plugins::merge_isinstance(self, expr, op, values);
}
if self.settings.enabled.contains(&CheckCode::SIM220) {
flake8_simplify::plugins::a_and_not_a(self, expr);
}
if self.settings.enabled.contains(&CheckCode::SIM221) {
flake8_simplify::plugins::a_or_not_a(self, expr);
}
if self.settings.enabled.contains(&CheckCode::SIM222) {
flake8_simplify::plugins::or_true(self, expr);
}
Expand Down
2 changes: 2 additions & 0 deletions src/flake8_simplify/mod.rs
View file
Open in desktop
Expand Up @@ -16,6 +16,8 @@ mod tests {
#[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")]
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
107 changes: 106 additions & 1 deletion src/flake8_simplify/plugins/bool_ops.rs
View file
Open in desktop
@@ -1,10 +1,115 @@
use rustpython_ast::{Boolop, Constant, Expr, ExprKind};
use rustpython_ast::{Boolop, Constant, Expr, ExprKind, Unaryop};

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

fn is_same_expr<'a>(a: &'a Expr, b: &'a Expr) -> Option<&'a str> {
if let (ExprKind::Name { id: a, .. }, ExprKind::Name { id: b, .. }) = (&a.node, &b.node) {
if a == b {
return Some(a);
}
}
None
}

/// SIM220
pub fn a_and_not_a(checker: &mut Checker, expr: &Expr) {
let ExprKind::BoolOp { op: Boolop::And, values, } = &expr.node else {
return;
};
if values.len() < 2 {
return;
}

// Collect all negated and non-negated expressions.
let mut negated_expr = vec![];
let mut non_negated_expr = vec![];
for expr in values {
if let ExprKind::UnaryOp {
op: Unaryop::Not,
operand,
} = &expr.node
{
negated_expr.push(operand);
} else {
non_negated_expr.push(expr);
}
}

if negated_expr.is_empty() {
return;
}

for negate_expr in negated_expr {
for non_negate_expr in &non_negated_expr {
if let Some(id) = is_same_expr(negate_expr, non_negate_expr) {
let mut check = Check::new(
CheckKind::AAndNotA(id.to_string()),
Range::from_located(expr),
);
if checker.patch(&CheckCode::SIM220) {
check.amend(Fix::replacement(
"False".to_string(),
expr.location,
expr.end_location.unwrap(),
));
}
checker.add_check(check);
}
}
}
}

/// SIM221
pub fn a_or_not_a(checker: &mut Checker, expr: &Expr) {
let ExprKind::BoolOp { op: Boolop::Or, values, } = &expr.node else {
return;
};
if values.len() < 2 {
return;
}

// Collect all negated and non-negated expressions.
let mut negated_expr = vec![];
let mut non_negated_expr = vec![];
for expr in values {
if let ExprKind::UnaryOp {
op: Unaryop::Not,
operand,
} = &expr.node
{
negated_expr.push(operand);
} else {
non_negated_expr.push(expr);
}
}

if negated_expr.is_empty() {
return;
}

for negate_expr in negated_expr {
for non_negate_expr in &non_negated_expr {
if let Some(id) = is_same_expr(negate_expr, non_negate_expr) {
let mut check = Check::new(
CheckKind::AOrNotA(id.to_string()),
Range::from_located(expr),
);
if checker.patch(&CheckCode::SIM220) {
check.amend(Fix::replacement(
"True".to_string(),
expr.location,
expr.end_location.unwrap(),
));
}
checker.add_check(check);
}
}
}
}

/// SIM222
pub fn or_true(checker: &mut Checker, expr: &Expr) {
let ExprKind::BoolOp { op: Boolop::Or, values, } = &expr.node else {
Expand Down
2 changes: 1 addition & 1 deletion src/flake8_simplify/plugins/mod.rs
View file
Open in desktop
@@ -1,4 +1,4 @@
pub use bool_ops::{and_false, or_true};
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 yoda_conditions::yoda_conditions;

Expand Down
56 changes: 56 additions & 0 deletions src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM220_SIM220.py.snap
View file
Open in desktop
@@ -0,0 +1,56 @@
---
source: src/flake8_simplify/mod.rs
expression: checks
---
- kind:
AAndNotA: a
location:
row: 1
column: 3
end_location:
row: 1
column: 14
fix:
content: "False"
location:
row: 1
column: 3
end_location:
row: 1
column: 14
parent: ~
- kind:
AAndNotA: a
location:
row: 4
column: 4
end_location:
row: 4
column: 15
fix:
content: "False"
location:
row: 4
column: 4
end_location:
row: 4
column: 15
parent: ~
- kind:
AAndNotA: a
location:
row: 7
column: 4
end_location:
row: 7
column: 15
fix:
content: "False"
location:
row: 7
column: 4
end_location:
row: 7
column: 15
parent: ~

35 changes: 35 additions & 0 deletions src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM221_SIM221.py.snap
View file
Open in desktop
@@ -0,0 +1,35 @@
---
source: src/flake8_simplify/mod.rs
expression: checks
---
- kind:
AOrNotA: a
location:
row: 1
column: 3
end_location:
row: 1
column: 13
fix: ~
parent: ~
- kind:
AOrNotA: a
location:
row: 4
column: 4
end_location:
row: 4
column: 14
fix: ~
parent: ~
- kind:
AOrNotA: a
location:
row: 7
column: 4
end_location:
row: 7
column: 14
fix: ~
parent: ~

18 changes: 17 additions & 1 deletion src/registry.rs
View file
Open in desktop
Expand Up @@ -217,6 +217,8 @@ pub enum CheckCode {
YTT302,
YTT303,
// flake8-simplify
SIM220,
SIM221,
SIM118,
SIM222,
SIM223,
Expand Down Expand Up @@ -945,6 +947,8 @@ pub enum CheckKind {
SysVersionCmpStr10,
SysVersionSlice1Referenced,
// flake8-simplify
AAndNotA(String),
AOrNotA(String),
KeyInDict(String, String),
AndFalse,
OrTrue,
Expand Down Expand Up @@ -1374,6 +1378,8 @@ impl CheckCode {
CheckCode::BLE001 => CheckKind::BlindExcept("Exception".to_string()),
// flake8-simplify
CheckCode::SIM118 => CheckKind::KeyInDict("key".to_string(), "dict".to_string()),
CheckCode::SIM220 => CheckKind::AAndNotA("...".to_string()),
CheckCode::SIM221 => CheckKind::AOrNotA("...".to_string()),
CheckCode::SIM222 => CheckKind::OrTrue,
CheckCode::SIM223 => CheckKind::AndFalse,
CheckCode::SIM300 => CheckKind::YodaConditions("left".to_string(), "right".to_string()),
Expand Down Expand Up @@ -1898,6 +1904,8 @@ impl CheckCode {
CheckCode::S107 => CheckCategory::Flake8Bandit,
// flake8-simplify
CheckCode::SIM118 => CheckCategory::Flake8Simplify,
CheckCode::SIM220 => CheckCategory::Flake8Simplify,
CheckCode::SIM221 => CheckCategory::Flake8Simplify,
CheckCode::SIM222 => CheckCategory::Flake8Simplify,
CheckCode::SIM223 => CheckCategory::Flake8Simplify,
CheckCode::SIM300 => CheckCategory::Flake8Simplify,
Expand Down Expand Up @@ -2147,6 +2155,8 @@ impl CheckKind {
CheckKind::SysVersionSlice1Referenced => &CheckCode::YTT303,
// flake8-simplify
CheckKind::KeyInDict(..) => &CheckCode::SIM118,
CheckKind::AAndNotA(..) => &CheckCode::SIM220,
CheckKind::AOrNotA(..) => &CheckCode::SIM221,
CheckKind::OrTrue => &CheckCode::SIM222,
CheckKind::AndFalse => &CheckCode::SIM223,
CheckKind::YodaConditions(..) => &CheckCode::SIM300,
Expand Down Expand Up @@ -2904,6 +2914,8 @@ impl CheckKind {
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::OrTrue => "Use `True` instead of `... or True`".to_string(),
CheckKind::AndFalse => "Use `False` instead of `... and False`".to_string(),
CheckKind::YodaConditions(left, right) => {
Expand Down Expand Up @@ -3495,9 +3507,11 @@ impl CheckKind {
pub fn fixable(&self) -> bool {
matches!(
self,
CheckKind::AmbiguousUnicodeCharacterString(..)
CheckKind::AAndNotA(..)
| CheckKind::AOrNotA(..)
| CheckKind::AmbiguousUnicodeCharacterComment(..)
| CheckKind::AmbiguousUnicodeCharacterDocstring(..)
| CheckKind::AmbiguousUnicodeCharacterString(..)
| CheckKind::AndFalse
| CheckKind::BlankLineAfterLastSection(..)
| CheckKind::BlankLineAfterSection(..)
Expand Down Expand Up @@ -3611,6 +3625,8 @@ impl CheckKind {
/// The message used to describe the fix action for a given `CheckKind`.
pub fn commit(&self) -> Option<String> {
match self {
CheckKind::AAndNotA(..) => Some("Replace with `False`".to_string()),
CheckKind::AOrNotA(..) => Some("Replace with `True`".to_string()),
CheckKind::AmbiguousUnicodeCharacterString(confusable, representant)
| CheckKind::AmbiguousUnicodeCharacterDocstring(confusable, representant)
| CheckKind::AmbiguousUnicodeCharacterComment(confusable, representant) => {
Expand Down