use crate::consts::constant;
-use crate::reexport::Name;
use crate::utils::paths;
+use crate::utils::sugg::Sugg;
use crate::utils::usage::{is_unused, mutated_variables};
use crate::utils::{
get_enclosing_block, get_parent_expr, get_trait_def_id, has_iter_method, higher, implements_trait,
- is_integer_const, is_no_std_crate, is_refutable, last_path_segment, match_trait_method, match_type, match_var,
- multispan_sugg, snippet, snippet_opt, snippet_with_applicability, span_lint, span_lint_and_help,
- span_lint_and_sugg, span_lint_and_then, SpanlessEq,
+ is_integer_const, is_no_std_crate, is_refutable, is_type_diagnostic_item, last_path_segment, match_trait_method,
+ match_type, match_var, multispan_sugg, qpath_res, snippet, snippet_opt, snippet_with_applicability,
+ snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, sugg,
+ SpanlessEq,
};
-use crate::utils::{is_type_diagnostic_item, qpath_res, sugg};
use if_chain::if_chain;
use rustc_ast::ast;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_hir::intravisit::{walk_block, walk_expr, walk_pat, walk_stmt, NestedVisitorMap, Visitor};
use rustc_hir::{
def_id, BinOpKind, BindingAnnotation, Block, BorrowKind, Expr, ExprKind, GenericArg, HirId, InlineAsmOperand,
- LoopSource, MatchSource, Mutability, Node, Pat, PatKind, QPath, Stmt, StmtKind,
+ Local, LoopSource, MatchSource, Mutability, Node, Pat, PatKind, QPath, Stmt, StmtKind,
};
use rustc_infer::infer::TyCtxtInferExt;
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::ty::{self, Ty, TyS};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Span;
-use rustc_span::symbol::Symbol;
+use rustc_span::symbol::{sym, Ident, Symbol};
use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId};
use std::iter::{once, Iterator};
use std::mem;
"variables used within while expression are not mutated in the body"
}
+declare_clippy_lint! {
+ /// **What it does:** Checks whether a for loop is being used to push a constant
+ /// value into a Vec.
+ ///
+ /// **Why is this bad?** This kind of operation can be expressed more succinctly with
+ /// `vec![item;SIZE]` or `vec.resize(NEW_SIZE, item)` and using these alternatives may also
+ /// have better performance.
+ /// **Known problems:** None
+ ///
+ /// **Example:**
+ /// ```rust
+ /// let item1 = 2;
+ /// let item2 = 3;
+ /// let mut vec: Vec<u8> = Vec::new();
+ /// for _ in 0..20 {
+ /// vec.push(item1);
+ /// }
+ /// for _ in 0..30 {
+ /// vec.push(item2);
+ /// }
+ /// ```
+ /// could be written as
+ /// ```rust
+ /// let item1 = 2;
+ /// let item2 = 3;
+ /// let mut vec: Vec<u8> = vec![item1; 20];
+ /// vec.resize(20 + 30, item2);
+ /// ```
+ pub SAME_ITEM_PUSH,
+ style,
+ "the same item is pushed inside of a for loop"
+}
+
declare_lint_pass!(Loops => [
MANUAL_MEMCPY,
NEEDLESS_RANGE_LOOP,
NEVER_LOOP,
MUT_RANGE_BOUND,
WHILE_IMMUTABLE_CONDITION,
+ SAME_ITEM_PUSH,
]);
-impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Loops {
+impl<'tcx> LateLintPass<'tcx> for Loops {
#[allow(clippy::too_many_lines)]
- fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
+ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let Some((pat, arg, body)) = higher::for_loop(expr) {
// we don't want to check expanded macros
// this check is not at the top of the function
if_chain! {
if let ExprKind::MethodCall(..) | ExprKind::Call(..) = iter_expr.kind;
if let Some(iter_def_id) = get_trait_def_id(cx, &paths::ITERATOR);
- if implements_trait(cx, cx.tables.expr_ty(iter_expr), iter_def_id, &[]);
+ if implements_trait(cx, cx.typeck_results().expr_ty(iter_expr), iter_def_id, &[]);
then {
return;
}
NeverLoopResult::AlwaysBreak
}
},
- ExprKind::Break(_, ref e) | ExprKind::Ret(ref e) => {
- if let Some(ref e) = *e {
- combine_seq(never_loop_expr(e, main_loop_id), NeverLoopResult::AlwaysBreak)
- } else {
- NeverLoopResult::AlwaysBreak
- }
- },
+ ExprKind::Break(_, ref e) | ExprKind::Ret(ref e) => e.as_ref().map_or(NeverLoopResult::AlwaysBreak, |e| {
+ combine_seq(never_loop_expr(e, main_loop_id), NeverLoopResult::AlwaysBreak)
+ }),
ExprKind::InlineAsm(ref asm) => asm
.operands
.iter()
.fold(NeverLoopResult::AlwaysBreak, combine_branches)
}
-fn check_for_loop<'a, 'tcx>(
- cx: &LateContext<'a, 'tcx>,
+fn check_for_loop<'tcx>(
+ cx: &LateContext<'tcx>,
pat: &'tcx Pat<'_>,
arg: &'tcx Expr<'_>,
body: &'tcx Expr<'_>,
check_for_loop_over_map_kv(cx, pat, arg, body, expr);
check_for_mut_range_bound(cx, arg, body);
detect_manual_memcpy(cx, pat, arg, body, expr);
+ detect_same_item_push(cx, pat, arg, body, expr);
}
-fn same_var<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &Expr<'_>, var: HirId) -> bool {
+fn same_var<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, var: HirId) -> bool {
if_chain! {
if let ExprKind::Path(qpath) = &expr.kind;
if let QPath::Resolved(None, path) = qpath;
offset: Offset,
}
-fn is_slice_like<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: Ty<'_>) -> bool {
+fn is_slice_like<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'_>) -> bool {
let is_slice = match ty.kind {
ty::Ref(_, subty, _) => is_slice_like(cx, subty),
ty::Slice(..) | ty::Array(..) => true,
}
}
-fn get_offset<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, idx: &Expr<'_>, var: HirId) -> Option<Offset> {
- fn extract_offset<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, e: &Expr<'_>, var: HirId) -> Option<String> {
+fn get_offset<'tcx>(cx: &LateContext<'tcx>, idx: &Expr<'_>, var: HirId) -> Option<Offset> {
+ fn extract_offset<'tcx>(cx: &LateContext<'tcx>, e: &Expr<'_>, var: HirId) -> Option<String> {
match &e.kind {
ExprKind::Lit(l) => match l.node {
ast::LitKind::Int(x, _ty) => Some(x.to_string()),
iter_a.into_iter().flatten().chain(iter_b.into_iter())
}
-fn build_manual_memcpy_suggestion<'a, 'tcx>(
- cx: &LateContext<'a, 'tcx>,
+fn build_manual_memcpy_suggestion<'tcx>(
+ cx: &LateContext<'tcx>,
start: &Expr<'_>,
end: &Expr<'_>,
limits: ast::RangeLimits,
}
/// Checks for for loops that sequentially copy items from one slice-like
/// object to another.
-fn detect_manual_memcpy<'a, 'tcx>(
- cx: &LateContext<'a, 'tcx>,
+fn detect_manual_memcpy<'tcx>(
+ cx: &LateContext<'tcx>,
pat: &'tcx Pat<'_>,
arg: &'tcx Expr<'_>,
body: &'tcx Expr<'_>,
start: Some(start),
end: Some(end),
limits,
- }) = higher::range(cx, arg)
+ }) = higher::range(arg)
{
// the var must be a single name
if let PatKind::Binding(_, canonical_id, _, _) = pat.kind {
if_chain! {
if let ExprKind::Index(seqexpr_left, idx_left) = lhs.kind;
if let ExprKind::Index(seqexpr_right, idx_right) = rhs.kind;
- if is_slice_like(cx, cx.tables.expr_ty(seqexpr_left))
- && is_slice_like(cx, cx.tables.expr_ty(seqexpr_right));
+ if is_slice_like(cx, cx.typeck_results().expr_ty(seqexpr_left))
+ && is_slice_like(cx, cx.typeck_results().expr_ty(seqexpr_right));
if let Some(offset_left) = get_offset(cx, &idx_left, canonical_id);
if let Some(offset_right) = get_offset(cx, &idx_right, canonical_id);
}
}
+// Scans the body of the for loop and determines whether lint should be given
+struct SameItemPushVisitor<'a, 'tcx> {
+ should_lint: bool,
+ // this field holds the last vec push operation visited, which should be the only push seen
+ vec_push: Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)>,
+ cx: &'a LateContext<'tcx>,
+}
+
+impl<'a, 'tcx> Visitor<'tcx> for SameItemPushVisitor<'a, 'tcx> {
+ type Map = Map<'tcx>;
+
+ fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
+ match &expr.kind {
+ // Non-determinism may occur ... don't give a lint
+ ExprKind::Loop(_, _, _) | ExprKind::Match(_, _, _) => self.should_lint = false,
+ ExprKind::Block(block, _) => self.visit_block(block),
+ _ => {},
+ }
+ }
+
+ fn visit_block(&mut self, b: &'tcx Block<'_>) {
+ for stmt in b.stmts.iter() {
+ self.visit_stmt(stmt);
+ }
+ }
+
+ fn visit_stmt(&mut self, s: &'tcx Stmt<'_>) {
+ let vec_push_option = get_vec_push(self.cx, s);
+ if vec_push_option.is_none() {
+ // Current statement is not a push so visit inside
+ match &s.kind {
+ StmtKind::Expr(expr) | StmtKind::Semi(expr) => self.visit_expr(&expr),
+ _ => {},
+ }
+ } else {
+ // Current statement is a push ...check whether another
+ // push had been previously done
+ if self.vec_push.is_none() {
+ self.vec_push = vec_push_option;
+ } else {
+ // There are multiple pushes ... don't lint
+ self.should_lint = false;
+ }
+ }
+ }
+
+ fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
+ NestedVisitorMap::None
+ }
+}
+
+// Given some statement, determine if that statement is a push on a Vec. If it is, return
+// the Vec being pushed into and the item being pushed
+fn get_vec_push<'tcx>(cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) -> Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)> {
+ if_chain! {
+ // Extract method being called
+ if let StmtKind::Semi(semi_stmt) = &stmt.kind;
+ if let ExprKind::MethodCall(path, _, args, _) = &semi_stmt.kind;
+ // Figure out the parameters for the method call
+ if let Some(self_expr) = args.get(0);
+ if let Some(pushed_item) = args.get(1);
+ // Check that the method being called is push() on a Vec
+ if match_type(cx, cx.typeck_results().expr_ty(self_expr), &paths::VEC);
+ if path.ident.name.as_str() == "push";
+ then {
+ return Some((self_expr, pushed_item))
+ }
+ }
+ None
+}
+
+/// Detects for loop pushing the same item into a Vec
+fn detect_same_item_push<'tcx>(
+ cx: &LateContext<'tcx>,
+ pat: &'tcx Pat<'_>,
+ _: &'tcx Expr<'_>,
+ body: &'tcx Expr<'_>,
+ _: &'tcx Expr<'_>,
+) {
+ fn emit_lint(cx: &LateContext<'_>, vec: &Expr<'_>, pushed_item: &Expr<'_>) {
+ let vec_str = snippet_with_macro_callsite(cx, vec.span, "");
+ let item_str = snippet_with_macro_callsite(cx, pushed_item.span, "");
+
+ span_lint_and_help(
+ cx,
+ SAME_ITEM_PUSH,
+ vec.span,
+ "it looks like the same item is being pushed into this Vec",
+ None,
+ &format!(
+ "try using vec![{};SIZE] or {}.resize(NEW_SIZE, {})",
+ item_str, vec_str, item_str
+ ),
+ )
+ }
+
+ if !matches!(pat.kind, PatKind::Wild) {
+ return;
+ }
+
+ // Determine whether it is safe to lint the body
+ let mut same_item_push_visitor = SameItemPushVisitor {
+ should_lint: true,
+ vec_push: None,
+ cx,
+ };
+ walk_expr(&mut same_item_push_visitor, body);
+ if same_item_push_visitor.should_lint {
+ if let Some((vec, pushed_item)) = same_item_push_visitor.vec_push {
+ let vec_ty = cx.typeck_results().expr_ty(vec);
+ let ty = vec_ty.walk().nth(1).unwrap().expect_ty();
+ if cx
+ .tcx
+ .lang_items()
+ .clone_trait()
+ .map_or(false, |id| implements_trait(cx, ty, id, &[]))
+ {
+ // Make sure that the push does not involve possibly mutating values
+ match pushed_item.kind {
+ ExprKind::Path(ref qpath) => {
+ match qpath_res(cx, qpath, pushed_item.hir_id) {
+ // immutable bindings that are initialized with literal or constant
+ Res::Local(hir_id) => {
+ if_chain! {
+ let node = cx.tcx.hir().get(hir_id);
+ if let Node::Binding(pat) = node;
+ if let PatKind::Binding(bind_ann, ..) = pat.kind;
+ if !matches!(bind_ann, BindingAnnotation::RefMut | BindingAnnotation::Mutable);
+ let parent_node = cx.tcx.hir().get_parent_node(hir_id);
+ if let Some(Node::Local(parent_let_expr)) = cx.tcx.hir().find(parent_node);
+ if let Some(init) = parent_let_expr.init;
+ then {
+ match init.kind {
+ // immutable bindings that are initialized with literal
+ ExprKind::Lit(..) => emit_lint(cx, vec, pushed_item),
+ // immutable bindings that are initialized with constant
+ ExprKind::Path(ref path) => {
+ if let Res::Def(DefKind::Const, ..) = qpath_res(cx, path, init.hir_id) {
+ emit_lint(cx, vec, pushed_item);
+ }
+ }
+ _ => {},
+ }
+ }
+ }
+ },
+ // constant
+ Res::Def(DefKind::Const, ..) => emit_lint(cx, vec, pushed_item),
+ _ => {},
+ }
+ },
+ ExprKind::Lit(..) => emit_lint(cx, vec, pushed_item),
+ _ => {},
+ }
+ }
+ }
+ }
+}
+
/// Checks for looping over a range and then indexing a sequence with it.
/// The iteratee must be a range literal.
#[allow(clippy::too_many_lines)]
-fn check_for_loop_range<'a, 'tcx>(
- cx: &LateContext<'a, 'tcx>,
+fn check_for_loop_range<'tcx>(
+ cx: &LateContext<'tcx>,
pat: &'tcx Pat<'_>,
arg: &'tcx Expr<'_>,
body: &'tcx Expr<'_>,
start: Some(start),
ref end,
limits,
- }) = higher::range(cx, arg)
+ }) = higher::range(arg)
{
// the var must be a single name
if let PatKind::Binding(_, canonical_id, ident, _) = pat.kind {
}
}
-fn is_len_call(expr: &Expr<'_>, var: Name) -> bool {
+fn is_len_call(expr: &Expr<'_>, var: Symbol) -> bool {
if_chain! {
if let ExprKind::MethodCall(ref method, _, ref len_args, _) = expr.kind;
if len_args.len() == 1;
}
fn is_end_eq_array_len<'tcx>(
- cx: &LateContext<'_, 'tcx>,
+ cx: &LateContext<'tcx>,
end: &Expr<'_>,
limits: ast::RangeLimits,
indexed_ty: Ty<'tcx>,
false
}
-fn lint_iter_method(cx: &LateContext<'_, '_>, args: &[Expr<'_>], arg: &Expr<'_>, method_name: &str) {
+fn lint_iter_method(cx: &LateContext<'_>, args: &[Expr<'_>], arg: &Expr<'_>, method_name: &str) {
let mut applicability = Applicability::MachineApplicable;
let object = snippet_with_applicability(cx, args[0].span, "_", &mut applicability);
let muta = if method_name == "iter_mut" { "mut " } else { "" };
)
}
-fn check_for_loop_arg(cx: &LateContext<'_, '_>, pat: &Pat<'_>, arg: &Expr<'_>, expr: &Expr<'_>) {
+fn check_for_loop_arg(cx: &LateContext<'_>, pat: &Pat<'_>, arg: &Expr<'_>, expr: &Expr<'_>) {
let mut next_loop_linted = false; // whether or not ITER_NEXT_LOOP lint was used
if let ExprKind::MethodCall(ref method, _, ref args, _) = arg.kind {
// just the receiver, no arguments
lint_iter_method(cx, args, arg, method_name);
}
} else if method_name == "into_iter" && match_trait_method(cx, arg, &paths::INTO_ITERATOR) {
- let receiver_ty = cx.tables.expr_ty(&args[0]);
- let receiver_ty_adjusted = cx.tables.expr_ty_adjusted(&args[0]);
+ let receiver_ty = cx.typeck_results().expr_ty(&args[0]);
+ let receiver_ty_adjusted = cx.typeck_results().expr_ty_adjusted(&args[0]);
if TyS::same_type(receiver_ty, receiver_ty_adjusted) {
let mut applicability = Applicability::MachineApplicable;
let object = snippet_with_applicability(cx, args[0].span, "_", &mut applicability);
}
/// Checks for `for` loops over `Option`s and `Result`s.
-fn check_arg_type(cx: &LateContext<'_, '_>, pat: &Pat<'_>, arg: &Expr<'_>) {
- let ty = cx.tables.expr_ty(arg);
+fn check_arg_type(cx: &LateContext<'_>, pat: &Pat<'_>, arg: &Expr<'_>) {
+ let ty = cx.typeck_results().expr_ty(arg);
if is_type_diagnostic_item(cx, ty, sym!(option_type)) {
span_lint_and_help(
cx,
}
}
-fn check_for_loop_explicit_counter<'a, 'tcx>(
- cx: &LateContext<'a, 'tcx>,
+fn check_for_loop_explicit_counter<'tcx>(
+ cx: &LateContext<'tcx>,
pat: &'tcx Pat<'_>,
arg: &'tcx Expr<'_>,
body: &'tcx Expr<'_>,
/// If `arg` was the argument to a `for` loop, return the "cleanest" way of writing the
/// actual `Iterator` that the loop uses.
-fn make_iterator_snippet(cx: &LateContext<'_, '_>, arg: &Expr<'_>, applic_ref: &mut Applicability) -> String {
- let impls_iterator = get_trait_def_id(cx, &paths::ITERATOR)
- .map_or(false, |id| implements_trait(cx, cx.tables.expr_ty(arg), id, &[]));
+fn make_iterator_snippet(cx: &LateContext<'_>, arg: &Expr<'_>, applic_ref: &mut Applicability) -> String {
+ let impls_iterator = get_trait_def_id(cx, &paths::ITERATOR).map_or(false, |id| {
+ implements_trait(cx, cx.typeck_results().expr_ty(arg), id, &[])
+ });
if impls_iterator {
format!(
"{}",
// (&mut x).into_iter() ==> x.iter_mut()
match &arg.kind {
ExprKind::AddrOf(BorrowKind::Ref, mutability, arg_inner)
- if has_iter_method(cx, cx.tables.expr_ty(&arg_inner)).is_some() =>
+ if has_iter_method(cx, cx.typeck_results().expr_ty(&arg_inner)).is_some() =>
{
let meth_name = match mutability {
Mutability::Mut => "iter_mut",
}
/// Checks for the `FOR_KV_MAP` lint.
-fn check_for_loop_over_map_kv<'a, 'tcx>(
- cx: &LateContext<'a, 'tcx>,
+fn check_for_loop_over_map_kv<'tcx>(
+ cx: &LateContext<'tcx>,
pat: &'tcx Pat<'_>,
arg: &'tcx Expr<'_>,
body: &'tcx Expr<'_>,
if let PatKind::Tuple(ref pat, _) = pat.kind {
if pat.len() == 2 {
let arg_span = arg.span;
- let (new_pat_span, kind, ty, mutbl) = match cx.tables.expr_ty(arg).kind {
+ let (new_pat_span, kind, ty, mutbl) = match cx.typeck_results().expr_ty(arg).kind {
ty::Ref(_, ty, mutbl) => match (&pat[0].kind, &pat[1].kind) {
(key, _) if pat_is_wild(key, body) => (pat[1].span, "value", ty, mutbl),
(_, value) if pat_is_wild(value, body) => (pat[0].span, "key", ty, Mutability::Not),
}
struct MutatePairDelegate<'a, 'tcx> {
- cx: &'a LateContext<'a, 'tcx>,
+ cx: &'a LateContext<'tcx>,
hir_id_low: Option<HirId>,
hir_id_high: Option<HirId>,
span_low: Option<Span>,
}
}
-fn check_for_mut_range_bound(cx: &LateContext<'_, '_>, arg: &Expr<'_>, body: &Expr<'_>) {
+fn check_for_mut_range_bound(cx: &LateContext<'_>, arg: &Expr<'_>, body: &Expr<'_>) {
if let Some(higher::Range {
start: Some(start),
end: Some(end),
..
- }) = higher::range(cx, arg)
+ }) = higher::range(arg)
{
let mut_ids = vec![check_for_mutability(cx, start), check_for_mutability(cx, end)];
if mut_ids[0].is_some() || mut_ids[1].is_some() {
}
}
-fn mut_warn_with_span(cx: &LateContext<'_, '_>, span: Option<Span>) {
+fn mut_warn_with_span(cx: &LateContext<'_>, span: Option<Span>) {
if let Some(sp) = span {
span_lint(
cx,
}
}
-fn check_for_mutability(cx: &LateContext<'_, '_>, bound: &Expr<'_>) -> Option<HirId> {
+fn check_for_mutability(cx: &LateContext<'_>, bound: &Expr<'_>) -> Option<HirId> {
if_chain! {
if let ExprKind::Path(ref qpath) = bound.kind;
if let QPath::Resolved(None, _) = *qpath;
None
}
-fn check_for_mutation<'a, 'tcx>(
- cx: &LateContext<'a, 'tcx>,
+fn check_for_mutation<'tcx>(
+ cx: &LateContext<'tcx>,
body: &Expr<'_>,
bound_ids: &[Option<HirId>],
) -> (Option<Span>, Option<Span>) {
};
let def_id = body.hir_id.owner.to_def_id();
cx.tcx.infer_ctxt().enter(|infcx| {
- ExprUseVisitor::new(&mut delegate, &infcx, def_id.expect_local(), cx.param_env, cx.tables).walk_expr(body);
+ ExprUseVisitor::new(
+ &mut delegate,
+ &infcx,
+ def_id.expect_local(),
+ cx.param_env,
+ cx.typeck_results(),
+ )
+ .walk_expr(body);
});
delegate.mutation_span()
}
}
struct LocalUsedVisitor<'a, 'tcx> {
- cx: &'a LateContext<'a, 'tcx>,
+ cx: &'a LateContext<'tcx>,
local: HirId,
used: bool,
}
struct VarVisitor<'a, 'tcx> {
/// context reference
- cx: &'a LateContext<'a, 'tcx>,
+ cx: &'a LateContext<'tcx>,
/// var name to look for as index
var: HirId,
/// indexed variables that are used mutably
- indexed_mut: FxHashSet<Name>,
+ indexed_mut: FxHashSet<Symbol>,
/// indirectly indexed variables (`v[(i + 4) % N]`), the extend is `None` for global
- indexed_indirectly: FxHashMap<Name, Option<region::Scope>>,
+ indexed_indirectly: FxHashMap<Symbol, Option<region::Scope>>,
/// subset of `indexed` of vars that are indexed directly: `v[i]`
/// this will not contain cases like `v[calc_index(i)]` or `v[(i + 4) % N]`
- indexed_directly: FxHashMap<Name, (Option<region::Scope>, Ty<'tcx>)>,
+ indexed_directly: FxHashMap<Symbol, (Option<region::Scope>, Ty<'tcx>)>,
/// Any names that are used outside an index operation.
/// Used to detect things like `&mut vec` used together with `vec[i]`
- referenced: FxHashSet<Name>,
+ referenced: FxHashSet<Symbol>,
/// has the loop variable been used in expressions other than the index of
/// an index op?
nonindex: bool,
if index_used_directly {
self.indexed_directly.insert(
seqvar.segments[0].ident.name,
- (Some(extent), self.cx.tables.node_type(seqexpr.hir_id)),
+ (Some(extent), self.cx.typeck_results().node_type(seqexpr.hir_id)),
);
}
return false; // no need to walk further *on the variable*
if index_used_directly {
self.indexed_directly.insert(
seqvar.segments[0].ident.name,
- (None, self.cx.tables.node_type(seqexpr.hir_id)),
+ (None, self.cx.typeck_results().node_type(seqexpr.hir_id)),
);
}
return false; // no need to walk further *on the variable*
ExprKind::Call(ref f, args) => {
self.visit_expr(f);
for expr in args {
- let ty = self.cx.tables.expr_ty_adjusted(expr);
+ let ty = self.cx.typeck_results().expr_ty_adjusted(expr);
self.prefer_mutable = false;
if let ty::Ref(_, _, mutbl) = ty.kind {
if mutbl == Mutability::Mut {
}
},
ExprKind::MethodCall(_, _, args, _) => {
- let def_id = self.cx.tables.type_dependent_def_id(expr.hir_id).unwrap();
+ let def_id = self.cx.typeck_results().type_dependent_def_id(expr.hir_id).unwrap();
for (ty, expr) in self.cx.tcx.fn_sig(def_id).inputs().skip_binder().iter().zip(args) {
self.prefer_mutable = false;
if let ty::Ref(_, _, mutbl) = ty.kind {
}
}
-fn is_used_inside<'a, 'tcx>(cx: &'a LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>, container: &'tcx Expr<'_>) -> bool {
+fn is_used_inside<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, container: &'tcx Expr<'_>) -> bool {
let def_id = match var_def_id(cx, expr) {
Some(id) => id,
None => return false,
false
}
-fn is_iterator_used_after_while_let<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, iter_expr: &'tcx Expr<'_>) -> bool {
+fn is_iterator_used_after_while_let<'tcx>(cx: &LateContext<'tcx>, iter_expr: &'tcx Expr<'_>) -> bool {
let def_id = match var_def_id(cx, iter_expr) {
Some(id) => id,
None => return false,
}
struct VarUsedAfterLoopVisitor<'a, 'tcx> {
- cx: &'a LateContext<'a, 'tcx>,
+ cx: &'a LateContext<'tcx>,
def_id: HirId,
iter_expr_id: HirId,
past_while_let: bool,
/// Returns `true` if the type of expr is one that provides `IntoIterator` impls
/// for `&T` and `&mut T`, such as `Vec`.
#[rustfmt::skip]
-fn is_ref_iterable_type(cx: &LateContext<'_, '_>, e: &Expr<'_>) -> bool {
+fn is_ref_iterable_type(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
// no walk_ptrs_ty: calling iter() on a reference can make sense because it
// will allow further borrows afterwards
- let ty = cx.tables.expr_ty(e);
+ let ty = cx.typeck_results().expr_ty(e);
is_iterable_array(ty, cx) ||
is_type_diagnostic_item(cx, ty, sym!(vec_type)) ||
match_type(cx, ty, &paths::LINKED_LIST) ||
match_type(cx, ty, &paths::BTREESET)
}
-fn is_iterable_array<'tcx>(ty: Ty<'tcx>, cx: &LateContext<'_, 'tcx>) -> bool {
+fn is_iterable_array<'tcx>(ty: Ty<'tcx>, cx: &LateContext<'tcx>) -> bool {
// IntoIterator is currently only implemented for array sizes <= 32 in rustc
match ty.kind {
- ty::Array(_, n) => {
- if let Some(val) = n.try_eval_usize(cx.tcx, cx.param_env) {
- (0..=32).contains(&val)
- } else {
- false
- }
- },
+ ty::Array(_, n) => n
+ .try_eval_usize(cx.tcx, cx.param_env)
+ .map_or(false, |val| (0..=32).contains(&val)),
_ => false,
}
}
return None;
}
if let StmtKind::Local(ref local) = block.stmts[0].kind {
- if let Some(expr) = local.init {
- Some(expr)
- } else {
- None
- }
+ local.init //.map(|expr| expr)
} else {
None
}
/// Scan a for loop for variables that are incremented exactly once.
struct IncrementVisitor<'a, 'tcx> {
- cx: &'a LateContext<'a, 'tcx>, // context reference
+ cx: &'a LateContext<'tcx>, // context reference
states: FxHashMap<HirId, VarState>, // incremented variables
depth: u32, // depth of conditional expressions
done: bool,
/// Checks whether a variable is initialized to zero at the start of a loop.
struct InitializeVisitor<'a, 'tcx> {
- cx: &'a LateContext<'a, 'tcx>, // context reference
- end_expr: &'tcx Expr<'tcx>, // the for loop. Stop scanning here.
+ cx: &'a LateContext<'tcx>, // context reference
+ end_expr: &'tcx Expr<'tcx>, // the for loop. Stop scanning here.
var_id: HirId,
state: VarState,
- name: Option<Name>,
+ name: Option<Symbol>,
depth: u32, // depth of conditional expressions
past_loop: bool,
}
if let PatKind::Binding(.., ident, _) = local.pat.kind {
self.name = Some(ident.name);
- self.state = if let Some(ref init) = local.init {
+ self.state = local.init.as_ref().map_or(VarState::Declared, |init| {
if is_integer_const(&self.cx, init, 0) {
VarState::Warn
} else {
VarState::Declared
}
- } else {
- VarState::Declared
- }
+ })
}
}
}
if self.state == VarState::DontWarn {
return;
}
- if SpanlessEq::new(self.cx).eq_expr(&expr, self.end_expr) {
+ if expr.hir_id == self.end_expr.hir_id {
self.past_loop = true;
return;
}
}
}
-fn var_def_id(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> Option<HirId> {
+fn var_def_id(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<HirId> {
if let ExprKind::Path(ref qpath) = expr.kind {
let path_res = qpath_res(cx, qpath, expr.hir_id);
if let Res::Local(hir_id) = path_res {
}
fn is_loop(expr: &Expr<'_>) -> bool {
- match expr.kind {
- ExprKind::Loop(..) => true,
- _ => false,
- }
+ matches!(expr.kind, ExprKind::Loop(..))
}
fn is_conditional(expr: &Expr<'_>) -> bool {
- match expr.kind {
- ExprKind::Match(..) => true,
- _ => false,
- }
+ matches!(expr.kind, ExprKind::Match(..))
}
-fn is_nested(cx: &LateContext<'_, '_>, match_expr: &Expr<'_>, iter_expr: &Expr<'_>) -> bool {
+fn is_nested(cx: &LateContext<'_>, match_expr: &Expr<'_>, iter_expr: &Expr<'_>) -> bool {
if_chain! {
if let Some(loop_block) = get_enclosing_block(cx, match_expr.hir_id);
let parent_node = cx.tcx.hir().get_parent_node(loop_block.hir_id);
false
}
-fn is_loop_nested(cx: &LateContext<'_, '_>, loop_expr: &Expr<'_>, iter_expr: &Expr<'_>) -> bool {
+fn is_loop_nested(cx: &LateContext<'_>, loop_expr: &Expr<'_>, iter_expr: &Expr<'_>) -> bool {
let mut id = loop_expr.hir_id;
let iter_name = if let Some(name) = path_name(iter_expr) {
name
struct LoopNestVisitor {
hir_id: HirId,
- iterator: Name,
+ iterator: Symbol,
nesting: Nesting,
}
}
}
-fn path_name(e: &Expr<'_>) -> Option<Name> {
+fn path_name(e: &Expr<'_>) -> Option<Symbol> {
if let ExprKind::Path(QPath::Resolved(_, ref path)) = e.kind {
let segments = &path.segments;
if segments.len() == 1 {
None
}
-fn check_infinite_loop<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, cond: &'tcx Expr<'_>, expr: &'tcx Expr<'_>) {
- if constant(cx, cx.tables, cond).is_some() {
+fn check_infinite_loop<'tcx>(cx: &LateContext<'tcx>, cond: &'tcx Expr<'_>, expr: &'tcx Expr<'_>) {
+ if constant(cx, cx.typeck_results(), cond).is_some() {
// A pure constant condition (e.g., `while false`) is not linted.
return;
}
/// Note: In some cases such as `self`, there are no mutable annotation,
/// All variables definition IDs are collected
struct VarCollectorVisitor<'a, 'tcx> {
- cx: &'a LateContext<'a, 'tcx>,
+ cx: &'a LateContext<'tcx>,
ids: FxHashSet<HirId>,
def_ids: FxHashMap<def_id::DefId, bool>,
skip: bool,
const NEEDLESS_COLLECT_MSG: &str = "avoid using `collect()` when not needed";
-fn check_needless_collect<'a, 'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'a, 'tcx>) {
+fn check_needless_collect<'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'tcx>) {
+ check_needless_collect_direct_usage(expr, cx);
+ check_needless_collect_indirect_usage(expr, cx);
+}
+fn check_needless_collect_direct_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'tcx>) {
if_chain! {
if let ExprKind::MethodCall(ref method, _, ref args, _) = expr.kind;
if let ExprKind::MethodCall(ref chain_method, _, _, _) = args[0].kind;
if let Some(ref generic_args) = chain_method.args;
if let Some(GenericArg::Type(ref ty)) = generic_args.args.get(0);
then {
- let ty = cx.tables.node_type(ty.hir_id);
+ let ty = cx.typeck_results().node_type(ty.hir_id);
if is_type_diagnostic_item(cx, ty, sym!(vec_type)) ||
is_type_diagnostic_item(cx, ty, sym!(vecdeque_type)) ||
match_type(cx, ty, &paths::BTREEMAP) ||
is_type_diagnostic_item(cx, ty, sym!(hashmap_type)) {
if method.ident.name == sym!(len) {
- let span = shorten_span(expr, sym!(collect));
+ let span = shorten_needless_collect_span(expr);
span_lint_and_sugg(
cx,
NEEDLESS_COLLECT,
);
}
if method.ident.name == sym!(is_empty) {
- let span = shorten_span(expr, sym!(iter));
+ let span = shorten_needless_collect_span(expr);
span_lint_and_sugg(
cx,
NEEDLESS_COLLECT,
span,
NEEDLESS_COLLECT_MSG,
"replace with",
- "get(0).is_none()".to_string(),
+ "next().is_none()".to_string(),
Applicability::MachineApplicable,
);
}
if method.ident.name == sym!(contains) {
let contains_arg = snippet(cx, args[1].span, "??");
- let span = shorten_span(expr, sym!(collect));
+ let span = shorten_needless_collect_span(expr);
span_lint_and_then(
cx,
NEEDLESS_COLLECT,
}
}
-fn shorten_span(expr: &Expr<'_>, target_fn_name: Symbol) -> Span {
- let mut current_expr = expr;
- while let ExprKind::MethodCall(ref path, ref span, ref args, _) = current_expr.kind {
- if path.ident.name == target_fn_name {
+fn check_needless_collect_indirect_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'tcx>) {
+ if let ExprKind::Block(ref block, _) = expr.kind {
+ for ref stmt in block.stmts {
+ if_chain! {
+ if let StmtKind::Local(
+ Local { pat: Pat { kind: PatKind::Binding(_, _, ident, .. ), .. },
+ init: Some(ref init_expr), .. }
+ ) = stmt.kind;
+ if let ExprKind::MethodCall(ref method_name, _, &[ref iter_source], ..) = init_expr.kind;
+ if method_name.ident.name == sym!(collect) && match_trait_method(cx, &init_expr, &paths::ITERATOR);
+ if let Some(ref generic_args) = method_name.args;
+ if let Some(GenericArg::Type(ref ty)) = generic_args.args.get(0);
+ if let ty = cx.typeck_results().node_type(ty.hir_id);
+ if is_type_diagnostic_item(cx, ty, sym::vec_type) ||
+ is_type_diagnostic_item(cx, ty, sym!(vecdeque_type)) ||
+ match_type(cx, ty, &paths::LINKED_LIST);
+ if let Some(iter_calls) = detect_iter_and_into_iters(block, *ident);
+ if iter_calls.len() == 1;
+ then {
+ // Suggest replacing iter_call with iter_replacement, and removing stmt
+ let iter_call = &iter_calls[0];
+ span_lint_and_then(
+ cx,
+ NEEDLESS_COLLECT,
+ stmt.span.until(iter_call.span),
+ NEEDLESS_COLLECT_MSG,
+ |diag| {
+ let iter_replacement = format!("{}{}", Sugg::hir(cx, iter_source, ".."), iter_call.get_iter_method(cx));
+ diag.multipart_suggestion(
+ iter_call.get_suggestion_text(),
+ vec![
+ (stmt.span, String::new()),
+ (iter_call.span, iter_replacement)
+ ],
+ Applicability::MachineApplicable,// MaybeIncorrect,
+ ).emit();
+ },
+ );
+ }
+ }
+ }
+ }
+}
+
+struct IterFunction {
+ func: IterFunctionKind,
+ span: Span,
+}
+impl IterFunction {
+ fn get_iter_method(&self, cx: &LateContext<'_>) -> String {
+ match &self.func {
+ IterFunctionKind::IntoIter => String::new(),
+ IterFunctionKind::Len => String::from(".count()"),
+ IterFunctionKind::IsEmpty => String::from(".next().is_none()"),
+ IterFunctionKind::Contains(span) => format!(".any(|x| x == {})", snippet(cx, *span, "..")),
+ }
+ }
+ fn get_suggestion_text(&self) -> &'static str {
+ match &self.func {
+ IterFunctionKind::IntoIter => {
+ "Use the original Iterator instead of collecting it and then producing a new one"
+ },
+ IterFunctionKind::Len => {
+ "Take the original Iterator's count instead of collecting it and finding the length"
+ },
+ IterFunctionKind::IsEmpty => {
+ "Check if the original Iterator has anything instead of collecting it and seeing if it's empty"
+ },
+ IterFunctionKind::Contains(_) => {
+ "Check if the original Iterator contains an element instead of collecting then checking"
+ },
+ }
+ }
+}
+enum IterFunctionKind {
+ IntoIter,
+ Len,
+ IsEmpty,
+ Contains(Span),
+}
+
+struct IterFunctionVisitor {
+ uses: Vec<IterFunction>,
+ seen_other: bool,
+ target: Ident,
+}
+impl<'tcx> Visitor<'tcx> for IterFunctionVisitor {
+ fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
+ // Check function calls on our collection
+ if_chain! {
+ if let ExprKind::MethodCall(method_name, _, ref args, _) = &expr.kind;
+ if let Some(Expr { kind: ExprKind::Path(QPath::Resolved(_, ref path)), .. }) = args.get(0);
+ if let &[name] = &path.segments;
+ if name.ident == self.target;
+ then {
+ let len = sym!(len);
+ let is_empty = sym!(is_empty);
+ let contains = sym!(contains);
+ match method_name.ident.name {
+ sym::into_iter => self.uses.push(
+ IterFunction { func: IterFunctionKind::IntoIter, span: expr.span }
+ ),
+ name if name == len => self.uses.push(
+ IterFunction { func: IterFunctionKind::Len, span: expr.span }
+ ),
+ name if name == is_empty => self.uses.push(
+ IterFunction { func: IterFunctionKind::IsEmpty, span: expr.span }
+ ),
+ name if name == contains => self.uses.push(
+ IterFunction { func: IterFunctionKind::Contains(args[1].span), span: expr.span }
+ ),
+ _ => self.seen_other = true,
+ }
+ return
+ }
+ }
+ // Check if the collection is used for anything else
+ if_chain! {
+ if let Expr { kind: ExprKind::Path(QPath::Resolved(_, ref path)), .. } = expr;
+ if let &[name] = &path.segments;
+ if name.ident == self.target;
+ then {
+ self.seen_other = true;
+ } else {
+ walk_expr(self, expr);
+ }
+ }
+ }
+
+ type Map = Map<'tcx>;
+ fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
+ NestedVisitorMap::None
+ }
+}
+
+/// Detect the occurences of calls to `iter` or `into_iter` for the
+/// given identifier
+fn detect_iter_and_into_iters<'tcx>(block: &'tcx Block<'tcx>, identifier: Ident) -> Option<Vec<IterFunction>> {
+ let mut visitor = IterFunctionVisitor {
+ uses: Vec::new(),
+ target: identifier,
+ seen_other: false,
+ };
+ visitor.visit_block(block);
+ if visitor.seen_other {
+ None
+ } else {
+ Some(visitor.uses)
+ }
+}
+
+fn shorten_needless_collect_span(expr: &Expr<'_>) -> Span {
+ if_chain! {
+ if let ExprKind::MethodCall(.., args, _) = &expr.kind;
+ if let ExprKind::MethodCall(_, span, ..) = &args[0].kind;
+ then {
return expr.span.with_lo(span.lo());
}
- current_expr = &args[0];
}
- unreachable!()
+ unreachable!();
}
projects / rust.git / blobdiff