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, 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,
+ contains_name, get_enclosing_block, get_parent_expr, get_trait_def_id, has_iter_method, higher, implements_trait,
+ indent_of, is_in_panic_handler, 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, single_segment_path,
+ snippet, snippet_with_applicability, snippet_with_macro_callsite, span_lint, span_lint_and_help,
+ span_lint_and_sugg, span_lint_and_then, sugg, SpanlessEq,
};
use if_chain::if_chain;
use rustc_ast::ast;
declare_clippy_lint! {
/// **What it does:** Checks for empty `loop` expressions.
///
- /// **Why is this bad?** Those busy loops burn CPU cycles without doing
- /// anything. Think of the environment and either block on something or at least
- /// make the thread sleep for some microseconds.
+ /// **Why is this bad?** These busy loops burn CPU cycles without doing
+ /// anything. It is _almost always_ a better idea to `panic!` than to have
+ /// a busy loop.
+ ///
+ /// If panicking isn't possible, think of the environment and either:
+ /// - block on something
+ /// - sleep the thread for some microseconds
+ /// - yield or pause the thread
+ ///
+ /// For `std` targets, this can be done with
+ /// [`std::thread::sleep`](https://doc.rust-lang.org/std/thread/fn.sleep.html)
+ /// or [`std::thread::yield_now`](https://doc.rust-lang.org/std/thread/fn.yield_now.html).
+ ///
+ /// For `no_std` targets, doing this is more complicated, especially because
+ /// `#[panic_handler]`s can't panic. To stop/pause the thread, you will
+ /// probably need to invoke some target-specific intrinsic. Examples include:
+ /// - [`x86_64::instructions::hlt`](https://docs.rs/x86_64/0.12.2/x86_64/instructions/fn.hlt.html)
+ /// - [`cortex_m::asm::wfi`](https://docs.rs/cortex-m/0.6.3/cortex_m/asm/fn.wfi.html)
///
/// **Known problems:** None.
///
"the same item is pushed inside of a for loop"
}
+declare_clippy_lint! {
+ /// **What it does:** Checks whether a for loop has a single element.
+ ///
+ /// **Why is this bad?** There is no reason to have a loop of a
+ /// single element.
+ /// **Known problems:** None
+ ///
+ /// **Example:**
+ /// ```rust
+ /// let item1 = 2;
+ /// for item in &[item1] {
+ /// println!("{}", item);
+ /// }
+ /// ```
+ /// could be written as
+ /// ```rust
+ /// let item1 = 2;
+ /// let item = &item1;
+ /// println!("{}", item);
+ /// ```
+ pub SINGLE_ELEMENT_LOOP,
+ complexity,
+ "there is no reason to have a single element loop"
+}
+
declare_lint_pass!(Loops => [
MANUAL_MEMCPY,
NEEDLESS_RANGE_LOOP,
MUT_RANGE_BOUND,
WHILE_IMMUTABLE_CONDITION,
SAME_ITEM_PUSH,
+ SINGLE_ELEMENT_LOOP,
]);
impl<'tcx> LateLintPass<'tcx> for Loops {
// (also matches an explicit "match" instead of "if let")
// (even if the "match" or "if let" is used for declaration)
if let ExprKind::Loop(ref block, _, LoopSource::Loop) = expr.kind {
- // also check for empty `loop {}` statements
- if block.stmts.is_empty() && block.expr.is_none() && !is_no_std_crate(cx.tcx.hir().krate()) {
- span_lint(
- cx,
- EMPTY_LOOP,
- expr.span,
- "empty `loop {}` detected. You may want to either use `panic!()` or add \
- `std::thread::sleep(..);` to the loop body.",
- );
+ // also check for empty `loop {}` statements, skipping those in #[panic_handler]
+ if block.stmts.is_empty() && block.expr.is_none() && !is_in_panic_handler(cx, expr) {
+ let msg = "empty `loop {}` wastes CPU cycles";
+ let help = if is_no_std_crate(cx.tcx.hir().krate()) {
+ "you should either use `panic!()` or add a call pausing or sleeping the thread to the loop body"
+ } else {
+ "you should either use `panic!()` or add `std::thread::sleep(..);` to the loop body"
+ };
+ span_lint_and_help(cx, EMPTY_LOOP, expr.span, msg, None, help);
}
// extract the expression from the first statement (if any) in a block
}
let lhs_constructor = last_path_segment(qpath);
- if method_path.ident.name == sym!(next)
+ if method_path.ident.name == sym::next
&& match_trait_method(cx, match_expr, &paths::ITERATOR)
- && lhs_constructor.ident.name == sym!(Some)
+ && lhs_constructor.ident.name == sym::Some
&& (pat_args.is_empty()
|| !is_refutable(cx, &pat_args[0])
&& !is_used_inside(cx, iter_expr, &arms[0].body)
| ExprKind::Closure(_, _, _, _, _)
| ExprKind::LlvmInlineAsm(_)
| ExprKind::Path(_)
+ | ExprKind::ConstBlock(_)
| ExprKind::Lit(_)
| ExprKind::Err => NeverLoopResult::Otherwise,
}
body: &'tcx Expr<'_>,
expr: &'tcx Expr<'_>,
) {
- check_for_loop_range(cx, pat, arg, body, expr);
+ let is_manual_memcpy_triggered = detect_manual_memcpy(cx, pat, arg, body, expr);
+ if !is_manual_memcpy_triggered {
+ check_for_loop_range(cx, pat, arg, body, expr);
+ check_for_loop_explicit_counter(cx, pat, arg, body, expr);
+ }
check_for_loop_arg(cx, pat, arg, expr);
- check_for_loop_explicit_counter(cx, pat, arg, body, 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);
+ check_for_single_element_loop(cx, pat, arg, body, expr);
detect_same_item_push(cx, pat, arg, body, expr);
}
+// this function assumes the given expression is a `for` loop.
+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!()
+ }
+}
+
fn same_var<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, var: HirId) -> bool {
if_chain! {
if let ExprKind::Path(qpath) = &expr.kind;
}
}
-#[derive(Clone, Copy)]
-enum OffsetSign {
- Positive,
- Negative,
-}
-
-struct Offset {
- value: MinifyingSugg<'static>,
- sign: OffsetSign,
-}
-
-impl Offset {
- fn negative(value: MinifyingSugg<'static>) -> Self {
- Self {
- value,
- sign: OffsetSign::Negative,
- }
- }
-
- fn positive(value: MinifyingSugg<'static>) -> Self {
- Self {
- value,
- sign: OffsetSign::Positive,
- }
- }
-
- fn empty() -> Self {
- Self::positive(MinifyingSugg::non_paren("0"))
- }
-}
-
-fn apply_offset(lhs: &MinifyingSugg<'static>, rhs: &Offset) -> MinifyingSugg<'static> {
- match rhs.sign {
- OffsetSign::Positive => lhs + &rhs.value,
- OffsetSign::Negative => lhs - &rhs.value,
- }
-}
-
+/// a wrapper of `Sugg`. Besides what `Sugg` do, this removes unnecessary `0`;
+/// and also, it avoids subtracting a variable from the same one by replacing it with `0`.
+/// it exists for the convenience of the overloaded operators while normal functions can do the
+/// same.
#[derive(Clone)]
-struct MinifyingSugg<'a>(sugg::Sugg<'a>);
-
-impl std::fmt::Display for MinifyingSugg<'_> {
- fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
- std::fmt::Display::fmt(&self.0, f)
- }
-}
+struct MinifyingSugg<'a>(Sugg<'a>);
impl<'a> MinifyingSugg<'a> {
fn as_str(&self) -> &str {
- let sugg::Sugg::NonParen(s) | sugg::Sugg::MaybeParen(s) | sugg::Sugg::BinOp(_, s) = &self.0;
+ let Sugg::NonParen(s) | Sugg::MaybeParen(s) | Sugg::BinOp(_, s) = &self.0;
s.as_ref()
}
- fn hir(cx: &LateContext<'_, '_>, expr: &Expr<'_>, default: &'a str) -> Self {
- Self(sugg::Sugg::hir(cx, expr, default))
- }
-
- fn maybe_par(self) -> Self {
- Self(self.0.maybe_par())
+ fn into_sugg(self) -> Sugg<'a> {
+ self.0
}
+}
- fn non_paren(str: impl Into<std::borrow::Cow<'a, str>>) -> Self {
- Self(sugg::Sugg::NonParen(str.into()))
+impl<'a> From<Sugg<'a>> for MinifyingSugg<'a> {
+ fn from(sugg: Sugg<'a>) -> Self {
+ Self(sugg)
}
}
match (self.as_str(), rhs.as_str()) {
("0", _) => rhs.clone(),
(_, "0") => self.clone(),
- (_, _) => MinifyingSugg(&self.0 + &rhs.0),
+ (_, _) => (&self.0 + &rhs.0).into(),
}
}
}
fn sub(self, rhs: &MinifyingSugg<'static>) -> MinifyingSugg<'static> {
match (self.as_str(), rhs.as_str()) {
(_, "0") => self.clone(),
- ("0", _) => MinifyingSugg(sugg::make_unop("-", rhs.0.clone())),
- (x, y) if x == y => MinifyingSugg::non_paren("0"),
- (_, _) => MinifyingSugg(&self.0 - &rhs.0),
+ ("0", _) => (-rhs.0.clone()).into(),
+ (x, y) if x == y => sugg::ZERO.into(),
+ (_, _) => (&self.0 - &rhs.0).into(),
}
}
}
match (self.as_str(), rhs.as_str()) {
("0", _) => rhs.clone(),
(_, "0") => self,
- (_, _) => MinifyingSugg(self.0 + &rhs.0),
+ (_, _) => (self.0 + &rhs.0).into(),
}
}
}
fn sub(self, rhs: &MinifyingSugg<'static>) -> MinifyingSugg<'static> {
match (self.as_str(), rhs.as_str()) {
(_, "0") => self,
- ("0", _) => MinifyingSugg(sugg::make_unop("-", rhs.0.clone())),
- (x, y) if x == y => MinifyingSugg::non_paren("0"),
- (_, _) => MinifyingSugg(self.0 - &rhs.0),
+ ("0", _) => (-rhs.0.clone()).into(),
+ (x, y) if x == y => sugg::ZERO.into(),
+ (_, _) => (self.0 - &rhs.0).into(),
+ }
+ }
+}
+
+/// a wrapper around `MinifyingSugg`, which carries a operator like currying
+/// so that the suggested code become more efficient (e.g. `foo + -bar` `foo - bar`).
+struct Offset {
+ value: MinifyingSugg<'static>,
+ sign: OffsetSign,
+}
+
+#[derive(Clone, Copy)]
+enum OffsetSign {
+ Positive,
+ Negative,
+}
+
+impl Offset {
+ fn negative(value: Sugg<'static>) -> Self {
+ Self {
+ value: value.into(),
+ sign: OffsetSign::Negative,
+ }
+ }
+
+ fn positive(value: Sugg<'static>) -> Self {
+ Self {
+ value: value.into(),
+ sign: OffsetSign::Positive,
}
}
+
+ fn empty() -> Self {
+ Self::positive(sugg::ZERO)
+ }
+}
+
+fn apply_offset(lhs: &MinifyingSugg<'static>, rhs: &Offset) -> MinifyingSugg<'static> {
+ match rhs.sign {
+ OffsetSign::Positive => lhs + &rhs.value,
+ OffsetSign::Negative => lhs - &rhs.value,
+ }
}
#[derive(Debug, Clone, Copy)]
_ => false,
};
- is_slice || is_type_diagnostic_item(cx, ty, sym!(vec_type)) || is_type_diagnostic_item(cx, ty, sym!(vecdeque_type))
+ is_slice || is_type_diagnostic_item(cx, ty, sym::vec_type) || is_type_diagnostic_item(cx, ty, sym!(vecdeque_type))
}
fn fetch_cloned_expr<'tcx>(expr: &'tcx Expr<'tcx>) -> &'tcx Expr<'tcx> {
if_chain! {
if let ExprKind::MethodCall(method, _, args, _) = expr.kind;
- if method.ident.name == sym!(clone);
+ if method.ident.name == sym::clone;
if args.len() == 1;
if let Some(arg) = args.get(0);
then { arg } else { expr }
}
}
-fn get_offset<'tcx>(
+fn get_details_from_idx<'tcx>(
cx: &LateContext<'tcx>,
idx: &Expr<'_>,
starts: &[Start<'tcx>],
) -> Option<(StartKind<'tcx>, Offset)> {
- fn extract_start<'tcx>(
- cx: &LateContext<'tcx>,
- expr: &Expr<'_>,
- starts: &[Start<'tcx>],
- ) -> Option<StartKind<'tcx>> {
- starts.iter().find(|var| same_var(cx, expr, var.id)).map(|v| v.kind)
- }
-
- fn extract_offset<'tcx>(
- cx: &LateContext<'tcx>,
- e: &Expr<'_>,
- starts: &[Start<'tcx>],
- ) -> Option<MinifyingSugg<'static>> {
+ fn get_start<'tcx>(cx: &LateContext<'tcx>, e: &Expr<'_>, starts: &[Start<'tcx>]) -> Option<StartKind<'tcx>> {
+ starts.iter().find_map(|start| {
+ if same_var(cx, e, start.id) {
+ Some(start.kind)
+ } else {
+ None
+ }
+ })
+ }
+
+ fn get_offset<'tcx>(cx: &LateContext<'tcx>, e: &Expr<'_>, starts: &[Start<'tcx>]) -> Option<Sugg<'static>> {
match &e.kind {
ExprKind::Lit(l) => match l.node {
- ast::LitKind::Int(x, _ty) => Some(MinifyingSugg::non_paren(x.to_string())),
+ ast::LitKind::Int(x, _ty) => Some(Sugg::NonParen(x.to_string().into())),
_ => None,
},
- ExprKind::Path(..) if extract_start(cx, e, starts).is_none() => {
- // `e` is always non paren as it's a `Path`
- Some(MinifyingSugg::non_paren(snippet(cx, e.span, "???")))
- },
+ ExprKind::Path(..) if get_start(cx, e, starts).is_none() => Some(Sugg::hir(cx, e, "???")),
_ => None,
}
}
match idx.kind {
ExprKind::Binary(op, lhs, rhs) => match op.node {
BinOpKind::Add => {
- let offset_opt = if let Some(s) = extract_start(cx, lhs, starts) {
- extract_offset(cx, rhs, starts).map(|o| (s, o))
- } else if let Some(s) = extract_start(cx, rhs, starts) {
- extract_offset(cx, lhs, starts).map(|o| (s, o))
- } else {
- None
- };
+ let offset_opt = get_start(cx, lhs, starts)
+ .and_then(|s| get_offset(cx, rhs, starts).map(|o| (s, o)))
+ .or_else(|| get_start(cx, rhs, starts).and_then(|s| get_offset(cx, lhs, starts).map(|o| (s, o))));
offset_opt.map(|(s, o)| (s, Offset::positive(o)))
},
- BinOpKind::Sub => extract_start(cx, lhs, starts)
- .and_then(|s| extract_offset(cx, rhs, starts).map(|o| (s, Offset::negative(o)))),
+ BinOpKind::Sub => {
+ get_start(cx, lhs, starts).and_then(|s| get_offset(cx, rhs, starts).map(|o| (s, Offset::negative(o))))
+ },
_ => None,
},
- ExprKind::Path(..) => extract_start(cx, idx, starts).map(|s| (s, Offset::empty())),
+ ExprKind::Path(..) => get_start(cx, idx, starts).map(|s| (s, Offset::empty())),
_ => None,
}
}
}
}
+/// Get assignments from the given block.
+/// The returned iterator yields `None` if no assignment expressions are there,
+/// filtering out the increments of the given whitelisted loop counters;
+/// because its job is to make sure there's nothing other than assignments and the increments.
fn get_assignments<'a: 'c, 'tcx: 'c, 'c>(
- cx: &'a LateContext<'a, 'tcx>,
- stmts: &'tcx [Stmt<'tcx>],
- expr: Option<&'tcx Expr<'tcx>>,
+ cx: &'a LateContext<'tcx>,
+ Block { stmts, expr, .. }: &'tcx Block<'tcx>,
loop_counters: &'c [Start<'tcx>],
) -> impl Iterator<Item = Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)>> + 'c {
+ // As the `filter` and `map` below do different things, I think putting together
+ // just increases complexity. (cc #3188 and #4193)
+ #[allow(clippy::filter_map)]
stmts
.iter()
.filter_map(move |stmt| match stmt.kind {
StmtKind::Local(..) | StmtKind::Item(..) => None,
- StmtKind::Expr(e) | StmtKind::Semi(e) => if_chain! {
- if let ExprKind::AssignOp(_, var, _) = e.kind;
- // skip StartKind::Range
- if loop_counters.iter().skip(1).any(|counter| Some(counter.id) == var_def_id(cx, var));
- then { None } else { Some(e) }
- },
+ StmtKind::Expr(e) | StmtKind::Semi(e) => Some(e),
+ })
+ .chain((*expr).into_iter())
+ .filter(move |e| {
+ if let ExprKind::AssignOp(_, place, _) = e.kind {
+ !loop_counters
+ .iter()
+ // skip the first item which should be `StartKind::Range`
+ // this makes it possible to use the slice with `StartKind::Range` in the same iterator loop.
+ .skip(1)
+ .any(|counter| same_var(cx, place, counter.id))
+ } else {
+ true
+ }
})
- .chain(expr.into_iter())
.map(get_assignment)
}
fn get_loop_counters<'a, 'tcx>(
- cx: &'a LateContext<'a, 'tcx>,
+ cx: &'a LateContext<'tcx>,
body: &'tcx Block<'tcx>,
expr: &'tcx Expr<'_>,
) -> Option<impl Iterator<Item = Start<'tcx>> + 'a> {
// For each candidate, check the parent block to see if
// it's initialized to zero at the start of the loop.
- if let Some(block) = get_enclosing_block(&cx, expr.hir_id) {
+ get_enclosing_block(&cx, expr.hir_id).and_then(|block| {
increment_visitor
.into_results()
.filter_map(move |var_id| {
})
})
.into()
- } else {
- None
- }
+ })
}
fn build_manual_memcpy_suggestion<'tcx>(
start: &Expr<'_>,
end: &Expr<'_>,
limits: ast::RangeLimits,
- dst: IndexExpr<'_>,
- src: IndexExpr<'_>,
+ dst: &IndexExpr<'_>,
+ src: &IndexExpr<'_>,
) -> String {
fn print_offset(offset: MinifyingSugg<'static>) -> MinifyingSugg<'static> {
if offset.as_str() == "0" {
- MinifyingSugg::non_paren("")
+ sugg::EMPTY.into()
} else {
offset
}
}
- let print_limit = |end: &Expr<'_>, end_str: &str, base: &Expr<'_>, sugg: MinifyingSugg<'static>| -> MinifyingSugg<'static> {
+ let print_limit = |end: &Expr<'_>, end_str: &str, base: &Expr<'_>, sugg: MinifyingSugg<'static>| {
if_chain! {
if let ExprKind::MethodCall(method, _, len_args, _) = end.kind;
if method.ident.name == sym!(len);
if var_def_id(cx, arg) == var_def_id(cx, base);
then {
if sugg.as_str() == end_str {
- MinifyingSugg::non_paren("")
+ sugg::EMPTY.into()
} else {
sugg
}
} else {
match limits {
ast::RangeLimits::Closed => {
- sugg + &MinifyingSugg::non_paren("1")
+ sugg + &sugg::ONE.into()
},
ast::RangeLimits::HalfOpen => sugg,
}
}
};
- let start_str = MinifyingSugg::hir(cx, start, "");
- let end_str = MinifyingSugg::hir(cx, end, "");
+ let start_str = Sugg::hir(cx, start, "").into();
+ let end_str: MinifyingSugg<'_> = Sugg::hir(cx, end, "").into();
let print_offset_and_limit = |idx_expr: &IndexExpr<'_>| match idx_expr.idx {
StartKind::Range => (
- print_offset(apply_offset(&start_str, &idx_expr.idx_offset)),
+ print_offset(apply_offset(&start_str, &idx_expr.idx_offset)).into_sugg(),
print_limit(
end,
end_str.as_str(),
idx_expr.base,
apply_offset(&end_str, &idx_expr.idx_offset),
- ),
+ )
+ .into_sugg(),
),
StartKind::Counter { initializer } => {
- let counter_start = MinifyingSugg::hir(cx, initializer, "");
+ let counter_start = Sugg::hir(cx, initializer, "").into();
(
- print_offset(apply_offset(&counter_start, &idx_expr.idx_offset)),
+ print_offset(apply_offset(&counter_start, &idx_expr.idx_offset)).into_sugg(),
print_limit(
end,
end_str.as_str(),
idx_expr.base,
apply_offset(&end_str, &idx_expr.idx_offset) + &counter_start - &start_str,
- ),
+ )
+ .into_sugg(),
)
},
};
let (dst_offset, dst_limit) = print_offset_and_limit(&dst);
let (src_offset, src_limit) = print_offset_and_limit(&src);
- let dst_base_str = snippet_opt(cx, dst.base.span).unwrap_or_else(|| "???".into());
- let src_base_str = snippet_opt(cx, src.base.span).unwrap_or_else(|| "???".into());
+ let dst_base_str = snippet(cx, dst.base.span, "???");
+ let src_base_str = snippet(cx, src.base.span, "???");
- let dst = if dst_offset.as_str() == "" && dst_limit.as_str() == "" {
+ let dst = if dst_offset == sugg::EMPTY && dst_limit == sugg::EMPTY {
dst_base_str
} else {
format!(
dst_offset.maybe_par(),
dst_limit.maybe_par()
)
+ .into()
};
format!(
- "{}.clone_from_slice(&{}[{}..{}])",
+ "{}.clone_from_slice(&{}[{}..{}]);",
dst,
src_base_str,
src_offset.maybe_par(),
arg: &'tcx Expr<'_>,
body: &'tcx Expr<'_>,
expr: &'tcx Expr<'_>,
-) {
+) -> bool {
if let Some(higher::Range {
start: Some(start),
end: Some(end),
if let Some(loop_counters) = get_loop_counters(cx, block, expr) {
starts.extend(loop_counters);
}
- iter_a = Some(get_assignments(cx, block.stmts, block.expr, &starts));
+ iter_a = Some(get_assignments(cx, block, &starts));
} else {
iter_b = Some(get_assignment(body));
}
- // The only statements in the for loops can be indexed assignments from
- // indexed retrievals.
let assignments = iter_a.into_iter().flatten().chain(iter_b.into_iter());
let big_sugg = assignments
+ // The only statements in the for loops can be indexed assignments from
+ // indexed retrievals (except increments of loop counters).
.map(|o| {
o.and_then(|(lhs, rhs)| {
let rhs = fetch_cloned_expr(rhs);
if let ExprKind::Index(base_right, idx_right) = rhs.kind;
if is_slice_like(cx, cx.typeck_results().expr_ty(base_left))
&& is_slice_like(cx, cx.typeck_results().expr_ty(base_right));
- if let Some((start_left, offset_left)) = get_offset(cx, &idx_left, &starts);
- if let Some((start_right, offset_right)) = get_offset(cx, &idx_right, &starts);
+ if let Some((start_left, offset_left)) = get_details_from_idx(cx, &idx_left, &starts);
+ if let Some((start_right, offset_right)) = get_details_from_idx(cx, &idx_right, &starts);
// Source and destination must be different
if var_def_id(cx, base_left) != var_def_id(cx, base_right);
}
})
})
- .map(|o| o.map(|(dst, src)| build_manual_memcpy_suggestion(cx, start, end, limits, dst, src)))
+ .map(|o| o.map(|(dst, src)| build_manual_memcpy_suggestion(cx, start, end, limits, &dst, &src)))
.collect::<Option<Vec<_>>>()
.filter(|v| !v.is_empty())
.map(|v| v.join("\n "));
span_lint_and_sugg(
cx,
MANUAL_MEMCPY,
- expr.span,
+ get_span_of_entire_for_loop(expr),
"it looks like you're manually copying between slices",
"try replacing the loop by",
big_sugg,
Applicability::Unspecified,
);
+ return true;
}
}
}
+ false
}
// Scans the body of the for loop and determines whether lint should be given
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 is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(self_expr), sym!(vec_type));
+ if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(self_expr), sym::vec_type);
if path.ident.name.as_str() == "push";
then {
return Some((self_expr, pushed_item))
let skip = if starts_at_zero {
String::new()
+ } else if visitor.indexed_mut.contains(&indexed) && contains_name(indexed, start) {
+ return;
} else {
format!(".skip({})", snippet(cx, start.span, ".."))
};
if is_len_call(end, indexed) || is_end_eq_array_len(cx, end, limits, indexed_ty) {
String::new()
+ } else if visitor.indexed_mut.contains(&indexed) && contains_name(indexed, take_expr) {
+ return;
} else {
match limits {
ast::RangeLimits::Closed => {
/// Checks for `for` loops over `Option`s and `Result`s.
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)) {
+ if is_type_diagnostic_item(cx, ty, sym::option_type) {
span_lint_and_help(
cx,
FOR_LOOPS_OVER_FALLIBLES,
snippet(cx, arg.span, "_")
),
);
- } else if is_type_diagnostic_item(cx, ty, sym!(result_type)) {
+ } else if is_type_diagnostic_item(cx, ty, sym::result_type) {
span_lint_and_help(
cx,
FOR_LOOPS_OVER_FALLIBLES,
then {
let mut applicability = Applicability::MachineApplicable;
- // for some reason this is the only way to get the `Span`
- // of the entire `for` loop
- let for_span = if let ExprKind::Match(_, arms, _) = &expr.kind {
- arms[0].body.span
- } else {
- unreachable!()
- };
+ let for_span = get_span_of_entire_for_loop(expr);
span_lint_and_sugg(
cx,
}
}
+fn check_for_single_element_loop<'tcx>(
+ cx: &LateContext<'tcx>,
+ pat: &'tcx Pat<'_>,
+ arg: &'tcx Expr<'_>,
+ body: &'tcx Expr<'_>,
+ expr: &'tcx Expr<'_>,
+) {
+ if_chain! {
+ if let ExprKind::AddrOf(BorrowKind::Ref, _, ref arg_expr) = arg.kind;
+ if let PatKind::Binding(.., target, _) = pat.kind;
+ if let ExprKind::Array(ref arg_expr_list) = arg_expr.kind;
+ if let [arg_expression] = arg_expr_list;
+ if let ExprKind::Path(ref list_item) = arg_expression.kind;
+ if let Some(list_item_name) = single_segment_path(list_item).map(|ps| ps.ident.name);
+ if let ExprKind::Block(ref block, _) = body.kind;
+ 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();
+
+
+ span_lint_and_sugg(
+ cx,
+ SINGLE_ELEMENT_LOOP,
+ for_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),
+ Applicability::MachineApplicable
+ )
+ }
+ }
+}
+
struct MutatePairDelegate<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
hir_id_low: Option<HirId>,
}
impl<'tcx> Delegate<'tcx> for MutatePairDelegate<'_, 'tcx> {
- fn consume(&mut self, _: &PlaceWithHirId<'tcx>, _: ConsumeMode) {}
+ fn consume(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId, _: ConsumeMode) {}
- fn borrow(&mut self, cmt: &PlaceWithHirId<'tcx>, bk: ty::BorrowKind) {
+ fn borrow(&mut self, cmt: &PlaceWithHirId<'tcx>, diag_expr_id: HirId, bk: ty::BorrowKind) {
if let ty::BorrowKind::MutBorrow = bk {
if let PlaceBase::Local(id) = cmt.place.base {
if Some(id) == self.hir_id_low {
- self.span_low = Some(self.cx.tcx.hir().span(cmt.hir_id))
+ self.span_low = Some(self.cx.tcx.hir().span(diag_expr_id))
}
if Some(id) == self.hir_id_high {
- self.span_high = Some(self.cx.tcx.hir().span(cmt.hir_id))
+ self.span_high = Some(self.cx.tcx.hir().span(diag_expr_id))
}
}
}
}
- fn mutate(&mut self, cmt: &PlaceWithHirId<'tcx>) {
+ fn mutate(&mut self, cmt: &PlaceWithHirId<'tcx>, diag_expr_id: HirId) {
if let PlaceBase::Local(id) = cmt.place.base {
if Some(id) == self.hir_id_low {
- self.span_low = Some(self.cx.tcx.hir().span(cmt.hir_id))
+ self.span_low = Some(self.cx.tcx.hir().span(diag_expr_id))
}
if Some(id) == self.hir_id_high {
- self.span_high = Some(self.cx.tcx.hir().span(cmt.hir_id))
+ self.span_high = Some(self.cx.tcx.hir().span(diag_expr_id))
}
}
}
span_low: None,
span_high: None,
};
- 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(),
+ body.hir_id.owner,
cx.param_env,
cx.typeck_results(),
)
if_chain! {
// a range index op
if let ExprKind::MethodCall(ref meth, _, ref args, _) = expr.kind;
- if (meth.ident.name == sym!(index) && match_trait_method(self.cx, expr, &paths::INDEX))
- || (meth.ident.name == sym!(index_mut) && match_trait_method(self.cx, expr, &paths::INDEX_MUT));
+ if (meth.ident.name == sym::index && match_trait_method(self.cx, expr, &paths::INDEX))
+ || (meth.ident.name == sym::index_mut && match_trait_method(self.cx, expr, &paths::INDEX_MUT));
if !self.check(&args[1], &args[0], expr);
then { return }
}
// will allow further borrows afterwards
let ty = cx.typeck_results().expr_ty(e);
is_iterable_array(ty, cx) ||
- is_type_diagnostic_item(cx, ty, sym!(vec_type)) ||
+ is_type_diagnostic_item(cx, ty, sym::vec_type) ||
match_type(cx, ty, &paths::LINKED_LIST) ||
is_type_diagnostic_item(cx, ty, sym!(hashmap_type)) ||
is_type_diagnostic_item(cx, ty, sym!(hashset_type)) ||
}
impl<'a, 'tcx> IncrementVisitor<'a, 'tcx> {
- fn new(cx: &'a LateContext<'a, 'tcx>) -> Self {
+ fn new(cx: &'a LateContext<'tcx>) -> Self {
Self {
cx,
states: FxHashMap::default(),
}
fn into_results(self) -> impl Iterator<Item = HirId> {
- self.states
- .into_iter()
- .filter(|(_, state)| *state == IncrementVisitorVarState::IncrOnce)
- .map(|(id, _)| id)
+ self.states.into_iter().filter_map(|(id, state)| {
+ if state == IncrementVisitorVarState::IncrOnce {
+ Some(id)
+ } else {
+ None
+ }
+ })
}
}
enum InitializeVisitorState<'hir> {
Initial, // Not examined yet
Declared(Symbol), // Declared but not (yet) initialized
- Initialized { name: Symbol, initializer: &'hir Expr<'hir> },
+ Initialized {
+ name: Symbol,
+ initializer: &'hir Expr<'hir>,
+ },
DontWarn,
}
}
impl<'a, 'tcx> InitializeVisitor<'a, 'tcx> {
- fn new(cx: &'a LateContext<'a, 'tcx>, end_expr: &'tcx Expr<'tcx>, var_id: HirId) -> Self {
+ fn new(cx: &'a LateContext<'tcx>, end_expr: &'tcx Expr<'tcx>, var_id: HirId) -> Self {
Self {
cx,
end_expr,
}
}
- fn get_result(&self) -> Option<(Name, &'tcx Expr<'tcx>)> {
+ fn get_result(&self) -> Option<(Symbol, &'tcx Expr<'tcx>)> {
if let InitializeVisitorState::Initialized { name, initializer } = self.state {
Some((name, initializer))
} else {
if local.pat.hir_id == self.var_id;
if let PatKind::Binding(.., ident, _) = local.pat.kind;
then {
- self.state = if let Some(ref init) = local.init {
+ self.state = local.init.map_or(InitializeVisitorState::Declared(ident.name), |init| {
InitializeVisitorState::Initialized {
initializer: init,
name: ident.name,
}
- } else {
- InitializeVisitorState::Declared(ident.name)
- }
+ })
}
}
walk_stmt(self, stmt);
if let Some(GenericArg::Type(ref ty)) = generic_args.args.get(0);
then {
let ty = cx.typeck_results().node_type(ty.hir_id);
- if is_type_diagnostic_item(cx, ty, sym!(vec_type)) ||
+ 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)) {
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, "..")),
+ IterFunctionKind::Contains(span) => {
+ let s = snippet(cx, *span, "..");
+ if let Some(stripped) = s.strip_prefix('&') {
+ format!(".any(|x| x == {})", stripped)
+ } else {
+ format!(".any(|x| x == *{})", s)
+ }
+ },
}
}
fn get_suggestion_text(&self) -> &'static str {
}
}
-/// Detect the occurences of calls to `iter` or `into_iter` for the
+/// Detect the occurrences 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 {