Skip to content

Commit

Permalink
Simplify iteration idioms (#13834)
Browse files Browse the repository at this point in the history
Remove unnecessary uses of `.as_ref()`, `.iter()`, `&**` and similar, mostly in situations when iterating over variables. Many of these changes are only possible following #13826, when we bumped our MSRV to 1.80: several useful implementations on `&Box<[T]>` were only stabilised in Rust 1.80. Some of these changes we could have done earlier, however.
  • Loading branch information
AlexWaygood authored Oct 20, 2024
1 parent 7fd8e30 commit 72adb09
Show file tree
Hide file tree
Showing 36 changed files with 72 additions and 73 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1012,7 +1012,7 @@ where

// Add symbols and definitions for the parameters to the lambda scope.
if let Some(parameters) = &lambda.parameters {
for parameter in &**parameters {
for parameter in parameters {
self.declare_parameter(parameter);
}
}
Expand Down
6 changes: 3 additions & 3 deletions crates/red_knot_python_semantic/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,7 @@ impl<'db> Type<'db> {
tuple
.elements(db)
.iter()
.zip(other_tuple.elements(db).iter())
.zip(other_tuple.elements(db))
.any(|(e1, e2)| e1.is_disjoint_from(db, *e2))
} else {
true
Expand Down Expand Up @@ -863,7 +863,7 @@ impl<'db> Type<'db> {
fn iterate(self, db: &'db dyn Db) -> IterationOutcome<'db> {
if let Type::Tuple(tuple_type) = self {
return IterationOutcome::Iterable {
element_ty: UnionType::from_elements(db, &**tuple_type.elements(db)),
element_ty: UnionType::from_elements(db, tuple_type.elements(db)),
};
}

Expand Down Expand Up @@ -1310,7 +1310,7 @@ impl<'db> CallOutcome<'db> {
let mut not_callable = vec![];
let mut union_builder = UnionBuilder::new(db);
let mut revealed = false;
for outcome in &**outcomes {
for outcome in outcomes {
let return_ty = match outcome {
Self::NotCallable { not_callable_ty } => {
not_callable.push(*not_callable_ty);
Expand Down
12 changes: 6 additions & 6 deletions crates/red_knot_python_semantic/src/types/infer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2809,7 +2809,7 @@ impl<'db> TypeInferenceBuilder<'db> {
} = compare;

self.infer_expression(left);
for right in comparators.as_ref() {
for right in comparators {
self.infer_expression(right);
}

Expand All @@ -2823,10 +2823,10 @@ impl<'db> TypeInferenceBuilder<'db> {
Self::infer_chained_boolean_types(
self.db,
ast::BoolOp::And,
std::iter::once(left.as_ref())
.chain(comparators.as_ref().iter())
std::iter::once(&**left)
.chain(comparators)
.tuple_windows::<(_, _)>()
.zip(ops.iter())
.zip(ops)
.map(|((left, right), op)| {
let left_ty = self.expression_ty(left);
let right_ty = self.expression_ty(right);
Expand Down Expand Up @@ -3036,8 +3036,8 @@ impl<'db> TypeInferenceBuilder<'db> {
}
(Type::Tuple(lhs), Type::Tuple(rhs)) => {
// Note: This only works on heterogeneous tuple types.
let lhs_elements = lhs.elements(self.db).as_ref();
let rhs_elements = rhs.elements(self.db).as_ref();
let lhs_elements = lhs.elements(self.db);
let rhs_elements = rhs.elements(self.db);

let mut lexicographic_type_comparison =
|op| self.infer_lexicographic_type_comparison(lhs_elements, op, rhs_elements);
Expand Down
2 changes: 1 addition & 1 deletion crates/red_knot_python_semantic/src/types/narrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ impl<'db> NarrowingConstraintsBuilder<'db> {
let symbol = self.symbols().symbol_id_by_name(id).unwrap();
let scope = self.scope();
let inference = infer_expression_types(self.db, expression);
for (op, comparator) in std::iter::zip(&**ops, &**comparators) {
for (op, comparator) in std::iter::zip(ops, comparators) {
let comp_ty = inference.expression_ty(comparator.scoped_ast_id(self.db, scope));
match op {
ast::CmpOp::IsNot => {
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_linter/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -732,7 +732,7 @@ impl<'a> Visitor<'a> for Checker<'a> {

// The default values of the parameters needs to be evaluated in the enclosing
// scope.
for parameter in &**parameters {
for parameter in parameters {
if let Some(expr) = parameter.default() {
self.visit_expr(expr);
}
Expand All @@ -744,7 +744,7 @@ impl<'a> Visitor<'a> for Checker<'a> {
self.visit_type_params(type_params);
}

for parameter in &**parameters {
for parameter in parameters {
if let Some(expr) = parameter.annotation() {
if singledispatch && !parameter.is_variadic() {
self.visit_runtime_required_annotation(expr);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ pub(crate) fn fastapi_unused_path_parameter(
.parameters
.args
.iter()
.chain(function_def.parameters.kwonlyargs.iter())
.chain(&function_def.parameters.kwonlyargs)
.map(|ParameterWithDefault { parameter, .. }| {
parameter_alias(parameter, checker.semantic())
.unwrap_or_else(|| parameter.name.as_str())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ fn is_infinite_iterator(arg: &Expr, semantic: &SemanticModel) -> bool {
}

// Ex) `iterools.repeat(1, times=None)`
for keyword in &**keywords {
for keyword in keywords {
if keyword.arg.as_ref().is_some_and(|name| name == "times") {
if keyword.value.is_none_literal_expr() {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ fn check_log_record_attr_clash(checker: &mut Checker, extra: &Keyword) {
..
}) => {
if checker.semantic().match_builtin_expr(func, "dict") {
for keyword in &**keywords {
for keyword in keywords {
if let Some(attr) = &keyword.arg {
if is_reserved_attr(attr) {
checker.diagnostics.push(Diagnostic::new(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ pub(crate) fn custom_type_var_return_type(
let Some(self_or_cls_annotation) = args
.posonlyargs
.iter()
.chain(args.args.iter())
.chain(&args.args)
.next()
.and_then(|parameter_with_default| parameter_with_default.parameter.annotation.as_ref())
else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ pub(crate) fn bad_exit_annotation(checker: &mut Checker, function: &StmtFunction
let non_self_positional_args: SmallVec<[&ParameterWithDefault; 3]> = parameters
.posonlyargs
.iter()
.chain(parameters.args.iter())
.chain(&parameters.args)
.skip(1)
.collect();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ impl UnittestAssert {
FxHashMap::with_capacity_and_hasher(args.len() + keywords.len(), FxBuildHasher);

// Process positional arguments.
for (arg_name, value) in arg_spec.iter().zip(args.iter()) {
for (arg_name, value) in arg_spec.iter().zip(args) {
args_map.insert(arg_name, value);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ pub(crate) fn if_with_same_arms(checker: &mut Checker, stmt_if: &ast::StmtIf) {
if !current_branch
.body
.iter()
.zip(following_branch.body.iter())
.zip(following_branch.body)
.all(|(stmt1, stmt2)| ComparableStmt::from(stmt1) == ComparableStmt::from(stmt2))
{
continue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,12 +248,7 @@ pub(crate) fn literal_comparisons(checker: &mut Checker, compare: &ast::ExprComp
}

// Check each comparator in order.
for (index, (op, next)) in compare
.ops
.iter()
.zip(compare.comparators.iter())
.enumerate()
{
for (index, (op, next)) in compare.ops.iter().zip(&compare.comparators).enumerate() {
if helpers::is_constant_non_singleton(comparator) {
comparator = next;
continue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ impl Violation for TypeComparison {

/// E721
pub(crate) fn type_comparison(checker: &mut Checker, compare: &ast::ExprCompare) {
for (left, right) in std::iter::once(compare.left.as_ref())
.chain(compare.comparators.iter())
for (left, right) in std::iter::once(&*compare.left)
.chain(&compare.comparators)
.tuple_windows()
.zip(compare.ops.iter())
.zip(&compare.ops)
.filter(|(_, op)| matches!(op, CmpOp::Eq | CmpOp::NotEq))
.map(|((left, right), _)| (left, right))
{
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/pyflakes/rules/strings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -870,7 +870,7 @@ pub(crate) fn string_dot_format_missing_argument(
let missing: Vec<String> = summary
.autos
.iter()
.chain(summary.indices.iter())
.chain(&summary.indices)
.filter(|&&i| i >= args.len())
.map(ToString::to_string)
.chain(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ pub(crate) fn compare_to_empty_string(

let mut first = true;
for ((lhs, rhs), op) in std::iter::once(left)
.chain(comparators.iter())
.chain(comparators)
.tuple_windows::<(&Expr, &Expr)>()
.zip(ops)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ pub(crate) fn comparison_of_constant(
comparators: &[Expr],
) {
for ((left, right), op) in std::iter::once(left)
.chain(comparators.iter())
.chain(comparators)
.tuple_windows()
.zip(ops)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ pub(crate) fn comparison_with_itself(
comparators: &[Expr],
) {
for ((left, right), op) in std::iter::once(left)
.chain(comparators.iter())
.chain(comparators)
.tuple_windows()
.zip(ops)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ pub(crate) fn duplicate_bases(checker: &mut Checker, name: &str, arguments: Opti
let bases = &arguments.args;

let mut seen: FxHashSet<&str> = FxHashSet::with_capacity_and_hasher(bases.len(), FxBuildHasher);
for base in &**bases {
for base in bases {
if let Expr::Name(ast::ExprName { id, .. }) = base {
if !seen.insert(id) {
let mut diagnostic = Diagnostic::new(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,18 +97,15 @@ fn is_magic_value(literal_expr: LiteralExpressionRef, allowed_types: &[ConstantT

/// PLR2004
pub(crate) fn magic_value_comparison(checker: &mut Checker, left: &Expr, comparators: &[Expr]) {
for (left, right) in std::iter::once(left)
.chain(comparators.iter())
.tuple_windows()
{
for (left, right) in std::iter::once(left).chain(comparators).tuple_windows() {
// If both of the comparators are literals, skip rule for the whole expression.
// R0133: comparison-of-constants
if as_literal(left).is_some() && as_literal(right).is_some() {
return;
}
}

for comparison_expr in std::iter::once(left).chain(comparators.iter()) {
for comparison_expr in std::iter::once(left).chain(comparators) {
if let Some(value) = as_literal(comparison_expr) {
if is_magic_value(value, &checker.settings.pylint.allow_magic_value_types) {
checker.diagnostics.push(Diagnostic::new(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ impl Violation for NanComparison {

/// PLW0177
pub(crate) fn nan_comparison(checker: &mut Checker, left: &Expr, comparators: &[Expr]) {
for expr in std::iter::once(left).chain(comparators.iter()) {
for expr in std::iter::once(left).chain(comparators) {
if let Some(qualified_name) = checker.semantic().resolve_qualified_name(expr) {
match qualified_name.segments() {
["numpy", "nan" | "NAN" | "NaN"] => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ pub(crate) fn repeated_equality_comparison(checker: &mut Checker, bool_op: &ast:
// Break into sequences of consecutive comparisons.
let mut sequences: Vec<(Vec<usize>, Vec<&Expr>)> = Vec::new();
let mut last = None;
for (index, comparator) in indices.iter().zip(comparators.iter()) {
for (index, comparator) in indices.iter().zip(comparators) {
if last.is_some_and(|last| last + 1 == *index) {
let (indices, comparators) = sequences.last_mut().unwrap();
indices.push(*index);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ pub(crate) fn unnecessary_lambda(checker: &mut Checker, lambda: &ExprLambda) {
if call_posargs.len() != lambda_posargs.len() {
return;
}
for (param, arg) in lambda_posargs.iter().zip(call_posargs.iter()) {
for (param, arg) in lambda_posargs.iter().zip(call_posargs) {
let Expr::Name(ast::ExprName { id, .. }) = arg else {
return;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,10 @@ fn concatenate_expressions(expr: &Expr) -> Option<(Expr, Type)> {
}
// If the splat element is itself a list/tuple, insert them in the other list/tuple.
Expr::List(ast::ExprList { elts, .. }) if matches!(type_, Type::List) => {
other_elements.iter().chain(elts.iter()).cloned().collect()
other_elements.iter().chain(elts).cloned().collect()
}
Expr::Tuple(ast::ExprTuple { elts, .. }) if matches!(type_, Type::Tuple) => {
other_elements.iter().chain(elts.iter()).cloned().collect()
other_elements.iter().chain(elts).cloned().collect()
}
_ => return None,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,12 +189,12 @@ fn should_be_fstring(
.filter_map(ast::Expr::as_call_expr)
{
let ast::Arguments { keywords, args, .. } = &expr.arguments;
for keyword in &**keywords {
for keyword in keywords {
if let Some(ident) = keyword.arg.as_ref() {
arg_names.insert(&ident.id);
}
}
for arg in &**args {
for arg in args {
if let ast::Expr::Name(ast::ExprName { id, .. }) = arg {
arg_names.insert(id);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ impl<'a> StatementVisitor<'a> for RaiseStatementVisitor<'a> {
Stmt::Try(ast::StmtTry {
body, finalbody, ..
}) => {
for stmt in body.iter().chain(finalbody.iter()) {
for stmt in body.iter().chain(finalbody) {
walk_stmt(self, stmt);
}
}
Expand Down
5 changes: 2 additions & 3 deletions crates/ruff_linter/src/source_kind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,8 @@ impl std::fmt::Display for SourceKindDiff<'_> {
}
DiffKind::IpyNotebook(original, modified) => {
// Cell indices are 1-based.
for ((idx, src_cell), dst_cell) in (1u32..)
.zip(original.cells().iter())
.zip(modified.cells().iter())
for ((idx, src_cell), dst_cell) in
(1u32..).zip(original.cells()).zip(modified.cells())
{
let (Cell::Code(src_cell), Cell::Code(dst_cell)) = (src_cell, dst_cell) else {
continue;
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_python_ast/src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2731,7 +2731,7 @@ impl AstNode for ast::ExprCompare {

visitor.visit_expr(left);

for (op, comparator) in ops.iter().zip(&**comparators) {
for (op, comparator) in ops.iter().zip(comparators) {
visitor.visit_cmp_op(op);
visitor.visit_expr(comparator);
}
Expand Down
10 changes: 9 additions & 1 deletion crates/ruff_python_ast/src/nodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2048,7 +2048,7 @@ impl PartialEq for ConcatenatedStringLiteral {
// The `zip` here is safe because we have checked the length of both parts.
self.strings
.iter()
.zip(other.strings.iter())
.zip(&other.strings)
.all(|(s1, s2)| s1 == s2)
}
}
Expand Down Expand Up @@ -3660,6 +3660,14 @@ impl<'a> IntoIterator for &'a Parameters {
}
}

impl<'a> IntoIterator for &'a Box<Parameters> {
type IntoIter = ParametersIterator<'a>;
type Item = AnyParameterRef<'a>;
fn into_iter(self) -> Self::IntoIter {
(&**self).into_iter()
}
}

/// An alternative type of AST `arg`. This is used for each function argument that might have a default value.
/// Used by `Arguments` original type.
///
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_python_ast/src/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -459,10 +459,10 @@ pub fn walk_expr<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, expr: &'a Expr) {
range: _,
}) => {
visitor.visit_expr(left);
for cmp_op in &**ops {
for cmp_op in ops {
visitor.visit_cmp_op(cmp_op);
}
for expr in &**comparators {
for expr in comparators {
visitor.visit_expr(expr);
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_python_codegen/src/generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1024,7 +1024,7 @@ impl<'a> Generator<'a> {
group_if!(precedence::CMP, {
let new_lvl = precedence::CMP + 1;
self.unparse_expr(left, new_lvl);
for (op, cmp) in ops.iter().zip(&**comparators) {
for (op, cmp) in ops.iter().zip(comparators) {
let op = match op {
CmpOp::Eq => " == ",
CmpOp::NotEq => " != ",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ impl FormatNodeRule<PatternArguments> for FormatPatternArguments {
pattern.format().with_options(Parentheses::Preserve),
)
}))
.nodes(item.keywords.iter());
.nodes(&item.keywords);
}
}

Expand Down
Loading

0 comments on commit 72adb09

Please sign in to comment.