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

Extend explicit_iter_loop and explicit_into_iter_loop #10416

Merged
merged 4 commits into from
Jun 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions clippy_lints/src/attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -808,7 +808,7 @@ fn check_deprecated_cfg_attr(cx: &EarlyContext<'_>, attr: &Attribute, msrv: &Msr
}

fn check_nested_cfg(cx: &EarlyContext<'_>, items: &[NestedMetaItem]) {
for item in items.iter() {
for item in items {
if let NestedMetaItem::MetaItem(meta) = item {
if !meta.has_name(sym::any) && !meta.has_name(sym::all) {
continue;
Expand Down Expand Up @@ -842,7 +842,7 @@ fn check_nested_cfg(cx: &EarlyContext<'_>, items: &[NestedMetaItem]) {
}

fn check_nested_misused_cfg(cx: &EarlyContext<'_>, items: &[NestedMetaItem]) {
for item in items.iter() {
for item in items {
if let NestedMetaItem::MetaItem(meta) = item {
if meta.has_name(sym!(features)) && let Some(val) = meta.value_str() {
span_lint_and_sugg(
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/default_numeric_fallback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ impl<'a, 'tcx> Visitor<'tcx> for NumericFallbackVisitor<'a, 'tcx> {
let fields_def = &variant.fields;

// Push field type then visit each field expr.
for field in fields.iter() {
for field in *fields {
let bound =
fields_def
.iter()
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
});
store.register_late_pass(|_| Box::<shadow::Shadow>::default());
store.register_late_pass(|_| Box::new(unit_types::UnitTypes));
store.register_late_pass(|_| Box::new(loops::Loops));
store.register_late_pass(move |_| Box::new(loops::Loops::new(msrv())));
store.register_late_pass(|_| Box::<main_recursion::MainRecursion>::default());
store.register_late_pass(|_| Box::new(lifetimes::Lifetimes));
store.register_late_pass(|_| Box::new(entry::HashMapPass));
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/lifetimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@ fn has_where_lifetimes<'tcx>(cx: &LateContext<'tcx>, generics: &'tcx Generics<'_
// if the bounds define new lifetimes, they are fine to occur
let allowed_lts = allowed_lts_from(pred.bound_generic_params);
// now walk the bounds
for bound in pred.bounds.iter() {
for bound in pred.bounds {
walk_param_bound(&mut visitor, bound);
}
// and check that all lifetimes are allowed
Expand Down
69 changes: 65 additions & 4 deletions clippy_lints/src/loops/explicit_into_iter_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,76 @@ use clippy_utils::source::snippet_with_applicability;
use rustc_errors::Applicability;
use rustc_hir::Expr;
use rustc_lint::LateContext;
use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability};
use rustc_span::symbol::sym;

#[derive(Clone, Copy)]
enum AdjustKind {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sugestion: would it be possible to make these adjustkind APIs generally useful in utils?

Copy link
Contributor Author

@Jarcho Jarcho Jun 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how much can be pulled out. Every time I've had to handle adjustments there's always been some weird cases to handle. Something I'll keep in mind to do if it ever comes up.

None,
Borrow,
BorrowMut,
Reborrow,
ReborrowMut,
}
impl AdjustKind {
fn borrow(mutbl: AutoBorrowMutability) -> Self {
match mutbl {
AutoBorrowMutability::Not => Self::Borrow,
AutoBorrowMutability::Mut { .. } => Self::BorrowMut,
}
}

fn reborrow(mutbl: AutoBorrowMutability) -> Self {
match mutbl {
AutoBorrowMutability::Not => Self::Reborrow,
AutoBorrowMutability::Mut { .. } => Self::ReborrowMut,
}
}

fn display(self) -> &'static str {
match self {
Self::None => "",
Self::Borrow => "&",
Self::BorrowMut => "&mut ",
Self::Reborrow => "&*",
Self::ReborrowMut => "&mut *",
}
}
}

pub(super) fn check(cx: &LateContext<'_>, self_arg: &Expr<'_>, call_expr: &Expr<'_>) {
let self_ty = cx.typeck_results().expr_ty(self_arg);
let self_ty_adjusted = cx.typeck_results().expr_ty_adjusted(self_arg);
if !(self_ty == self_ty_adjusted && is_trait_method(cx, call_expr, sym::IntoIterator)) {
if !is_trait_method(cx, call_expr, sym::IntoIterator) {
return;
}

let typeck = cx.typeck_results();
let self_ty = typeck.expr_ty(self_arg);
let adjust = match typeck.expr_adjustments(self_arg) {
[] => AdjustKind::None,
&[
Adjustment {
kind: Adjust::Borrow(AutoBorrow::Ref(_, mutbl)),
..
},
] => AdjustKind::borrow(mutbl),
&[
Adjustment {
kind: Adjust::Deref(_), ..
},
Adjustment {
kind: Adjust::Borrow(AutoBorrow::Ref(_, mutbl)),
target,
},
] => {
if self_ty == target && matches!(mutbl, AutoBorrowMutability::Not) {
AdjustKind::None
} else {
AdjustKind::reborrow(mutbl)
}
},
_ => return,
};

let mut applicability = Applicability::MachineApplicable;
let object = snippet_with_applicability(cx, self_arg.span, "_", &mut applicability);
span_lint_and_sugg(
Expand All @@ -23,7 +84,7 @@ pub(super) fn check(cx: &LateContext<'_>, self_arg: &Expr<'_>, call_expr: &Expr<
"it is more concise to loop over containers instead of using explicit \
iteration methods",
"to write this more concisely, try",
object.to_string(),
format!("{}{object}", adjust.display()),
applicability,
);
}
Loading