// use rustc::middle::region::CodeExtent;
use crate::consts::{constant, Constant};
use crate::utils::usage::mutated_variables;
-use crate::utils::{in_macro_or_desugar, sext, sugg};
+use crate::utils::{is_type_diagnostic_item, qpath_res, sext, sugg};
use rustc::middle::expr_use_visitor::*;
use rustc::middle::mem_categorization::cmt_;
use rustc::middle::mem_categorization::Categorization;
use std::mem;
use syntax::ast;
use syntax::source_map::Span;
-use syntax_pos::BytePos;
+use syntax_pos::{BytePos, Symbol};
use crate::utils::paths;
use crate::utils::{
- get_enclosing_block, get_parent_expr, has_iter_method, higher, is_integer_literal, is_refutable, last_path_segment,
+ get_enclosing_block, get_parent_expr, has_iter_method, higher, is_integer_const, is_refutable, last_path_segment,
match_trait_method, match_type, match_var, multispan_sugg, snippet, snippet_opt, snippet_with_applicability,
span_help_and_lint, span_lint, span_lint_and_sugg, span_lint_and_then, SpanlessEq,
};
/// **Known problems:** None.
///
/// **Example:**
- /// ```ignore
+ /// ```rust
+ /// # let src = vec![1];
+ /// # let mut dst = vec![0; 65];
/// for i in 0..src.len() {
/// dst[i + 64] = src[i];
/// }
/// ```
+ /// Could be written as:
+ /// ```rust
+ /// # let src = vec![1];
+ /// # let mut dst = vec![0; 65];
+ /// dst[64..(src.len() + 64)].clone_from_slice(&src[..]);
+ /// ```
pub MANUAL_MEMCPY,
perf,
"manually copying items between slices"
/// types.
///
/// **Example:**
- /// ```ignore
+ /// ```rust
/// // with `y` a `Vec` or slice:
+ /// # let y = vec![1];
/// for x in y.iter() {
- /// ..
+ /// // ..
/// }
/// ```
/// can be rewritten to
/// ```rust
+ /// # let y = vec![1];
/// for x in &y {
- /// ..
+ /// // ..
/// }
/// ```
pub EXPLICIT_ITER_LOOP,
/// **Known problems:** None
///
/// **Example:**
- /// ```ignore
+ /// ```rust
+ /// # let y = vec![1];
/// // with `y` a `Vec` or slice:
/// for x in y.into_iter() {
- /// ..
+ /// // ..
/// }
/// ```
/// can be rewritten to
- /// ```ignore
+ /// ```rust
+ /// # let y = vec![1];
/// for x in y {
- /// ..
+ /// // ..
/// }
/// ```
pub EXPLICIT_INTO_ITER_LOOP,
/// **Known problems:** Sometimes the wrong binding is displayed (#383).
///
/// **Example:**
- /// ```rust
+ /// ```rust,no_run
+ /// # let y = Some(1);
/// loop {
/// let x = match y {
/// Some(x) => x,
/// None => break,
- /// }
+ /// };
/// // .. do something with x
/// }
/// // is easier written as
/// while let Some(x) = y {
/// // .. do something with x
- /// }
+ /// };
/// ```
pub WHILE_LET_LOOP,
complexity,
"`loop { if let { ... } else break }`, which can be written as a `while let` loop"
}
-declare_clippy_lint! {
- /// **What it does:** Checks for using `collect()` on an iterator without using
- /// the result.
- ///
- /// **Why is this bad?** It is more idiomatic to use a `for` loop over the
- /// iterator instead.
- ///
- /// **Known problems:** None.
- ///
- /// **Example:**
- /// ```ignore
- /// vec.iter().map(|x| /* some operation returning () */).collect::<Vec<_>>();
- /// ```
- pub UNUSED_COLLECT,
- perf,
- "`collect()`ing an iterator without using the result; this is usually better written as a for loop"
-}
-
declare_clippy_lint! {
/// **What it does:** Checks for functions collecting an iterator when collect
/// is not needed.
/// None
///
/// **Example:**
- /// ```ignore
- /// let len = iterator.collect::<Vec<_>>().len();
+ /// ```rust
+ /// # let iterator = vec![1].into_iter();
+ /// let len = iterator.clone().collect::<Vec<_>>().len();
/// // should be
/// let len = iterator.count();
/// ```
/// **What it does:** Checks `for` loops over slices with an explicit counter
/// and suggests the use of `.enumerate()`.
///
- /// **Why is it bad?** Not only is the version using `.enumerate()` more
- /// readable, the compiler is able to remove bounds checks which can lead to
- /// faster code in some instances.
+ /// **Why is it bad?** Using `.enumerate()` makes the intent more clear,
+ /// declutters the code and may be faster in some instances.
///
/// **Known problems:** None.
///
/// **Example:**
- /// ```ignore
- /// for i in 0..v.len() { foo(v[i]);
- /// for i in 0..v.len() { bar(i, v[i]); }
+ /// ```rust
+ /// # let v = vec![1];
+ /// # fn bar(bar: usize, baz: usize) {}
+ /// let mut i = 0;
+ /// for item in &v {
+ /// bar(i, *item);
+ /// i += 1;
+ /// }
+ /// ```
+ /// Could be written as
+ /// ```rust
+ /// # let v = vec![1];
+ /// # fn bar(bar: usize, baz: usize) {}
+ /// for (i, item) in v.iter().enumerate() { bar(i, *item); }
/// ```
pub EXPLICIT_COUNTER_LOOP,
complexity,
FOR_LOOP_OVER_RESULT,
FOR_LOOP_OVER_OPTION,
WHILE_LET_LOOP,
- UNUSED_COLLECT,
NEEDLESS_COLLECT,
REVERSE_RANGE_LOOP,
EXPLICIT_COUNTER_LOOP,
#[allow(clippy::too_many_lines)]
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
// we don't want to check expanded macros
- if in_macro_or_desugar(expr.span) {
+ if expr.span.from_expansion() {
return;
}
}
// check for never_loop
- match expr.node {
- ExprKind::While(_, ref block, _) | ExprKind::Loop(ref block, _, _) => {
- match never_loop_block(block, expr.hir_id) {
- NeverLoopResult::AlwaysBreak => {
- span_lint(cx, NEVER_LOOP, expr.span, "this loop never actually loops")
- },
- NeverLoopResult::MayContinueMainLoop | NeverLoopResult::Otherwise => (),
- }
- },
- _ => (),
+ if let ExprKind::Loop(ref block, _, _) = expr.node {
+ match never_loop_block(block, expr.hir_id) {
+ NeverLoopResult::AlwaysBreak => span_lint(cx, NEVER_LOOP, expr.span, "this loop never actually loops"),
+ NeverLoopResult::MayContinueMainLoop | NeverLoopResult::Otherwise => (),
+ }
}
// check for `loop { if let {} else break }` that could be `while let`
}
}
- // check for while loops which conditions never change
- if let ExprKind::While(ref cond, _, _) = expr.node {
- check_infinite_loop(cx, cond, expr);
+ if let Some((cond, body)) = higher::while_loop(&expr) {
+ check_infinite_loop(cx, cond, body);
}
check_needless_collect(expr, cx);
}
-
- fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, stmt: &'tcx Stmt) {
- if let StmtKind::Semi(ref expr) = stmt.node {
- if let ExprKind::MethodCall(ref method, _, ref args) = expr.node {
- if args.len() == 1
- && method.ident.name == sym!(collect)
- && match_trait_method(cx, expr, &paths::ITERATOR)
- {
- span_lint(
- cx,
- UNUSED_COLLECT,
- expr.span,
- "you are collect()ing an iterator and throwing away the result. \
- Consider using an explicit for loop to exhaust the iterator",
- );
- }
- }
- }
- }
}
enum NeverLoopResult {
// Break can come from the inner loop so remove them.
absorb_break(&never_loop_block(b, main_loop_id))
},
- ExprKind::While(ref e, ref b, _) => {
- let e = never_loop_expr(e, main_loop_id);
- let result = never_loop_block(b, main_loop_id);
- // Break can come from the inner loop so remove them.
- combine_seq(e, absorb_break(&result))
- },
ExprKind::Match(ref e, ref arms, _) => {
let e = never_loop_expr(e, main_loop_id);
if arms.is_empty() {
NeverLoopResult::AlwaysBreak
}
},
- ExprKind::Break(_, _) => NeverLoopResult::AlwaysBreak,
- ExprKind::Ret(ref e) => {
+ 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 {
if let ExprKind::Path(ref qpath) = expr.node;
if let QPath::Resolved(None, ref path) = *qpath;
if path.segments.len() == 1;
- if let Res::Local(local_id) = cx.tables.qpath_res(qpath, expr.hir_id);
+ if let Res::Local(local_id) = qpath_res(cx, qpath, expr.hir_id);
// our variable!
if local_id == var;
then {
_ => false,
};
- is_slice || match_type(cx, ty, &paths::VEC) || match_type(cx, ty, &paths::VEC_DEQUE)
+ is_slice || is_type_diagnostic_item(cx, ty, Symbol::intern("vec_type")) || match_type(cx, ty, &paths::VEC_DEQUE)
}
fn get_fixed_offset_var<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &Expr, var: HirId) -> Option<FixedOffsetVar> {
body: &'tcx Expr,
expr: &'tcx Expr,
) {
- if in_macro_or_desugar(expr.span) {
+ if expr.span.from_expansion() {
return;
}
// ensure that the indexed variable was declared before the loop, see #601
if let Some(indexed_extent) = indexed_extent {
let parent_id = cx.tcx.hir().get_parent_item(expr.hir_id);
- let parent_def_id = cx.tcx.hir().local_def_id_from_hir_id(parent_id);
+ let parent_def_id = cx.tcx.hir().local_def_id(parent_id);
let region_scope_tree = cx.tcx.region_scope_tree(parent_def_id);
let pat_extent = region_scope_tree.var_scope(pat.hir_id.local_id);
if region_scope_tree.is_subscope_of(indexed_extent, pat_extent) {
return;
}
- let starts_at_zero = is_integer_literal(start, 0);
+ let starts_at_zero = is_integer_const(cx, start, 0);
let skip = if starts_at_zero {
String::new()
if let ExprKind::Lit(ref lit) = end.node;
if let ast::LitKind::Int(end_int, _) = lit.node;
if let ty::Array(_, arr_len_const) = indexed_ty.sty;
- if let Some(arr_len) = arr_len_const.assert_usize(cx.tcx);
+ if let Some(arr_len) = arr_len_const.try_eval_usize(cx.tcx, cx.param_env);
then {
return match limits {
ast::RangeLimits::Closed => end_int + 1 >= arr_len.into(),
match cx.tables.expr_ty(&args[0]).sty {
// If the length is greater than 32 no traits are implemented for array and
// therefore we cannot use `&`.
- ty::Array(_, size) if size.assert_usize(cx.tcx).expect("array size") > 32 => (),
+ ty::Array(_, size) if size.eval_usize(cx.tcx, cx.param_env) > 32 => {},
_ => lint_iter_method(cx, args, arg, method_name),
};
} else {
if let ExprKind::Path(ref qpath) = bound.node;
if let QPath::Resolved(None, _) = *qpath;
then {
- let res = cx.tables.qpath_res(qpath, bound.hir_id);
+ let res = qpath_res(cx, qpath, bound.hir_id);
if let Res::Local(node_id) = res {
let node_str = cx.tcx.hir().get(node_id);
if_chain! {
if self.prefer_mutable {
self.indexed_mut.insert(seqvar.segments[0].ident.name);
}
- let res = self.cx.tables.qpath_res(seqpath, seqexpr.hir_id);
+ let res = qpath_res(self.cx, seqpath, seqexpr.hir_id);
match res {
Res::Local(hir_id) => {
let parent_id = self.cx.tcx.hir().get_parent_item(expr.hir_id);
- let parent_def_id = self.cx.tcx.hir().local_def_id_from_hir_id(parent_id);
+ let parent_def_id = self.cx.tcx.hir().local_def_id(parent_id);
let extent = self.cx.tcx.region_scope_tree(parent_def_id).var_scope(hir_id.local_id);
if indexed_indirectly {
self.indexed_indirectly.insert(seqvar.segments[0].ident.name, Some(extent));
if let QPath::Resolved(None, ref path) = *qpath;
if path.segments.len() == 1;
then {
- if let Res::Local(local_id) = self.cx.tables.qpath_res(qpath, expr.hir_id) {
+ if let Res::Local(local_id) = qpath_res(self.cx, qpath, expr.hir_id) {
if local_id == self.var {
self.nonindex = true;
} else {
// will allow further borrows afterwards
let ty = cx.tables.expr_ty(e);
is_iterable_array(ty, cx) ||
- match_type(cx, ty, &paths::VEC) ||
+ is_type_diagnostic_item(cx, ty, Symbol::intern("vec_type")) ||
match_type(cx, ty, &paths::LINKED_LIST) ||
match_type(cx, ty, &paths::HASHMAP) ||
match_type(cx, ty, &paths::HASHSET) ||
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.sty {
- ty::Array(_, n) => (0..=32).contains(&n.assert_usize(cx.tcx).expect("array length")),
+ ty::Array(_, n) => (0..=32).contains(&n.eval_usize(cx.tcx, cx.param_env)),
_ => false,
}
}
fn is_simple_break_expr(expr: &Expr) -> bool {
match expr.node {
ExprKind::Break(dest, ref passed_expr) if dest.label.is_none() && passed_expr.is_none() => true,
- ExprKind::Block(ref b, _) => match extract_first_expr(b) {
- Some(subexpr) => is_simple_break_expr(subexpr),
- None => false,
- },
+ ExprKind::Block(ref b, _) => extract_first_expr(b).map_or(false, |subexpr| is_simple_break_expr(subexpr)),
_ => false,
}
}
match parent.node {
ExprKind::AssignOp(op, ref lhs, ref rhs) => {
if lhs.hir_id == expr.hir_id {
- if op.node == BinOpKind::Add && is_integer_literal(rhs, 1) {
+ if op.node == BinOpKind::Add && is_integer_const(self.cx, rhs, 1) {
*state = match *state {
VarState::Initial if self.depth == 0 => VarState::IncrOnce,
_ => VarState::DontWarn,
self.name = Some(ident.name);
self.state = if let Some(ref init) = local.init {
- if is_integer_literal(init, 0) {
+ if is_integer_const(&self.cx, init, 0) {
VarState::Warn
} else {
VarState::Declared
self.state = VarState::DontWarn;
},
ExprKind::Assign(ref lhs, ref rhs) if lhs.hir_id == expr.hir_id => {
- self.state = if is_integer_literal(rhs, 0) && self.depth == 0 {
+ self.state = if is_integer_const(&self.cx, rhs, 0) && self.depth == 0 {
VarState::Warn
} else {
VarState::DontWarn
fn var_def_id(cx: &LateContext<'_, '_>, expr: &Expr) -> Option<HirId> {
if let ExprKind::Path(ref qpath) = expr.node {
- let path_res = cx.tables.qpath_res(qpath, expr.hir_id);
+ let path_res = qpath_res(cx, qpath, expr.hir_id);
if let Res::Local(node_id) = path_res {
return Some(node_id);
}
fn is_loop(expr: &Expr) -> bool {
match expr.node {
- ExprKind::Loop(..) | ExprKind::While(..) => true,
+ ExprKind::Loop(..) => true,
_ => false,
}
}
return false;
}
match cx.tcx.hir().find(parent) {
- Some(Node::Expr(expr)) => match expr.node {
- ExprKind::Loop(..) | ExprKind::While(..) => {
+ Some(Node::Expr(expr)) => {
+ if let ExprKind::Loop(..) = expr.node {
return true;
- },
- _ => (),
+ };
},
Some(Node::Block(block)) => {
let mut block_visitor = LoopNestVisitor {
if_chain! {
if let ExprKind::Path(ref qpath) = ex.node;
if let QPath::Resolved(None, _) = *qpath;
- let res = self.cx.tables.qpath_res(qpath, ex.hir_id);
+ let res = qpath_res(self.cx, qpath, ex.hir_id);
then {
match res {
Res::Local(node_id) => {
if let Some(GenericArg::Type(ref ty)) = generic_args.args.get(0);
then {
let ty = cx.tables.node_type(ty.hir_id);
- if match_type(cx, ty, &paths::VEC) ||
+ if is_type_diagnostic_item(cx, ty, Symbol::intern("vec_type")) ||
match_type(cx, ty, &paths::VEC_DEQUE) ||
match_type(cx, ty, &paths::BTREEMAP) ||
match_type(cx, ty, &paths::HASHMAP) {
projects / rust.git / blobdiff