compiler/rustc_lint/src/for_loops_over_fallibles.rs

   1 use crate::{LateContext, LateLintPass, LintContext};
   2
   3 use hir::{Expr, Pat};
   4 use rustc_errors::{Applicability, DelayDm};
   5 use rustc_hir as hir;
   6 use rustc_infer::traits::TraitEngine;
   7 use rustc_infer::{infer::TyCtxtInferExt, traits::ObligationCause};
   8 use rustc_middle::ty::{self, List};
   9 use rustc_span::{sym, Span};
  10 use rustc_trait_selection::traits::TraitEngineExt;
  11
  12 declare_lint! {
  13     /// The `for_loops_over_fallibles` lint checks for `for` loops over `Option` or `Result` values.
  14     ///
  15     /// ### Example
  16     ///
  17     /// ```rust
  18     /// let opt = Some(1);
  19     /// for x in opt { /* ... */}
  20     /// ```
  21     ///
  22     /// {{produces}}
  23     ///
  24     /// ### Explanation
  25     ///
  26     /// Both `Option` and `Result` implement `IntoIterator` trait, which allows using them in a `for` loop.
  27     /// `for` loop over `Option` or `Result` will iterate either 0 (if the value is `None`/`Err(_)`)
  28     /// or 1 time (if the value is `Some(_)`/`Ok(_)`). This is not very useful and is more clearly expressed
  29     /// via `if let`.
  30     ///
  31     /// `for` loop can also be accidentally written with the intention to call a function multiple times,
  32     /// while the function returns `Some(_)`, in these cases `while let` loop should be used instead.
  33     ///
  34     /// The "intended" use of `IntoIterator` implementations for `Option` and `Result` is passing them to
  35     /// generic code that expects something implementing `IntoIterator`. For example using `.chain(option)`
  36     /// to optionally add a value to an iterator.
  37     pub FOR_LOOPS_OVER_FALLIBLES,
  38     Warn,
  39     "for-looping over an `Option` or a `Result`, which is more clearly expressed as an `if let`"
  40 }
  41
  42 declare_lint_pass!(ForLoopsOverFallibles => [FOR_LOOPS_OVER_FALLIBLES]);
  43
  44 impl<'tcx> LateLintPass<'tcx> for ForLoopsOverFallibles {
  45     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
  46         let Some((pat, arg)) = extract_for_loop(expr) else { return };
  47
  48         let ty = cx.typeck_results().expr_ty(arg);
  49
  50         let &ty::Adt(adt, substs) = ty.kind() else { return };
  51
  52         let (article, ty, var) = match adt.did() {
  53             did if cx.tcx.is_diagnostic_item(sym::Option, did) => ("an", "Option", "Some"),
  54             did if cx.tcx.is_diagnostic_item(sym::Result, did) => ("a", "Result", "Ok"),
  55             _ => return,
  56         };
  57
  58         let msg = DelayDm(|| {
  59             format!(
  60                 "for loop over {article} `{ty}`. This is more readably written as an `if let` statement",
  61             )
  62         });
  63
  64         cx.struct_span_lint(FOR_LOOPS_OVER_FALLIBLES, arg.span, msg, |lint| {
  65             if let Some(recv) = extract_iterator_next_call(cx, arg)
  66             && let Ok(recv_snip) = cx.sess().source_map().span_to_snippet(recv.span)
  67             {
  68                 lint.span_suggestion(
  69                     recv.span.between(arg.span.shrink_to_hi()),
  70                     format!("to iterate over `{recv_snip}` remove the call to `next`"),
  71                     ".by_ref()",
  72                     Applicability::MaybeIncorrect
  73                 );
  74             } else {
  75                 lint.multipart_suggestion_verbose(
  76                     format!("to check pattern in a loop use `while let`"),
  77                     vec![
  78                         // NB can't use `until` here because `expr.span` and `pat.span` have different syntax contexts
  79                         (expr.span.with_hi(pat.span.lo()), format!("while let {var}(")),
  80                         (pat.span.between(arg.span), format!(") = ")),
  81                     ],
  82                     Applicability::MaybeIncorrect
  83                 );
  84             }
  85
  86             if suggest_question_mark(cx, adt, substs, expr.span) {
  87                 lint.span_suggestion(
  88                     arg.span.shrink_to_hi(),
  89                     "consider unwrapping the `Result` with `?` to iterate over its contents",
  90                     "?",
  91                     Applicability::MaybeIncorrect,
  92                 );
  93             }
  94
  95             lint.multipart_suggestion_verbose(
  96                 "consider using `if let` to clear intent",
  97                 vec![
  98                     // NB can't use `until` here because `expr.span` and `pat.span` have different syntax contexts
  99                     (expr.span.with_hi(pat.span.lo()), format!("if let {var}(")),
 100                     (pat.span.between(arg.span), format!(") = ")),
 101                 ],
 102                 Applicability::MaybeIncorrect,
 103             )
 104         })
 105     }
 106 }
 107
 108 fn extract_for_loop<'tcx>(expr: &Expr<'tcx>) -> Option<(&'tcx Pat<'tcx>, &'tcx Expr<'tcx>)> {
 109     if let hir::ExprKind::DropTemps(e) = expr.kind
 110     && let hir::ExprKind::Match(iterexpr, [arm], hir::MatchSource::ForLoopDesugar) = e.kind
 111     && let hir::ExprKind::Call(_, [arg]) = iterexpr.kind
 112     && let hir::ExprKind::Loop(block, ..) = arm.body.kind
 113     && let [stmt] = block.stmts
 114     && let hir::StmtKind::Expr(e) = stmt.kind
 115     && let hir::ExprKind::Match(_, [_, some_arm], _) = e.kind
 116     && let hir::PatKind::Struct(_, [field], _) = some_arm.pat.kind
 117     {
 118         Some((field.pat, arg))
 119     } else {
 120         None
 121     }
 122 }
 123
 124 fn extract_iterator_next_call<'tcx>(
 125     cx: &LateContext<'_>,
 126     expr: &Expr<'tcx>,
 127 ) -> Option<&'tcx Expr<'tcx>> {
 128     // This won't work for `Iterator::next(iter)`, is this an issue?
 129     if let hir::ExprKind::MethodCall(_, recv, _, _) = expr.kind
 130     && cx.typeck_results().type_dependent_def_id(expr.hir_id) == cx.tcx.lang_items().next_fn()
 131     {
 132         Some(recv)
 133     } else {
 134         return None
 135     }
 136 }
 137
 138 fn suggest_question_mark<'tcx>(
 139     cx: &LateContext<'tcx>,
 140     adt: ty::AdtDef<'tcx>,
 141     substs: &List<ty::GenericArg<'tcx>>,
 142     span: Span,
 143 ) -> bool {
 144     let Some(body_id) = cx.enclosing_body else { return false };
 145     let Some(into_iterator_did) = cx.tcx.get_diagnostic_item(sym::IntoIterator) else { return false };
 146
 147     if !cx.tcx.is_diagnostic_item(sym::Result, adt.did()) {
 148         return false;
 149     }
 150
 151     // Check that the function/closure/constant we are in has a `Result` type.
 152     // Otherwise suggesting using `?` may not be a good idea.
 153     {
 154         let ty = cx.typeck_results().expr_ty(&cx.tcx.hir().body(body_id).value);
 155         let ty::Adt(ret_adt, ..) = ty.kind() else { return false };
 156         if !cx.tcx.is_diagnostic_item(sym::Result, ret_adt.did()) {
 157             return false;
 158         }
 159     }
 160
 161     let ty = substs.type_at(0);
 162     let infcx = cx.tcx.infer_ctxt().build();
 163     let mut fulfill_cx = <dyn TraitEngine<'_>>::new(infcx.tcx);
 164
 165     let cause = ObligationCause::new(
 166         span,
 167         body_id.hir_id,
 168         rustc_infer::traits::ObligationCauseCode::MiscObligation,
 169     );
 170     fulfill_cx.register_bound(
 171         &infcx,
 172         ty::ParamEnv::empty(),
 173         // Erase any region vids from the type, which may not be resolved
 174         infcx.tcx.erase_regions(ty),
 175         into_iterator_did,
 176         cause,
 177     );
 178
 179     // Select all, including ambiguous predicates
 180     let errors = fulfill_cx.select_all_or_error(&infcx);
 181
 182     errors.is_empty()
 183 }