-use crate::consts::{constant, Constant};
+use crate::consts::constant;
use crate::reexport::Name;
use crate::utils::paths;
use crate::utils::usage::{is_unused, mutated_variables};
multispan_sugg, snippet, snippet_opt, snippet_with_applicability, span_lint, span_lint_and_help,
span_lint_and_sugg, span_lint_and_then, SpanlessEq,
};
-use crate::utils::{is_type_diagnostic_item, qpath_res, same_tys, sext, sugg};
+use crate::utils::{is_type_diagnostic_item, qpath_res, same_tys, sugg};
use if_chain::if_chain;
use rustc_ast::ast;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_hir::def::{DefKind, Res};
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, LoopSource,
- MatchSource, Mutability, Node, Pat, PatKind, QPath, Stmt, StmtKind,
+ def_id, BinOpKind, BindingAnnotation, Block, BorrowKind, Expr, ExprKind, GenericArg, HirId, InlineAsmOperand,
+ LoopSource, MatchSource, Mutability, Node, Pat, PatKind, QPath, Stmt, StmtKind,
};
use rustc_infer::infer::TyCtxtInferExt;
use rustc_lint::{LateContext, LateLintPass, LintContext};
}
declare_clippy_lint! {
- /// **What it does:** Checks for `for` loops over `Option` values.
+ /// **What it does:** Checks for `for` loops over `Option` or `Result` values.
///
/// **Why is this bad?** Readability. This is more clearly expressed as an `if
/// let`.
/// **Known problems:** None.
///
/// **Example:**
- /// ```ignore
- /// for x in option {
- /// ..
+ /// ```rust
+ /// # let opt = Some(1);
+ ///
+ /// // Bad
+ /// for x in opt {
+ /// // ..
/// }
- /// ```
///
- /// This should be
- /// ```ignore
- /// if let Some(x) = option {
- /// ..
+ /// // Good
+ /// if let Some(x) = opt {
+ /// // ..
/// }
/// ```
- pub FOR_LOOP_OVER_OPTION,
- correctness,
- "for-looping over an `Option`, which is more clearly expressed as an `if let`"
-}
-
-declare_clippy_lint! {
- /// **What it does:** Checks for `for` loops over `Result` values.
///
- /// **Why is this bad?** Readability. This is more clearly expressed as an `if
- /// let`.
+ /// // or
///
- /// **Known problems:** None.
+ /// ```rust
+ /// # let res: Result<i32, std::io::Error> = Ok(1);
///
- /// **Example:**
- /// ```ignore
- /// for x in result {
- /// ..
+ /// // Bad
+ /// for x in &res {
+ /// // ..
/// }
- /// ```
///
- /// This should be
- /// ```ignore
- /// if let Ok(x) = result {
- /// ..
+ /// // Good
+ /// if let Ok(x) = res {
+ /// // ..
/// }
/// ```
- pub FOR_LOOP_OVER_RESULT,
+ pub FOR_LOOPS_OVER_FALLIBLES,
correctness,
- "for-looping over a `Result`, which is more clearly expressed as an `if let`"
+ "for-looping over an `Option` or a `Result`, which is more clearly expressed as an `if let`"
}
declare_clippy_lint! {
"collecting an iterator when collect is not needed"
}
-declare_clippy_lint! {
- /// **What it does:** Checks for loops over ranges `x..y` where both `x` and `y`
- /// are constant and `x` is greater or equal to `y`, unless the range is
- /// reversed or has a negative `.step_by(_)`.
- ///
- /// **Why is it bad?** Such loops will either be skipped or loop until
- /// wrap-around (in debug code, this may `panic!()`). Both options are probably
- /// not intended.
- ///
- /// **Known problems:** The lint cannot catch loops over dynamically defined
- /// ranges. Doing this would require simulating all possible inputs and code
- /// paths through the program, which would be complex and error-prone.
- ///
- /// **Example:**
- /// ```ignore
- /// for x in 5..10 - 5 {
- /// ..
- /// } // oops, stray `-`
- /// ```
- pub REVERSE_RANGE_LOOP,
- correctness,
- "iteration over an empty range, such as `10..0` or `5..5`"
-}
-
declare_clippy_lint! {
/// **What it does:** Checks `for` loops over slices with an explicit counter
/// and suggests the use of `.enumerate()`.
EXPLICIT_ITER_LOOP,
EXPLICIT_INTO_ITER_LOOP,
ITER_NEXT_LOOP,
- FOR_LOOP_OVER_RESULT,
- FOR_LOOP_OVER_OPTION,
+ FOR_LOOPS_OVER_FALLIBLES,
WHILE_LET_LOOP,
NEEDLESS_COLLECT,
- REVERSE_RANGE_LOOP,
EXPLICIT_COUNTER_LOOP,
EMPTY_LOOP,
WHILE_LET_ON_ITERATOR,
NeverLoopResult::AlwaysBreak
}
},
+ ExprKind::InlineAsm(ref asm) => asm
+ .operands
+ .iter()
+ .map(|o| match o {
+ InlineAsmOperand::In { expr, .. }
+ | InlineAsmOperand::InOut { expr, .. }
+ | InlineAsmOperand::Const { expr }
+ | InlineAsmOperand::Sym { expr } => never_loop_expr(expr, main_loop_id),
+ InlineAsmOperand::Out { expr, .. } => never_loop_expr_all(&mut expr.iter(), main_loop_id),
+ InlineAsmOperand::SplitInOut { in_expr, out_expr, .. } => {
+ never_loop_expr_all(&mut once(in_expr).chain(out_expr.iter()), main_loop_id)
+ },
+ })
+ .fold(NeverLoopResult::Otherwise, combine_both),
ExprKind::Struct(_, _, None)
| ExprKind::Yield(_, _)
| ExprKind::Closure(_, _, _, _, _)
expr: &'tcx Expr<'_>,
) {
check_for_loop_range(cx, pat, arg, body, expr);
- check_for_loop_reverse_range(cx, arg, 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);
|diag| {
multispan_sugg(
diag,
- "consider using an iterator".to_string(),
+ "consider using an iterator",
vec![
(pat.span, format!("({}, <item>)", ident.name)),
(
|diag| {
multispan_sugg(
diag,
- "consider using an iterator".to_string(),
+ "consider using an iterator",
vec![(pat.span, "<item>".to_string()), (arg.span, repl)],
);
},
false
}
-fn check_for_loop_reverse_range<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, arg: &'tcx Expr<'_>, expr: &'tcx Expr<'_>) {
- // if this for loop is iterating over a two-sided range...
- if let Some(higher::Range {
- start: Some(start),
- end: Some(end),
- limits,
- }) = higher::range(cx, arg)
- {
- // ...and both sides are compile-time constant integers...
- if let Some((start_idx, _)) = constant(cx, cx.tables, start) {
- if let Some((end_idx, _)) = constant(cx, cx.tables, end) {
- // ...and the start index is greater than the end index,
- // this loop will never run. This is often confusing for developers
- // who think that this will iterate from the larger value to the
- // smaller value.
- let ty = cx.tables.expr_ty(start);
- let (sup, eq) = match (start_idx, end_idx) {
- (Constant::Int(start_idx), Constant::Int(end_idx)) => (
- match ty.kind {
- ty::Int(ity) => sext(cx.tcx, start_idx, ity) > sext(cx.tcx, end_idx, ity),
- ty::Uint(_) => start_idx > end_idx,
- _ => false,
- },
- start_idx == end_idx,
- ),
- _ => (false, false),
- };
-
- if sup {
- let start_snippet = snippet(cx, start.span, "_");
- let end_snippet = snippet(cx, end.span, "_");
- let dots = if limits == ast::RangeLimits::Closed {
- "..="
- } else {
- ".."
- };
-
- span_lint_and_then(
- cx,
- REVERSE_RANGE_LOOP,
- expr.span,
- "this range is empty so this for loop will never run",
- |diag| {
- diag.span_suggestion(
- arg.span,
- "consider using the following if you are attempting to iterate over this \
- range in reverse",
- format!(
- "({end}{dots}{start}).rev()",
- end = end_snippet,
- dots = dots,
- start = start_snippet
- ),
- Applicability::MaybeIncorrect,
- );
- },
- );
- } else if eq && limits != ast::RangeLimits::Closed {
- // if they are equal, it's also problematic - this loop
- // will never run.
- span_lint(
- cx,
- REVERSE_RANGE_LOOP,
- expr.span,
- "this range is empty so this for loop will never run",
- );
- }
- }
- }
- }
-}
-
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);
ITER_NEXT_LOOP,
expr.span,
"you are iterating over `Iterator::next()` which is an Option; this will compile but is \
- probably not what you want",
+ probably not what you want",
);
next_loop_linted = true;
}
if is_type_diagnostic_item(cx, ty, sym!(option_type)) {
span_lint_and_help(
cx,
- FOR_LOOP_OVER_OPTION,
+ FOR_LOOPS_OVER_FALLIBLES,
arg.span,
&format!(
"for loop over `{0}`, which is an `Option`. This is more readably written as an \
- `if let` statement.",
+ `if let` statement.",
snippet(cx, arg.span, "_")
),
None,
} else if is_type_diagnostic_item(cx, ty, sym!(result_type)) {
span_lint_and_help(
cx,
- FOR_LOOP_OVER_RESULT,
+ FOR_LOOPS_OVER_FALLIBLES,
arg.span,
&format!(
"for loop over `{0}`, which is a `Result`. This is more readably written as an \
- `if let` statement.",
+ `if let` statement.",
snippet(cx, arg.span, "_")
),
None,
let map = sugg::Sugg::hir(cx, arg, "map");
multispan_sugg(
diag,
- "use the corresponding method".into(),
+ "use the corresponding method",
vec
projects / rust.git / blobdiff