From 6ffe7672523f1df5f4930f31e694e7b211137ad7 Mon Sep 17 00:00:00 2001 From: Harutaka Kawamura Date: Sat, 19 Nov 2022 02:16:11 +0900 Subject: [PATCH] Implement autofix for E713 and E714 (#804) --- README.md | 8 +-- src/check_ast.rs | 10 ++- src/pycodestyle/checks.rs | 40 +----------- src/pycodestyle/plugins.rs | 61 ++++++++++++++++++- .../ruff__linter__tests__E713_E713.py.snap | 55 +++++++++++++++-- .../ruff__linter__tests__E714_E714.py.snap | 28 +++++++-- 6 files changed, 145 insertions(+), 57 deletions(-) diff --git a/README.md b/README.md index b4edd7a05f..00aa827487 100644 --- a/README.md +++ b/README.md @@ -361,10 +361,10 @@ For more, see [pycodestyle](https://pypi.org/project/pycodestyle/2.9.1/) on PyPI | ---- | ---- | ------- | --- | | E402 | ModuleImportNotAtTopOfFile | Module level import not at top of file | | | E501 | LineTooLong | Line too long (89 > 88 characters) | | -| E711 | NoneComparison | Comparison to `None` should be `cond is None` | | -| E712 | TrueFalseComparison | Comparison to `True` should be `cond is True` | | -| E713 | NotInTest | Test for membership should be `not in` | | -| E714 | NotIsTest | Test for object identity should be `is not` | | +| E711 | NoneComparison | Comparison to `None` should be `cond is None` | 🛠 | +| E712 | TrueFalseComparison | Comparison to `True` should be `cond is True` | 🛠 | +| E713 | NotInTest | Test for membership should be `not in` | 🛠 | +| E714 | NotIsTest | Test for object identity should be `is not` | 🛠 | | E721 | TypeComparison | Do not compare types, use `isinstance()` | | | E722 | DoNotUseBareExcept | Do not use bare `except` | | | E731 | DoNotAssignLambda | Do not assign a lambda expression, use a def | | diff --git a/src/check_ast.rs b/src/check_ast.rs index a9b85f2df9..d76f6a4818 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -1525,9 +1525,13 @@ where let check_not_in = self.settings.enabled.contains(&CheckCode::E713); let check_not_is = self.settings.enabled.contains(&CheckCode::E714); if check_not_in || check_not_is { - self.add_checks( - pycodestyle::checks::not_tests(op, operand, check_not_in, check_not_is) - .into_iter(), + pycodestyle::plugins::not_tests( + self, + expr, + op, + operand, + check_not_in, + check_not_is, ); } diff --git a/src/pycodestyle/checks.rs b/src/pycodestyle/checks.rs index 293dcfc581..6b76617194 100644 --- a/src/pycodestyle/checks.rs +++ b/src/pycodestyle/checks.rs @@ -1,6 +1,6 @@ use itertools::izip; use rustpython_ast::Location; -use rustpython_parser::ast::{Cmpop, Expr, ExprKind, Stmt, Unaryop}; +use rustpython_parser::ast::{Cmpop, Expr, ExprKind, Stmt}; use crate::ast::types::Range; use crate::checks::{Check, CheckKind}; @@ -59,44 +59,6 @@ pub fn do_not_assign_lambda(target: &Expr, value: &Expr, stmt: &Stmt) -> Option< None } -/// E713, E714 -pub fn not_tests( - op: &Unaryop, - operand: &Expr, - check_not_in: bool, - check_not_is: bool, -) -> Vec { - let mut checks: Vec = vec![]; - - if matches!(op, Unaryop::Not) { - if let ExprKind::Compare { ops, .. } = &operand.node { - for op in ops { - match op { - Cmpop::In => { - if check_not_in { - checks.push(Check::new( - CheckKind::NotInTest, - Range::from_located(operand), - )); - } - } - Cmpop::Is => { - if check_not_is { - checks.push(Check::new( - CheckKind::NotIsTest, - Range::from_located(operand), - )); - } - } - _ => {} - } - } - } - } - - checks -} - /// E721 pub fn type_comparison(ops: &[Cmpop], comparators: &[Expr], location: Range) -> Vec { let mut checks: Vec = vec![]; diff --git a/src/pycodestyle/plugins.rs b/src/pycodestyle/plugins.rs index d83d10b7df..d1b2fddf35 100644 --- a/src/pycodestyle/plugins.rs +++ b/src/pycodestyle/plugins.rs @@ -1,6 +1,6 @@ use fnv::FnvHashMap; use itertools::izip; -use rustpython_parser::ast::{Cmpop, Constant, Expr, ExprKind}; +use rustpython_parser::ast::{Cmpop, Constant, Expr, ExprKind, Unaryop}; use crate::ast::types::Range; use crate::autofix::Fix; @@ -201,3 +201,62 @@ pub fn literal_comparisons( checker.add_checks(checks.into_iter()); } + +/// E713, E714 +pub fn not_tests( + checker: &mut Checker, + expr: &Expr, + op: &Unaryop, + operand: &Expr, + check_not_in: bool, + check_not_is: bool, +) { + if matches!(op, Unaryop::Not) { + if let ExprKind::Compare { + left, + ops, + comparators, + .. + } = &operand.node + { + let should_fix = ops.len() == 1; + for op in ops.iter() { + match op { + Cmpop::In => { + if check_not_in { + let mut check = + Check::new(CheckKind::NotInTest, Range::from_located(operand)); + if checker.patch() && should_fix { + if let Some(content) = compare(left, &[Cmpop::NotIn], comparators) { + check.amend(Fix::replacement( + content, + expr.location, + expr.end_location.unwrap(), + )); + } + } + checker.add_check(check); + } + } + Cmpop::Is => { + if check_not_is { + let mut check = + Check::new(CheckKind::NotInTest, Range::from_located(operand)); + if checker.patch() && should_fix { + if let Some(content) = compare(left, &[Cmpop::IsNot], comparators) { + check.amend(Fix::replacement( + content, + expr.location, + expr.end_location.unwrap(), + )); + } + } + checker.add_check(check); + } + } + _ => {} + } + } + } + } +} diff --git a/src/snapshots/ruff__linter__tests__E713_E713.py.snap b/src/snapshots/ruff__linter__tests__E713_E713.py.snap index 06c996e6d8..52dbaba3d9 100644 --- a/src/snapshots/ruff__linter__tests__E713_E713.py.snap +++ b/src/snapshots/ruff__linter__tests__E713_E713.py.snap @@ -9,7 +9,16 @@ expression: checks end_location: row: 2 column: 13 - fix: ~ + fix: + patch: + content: X not in Y + location: + row: 2 + column: 3 + end_location: + row: 2 + column: 13 + applied: false - kind: NotInTest location: row: 5 @@ -17,7 +26,16 @@ expression: checks end_location: row: 5 column: 15 - fix: ~ + fix: + patch: + content: X.B not in Y + location: + row: 5 + column: 3 + end_location: + row: 5 + column: 15 + applied: false - kind: NotInTest location: row: 8 @@ -25,7 +43,16 @@ expression: checks end_location: row: 8 column: 13 - fix: ~ + fix: + patch: + content: X not in Y + location: + row: 8 + column: 3 + end_location: + row: 8 + column: 13 + applied: false - kind: NotInTest location: row: 11 @@ -33,7 +60,16 @@ expression: checks end_location: row: 11 column: 28 - fix: ~ + fix: + patch: + content: Y not in Z + location: + row: 11 + column: 18 + end_location: + row: 11 + column: 28 + applied: false - kind: NotInTest location: row: 14 @@ -41,5 +77,14 @@ expression: checks end_location: row: 14 column: 14 - fix: ~ + fix: + patch: + content: X not in Y + location: + row: 14 + column: 3 + end_location: + row: 14 + column: 15 + applied: false diff --git a/src/snapshots/ruff__linter__tests__E714_E714.py.snap b/src/snapshots/ruff__linter__tests__E714_E714.py.snap index ddf8d5bb55..69f406b624 100644 --- a/src/snapshots/ruff__linter__tests__E714_E714.py.snap +++ b/src/snapshots/ruff__linter__tests__E714_E714.py.snap @@ -2,23 +2,41 @@ source: src/linter.rs expression: checks --- -- kind: NotIsTest +- kind: NotInTest location: row: 2 column: 7 end_location: row: 2 column: 13 - fix: ~ -- kind: NotIsTest + fix: + patch: + content: X is not Y + location: + row: 2 + column: 3 + end_location: + row: 2 + column: 13 + applied: false +- kind: NotInTest location: row: 5 column: 7 end_location: row: 5 column: 15 - fix: ~ -- kind: NotIsTest + fix: + patch: + content: X.B is not Y + location: + row: 5 + column: 3 + end_location: + row: 5 + column: 15 + applied: false +- kind: NotInTest location: row: 8 column: 7