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.

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

Replace misplaced-comparison-constant with SIM300 #1980

Merged
merged 1 commit into from
Jan 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions BREAKING_CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
# Breaking Changes

## 0.0.226

### `misplaced-comparison-constant` (`PLC2201`) was deprecated in favor of `SIM300` ([#1980](https://github.com/charliermarsh/ruff/pull/1980))

These two rules contain (nearly) identical logic. To deduplicate the rule set, we've upgraded
`SIM300` to handle a few more cases, and deprecated `PLC2201` in favor of `SIM300`.

## 0.0.225

### `@functools.cache` rewrites have been moved to a standalone rule (`UP033`) ([#1938](https://github.com/charliermarsh/ruff/pull/1938))
Expand Down
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1112,7 +1112,6 @@ For more, see [Pylint](https://pypi.org/project/pylint/2.15.7/) on PyPI.
| Code | Name | Message | Fix |
| ---- | ---- | ------- | --- |
| PLC0414 | UselessImportAlias | Import alias does not rename original package | 🛠 |
| PLC2201 | MisplacedComparisonConstant | Comparison should be ... | 🛠 |
| PLC3002 | UnnecessaryDirectLambdaCall | Lambda expression called directly. Execute the expression inline instead. | |

#### Error (PLE)
Expand Down
3 changes: 3 additions & 0 deletions resources/test/fixtures/flake8_simplify/SIM300.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
"yoda" == compare # SIM300
'yoda' == compare # SIM300
42 == age # SIM300
"yoda" <= compare # SIM300
'yoda' < compare # SIM300
42 > age # SIM300

# OK
compare == "yoda"
Expand Down
4 changes: 0 additions & 4 deletions ruff.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1484,10 +1484,6 @@
"PLC04",
"PLC041",
"PLC0414",
"PLC2",
"PLC22",
"PLC220",
"PLC2201",
"PLC3",
"PLC30",
"PLC300",
Expand Down
10 changes: 0 additions & 10 deletions src/checkers/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2552,16 +2552,6 @@ where
);
}

if self.settings.rules.enabled(&RuleCode::PLC2201) {
pylint::rules::misplaced_comparison_constant(
self,
expr,
left,
ops,
comparators,
);
}

if self.settings.rules.enabled(&RuleCode::PLR0133) {
pylint::rules::constant_comparison(self, left, ops, comparators);
}
Expand Down
1 change: 0 additions & 1 deletion src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ ruff_macros::define_rule_mapping!(
F901 => violations::RaiseNotImplemented,
// pylint
PLC0414 => violations::UselessImportAlias,
PLC2201 => violations::MisplacedComparisonConstant,
PLC3002 => violations::UnnecessaryDirectLambdaCall,
PLE0117 => violations::NonlocalWithoutBinding,
PLE0118 => violations::UsedPriorGlobalDeclaration,
Expand Down
32 changes: 23 additions & 9 deletions src/rules/flake8_simplify/rules/yoda_conditions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,20 @@ pub fn yoda_conditions(
ops: &[Cmpop],
comparators: &[Expr],
) {
if !matches!(ops[..], [Cmpop::Eq]) {
let ([op], [right]) = (ops, comparators) else {
return;
}
if comparators.len() != 1 {
};

if !matches!(
op,
Cmpop::Eq | Cmpop::NotEq | Cmpop::Lt | Cmpop::LtE | Cmpop::Gt | Cmpop::GtE,
) {
return;
}
if !matches!(left.node, ExprKind::Constant { .. }) {
if !matches!(&left.node, &ExprKind::Constant { .. }) {
return;
}
let right = comparators.first().unwrap();
if matches!(right.node, ExprKind::Constant { .. }) {
if matches!(&right.node, &ExprKind::Constant { .. }) {
return;
}

Expand All @@ -36,16 +39,27 @@ pub fn yoda_conditions(
.locator
.slice_source_code_range(&Range::from_located(right));

// Reverse the operation.
let reversed_op = match op {
Cmpop::Eq => "==",
Cmpop::NotEq => "!=",
Cmpop::Lt => ">",
Cmpop::LtE => ">=",
Cmpop::Gt => "<",
Cmpop::GtE => "<=",
_ => unreachable!("Expected comparison operator"),
};

let suggestion = format!("{variable} {reversed_op} {constant}");
let mut diagnostic = Diagnostic::new(
violations::YodaConditions {
constant: constant.to_string(),
variable: variable.to_string(),
suggestion: suggestion.to_string(),
},
Range::from_located(expr),
);
if checker.patch(diagnostic.kind.code()) {
diagnostic.amend(Fix::replacement(
format!("{variable} == {constant}"),
suggestion,
left.location,
right.end_location.unwrap(),
));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@ expression: diagnostics
---
- kind:
YodaConditions:
variable: compare
constant: "\"yoda\""
suggestion: "compare == \"yoda\""
location:
row: 2
column: 0
Expand All @@ -23,8 +22,7 @@ expression: diagnostics
parent: ~
- kind:
YodaConditions:
variable: compare
constant: "'yoda'"
suggestion: "compare == 'yoda'"
location:
row: 3
column: 0
Expand All @@ -42,8 +40,7 @@ expression: diagnostics
parent: ~
- kind:
YodaConditions:
variable: age
constant: "42"
suggestion: age == 42
location:
row: 4
column: 0
Expand All @@ -59,4 +56,58 @@ expression: diagnostics
row: 4
column: 9
parent: ~
- kind:
YodaConditions:
suggestion: "compare >= \"yoda\""
location:
row: 5
column: 0
end_location:
row: 5
column: 17
fix:
content: "compare >= \"yoda\""
location:
row: 5
column: 0
end_location:
row: 5
column: 17
parent: ~
- kind:
YodaConditions:
suggestion: "compare > 'yoda'"
location:
row: 6
column: 0
end_location:
row: 6
column: 16
fix:
content: "compare > 'yoda'"
location:
row: 6
column: 0
end_location:
row: 6
column: 16
parent: ~
- kind:
YodaConditions:
suggestion: age < 42
location:
row: 7
column: 0
end_location:
row: 7
column: 8
fix:
content: age < 42
location:
row: 7
column: 0
end_location:
row: 7
column: 8
parent: ~

1 change: 0 additions & 1 deletion src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ mod tests {
use crate::settings::Settings;

#[test_case(RuleCode::PLC0414, Path::new("import_aliasing.py"); "PLC0414")]
#[test_case(RuleCode::PLC2201, Path::new("misplaced_comparison_constant.py"); "PLC2201")]
#[test_case(RuleCode::PLC3002, Path::new("unnecessary_direct_lambda_call.py"); "PLC3002")]
#[test_case(RuleCode::PLE0117, Path::new("nonlocal_without_binding.py"); "PLE0117")]
#[test_case(RuleCode::PLE0118, Path::new("used_prior_global_declaration.py"); "PLE0118")]
Expand Down
56 changes: 0 additions & 56 deletions src/rules/pylint/rules/misplaced_comparison_constant.rs

This file was deleted.

2 changes: 0 additions & 2 deletions src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ pub use await_outside_async::await_outside_async;
pub use constant_comparison::constant_comparison;
pub use magic_value_comparison::magic_value_comparison;
pub use merge_isinstance::merge_isinstance;
pub use misplaced_comparison_constant::misplaced_comparison_constant;
pub use property_with_parameters::property_with_parameters;
pub use unnecessary_direct_lambda_call::unnecessary_direct_lambda_call;
pub use use_from_import::use_from_import;
Expand All @@ -15,7 +14,6 @@ mod await_outside_async;
mod constant_comparison;
mod magic_value_comparison;
mod merge_isinstance;
mod misplaced_comparison_constant;
mod property_with_parameters;
mod unnecessary_direct_lambda_call;
mod use_from_import;
Expand Down
Loading