Implement autofix for E713 and E714 (#804)

This commit is contained in:
Harutaka Kawamura 2022-11-19 02:16:11 +09:00 committed by GitHub
parent 2f894e3951
commit 6ffe767252
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 145 additions and 57 deletions

View file

@ -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 | |

View file

@ -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,
);
}

View file

@ -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<Check> {
let mut checks: Vec<Check> = 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<Check> {
let mut checks: Vec<Check> = vec![];

View file

@ -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);
}
}
_ => {}
}
}
}
}
}

View file

@ -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

View file

@ -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