Simplify iteration idioms (#13834)

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.
This commit is contained in:
Alex Waygood 2024-10-20 22:25:27 +01:00 committed by GitHub
parent 7fd8e30eed
commit 72adb09bf3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
36 changed files with 72 additions and 73 deletions

View file

@ -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);
}
}

View file

@ -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
@ -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)),
};
}
@ -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);

View file

@ -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);
}
@ -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);
@ -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);

View file

@ -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 => {

View file

@ -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);
}
@ -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);

View file

@ -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())

View file

@ -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;

View file

@ -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(

View file

@ -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 {

View file

@ -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();

View file

@ -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);
}

View file

@ -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;

View file

@ -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;

View file

@ -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))
{

View file

@ -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(

View file

@ -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)
{

View file

@ -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)
{

View file

@ -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)
{

View file

@ -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(

View file

@ -97,10 +97,7 @@ 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() {
@ -108,7 +105,7 @@ pub(crate) fn magic_value_comparison(checker: &mut Checker, left: &Expr, compara
}
}
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(

View file

@ -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"] => {

View file

@ -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);

View file

@ -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;
};

View file

@ -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,
};

View file

@ -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);
}

View file

@ -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);
}
}

View file

@ -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;

View file

@ -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);
}

View file

@ -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)
}
}
@ -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.
///

View file

@ -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);
}
}

View file

@ -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 => " != ",

View file

@ -48,7 +48,7 @@ impl FormatNodeRule<PatternArguments> for FormatPatternArguments {
pattern.format().with_options(Parentheses::Preserve),
)
}))
.nodes(item.keywords.iter());
.nodes(&item.keywords);
}
}

View file

@ -487,7 +487,7 @@ fn check_type<T: TypeChecker>(binding: &Binding, semantic: &SemanticModel) -> bo
// The type checker might know how to infer the type based on `init_expr`.
Some(Stmt::Assign(ast::StmtAssign { targets, value, .. })) => targets
.iter()
.find_map(|target| match_value(binding, target, value.as_ref()))
.find_map(|target| match_value(binding, target, value))
.is_some_and(|value| T::match_initializer(value, semantic)),
// ```python
@ -496,7 +496,7 @@ fn check_type<T: TypeChecker>(binding: &Binding, semantic: &SemanticModel) -> bo
//
// In this situation, we check only the annotation.
Some(Stmt::AnnAssign(ast::StmtAnnAssign { annotation, .. })) => {
T::match_annotation(annotation.as_ref(), semantic)
T::match_annotation(annotation, semantic)
}
_ => false,
@ -512,7 +512,7 @@ fn check_type<T: TypeChecker>(binding: &Binding, semantic: &SemanticModel) -> bo
.expressions(source)
.find_map(|expr| expr.as_named_expr())
.and_then(|ast::ExprNamed { target, value, .. }| {
match_value(binding, target.as_ref(), value.as_ref())
match_value(binding, target, value)
})
.is_some_and(|value| T::match_initializer(value, semantic))
})
@ -543,13 +543,13 @@ fn check_type<T: TypeChecker>(binding: &Binding, semantic: &SemanticModel) -> bo
//
// We trust the annotation and see if the type checker matches the annotation.
Some(Stmt::FunctionDef(ast::StmtFunctionDef { parameters, .. })) => {
let Some(parameter) = find_parameter(parameters.as_ref(), binding) else {
let Some(parameter) = find_parameter(parameters, binding) else {
return false;
};
let Some(ref annotation) = parameter.parameter.annotation else {
return false;
};
T::match_annotation(annotation.as_ref(), semantic)
T::match_annotation(annotation, semantic)
}
_ => false,
@ -562,7 +562,7 @@ fn check_type<T: TypeChecker>(binding: &Binding, semantic: &SemanticModel) -> bo
//
// It's a typed declaration, type annotation is the only source of information.
Some(Stmt::AnnAssign(ast::StmtAnnAssign { annotation, .. })) => {
T::match_annotation(annotation.as_ref(), semantic)
T::match_annotation(annotation, semantic)
}
_ => false,
},
@ -726,7 +726,7 @@ impl TypeChecker for IoBaseChecker {
// Ex) `open("file.txt")`
semantic
.resolve_qualified_name(func.as_ref())
.resolve_qualified_name(func)
.is_some_and(|qualified_name| {
matches!(
qualified_name.segments(),
@ -899,7 +899,7 @@ pub fn find_binding_value<'a>(binding: &Binding, semantic: &'a SemanticModel) ->
.expressions(parent_id)
.find_map(|expr| expr.as_named_expr());
if let Some(ast::ExprNamed { target, value, .. }) = parent {
return match_value(binding, target.as_ref(), value.as_ref());
return match_value(binding, target, value);
}
}
// Ex) `x = 1`
@ -907,14 +907,14 @@ pub fn find_binding_value<'a>(binding: &Binding, semantic: &'a SemanticModel) ->
Some(Stmt::Assign(ast::StmtAssign { value, targets, .. })) => {
return targets
.iter()
.find_map(|target| match_value(binding, target, value.as_ref()))
.find_map(|target| match_value(binding, target, value))
}
Some(Stmt::AnnAssign(ast::StmtAnnAssign {
value: Some(value),
target,
..
})) => {
return match_value(binding, target, value.as_ref());
return match_value(binding, target, value);
}
_ => {}
},
@ -958,7 +958,7 @@ fn match_value<'a>(binding: &Binding, target: &Expr, value: &'a Expr) -> Option<
/// Given a target and value, find the value that's assigned to the given symbol.
fn match_target<'a>(binding: &Binding, targets: &[Expr], values: &'a [Expr]) -> Option<&'a Expr> {
for (target, value) in targets.iter().zip(values.iter()) {
for (target, value) in targets.iter().zip(values) {
match target {
Expr::Tuple(ast::ExprTuple {
elts: target_elts, ..

View file

@ -795,7 +795,7 @@ impl<'a> SemanticModel<'a> {
let resolved: QualifiedName = qualified_name
.segments()
.iter()
.chain(tail.iter())
.chain(tail)
.copied()
.collect();
Some(resolved)
@ -809,7 +809,7 @@ impl<'a> SemanticModel<'a> {
.segments()
.iter()
.take(1)
.chain(tail.iter())
.chain(tail)
.copied()
.collect(),
)
@ -832,7 +832,7 @@ impl<'a> SemanticModel<'a> {
qualified_name
.segments()
.iter()
.chain(tail.iter())
.chain(tail)
.copied()
.collect()
};

View file

@ -803,7 +803,7 @@ impl LintConfiguration {
.select
.iter()
.flatten()
.chain(selection.extend_select.iter())
.chain(&selection.extend_select)
.filter(|s| s.specificity() == spec)
{
for rule in selector.rules(&preview) {
@ -830,7 +830,7 @@ impl LintConfiguration {
.fixable
.iter()
.flatten()
.chain(selection.extend_fixable.iter())
.chain(&selection.extend_fixable)
.filter(|s| s.specificity() == spec)
{
for rule in selector.all_rules() {

View file

@ -219,7 +219,7 @@ impl<'a> Resolver<'a> {
/// Return an iterator over the resolved [`Settings`] in this [`Resolver`].
pub fn settings(&self) -> impl Iterator<Item = &Settings> {
std::iter::once(&self.pyproject_config.settings).chain(self.settings.iter())
std::iter::once(&self.pyproject_config.settings).chain(&self.settings)
}
}