From 33d2457909d476c61f041c25e1eee267b04c1607 Mon Sep 17 00:00:00 2001 From: Evan Rittenhouse Date: Thu, 16 Mar 2023 22:50:45 -0500 Subject: [PATCH] Prefer `itertools.pairwise()` over `zip()` for successive pairs (`RUF007`) (#3501) --- .../resources/test/fixtures/ruff/RUF007.py | 19 +++ crates/ruff/src/checkers/ast/mod.rs | 6 + crates/ruff/src/codes.rs | 1 + crates/ruff/src/registry.rs | 1 + crates/ruff/src/rules/ruff/mod.rs | 10 ++ crates/ruff/src/rules/ruff/rules/mod.rs | 12 +- .../rules/ruff/rules/pairwise_over_zipped.rs | 132 ++++++++++++++++++ ...uff__tests__ruff_pairwise_over_zipped.snap | 96 +++++++++++++ ruff.schema.json | 1 + 9 files changed, 273 insertions(+), 5 deletions(-) create mode 100644 crates/ruff/resources/test/fixtures/ruff/RUF007.py create mode 100644 crates/ruff/src/rules/ruff/rules/pairwise_over_zipped.rs create mode 100644 crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__ruff_pairwise_over_zipped.snap diff --git a/crates/ruff/resources/test/fixtures/ruff/RUF007.py b/crates/ruff/resources/test/fixtures/ruff/RUF007.py new file mode 100644 index 0000000000..45f506b729 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/ruff/RUF007.py @@ -0,0 +1,19 @@ +input = [1, 2, 3] +otherInput = [2, 3, 4] + +# OK +zip(input, otherInput) # different inputs +zip(input, otherInput[1:]) # different inputs +zip(input, input[2:]) # not successive +zip(input[:-1], input[2:]) # not successive +list(zip(input, otherInput)) # nested call +zip(input, input[1::2]) # not successive + +# Errors +zip(input, input[1:]) +zip(input, input[1::1]) +zip(input[:-1], input[1:]) +zip(input[1:], input[2:]) +zip(input[1:-1], input[2:]) +list(zip(input, input[1:])) +list(zip(input[:-1], input[1:])) diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 9b6088b1a7..279387f783 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -2866,6 +2866,12 @@ where flake8_pytest_style::rules::fail_call(self, func, args, keywords); } + if self.settings.rules.enabled(Rule::PairwiseOverZipped) { + if self.settings.target_version >= PythonVersion::Py310 { + ruff::rules::pairwise_over_zipped(self, func, args); + } + } + // flake8-simplify if self .settings diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index d74df55579..f4d2f5592b 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -661,6 +661,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option { (Ruff, "003") => Rule::AmbiguousUnicodeCharacterComment, (Ruff, "005") => Rule::UnpackInsteadOfConcatenatingToCollectionLiteral, (Ruff, "006") => Rule::AsyncioDanglingTask, + (Ruff, "007") => Rule::PairwiseOverZipped, (Ruff, "100") => Rule::UnusedNOQA, // flake8-django diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index 5b448804ac..04e5bfc147 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -602,6 +602,7 @@ ruff_macros::register_rules!( rules::ruff::rules::UnpackInsteadOfConcatenatingToCollectionLiteral, rules::ruff::rules::AsyncioDanglingTask, rules::ruff::rules::UnusedNOQA, + rules::ruff::rules::PairwiseOverZipped, // flake8-django rules::flake8_django::rules::NullableModelStringField, rules::flake8_django::rules::LocalsInRenderFunction, diff --git a/crates/ruff/src/rules/ruff/mod.rs b/crates/ruff/src/rules/ruff/mod.rs index 4fa134da43..bfee0c2be6 100644 --- a/crates/ruff/src/rules/ruff/mod.rs +++ b/crates/ruff/src/rules/ruff/mod.rs @@ -142,4 +142,14 @@ mod tests { assert_yaml_snapshot!(diagnostics); Ok(()) } + + #[test] + fn ruff_pairwise_over_zipped() -> Result<()> { + let diagnostics = test_path( + Path::new("ruff/RUF007.py"), + &settings::Settings::for_rules(vec![Rule::PairwiseOverZipped]), + )?; + assert_yaml_snapshot!(diagnostics); + Ok(()) + } } diff --git a/crates/ruff/src/rules/ruff/rules/mod.rs b/crates/ruff/src/rules/ruff/rules/mod.rs index 1cc2d35fae..0460ae4a77 100644 --- a/crates/ruff/src/rules/ruff/rules/mod.rs +++ b/crates/ruff/src/rules/ruff/rules/mod.rs @@ -1,19 +1,21 @@ +mod ambiguous_unicode_character; +mod asyncio_dangling_task; +mod pairwise_over_zipped; +mod unpack_instead_of_concatenating_to_collection_literal; +mod unused_noqa; + pub use ambiguous_unicode_character::{ ambiguous_unicode_character, AmbiguousUnicodeCharacterComment, AmbiguousUnicodeCharacterDocstring, AmbiguousUnicodeCharacterString, }; pub use asyncio_dangling_task::{asyncio_dangling_task, AsyncioDanglingTask}; +pub use pairwise_over_zipped::{pairwise_over_zipped, PairwiseOverZipped}; pub use unpack_instead_of_concatenating_to_collection_literal::{ unpack_instead_of_concatenating_to_collection_literal, UnpackInsteadOfConcatenatingToCollectionLiteral, }; pub use unused_noqa::{UnusedCodes, UnusedNOQA}; -mod ambiguous_unicode_character; -mod asyncio_dangling_task; -mod unpack_instead_of_concatenating_to_collection_literal; -mod unused_noqa; - #[derive(Clone, Copy)] pub enum Context { String, diff --git a/crates/ruff/src/rules/ruff/rules/pairwise_over_zipped.rs b/crates/ruff/src/rules/ruff/rules/pairwise_over_zipped.rs new file mode 100644 index 0000000000..651fdf693d --- /dev/null +++ b/crates/ruff/src/rules/ruff/rules/pairwise_over_zipped.rs @@ -0,0 +1,132 @@ +use num_traits::ToPrimitive; +use rustpython_parser::ast::{Constant, Expr, ExprKind, Unaryop}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; + +use crate::checkers::ast::Checker; +use crate::Range; + +#[violation] +pub struct PairwiseOverZipped; + +impl Violation for PairwiseOverZipped { + #[derive_message_formats] + fn message(&self) -> String { + format!("Prefer `itertools.pairwise()` over `zip()` when iterating over successive pairs") + } +} + +#[derive(Debug)] +struct SliceInfo { + arg_name: String, + slice_start: Option, +} + +impl SliceInfo { + pub fn new(arg_name: String, slice_start: Option) -> Self { + Self { + arg_name, + slice_start, + } + } +} + +/// Return the argument name, lower bound, and upper bound for an expression, if it's a slice. +fn match_slice_info(expr: &Expr) -> Option { + let ExprKind::Subscript { value, slice, .. } = &expr.node else { + return None; + }; + + let ExprKind::Name { id: arg_id, .. } = &value.node else { + return None; + }; + + let ExprKind::Slice { lower, step, .. } = &slice.node else { + return None; + }; + + // Avoid false positives for slices with a step. + if let Some(step) = step { + if let Some(step) = to_bound(step) { + if step != 1 { + return None; + } + } else { + return None; + } + } + + Some(SliceInfo::new( + arg_id.to_string(), + lower.as_ref().and_then(|expr| to_bound(expr)), + )) +} + +fn to_bound(expr: &Expr) -> Option { + match &expr.node { + ExprKind::Constant { + value: Constant::Int(value), + .. + } => value.to_i64(), + ExprKind::UnaryOp { + op: Unaryop::USub | Unaryop::Invert, + operand, + } => { + if let ExprKind::Constant { + value: Constant::Int(value), + .. + } = &operand.node + { + value.to_i64().map(|v| -v) + } else { + None + } + } + _ => None, + } +} + +/// RUF007 +pub fn pairwise_over_zipped(checker: &mut Checker, func: &Expr, args: &[Expr]) { + let ExprKind::Name { id, .. } = &func.node else { + return; + }; + + if !(args.len() > 1 && id == "zip" && checker.ctx.is_builtin(id)) { + return; + }; + + // Allow the first argument to be a `Name` or `Subscript`. + let Some(first_arg_info) = ({ + if let ExprKind::Name { id, .. } = &args[0].node { + Some(SliceInfo::new(id.to_string(), None)) + } else { + match_slice_info(&args[0]) + } + }) else { + return; + }; + + // Require second argument to be a `Subscript`. + let ExprKind::Subscript { .. } = &args[1].node else { + return; + }; + let Some(second_arg_info) = match_slice_info(&args[1]) else { + return; + }; + + // Verify that the arguments match the same name. + if first_arg_info.arg_name != second_arg_info.arg_name { + return; + } + + // Verify that the arguments are successive. + if second_arg_info.slice_start.unwrap_or(0) - first_arg_info.slice_start.unwrap_or(0) != 1 { + return; + } + + checker + .diagnostics + .push(Diagnostic::new(PairwiseOverZipped, Range::from(func))); +} diff --git a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__ruff_pairwise_over_zipped.snap b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__ruff_pairwise_over_zipped.snap new file mode 100644 index 0000000000..944d3bb391 --- /dev/null +++ b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__ruff_pairwise_over_zipped.snap @@ -0,0 +1,96 @@ +--- +source: crates/ruff/src/rules/ruff/mod.rs +expression: diagnostics +--- +- kind: + name: PairwiseOverZipped + body: "Prefer `itertools.pairwise()` over `zip()` when iterating over successive pairs" + suggestion: ~ + fixable: false + location: + row: 13 + column: 0 + end_location: + row: 13 + column: 3 + fix: ~ + parent: ~ +- kind: + name: PairwiseOverZipped + body: "Prefer `itertools.pairwise()` over `zip()` when iterating over successive pairs" + suggestion: ~ + fixable: false + location: + row: 14 + column: 0 + end_location: + row: 14 + column: 3 + fix: ~ + parent: ~ +- kind: + name: PairwiseOverZipped + body: "Prefer `itertools.pairwise()` over `zip()` when iterating over successive pairs" + suggestion: ~ + fixable: false + location: + row: 15 + column: 0 + end_location: + row: 15 + column: 3 + fix: ~ + parent: ~ +- kind: + name: PairwiseOverZipped + body: "Prefer `itertools.pairwise()` over `zip()` when iterating over successive pairs" + suggestion: ~ + fixable: false + location: + row: 16 + column: 0 + end_location: + row: 16 + column: 3 + fix: ~ + parent: ~ +- kind: + name: PairwiseOverZipped + body: "Prefer `itertools.pairwise()` over `zip()` when iterating over successive pairs" + suggestion: ~ + fixable: false + location: + row: 17 + column: 0 + end_location: + row: 17 + column: 3 + fix: ~ + parent: ~ +- kind: + name: PairwiseOverZipped + body: "Prefer `itertools.pairwise()` over `zip()` when iterating over successive pairs" + suggestion: ~ + fixable: false + location: + row: 18 + column: 5 + end_location: + row: 18 + column: 8 + fix: ~ + parent: ~ +- kind: + name: PairwiseOverZipped + body: "Prefer `itertools.pairwise()` over `zip()` when iterating over successive pairs" + suggestion: ~ + fixable: false + location: + row: 19 + column: 5 + end_location: + row: 19 + column: 8 + fix: ~ + parent: ~ + diff --git a/ruff.schema.json b/ruff.schema.json index ac985090be..2d5f187c93 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2013,6 +2013,7 @@ "RUF003", "RUF005", "RUF006", + "RUF007", "RUF1", "RUF10", "RUF100",