Skip to content

Rustup #8025

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

Merged
merged 5 commits into from
Nov 23, 2021
Merged

Rustup #8025

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
12 changes: 4 additions & 8 deletions clippy_lints/src/loops/explicit_counter_loop.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
use super::{
get_span_of_entire_for_loop, make_iterator_snippet, IncrementVisitor, InitializeVisitor, EXPLICIT_COUNTER_LOOP,
};
use super::{make_iterator_snippet, IncrementVisitor, InitializeVisitor, EXPLICIT_COUNTER_LOOP};
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::{get_enclosing_block, is_integer_const};
Expand Down Expand Up @@ -38,15 +36,13 @@ pub(super) fn check<'tcx>(
then {
let mut applicability = Applicability::MachineApplicable;

let for_span = get_span_of_entire_for_loop(expr);

let int_name = match ty.map(ty::TyS::kind) {
// usize or inferred
Some(ty::Uint(UintTy::Usize)) | None => {
span_lint_and_sugg(
cx,
EXPLICIT_COUNTER_LOOP,
for_span.with_hi(arg.span.hi()),
expr.span.with_hi(arg.span.hi()),
&format!("the variable `{}` is used as a loop counter", name),
"consider using",
format!(
Expand All @@ -67,11 +63,11 @@ pub(super) fn check<'tcx>(
span_lint_and_then(
cx,
EXPLICIT_COUNTER_LOOP,
for_span.with_hi(arg.span.hi()),
expr.span.with_hi(arg.span.hi()),
&format!("the variable `{}` is used as a loop counter", name),
|diag| {
diag.span_suggestion(
for_span.with_hi(arg.span.hi()),
expr.span.with_hi(arg.span.hi()),
"consider using",
format!(
"for ({}, {}) in (0_{}..).zip({})",
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/loops/manual_memcpy.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::{get_span_of_entire_for_loop, IncrementVisitor, InitializeVisitor, MANUAL_MEMCPY};
use super::{IncrementVisitor, InitializeVisitor, MANUAL_MEMCPY};
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet;
use clippy_utils::sugg::Sugg;
Expand Down Expand Up @@ -86,7 +86,7 @@ pub(super) fn check<'tcx>(
span_lint_and_sugg(
cx,
MANUAL_MEMCPY,
get_span_of_entire_for_loop(expr),
expr.span,
"it looks like you're manually copying between slices",
"try replacing the loop by",
big_sugg,
Expand Down
19 changes: 16 additions & 3 deletions clippy_lints/src/loops/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use rustc_hir::{Expr, ExprKind, LoopSource, Pat};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Span;
use utils::{get_span_of_entire_for_loop, make_iterator_snippet, IncrementVisitor, InitializeVisitor};
use utils::{make_iterator_snippet, IncrementVisitor, InitializeVisitor};

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -584,14 +584,25 @@ declare_lint_pass!(Loops => [
impl<'tcx> LateLintPass<'tcx> for Loops {
#[allow(clippy::too_many_lines)]
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let Some(higher::ForLoop { pat, arg, body, span }) = higher::ForLoop::hir(expr) {
let for_loop = higher::ForLoop::hir(expr);
if let Some(higher::ForLoop {
pat,
arg,
body,
loop_id,
span,
}) = for_loop
{
// we don't want to check expanded macros
// this check is not at the top of the function
// since higher::for_loop expressions are marked as expansions
if body.span.from_expansion() {
return;
}
check_for_loop(cx, pat, arg, body, expr, span);
if let ExprKind::Block(block, _) = body.kind {
never_loop::check(cx, block, loop_id, span, for_loop.as_ref());
}
}

// we don't want to check expanded macros
Expand All @@ -600,7 +611,9 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
}

// check for never_loop
never_loop::check(cx, expr);
if let ExprKind::Loop(block, ..) = expr.kind {
never_loop::check(cx, block, expr.hir_id, expr.span, None);
}

// check for `loop { if let {} else break }` that could be `while let`
// (also matches an explicit "match" instead of "if let")
Expand Down
58 changes: 32 additions & 26 deletions clippy_lints/src/loops/never_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,35 +4,41 @@ use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::higher::ForLoop;
use clippy_utils::source::snippet;
use rustc_errors::Applicability;
use rustc_hir::{Block, Expr, ExprKind, HirId, InlineAsmOperand, LoopSource, Node, Pat, Stmt, StmtKind};
use rustc_hir::{Block, Expr, ExprKind, HirId, InlineAsmOperand, Pat, Stmt, StmtKind};
use rustc_lint::LateContext;
use rustc_span::Span;
use std::iter::{once, Iterator};

pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let ExprKind::Loop(block, _, source, _) = expr.kind {
match never_loop_block(block, expr.hir_id) {
NeverLoopResult::AlwaysBreak => {
span_lint_and_then(cx, NEVER_LOOP, expr.span, "this loop never actually loops", |diag| {
if_chain! {
if source == LoopSource::ForLoop;
if let Some((_, Node::Expr(parent_match))) = cx.tcx.hir().parent_iter(expr.hir_id).nth(1);
if let Some(ForLoop { arg: iterator, pat, span: for_span, .. }) = ForLoop::hir(parent_match);
then {
// Suggests using an `if let` instead. This is `Unspecified` because the
// loop may (probably) contain `break` statements which would be invalid
// in an `if let`.
diag.span_suggestion_verbose(
for_span.with_hi(iterator.span.hi()),
"if you need the first element of the iterator, try writing",
for_to_if_let_sugg(cx, iterator, pat),
Applicability::Unspecified,
);
}
};
});
},
NeverLoopResult::MayContinueMainLoop | NeverLoopResult::Otherwise => (),
}
pub(super) fn check(
cx: &LateContext<'tcx>,
block: &'tcx Block<'_>,
loop_id: HirId,
span: Span,
for_loop: Option<&ForLoop<'_>>,
) {
match never_loop_block(block, loop_id) {
NeverLoopResult::AlwaysBreak => {
span_lint_and_then(cx, NEVER_LOOP, span, "this loop never actually loops", |diag| {
if let Some(ForLoop {
arg: iterator,
pat,
span: for_span,
..
}) = for_loop
{
// Suggests using an `if let` instead. This is `Unspecified` because the
// loop may (probably) contain `break` statements which would be invalid
// in an `if let`.
diag.span_suggestion_verbose(
for_span.with_hi(iterator.span.hi()),
"if you need the first element of the iterator, try writing",
for_to_if_let_sugg(cx, iterator, pat),
Applicability::Unspecified,
);
}
});
},
NeverLoopResult::MayContinueMainLoop | NeverLoopResult::Otherwise => (),
}
}

Expand Down
5 changes: 2 additions & 3 deletions clippy_lints/src/loops/single_element_loop.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::{get_span_of_entire_for_loop, SINGLE_ELEMENT_LOOP};
use super::SINGLE_ELEMENT_LOOP;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::single_segment_path;
use clippy_utils::source::{indent_of, snippet};
Expand Down Expand Up @@ -30,7 +30,6 @@ pub(super) fn check<'tcx>(
if !block.stmts.is_empty();

then {
let for_span = get_span_of_entire_for_loop(expr);
let mut block_str = snippet(cx, block.span, "..").into_owned();
block_str.remove(0);
block_str.pop();
Expand All @@ -39,7 +38,7 @@ pub(super) fn check<'tcx>(
span_lint_and_sugg(
cx,
SINGLE_ELEMENT_LOOP,
for_span,
expr.span,
"for loop over a single element",
"try",
format!("{{\n{}let {} = &{};{}}}", " ".repeat(indent_of(cx, block.stmts[0].span).unwrap_or(0)), target.name, list_item_name, block_str),
Expand Down
12 changes: 0 additions & 12 deletions clippy_lints/src/loops/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, HirId, HirIdMap, Local, M
use rustc_lint::LateContext;
use rustc_middle::hir::map::Map;
use rustc_middle::ty::Ty;
use rustc_span::source_map::Span;
use rustc_span::source_map::Spanned;
use rustc_span::symbol::{sym, Symbol};
use rustc_typeck::hir_ty_to_ty;
Expand Down Expand Up @@ -330,17 +329,6 @@ impl<'tcx> Visitor<'tcx> for LoopNestVisitor {
}
}

// this function assumes the given expression is a `for` loop.
pub(super) fn get_span_of_entire_for_loop(expr: &Expr<'_>) -> Span {
// for some reason this is the only way to get the `Span`
// of the entire `for` loop
if let ExprKind::Match(_, arms, _) = &expr.kind {
arms[0].body.span
} else {
unreachable!()
}
}

/// If `arg` was the argument to a `for` loop, return the "cleanest" way of writing the
/// actual `Iterator` that the loop uses.
pub(super) fn make_iterator_snippet(cx: &LateContext<'_>, arg: &Expr<'_>, applic_ref: &mut Applicability) -> String {
Expand Down
5 changes: 2 additions & 3 deletions clippy_lints/src/misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ use rustc_span::symbol::sym;
use clippy_utils::consts::{constant, Constant};
use clippy_utils::sugg::Sugg;
use clippy_utils::{
expr_path_res, get_item_name, get_parent_expr, higher, in_constant, is_diag_trait_item, is_integer_const,
iter_input_pats, last_path_segment, match_any_def_paths, paths, unsext, SpanlessEq,
expr_path_res, get_item_name, get_parent_expr, in_constant, is_diag_trait_item, is_integer_const, iter_input_pats,
last_path_segment, match_any_def_paths, paths, unsext, SpanlessEq,
};

declare_clippy_lint! {
Expand Down Expand Up @@ -321,7 +321,6 @@ impl<'tcx> LateLintPass<'tcx> for MiscLints {
if let StmtKind::Local(local) = stmt.kind;
if let PatKind::Binding(an, .., name, None) = local.pat.kind;
if let Some(init) = local.init;
if !higher::is_from_for_desugar(local);
if an == BindingAnnotation::Ref || an == BindingAnnotation::RefMut;
then {
// use the macro callsite when the init span (but not the whole local span)
Expand Down
Loading