mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #7950 - Serial-ATA:issue-7920, r=llogiq

Browse Source 
Fix `explicit_counter_loop` suggestion for non-usize types

changelog: Add a new suggestion for non-usize types in [`explicit_counter_loop`]

closes: #7920
This commit is contained in:
bors 2021-11-09 19:14:24 +00:00
commit f69721f37c
5 changed files with 145 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

58
clippy_lints/src/loops/explicit_counter_loop.rs
View File

@ -1,7 +1,7 @@
use super::{
get_span_of_entire_for_loop, make_iterator_snippet, IncrementVisitor, InitializeVisitor, EXPLICIT_COUNTER_LOOP,
};
use clippy_utils::diagnostics::span_lint_and_sugg;
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};
use if_chain::if_chain;
@ -9,6 +9,7 @@
use rustc_hir::intravisit::{walk_block, walk_expr};
use rustc_hir::{Expr, Pat};
use rustc_lint::LateContext;
use rustc_middle::ty::{self, UintTy};
// To trigger the EXPLICIT_COUNTER_LOOP lint, a variable must be
// incremented exactly once in the loop body, and initialized to zero
@ -32,26 +33,61 @@ pub(super) fn check<'tcx>(
walk_block(&mut initialize_visitor, block);
if_chain! {
if let Some((name, initializer)) = initialize_visitor.get_result();
if let Some((name, ty, initializer)) = initialize_visitor.get_result();
if is_integer_const(cx, initializer, 0);
then {
let mut applicability = Applicability::MachineApplicable;
let for_span = get_span_of_entire_for_loop(expr);
span_lint_and_sugg(
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()),
&format!("the variable `{}` is used as a loop counter", name),
"consider using",
format!(
"for ({}, {}) in {}.enumerate()",
name,
snippet_with_applicability(cx, pat.span, "item", &mut applicability),
make_iterator_snippet(cx, arg, &mut applicability),
),
applicability,
);
return;
}
Some(ty::Int(int_ty)) => int_ty.name_str(),
Some(ty::Uint(uint_ty)) => uint_ty.name_str(),
_ => return,
};
span_lint_and_then(
cx,
EXPLICIT_COUNTER_LOOP,
for_span.with_hi(arg.span.hi()),
&format!("the variable `{}` is used as a loop counter", name),
"consider using",
format!(
"for ({}, {}) in {}.enumerate()",
name,
snippet_with_applicability(cx, pat.span, "item", &mut applicability),
make_iterator_snippet(cx, arg, &mut applicability),
),
applicability,
|diag| {
diag.span_suggestion(
for_span.with_hi(arg.span.hi()),
"consider using",
format!(
"for ({}, {}) in (0_{}..).zip({})",
name,
snippet_with_applicability(cx, pat.span, "item", &mut applicability),
int_name,
make_iterator_snippet(cx, arg, &mut applicability),
),
applicability,
);
diag.note(&format!(
"`{}` is of type `{}`, making it ineligible for `Iterator::enumerate`",
name, int_name
));
},
);
}
}

2
clippy_lints/src/loops/manual_memcpy.rs
View File

@ -442,7 +442,7 @@ fn get_loop_counters<'a, 'tcx>(
let mut initialize_visitor = InitializeVisitor::new(cx, expr, var_id);
walk_block(&mut initialize_visitor, block);
initialize_visitor.get_result().map(|(_, initializer)| Start {
initialize_visitor.get_result().map(|(_, _, initializer)| Start {
id: var_id,
kind: StartKind::Counter { initializer },
})

74
clippy_lints/src/loops/utils.rs
View File

@ -1,14 +1,17 @@
use clippy_utils::ty::{has_iter_method, implements_trait};
use clippy_utils::{get_parent_expr, is_integer_const, path_to_local, path_to_local_id, sugg};
use if_chain::if_chain;
use rustc_ast::ast::{LitIntType, LitKind};
use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_expr, walk_pat, walk_stmt, NestedVisitorMap, Visitor};
use rustc_hir::HirIdMap;
use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, HirId, Mutability, Pat, PatKind, Stmt, StmtKind};
use rustc_hir::intravisit::{walk_expr, walk_local, walk_pat, walk_stmt, NestedVisitorMap, Visitor};
use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, HirId, HirIdMap, Local, Mutability, Pat, PatKind, Stmt};
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;
use std::iter::Iterator;
#[derive(Debug, PartialEq)]
@ -106,10 +109,11 @@ fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
}
enum InitializeVisitorState<'hir> {
Initial, // Not examined yet
Declared(Symbol), // Declared but not (yet) initialized
Initial, // Not examined yet
Declared(Symbol, Option<Ty<'hir>>), // Declared but not (yet) initialized
Initialized {
name: Symbol,
ty: Option<Ty<'hir>>,
initializer: &'hir Expr<'hir>,
},
DontWarn,
@ -138,9 +142,9 @@ pub(super) fn new(cx: &'a LateContext<'tcx>, end_expr: &'tcx Expr<'tcx>, var_id:
}
}
pub(super) fn get_result(&self) -> Option<(Symbol, &'tcx Expr<'tcx>)> {
if let InitializeVisitorState::Initialized { name, initializer } = self.state {
Some((name, initializer))
pub(super) fn get_result(&self) -> Option<(Symbol, Option<Ty<'tcx>>, &'tcx Expr<'tcx>)> {
if let InitializeVisitorState::Initialized { name, ty, initializer } = self.state {
Some((name, ty, initializer))
} else {
None
}
@ -150,22 +154,25 @@ pub(super) fn get_result(&self) -> Option<(Symbol, &'tcx Expr<'tcx>)> {
impl<'a, 'tcx> Visitor<'tcx> for InitializeVisitor<'a, 'tcx> {
type Map = Map<'tcx>;
fn visit_stmt(&mut self, stmt: &'tcx Stmt<'_>) {
fn visit_local(&mut self, l: &'tcx Local<'_>) {
// Look for declarations of the variable
if_chain! {
if let StmtKind::Local(local) = stmt.kind;
if local.pat.hir_id == self.var_id;
if let PatKind::Binding(.., ident, _) = local.pat.kind;
if l.pat.hir_id == self.var_id;
if let PatKind::Binding(.., ident, _) = l.pat.kind;
then {
self.state = local.init.map_or(InitializeVisitorState::Declared(ident.name), |init| {
let ty = l.ty.map(|ty| hir_ty_to_ty(self.cx.tcx, ty));
self.state = l.init.map_or(InitializeVisitorState::Declared(ident.name, ty), |init| {
InitializeVisitorState::Initialized {
initializer: init,
ty,
name: ident.name,
}
})
}
}
walk_stmt(self, stmt);
walk_local(self, l);
}
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
@ -195,15 +202,38 @@ fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
self.state = InitializeVisitorState::DontWarn;
},
ExprKind::Assign(lhs, rhs, _) if lhs.hir_id == expr.hir_id => {
self.state = if_chain! {
if self.depth == 0;
if let InitializeVisitorState::Declared(name)
| InitializeVisitorState::Initialized { name, ..} = self.state;
then {
InitializeVisitorState::Initialized { initializer: rhs, name }
} else {
InitializeVisitorState::DontWarn
self.state = if self.depth == 0 {
match self.state {
InitializeVisitorState::Declared(name, mut ty) => {
if ty.is_none() {
if let ExprKind::Lit(Spanned {
node: LitKind::Int(_, LitIntType::Unsuffixed),
..
}) = rhs.kind
{
ty = None;
} else {
ty = self.cx.typeck_results().expr_ty_opt(rhs);
}
}
InitializeVisitorState::Initialized {
initializer: rhs,
ty,
name,
}
},
InitializeVisitorState::Initialized { ty, name, .. } => {
InitializeVisitorState::Initialized {
initializer: rhs,
ty,
name,
}
},
_ => InitializeVisitorState::DontWarn,
}
} else {
InitializeVisitorState::DontWarn
}
},
ExprKind::AddrOf(BorrowKind::Ref, mutability, _) if mutability == Mutability::Mut => {

30
tests/ui/explicit_counter_loop.rs
View File

@ -158,3 +158,33 @@ pub fn test() {
}
}
}
mod issue_7920 {
pub fn test() {
let slice = &[1, 2, 3];
let index_usize: usize = 0;
let mut idx_usize: usize = 0;
// should suggest `enumerate`
for _item in slice {
if idx_usize == index_usize {
break;
}
idx_usize += 1;
}
let index_u32: u32 = 0;
let mut idx_u32: u32 = 0;
// should suggest `zip`
for _item in slice {
if idx_u32 == index_u32 {
break;
}
idx_u32 += 1;
}
}
}

16
tests/ui/explicit_counter_loop.stderr
View File

@ -42,5 +42,19 @@ error: the variable `count` is used as a loop counter
LL | for _i in 3..10 {
| ^^^^^^^^^^^^^^^ help: consider using: `for (count, _i) in (3..10).enumerate()`
error: aborting due to 7 previous errors
error: the variable `idx_usize` is used as a loop counter
--> $DIR/explicit_counter_loop.rs:170:9
|
LL | for _item in slice {
| ^^^^^^^^^^^^^^^^^^ help: consider using: `for (idx_usize, _item) in slice.into_iter().enumerate()`
error: the variable `idx_u32` is used as a loop counter
--> $DIR/explicit_counter_loop.rs:182:9
|
LL | for _item in slice {
| ^^^^^^^^^^^^^^^^^^ help: consider using: `for (idx_u32, _item) in (0_u32..).zip(slice.into_iter())`
|
= note: `idx_u32` is of type `u32`, making it ineligible for `Iterator::enumerate`
error: aborting due to 9 previous errors