Skip to content

Commit

Permalink
Merge convert-loop-to-any & convert-loop-to-all to reimplemented-builtin
Browse files Browse the repository at this point in the history
  • Loading branch information
not-my-profile authored and charliermarsh committed Feb 15, 2023
1 parent f8d46d0 commit 7b09972
Show file tree
Hide file tree
Showing 12 changed files with 378 additions and 205 deletions.
3 changes: 1 addition & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1303,8 +1303,7 @@ For more, see [flake8-simplify](https://pypi.org/project/flake8-simplify/) on Py
| SIM107 | return-in-try-except-finally | Don't use `return` in `try`/`except` and `finally` | |
| SIM108 | use-ternary-operator | Use ternary operator `{contents}` instead of if-else-block | 🛠 |
| SIM109 | compare-with-tuple | Use `{replacement}` instead of multiple equality comparisons | 🛠 |
| SIM110 | convert-loop-to-any | Use `{any}` instead of `for` loop | 🛠 |
| SIM111 | convert-loop-to-all | Use `{all}` instead of `for` loop | 🛠 |
| SIM110 | reimplemented-builtin | Use `{repl}` instead of `for` loop | 🛠 |
| SIM112 | use-capital-environment-variables | Use capitalized environment variable `{expected}` instead of `{original}` | 🛠 |
| SIM114 | [if-with-same-arms](https://beta.ruff.rs/docs/rules/if-with-same-arms/) | Combine `if` branches using logical `or` operator | |
| SIM115 | open-file-with-context-handler | Use context handler for opening files | |
Expand Down
4 changes: 1 addition & 3 deletions crates/ruff/src/checkers/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1643,9 +1643,7 @@ where
pylint::rules::useless_else_on_loop(self, stmt, body, orelse);
}
if matches!(stmt.node, StmtKind::For { .. }) {
if self.settings.rules.enabled(&Rule::ConvertLoopToAny)
|| self.settings.rules.enabled(&Rule::ConvertLoopToAll)
{
if self.settings.rules.enabled(&Rule::ReimplementedBuiltin) {
flake8_simplify::rules::convert_for_loop_to_any_all(
self,
stmt,
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,8 +270,8 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
(Flake8Simplify, "107") => Rule::ReturnInTryExceptFinally,
(Flake8Simplify, "108") => Rule::UseTernaryOperator,
(Flake8Simplify, "109") => Rule::CompareWithTuple,
(Flake8Simplify, "110") => Rule::ConvertLoopToAny,
(Flake8Simplify, "111") => Rule::ConvertLoopToAll,
(Flake8Simplify, "110") => Rule::ReimplementedBuiltin,
// (Flake8Simplify, "111") => Rule::ReimplementedBuiltin,
(Flake8Simplify, "112") => Rule::UseCapitalEnvironmentVariables,
(Flake8Simplify, "114") => Rule::IfWithSameArms,
(Flake8Simplify, "115") => Rule::OpenFileWithContextHandler,
Expand Down
3 changes: 1 addition & 2 deletions crates/ruff/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,8 +259,7 @@ ruff_macros::register_rules!(
rules::flake8_simplify::rules::ReturnInTryExceptFinally,
rules::flake8_simplify::rules::UseTernaryOperator,
rules::flake8_simplify::rules::CompareWithTuple,
rules::flake8_simplify::rules::ConvertLoopToAny,
rules::flake8_simplify::rules::ConvertLoopToAll,
rules::flake8_simplify::rules::ReimplementedBuiltin,
rules::flake8_simplify::rules::UseCapitalEnvironmentVariables,
rules::flake8_simplify::rules::IfWithSameArms,
rules::flake8_simplify::rules::OpenFileWithContextHandler,
Expand Down
3 changes: 3 additions & 0 deletions crates/ruff/src/rule_redirects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ pub(crate) fn get_redirect(code: &str) -> Option<(&'static str, &'static str)> {

static REDIRECTS: Lazy<HashMap<&'static str, &'static str>> = Lazy::new(|| {
HashMap::from_iter([
// The following are here because we don't yet have the many-to-one mapping enabled.
("SIM111", "SIM110"),
// The following are deprecated.
("C", "C4"),
("C9", "C90"),
("T", "T10"),
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff/src/rules/flake8_simplify/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ mod tests {
#[test_case(Rule::ReturnInTryExceptFinally, Path::new("SIM107.py"); "SIM107")]
#[test_case(Rule::UseTernaryOperator, Path::new("SIM108.py"); "SIM108")]
#[test_case(Rule::CompareWithTuple, Path::new("SIM109.py"); "SIM109")]
#[test_case(Rule::ConvertLoopToAny, Path::new("SIM110.py"); "SIM110")]
#[test_case(Rule::ConvertLoopToAll, Path::new("SIM111.py"); "SIM111")]
#[test_case(Rule::ReimplementedBuiltin, Path::new("SIM110.py"); "SIM110")]
#[test_case(Rule::ReimplementedBuiltin, Path::new("SIM111.py"); "SIM111")]
#[test_case(Rule::UseCapitalEnvironmentVariables, Path::new("SIM112.py"); "SIM112")]
#[test_case(Rule::OpenFileWithContextHandler, Path::new("SIM115.py"); "SIM115")]
#[test_case(Rule::MultipleWithStatements, Path::new("SIM117.py"); "SIM117")]
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff/src/rules/flake8_simplify/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ pub use ast_bool_op::{
AAndNotA, AOrNotA, AndFalse, CompareWithTuple, DuplicateIsinstanceCall, OrTrue,
};
pub use ast_expr::{use_capital_environment_variables, UseCapitalEnvironmentVariables};
pub use ast_for::{convert_for_loop_to_any_all, ConvertLoopToAll, ConvertLoopToAny};
pub use ast_if::{
if_with_same_arms, nested_if_statements, return_bool_condition_directly,
use_dict_get_with_default, use_ternary_operator, CollapsibleIf, DictGetWithDefault,
Expand All @@ -22,13 +21,13 @@ pub use key_in_dict::{key_in_dict_compare, key_in_dict_for, KeyInDict};
pub use open_file_with_context_handler::{
open_file_with_context_handler, OpenFileWithContextHandler,
};
pub use reimplemented_builtin::{convert_for_loop_to_any_all, ReimplementedBuiltin};
pub use return_in_try_except_finally::{return_in_try_except_finally, ReturnInTryExceptFinally};
pub use use_contextlib_suppress::{use_contextlib_suppress, UseContextlibSuppress};
pub use yoda_conditions::{yoda_conditions, YodaConditions};

mod ast_bool_op;
mod ast_expr;
mod ast_for;
mod ast_if;
mod ast_ifexp;
mod ast_unary_op;
Expand All @@ -37,6 +36,7 @@ mod fix_if;
mod fix_with;
mod key_in_dict;
mod open_file_with_context_handler;
mod reimplemented_builtin;
mod return_in_try_except_finally;
mod use_contextlib_suppress;
mod yoda_conditions;
Original file line number Diff line number Diff line change
Expand Up @@ -12,38 +12,20 @@ use crate::source_code::Stylist;
use crate::violation::AlwaysAutofixableViolation;

define_violation!(
pub struct ConvertLoopToAny {
pub any: String,
pub struct ReimplementedBuiltin {
pub repl: String,
}
);
impl AlwaysAutofixableViolation for ConvertLoopToAny {
impl AlwaysAutofixableViolation for ReimplementedBuiltin {
#[derive_message_formats]
fn message(&self) -> String {
let ConvertLoopToAny { any } = self;
format!("Use `{any}` instead of `for` loop")
let ReimplementedBuiltin { repl } = self;
format!("Use `{repl}` instead of `for` loop")
}

fn autofix_title(&self) -> String {
let ConvertLoopToAny { any } = self;
format!("Replace with `{any}`")
}
}

define_violation!(
pub struct ConvertLoopToAll {
pub all: String,
}
);
impl AlwaysAutofixableViolation for ConvertLoopToAll {
#[derive_message_formats]
fn message(&self) -> String {
let ConvertLoopToAll { all } = self;
format!("Use `{all}` instead of `for` loop")
}

fn autofix_title(&self) -> String {
let ConvertLoopToAll { all } = self;
format!("Replace with `{all}`")
let ReimplementedBuiltin { repl } = self;
format!("Replace with `{repl}`")
}
}

Expand Down Expand Up @@ -219,7 +201,7 @@ pub fn convert_for_loop_to_any_all(checker: &mut Checker, stmt: &Stmt, sibling:
.or_else(|| sibling.and_then(|sibling| return_values_for_siblings(stmt, sibling)))
{
if loop_info.return_value && !loop_info.next_return_value {
if checker.settings.rules.enabled(&Rule::ConvertLoopToAny) {
if checker.settings.rules.enabled(&Rule::ReimplementedBuiltin) {
let contents = return_stmt(
"any",
loop_info.test,
Expand All @@ -234,8 +216,8 @@ pub fn convert_for_loop_to_any_all(checker: &mut Checker, stmt: &Stmt, sibling:
}

let mut diagnostic = Diagnostic::new(
ConvertLoopToAny {
any: contents.clone(),
ReimplementedBuiltin {
repl: contents.clone(),
},
Range::from_located(stmt),
);
Expand All @@ -251,7 +233,7 @@ pub fn convert_for_loop_to_any_all(checker: &mut Checker, stmt: &Stmt, sibling:
}

if !loop_info.return_value && loop_info.next_return_value {
if checker.settings.rules.enabled(&Rule::ConvertLoopToAll) {
if checker.settings.rules.enabled(&Rule::ReimplementedBuiltin) {
// Invert the condition.
let test = {
if let ExprKind::UnaryOp {
Expand Down Expand Up @@ -311,8 +293,8 @@ pub fn convert_for_loop_to_any_all(checker: &mut Checker, stmt: &Stmt, sibling:
}

let mut diagnostic = Diagnostic::new(
ConvertLoopToAll {
all: contents.clone(),
ReimplementedBuiltin {
repl: contents.clone(),
},
Range::from_located(stmt),
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
---
source: src/rules/flake8_simplify/mod.rs
source: crates/ruff/src/rules/flake8_simplify/mod.rs
expression: diagnostics
---
- kind:
ConvertLoopToAny:
any: return any(check(x) for x in iterable)
ReimplementedBuiltin:
repl: return any(check(x) for x in iterable)
location:
row: 3
column: 4
Expand All @@ -22,8 +22,46 @@ expression: diagnostics
column: 16
parent: ~
- kind:
ConvertLoopToAny:
any: return any(check(x) for x in iterable)
ReimplementedBuiltin:
repl: return all(not check(x) for x in iterable)
location:
row: 25
column: 4
end_location:
row: 27
column: 24
fix:
content:
- return all(not check(x) for x in iterable)
location:
row: 25
column: 4
end_location:
row: 28
column: 15
parent: ~
- kind:
ReimplementedBuiltin:
repl: return all(x.is_empty() for x in iterable)
location:
row: 33
column: 4
end_location:
row: 35
column: 24
fix:
content:
- return all(x.is_empty() for x in iterable)
location:
row: 33
column: 4
end_location:
row: 36
column: 15
parent: ~
- kind:
ReimplementedBuiltin:
repl: return any(check(x) for x in iterable)
location:
row: 55
column: 4
Expand All @@ -41,8 +79,27 @@ expression: diagnostics
column: 20
parent: ~
- kind:
ConvertLoopToAny:
any: return any(check(x) for x in iterable)
ReimplementedBuiltin:
repl: return all(not check(x) for x in iterable)
location:
row: 64
column: 4
end_location:
row: 68
column: 19
fix:
content:
- return all(not check(x) for x in iterable)
location:
row: 64
column: 4
end_location:
row: 68
column: 19
parent: ~
- kind:
ReimplementedBuiltin:
repl: return any(check(x) for x in iterable)
location:
row: 73
column: 4
Expand All @@ -60,8 +117,27 @@ expression: diagnostics
column: 20
parent: ~
- kind:
ConvertLoopToAny:
any: return any(check(x) for x in iterable)
ReimplementedBuiltin:
repl: return all(not check(x) for x in iterable)
location:
row: 83
column: 4
end_location:
row: 87
column: 19
fix:
content:
- return all(not check(x) for x in iterable)
location:
row: 83
column: 4
end_location:
row: 87
column: 19
parent: ~
- kind:
ReimplementedBuiltin:
repl: return any(check(x) for x in iterable)
location:
row: 124
column: 4
Expand All @@ -71,8 +147,19 @@ expression: diagnostics
fix: ~
parent: ~
- kind:
ConvertLoopToAny:
any: return any(check(x) for x in iterable)
ReimplementedBuiltin:
repl: return all(not check(x) for x in iterable)
location:
row: 134
column: 4
end_location:
row: 136
column: 24
fix: ~
parent: ~
- kind:
ReimplementedBuiltin:
repl: return any(check(x) for x in iterable)
location:
row: 144
column: 4
Expand All @@ -89,4 +176,23 @@ expression: diagnostics
row: 147
column: 16
parent: ~
- kind:
ReimplementedBuiltin:
repl: return all(not check(x) for x in iterable)
location:
row: 154
column: 4
end_location:
row: 156
column: 24
fix:
content:
- return all(not check(x) for x in iterable)
location:
row: 154
column: 4
end_location:
row: 157
column: 15
parent: ~

Loading

0 comments on commit 7b09972

Please sign in to comment.