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

Prefer itertools.pairwise() over zip() for successive pairs (RUF007) #3501

Merged
merged 12 commits into from
Mar 17, 2023
19 changes: 19 additions & 0 deletions crates/ruff/resources/test/fixtures/ruff/RUF007.py
Original file line number Diff line number Diff line change
@@ -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:]))
6 changes: 6 additions & 0 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2858,6 +2858,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
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -660,6 +660,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
(Ruff, "003") => Rule::AmbiguousUnicodeCharacterComment,
(Ruff, "005") => Rule::UnpackInsteadOfConcatenatingToCollectionLiteral,
(Ruff, "006") => Rule::AsyncioDanglingTask,
(Ruff, "007") => Rule::PairwiseOverZipped,
(Ruff, "100") => Rule::UnusedNOQA,

// flake8-django
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -601,6 +601,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,
Expand Down
10 changes: 10 additions & 0 deletions crates/ruff/src/rules/ruff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
}
12 changes: 7 additions & 5 deletions crates/ruff/src/rules/ruff/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down
132 changes: 132 additions & 0 deletions crates/ruff/src/rules/ruff/rules/pairwise_over_zipped.rs
Original file line number Diff line number Diff line change
@@ -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<i64>,
}

impl SliceInfo {
pub fn new(arg_name: String, slice_start: Option<i64>) -> 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<SliceInfo> {
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<i64> {
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)));
}
Original file line number Diff line number Diff line change
@@ -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: ~

1 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.