Skip to content

Commit

Permalink
Done
Browse files Browse the repository at this point in the history
  • Loading branch information
evanrittenhouse committed Mar 14, 2023
1 parent 84587d7 commit 980b098
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 33 deletions.
13 changes: 13 additions & 0 deletions crates/ruff/resources/test/fixtures/ruff/RUF007.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
input = [1, 2, 3]
otherInput = [2, 3, 4]

# Don't trigger
zip(input, otherInput)
list(zip(input, otherInput))

# Error - prefer pairwise here
zip(input, input[1:])
list(zip(input, input[1:]))

# Don't want the error triggered here since it's not successive - pairwise() is not a valid substitute!
zip(input, input[2:])
58 changes: 25 additions & 33 deletions crates/ruff/src/rules/ruff/rules/pairwise_over_zipped.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use ruff_macros::{define_violation, derive_message_formats};
use rustpython_parser::ast::{Expr, ExprKind};

use crate::ast::helpers::unparse_constant;
use crate::checkers::ast::Checker;
use crate::registry::Diagnostic;
use crate::violation::Violation;
Expand All @@ -20,46 +21,37 @@ impl Violation for PairwiseOverZipped {
}
}

// have to implement a fix here as well
pub fn pairwise_over_zipped(checker: &mut Checker, func: &Expr, args: &[Expr]) {
if let ExprKind::Name { id, .. } = &func.node {
if id == "zip" {
// combine all Names and Constants into separate lists here
let mut names = Vec::from_iter(args.iter().filter_map(|arg| match &arg.node {
ExprKind::Name { id, .. } => Some(id),
_ => None,
}));
let num_names = names.len();
names.dedup();
let num_unique_names = names.len();
let ExprKind::Name { id: first_arg_id, .. } = &args[0].node else {
return;
};

let mut constants = Vec::from_iter(args.iter().filter_map(|arg| match &arg.node {
ExprKind::Constant { value, .. } => Some(value),
_ => None,
}));
let num_constants = constants.len();
constants.dedup();
let num_unique_constants = constants.len();
let ExprKind::Subscript { value: second_arg, slice, .. } = &args[1].node else {
return;
};

if num_names > num_unique_names || num_constants > num_unique_constants {
checker.diagnostics.push(Diagnostic::new(
PairwiseOverZipped {
expr: String::from("Something"),
},
Range::from_located(func),
));
} else {
let ExprKind::Name { id: second_arg_id, .. } = &second_arg.node else {
return;
};

// Make sure that the lower end of the slice is 1, or else we can't guarantee that
// successive pairs are desired
let mut lower_bound = 0;
if let ExprKind::Slice { lower, .. } = &slice.node {
// If there's no lower bound, it can't be a successive pair request
let ExprKind::Constant { value, .. } = &lower.as_ref().unwrap().node else {
return;
};

lower_bound = unparse_constant(value, checker.stylist).parse::<i32>().unwrap();
}

if first_arg_id == second_arg_id && lower_bound == 1 {
checker.diagnostics.push(Diagnostic::new(
PairwiseOverZipped {
expr: format!(
"{:?}:{}|{} {:?}:{}|{}",
names,
num_names,
num_unique_names,
constants,
num_constants,
num_unique_constants
),
expr: String::from("Prefer `itertools.pairwise()` to `zip()` if trying to fetch an iterable's successive overlapping pairs."),
},
Range::from_located(func),
));
Expand Down

0 comments on commit 980b098

Please sign in to comment.