mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-09-26 11:59:35 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Implement misplaced-comparison-constant (#1023)

Browse source
This commit is contained in:
Harutaka Kawamura 2022-12-04 14:10:07 +09:00 • committed by GitHub
parent 6cb84d29f0
commit 20e6ff112a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 228 additions and 3 deletions
Download patch file Download diff file Expand all files Collapse all files

1
README.md
View file

@ -767,6 +767,7 @@ For more, see [Pylint](https://pypi.org/project/pylint/2.15.7/) on PyPI.
| Code | Name | Message | Fix |
| ---- | ---- | ------- | --- |
| PLC2201 | MisplacedComparisonConstant | Comparison should be ... | |
| PLC3002 | UnnecessaryDirectLambdaCall | Lambda expression called directly. Execute the expression inline instead. | |
| PLE1142 | AwaitOutsideAsync | `await` should be used within an async function | |
| PLR0206 | PropertyWithParameters | Cannot have defined parameters for properties | |

50
resources/test/fixtures/pylint/misplaced_comparison_constant.py vendored Normal file
View file

@ -0,0 +1,50 @@
"""Check that the constants are on the right side of the comparisons"""
# pylint: disable=singleton-comparison, missing-docstring, too-few-public-methods
# pylint: disable=comparison-of-constants
class MyClass:
def __init__(self):
self.attr = 1
def dummy_return(self):
return self.attr
def dummy_return():
return 2
def bad_comparisons():
"""this is not ok"""
instance = MyClass()
for i in range(10):
if 5 <= i: # [misplaced-comparison-constant]
pass
if 1 == i: # [misplaced-comparison-constant]
pass
if 3 < dummy_return(): # [misplaced-comparison-constant]
pass
if 4 != instance.dummy_return(): # [misplaced-comparison-constant]
pass
if 1 == instance.attr: # [misplaced-comparison-constant]
pass
if "aaa" == instance.attr: # [misplaced-comparison-constant]
pass
def good_comparison():
"""this is ok"""
for i in range(10):
if i == 5:
pass
def double_comparison():
"""Check that we return early for non-binary comparison"""
for i in range(10):
if i == 1 == 2:
pass
if 2 <= i <= 8:
print("Between 2 and 8 inclusive")
def const_comparison():
"""Check that we return early for comparison of two constants"""
if 1 == 2:
pass

10
src/check_ast.rs
View file

@ -2001,6 +2001,16 @@ where
.into_iter(),
);
}
if self.settings.enabled.contains(&CheckCode::PLC2201) {
pylint::plugins::misplaced_comparison_constant(
self,
expr,
left,
ops,
comparators,
);
}
}
ExprKind::Constant {
value: Constant::Str(value),

10
src/checks.rs
View file

@ -93,6 +93,7 @@ pub enum CheckCode {
F841,
F901,
// pylint
PLC2201,
PLC3002,
PLE1142,
PLR0206,
@ -573,6 +574,7 @@ pub enum CheckKind {
YieldOutsideFunction(DeferralKeyword),
// pylint
ConsiderMergingIsinstance(String, Vec<String>),
MisplacedComparisonConstant(String),
UnnecessaryDirectLambdaCall,
PropertyWithParameters,
ConsiderUsingFromImport(String, String),
@ -869,6 +871,7 @@ impl CheckCode {
CheckCode::F841 => CheckKind::UnusedVariable("...".to_string()),
CheckCode::F901 => CheckKind::RaiseNotImplemented,
// pylint
CheckCode::PLC2201 => CheckKind::MisplacedComparisonConstant("...".to_string()),
CheckCode::PLC3002 => CheckKind::UnnecessaryDirectLambdaCall,
CheckCode::PLE1142 => CheckKind::AwaitOutsideAsync,
CheckCode::PLR0402 => {
@ -1296,6 +1299,7 @@ impl CheckCode {
CheckCode::N817 => CheckCategory::PEP8Naming,
CheckCode::N818 => CheckCategory::PEP8Naming,
CheckCode::PGH001 => CheckCategory::PygrepHooks,
CheckCode::PLC2201 => CheckCategory::Pylint,
CheckCode::PLC3002 => CheckCategory::Pylint,
CheckCode::PLE1142 => CheckCategory::Pylint,
CheckCode::PLR0206 => CheckCategory::Pylint,
@ -1422,11 +1426,12 @@ impl CheckKind {
CheckKind::NoNewLineAtEndOfFile => &CheckCode::W292,
CheckKind::InvalidEscapeSequence(_) => &CheckCode::W605,
// pylint
CheckKind::MisplacedComparisonConstant(..) => &CheckCode::PLC2201,
CheckKind::UnnecessaryDirectLambdaCall => &CheckCode::PLC3002,
CheckKind::AwaitOutsideAsync => &CheckCode::PLE1142,
CheckKind::ConsiderMergingIsinstance(..) => &CheckCode::PLR1701,
CheckKind::PropertyWithParameters => &CheckCode::PLR0206,
CheckKind::ConsiderUsingFromImport(..) => &CheckCode::PLR0402,
CheckKind::UnnecessaryDirectLambdaCall => &CheckCode::PLC3002,
// flake8-builtins
CheckKind::BuiltinVariableShadowing(_) => &CheckCode::A001,
CheckKind::BuiltinArgumentShadowing(_) => &CheckCode::A002,
@ -1811,6 +1816,9 @@ impl CheckKind {
let types = types.join(", ");
format!("Consider merging these isinstance calls: `isinstance({obj}, ({types}))`")
}
CheckKind::MisplacedComparisonConstant(comprison) => {
format!("Comparison should be {comprison}")
}
CheckKind::UnnecessaryDirectLambdaCall => "Lambda expression called directly. Execute \
the expression inline instead."
.to_string(),

14
src/checks_gen.rs
View file

@ -280,6 +280,10 @@ pub enum CheckCodePrefix {
PGH00,
PGH001,
PLC,
PLC2,
PLC22,
PLC220,
PLC2201,
PLC3,
PLC30,
PLC300,
@ -1196,7 +1200,11 @@ impl CheckCodePrefix {
CheckCodePrefix::PGH0 => vec![CheckCode::PGH001],
CheckCodePrefix::PGH00 => vec![CheckCode::PGH001],
CheckCodePrefix::PGH001 => vec![CheckCode::PGH001],
CheckCodePrefix::PLC => vec![CheckCode::PLC3002],
CheckCodePrefix::PLC => vec![CheckCode::PLC2201, CheckCode::PLC3002],
CheckCodePrefix::PLC2 => vec![CheckCode::PLC2201],
CheckCodePrefix::PLC22 => vec![CheckCode::PLC2201],
CheckCodePrefix::PLC220 => vec![CheckCode::PLC2201],
CheckCodePrefix::PLC2201 => vec![CheckCode::PLC2201],
CheckCodePrefix::PLC3 => vec![CheckCode::PLC3002],
CheckCodePrefix::PLC30 => vec![CheckCode::PLC3002],
CheckCodePrefix::PLC300 => vec![CheckCode::PLC3002],
@ -1909,6 +1917,10 @@ impl CheckCodePrefix {
CheckCodePrefix::PGH00 => SuffixLength::Two,
CheckCodePrefix::PGH001 => SuffixLength::Three,
CheckCodePrefix::PLC => SuffixLength::Zero,
CheckCodePrefix::PLC2 => SuffixLength::One,
CheckCodePrefix::PLC22 => SuffixLength::Two,
CheckCodePrefix::PLC220 => SuffixLength::Three,
CheckCodePrefix::PLC2201 => SuffixLength::Four,
CheckCodePrefix::PLC3 => SuffixLength::One,
CheckCodePrefix::PLC30 => SuffixLength::Two,
CheckCodePrefix::PLC300 => SuffixLength::Three,

1
src/pylint/mod.rs
View file

@ -11,6 +11,7 @@ mod tests {
use crate::linter::test_path;
use crate::Settings;
#[test_case(CheckCode::PLC2201, Path::new("misplaced_comparison_constant.py"); "PLC2201")]
#[test_case(CheckCode::PLC3002, Path::new("unnecessary_direct_lambda_call.py"); "PLC3002")]
#[test_case(CheckCode::PLE1142, Path::new("await_outside_async.py"); "PLE1142")]
#[test_case(CheckCode::PLR0206, Path::new("property_with_parameters.py"); "PLR0206")]

44
src/pylint/plugins.rs
View file

@ -1,12 +1,54 @@
use itertools::Itertools;
use rustc_hash::{FxHashMap, FxHashSet};
use rustpython_ast::{Alias, Arguments, Boolop, Expr, ExprKind, Stmt};
use rustpython_ast::{Alias, Arguments, Boolop, Cmpop, Expr, ExprKind, Stmt};
use crate::ast::types::{FunctionScope, Range, ScopeKind};
use crate::autofix::Fix;
use crate::check_ast::Checker;
use crate::checks::CheckKind;
use crate::Check;
/// PLC2201
pub fn misplaced_comparison_constant(
checker: &mut Checker,
expr: &Expr,
left: &Expr,
ops: &[Cmpop],
comparators: &[Expr],
) {
if let ([op], [right]) = (ops, comparators) {
if matches!(
op,
Cmpop::Eq | Cmpop::NotEq | Cmpop::Lt | Cmpop::LtE | Cmpop::Gt | Cmpop::GtE,
) && matches!(&left.node, &ExprKind::Constant { .. })
&& !matches!(&right.node, &ExprKind::Constant { .. })
{
let reversed_op = match op {
Cmpop::Eq => "==",
Cmpop::NotEq => "!=",
Cmpop::Lt => ">",
Cmpop::LtE => ">=",
Cmpop::Gt => "<",
Cmpop::GtE => "<=",
_ => unreachable!("Expected comparison operator"),
};
let suggestion = format!("{right} {reversed_op} {left}");
let mut check = Check::new(
CheckKind::MisplacedComparisonConstant(suggestion.clone()),
Range::from_located(expr),
);
if checker.patch(check.kind.code()) {
check.amend(Fix::replacement(
suggestion,
expr.location,
expr.end_location.unwrap(),
));
}
checker.add_check(check);
}
}
}
/// PLC3002
pub fn unnecessary_direct_lambda_call(checker: &mut Checker, expr: &Expr, func: &Expr) {
if let ExprKind::Lambda { .. } = &func.node {

101
src/pylint/snapshots/ruff__pylint__tests__misplaced_comparison_constant.py.snap Normal file
View file

@ -0,0 +1,101 @@
---
source: src/pylint/mod.rs
expression: checks
---
- kind:
MisplacedComparisonConstant: i >= 5
location:
row: 20
column: 11
end_location:
row: 20
column: 17
fix:
content: i >= 5
location:
row: 20
column: 11
end_location:
row: 20
column: 17
- kind:
MisplacedComparisonConstant: i == 1
location:
row: 22
column: 11
end_location:
row: 22
column: 17
fix:
content: i == 1
location:
row: 22
column: 11
end_location:
row: 22
column: 17
- kind:
MisplacedComparisonConstant: dummy_return() > 3
location:
row: 24
column: 11
end_location:
row: 24
column: 29
fix:
content: dummy_return() > 3
location:
row: 24
column: 11
end_location:
row: 24
column: 29
- kind:
MisplacedComparisonConstant: instance.dummy_return() != 4
location:
row: 26
column: 11
end_location:
row: 26
column: 39
fix:
content: instance.dummy_return() != 4
location:
row: 26
column: 11
end_location:
row: 26
column: 39
- kind:
MisplacedComparisonConstant: instance.attr == 1
location:
row: 28
column: 11
end_location:
row: 28
column: 29
fix:
content: instance.attr == 1
location:
row: 28
column: 11
end_location:
row: 28
column: 29
- kind:
MisplacedComparisonConstant: "instance.attr == 'aaa'"
location:
row: 30
column: 11
end_location:
row: 30
column: 33
fix:
content: "instance.attr == 'aaa'"
location:
row: 30
column: 11
end_location:
row: 30
column: 33