Rollup of 6 pull requests

compiler/rustc_ast/src/ast.rs

@@ -2953,7 +2953,7 @@ pub enum AssocItemKind {
     /// An associated function.
     Fn(Box<Fn>),
     /// An associated type.
-    TyAlias(Box<TyAlias>),
+    Type(Box<TyAlias>),
     /// A macro expanding to associated items.
     MacCall(P<MacCall>),
 }
@@ -2963,7 +2963,7 @@ impl AssocItemKind {
         match *self {
             Self::Const(defaultness, ..)
             | Self::Fn(box Fn { defaultness, .. })
-            | Self::TyAlias(box TyAlias { defaultness, .. }) => defaultness,
+            | Self::Type(box TyAlias { defaultness, .. }) => defaultness,
             Self::MacCall(..) => Defaultness::Final,
         }
     }
@@ -2974,7 +2974,7 @@ impl From<AssocItemKind> for ItemKind {
         match assoc_item_kind {
             AssocItemKind::Const(a, b, c) => ItemKind::Const(a, b, c),
             AssocItemKind::Fn(fn_kind) => ItemKind::Fn(fn_kind),
-            AssocItemKind::TyAlias(ty_alias_kind) => ItemKind::TyAlias(ty_alias_kind),
+            AssocItemKind::Type(ty_alias_kind) => ItemKind::TyAlias(ty_alias_kind),
             AssocItemKind::MacCall(a) => ItemKind::MacCall(a),
         }
     }
@@ -2987,7 +2987,7 @@ impl TryFrom<ItemKind> for AssocItemKind {
         Ok(match item_kind {
             ItemKind::Const(a, b, c) => AssocItemKind::Const(a, b, c),
             ItemKind::Fn(fn_kind) => AssocItemKind::Fn(fn_kind),
-            ItemKind::TyAlias(ty_alias_kind) => AssocItemKind::TyAlias(ty_alias_kind),
+            ItemKind::TyAlias(ty_kind) => AssocItemKind::Type(ty_kind),
             ItemKind::MacCall(a) => AssocItemKind::MacCall(a),
             _ => return Err(item_kind),
         })

compiler/rustc_ast/src/mut_visit.rs

@@ -1106,7 +1106,7 @@ pub fn noop_flat_map_assoc_item<T: MutVisitor>(
             visit_fn_sig(sig, visitor);
             visit_opt(body, |body| visitor.visit_block(body));
         }
-        AssocItemKind::TyAlias(box TyAlias {
+        AssocItemKind::Type(box TyAlias {
             defaultness,
             generics,
             where_clauses,

compiler/rustc_ast/src/visit.rs

@@ -244,14 +244,12 @@ pub trait Visitor<'ast>: Sized {
 
 #[macro_export]
 macro_rules! walk_list {
-    ($visitor: expr, $method: ident, $list: expr) => {
-        for elem in $list {
-            $visitor.$method(elem)
-        }
-    };
-    ($visitor: expr, $method: ident, $list: expr, $($extra_args: expr),*) => {
-        for elem in $list {
-            $visitor.$method(elem, $($extra_args,)*)
+    ($visitor: expr, $method: ident, $list: expr $(, $($extra_args: expr),* )?) => {
+        {
+            #[cfg_attr(not(bootstrap), allow(for_loops_over_fallibles))]
+            for elem in $list {
+                $visitor.$method(elem $(, $($extra_args,)* )?)
+            }
         }
     }
 }
@@ -685,7 +683,7 @@ pub fn walk_assoc_item<'a, V: Visitor<'a>>(visitor: &mut V, item: &'a AssocItem,
             let kind = FnKind::Fn(FnCtxt::Assoc(ctxt), ident, sig, vis, generics, body.as_deref());
             visitor.visit_fn(kind, span, id);
         }
-        AssocItemKind::TyAlias(box TyAlias { generics, bounds, ty, .. }) => {
+        AssocItemKind::Type(box TyAlias { generics, bounds, ty, .. }) => {
             visitor.visit_generics(generics);
             walk_list!(visitor, visit_param_bound, bounds, BoundKind::Bound);
             walk_list!(visitor, visit_ty, ty);

compiler/rustc_ast_lowering/src/item.rs

@@ -804,7 +804,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 );
                 (generics, hir::TraitItemKind::Fn(sig, hir::TraitFn::Provided(body_id)), true)
             }
-            AssocItemKind::TyAlias(box TyAlias {
+            AssocItemKind::Type(box TyAlias {
                 ref generics,
                 where_clauses,
                 ref bounds,
@@ -850,7 +850,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
     fn lower_trait_item_ref(&mut self, i: &AssocItem) -> hir::TraitItemRef {
         let kind = match &i.kind {
             AssocItemKind::Const(..) => hir::AssocItemKind::Const,
-            AssocItemKind::TyAlias(..) => hir::AssocItemKind::Type,
+            AssocItemKind::Type(..) => hir::AssocItemKind::Type,
             AssocItemKind::Fn(box Fn { sig, .. }) => {
                 hir::AssocItemKind::Fn { has_self: sig.decl.has_self() }
             }
@@ -898,7 +898,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
 
                 (generics, hir::ImplItemKind::Fn(sig, body_id))
             }
-            AssocItemKind::TyAlias(box TyAlias { generics, where_clauses, ty, .. }) => {
+            AssocItemKind::Type(box TyAlias { generics, where_clauses, ty, .. }) => {
                 let mut generics = generics.clone();
                 add_ty_alias_where_clause(&mut generics, *where_clauses, false);
                 self.lower_generics(
@@ -941,7 +941,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
             span: self.lower_span(i.span),
             kind: match &i.kind {
                 AssocItemKind::Const(..) => hir::AssocItemKind::Const,
-                AssocItemKind::TyAlias(..) => hir::AssocItemKind::Type,
+                AssocItemKind::Type(..) => hir::AssocItemKind::Type,
                 AssocItemKind::Fn(box Fn { sig, .. }) => {
                     hir::AssocItemKind::Fn { has_self: sig.decl.has_self() }
                 }

compiler/rustc_ast_passes/src/ast_validation.rs

@@ -1556,7 +1556,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                         });
                     }
                 }
-                AssocItemKind::TyAlias(box TyAlias {
+                AssocItemKind::Type(box TyAlias {
                     generics,
                     where_clauses,
                     where_predicates_split,
@@ -1595,7 +1595,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
         }
 
         match item.kind {
-            AssocItemKind::TyAlias(box TyAlias { ref generics, ref bounds, ref ty, .. })
+            AssocItemKind::Type(box TyAlias { ref generics, ref bounds, ref ty, .. })
                 if ctxt == AssocCtxt::Trait =>
             {
                 self.visit_vis(&item.vis);

compiler/rustc_ast_passes/src/feature_gate.rs

@@ -517,7 +517,7 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
     fn visit_assoc_item(&mut self, i: &'a ast::AssocItem, ctxt: AssocCtxt) {
         let is_fn = match i.kind {
             ast::AssocItemKind::Fn(_) => true,
-            ast::AssocItemKind::TyAlias(box ast::TyAlias { ref ty, .. }) => {
+            ast::AssocItemKind::Type(box ast::TyAlias { ref ty, .. }) => {
                 if let (Some(_), AssocCtxt::Trait) = (ty, ctxt) {
                     gate_feature_post!(
                         &self,

compiler/rustc_ast_pretty/src/pprust/state/item.rs

@@ -516,7 +516,7 @@ impl<'a> State<'a> {
             ast::AssocItemKind::Const(def, ty, body) => {
                 self.print_item_const(ident, None, ty, body.as_deref(), vis, *def);
             }
-            ast::AssocItemKind::TyAlias(box ast::TyAlias {
+            ast::AssocItemKind::Type(box ast::TyAlias {
                 defaultness,
                 generics,
                 where_clauses,

compiler/rustc_builtin_macros/src/deriving/generic/mod.rs

@@ -566,7 +566,7 @@ impl<'a> TraitDef<'a> {
                     tokens: None,
                 },
                 attrs: ast::AttrVec::new(),
-                kind: ast::AssocItemKind::TyAlias(Box::new(ast::TyAlias {
+                kind: ast::AssocItemKind::Type(Box::new(ast::TyAlias {
                     defaultness: ast::Defaultness::Final,
                     generics: Generics::default(),
                     where_clauses: (

compiler/rustc_lint/src/for_loops_over_fallibles.rs

@@ -0,0 +1,183 @@
+use crate::{LateContext, LateLintPass, LintContext};
+
+use hir::{Expr, Pat};
+use rustc_errors::{Applicability, DelayDm};
+use rustc_hir as hir;
+use rustc_infer::traits::TraitEngine;
+use rustc_infer::{infer::TyCtxtInferExt, traits::ObligationCause};
+use rustc_middle::ty::{self, List};
+use rustc_span::{sym, Span};
+use rustc_trait_selection::traits::TraitEngineExt;
+
+declare_lint! {
+    /// The `for_loops_over_fallibles` lint checks for `for` loops over `Option` or `Result` values.
+    ///
+    /// ### Example
+    ///
+    /// ```rust
+    /// let opt = Some(1);
+    /// for x in opt { /* ... */}
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// Both `Option` and `Result` implement `IntoIterator` trait, which allows using them in a `for` loop.
+    /// `for` loop over `Option` or `Result` will iterate either 0 (if the value is `None`/`Err(_)`)
+    /// or 1 time (if the value is `Some(_)`/`Ok(_)`). This is not very useful and is more clearly expressed
+    /// via `if let`.
+    ///
+    /// `for` loop can also be accidentally written with the intention to call a function multiple times,
+    /// while the function returns `Some(_)`, in these cases `while let` loop should be used instead.
+    ///
+    /// The "intended" use of `IntoIterator` implementations for `Option` and `Result` is passing them to
+    /// generic code that expects something implementing `IntoIterator`. For example using `.chain(option)`
+    /// to optionally add a value to an iterator.
+    pub FOR_LOOPS_OVER_FALLIBLES,
+    Warn,
+    "for-looping over an `Option` or a `Result`, which is more clearly expressed as an `if let`"
+}
+
+declare_lint_pass!(ForLoopsOverFallibles => [FOR_LOOPS_OVER_FALLIBLES]);
+
+impl<'tcx> LateLintPass<'tcx> for ForLoopsOverFallibles {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+        let Some((pat, arg)) = extract_for_loop(expr) else { return };
+
+        let ty = cx.typeck_results().expr_ty(arg);
+
+        let &ty::Adt(adt, substs) = ty.kind() else { return };
+
+        let (article, ty, var) = match adt.did() {
+            did if cx.tcx.is_diagnostic_item(sym::Option, did) => ("an", "Option", "Some"),
+            did if cx.tcx.is_diagnostic_item(sym::Result, did) => ("a", "Result", "Ok"),
+            _ => return,
+        };
+
+        let msg = DelayDm(|| {
+            format!(
+                "for loop over {article} `{ty}`. This is more readably written as an `if let` statement",
+            )
+        });
+
+        cx.struct_span_lint(FOR_LOOPS_OVER_FALLIBLES, arg.span, msg, |lint| {
+            if let Some(recv) = extract_iterator_next_call(cx, arg)
+            && let Ok(recv_snip) = cx.sess().source_map().span_to_snippet(recv.span)
+            {
+                lint.span_suggestion(
+                    recv.span.between(arg.span.shrink_to_hi()),
+                    format!("to iterate over `{recv_snip}` remove the call to `next`"),
+                    ".by_ref()",
+                    Applicability::MaybeIncorrect
+                );
+            } else {
+                lint.multipart_suggestion_verbose(
+                    format!("to check pattern in a loop use `while let`"),
+                    vec![
+                        // NB can't use `until` here because `expr.span` and `pat.span` have different syntax contexts
+                        (expr.span.with_hi(pat.span.lo()), format!("while let {var}(")),
+                        (pat.span.between(arg.span), format!(") = ")),
+                    ],
+                    Applicability::MaybeIncorrect
+                );
+            }
+
+            if suggest_question_mark(cx, adt, substs, expr.span) {
+                lint.span_suggestion(
+                    arg.span.shrink_to_hi(),
+                    "consider unwrapping the `Result` with `?` to iterate over its contents",
+                    "?",
+                    Applicability::MaybeIncorrect,
+                );
+            }
+
+            lint.multipart_suggestion_verbose(
+                "consider using `if let` to clear intent",
+                vec![
+                    // NB can't use `until` here because `expr.span` and `pat.span` have different syntax contexts
+                    (expr.span.with_hi(pat.span.lo()), format!("if let {var}(")),
+                    (pat.span.between(arg.span), format!(") = ")),
+                ],
+                Applicability::MaybeIncorrect,
+            )
+        })
+    }
+}
+
+fn extract_for_loop<'tcx>(expr: &Expr<'tcx>) -> Option<(&'tcx Pat<'tcx>, &'tcx Expr<'tcx>)> {
+    if let hir::ExprKind::DropTemps(e) = expr.kind
+    && let hir::ExprKind::Match(iterexpr, [arm], hir::MatchSource::ForLoopDesugar) = e.kind
+    && let hir::ExprKind::Call(_, [arg]) = iterexpr.kind
+    && let hir::ExprKind::Loop(block, ..) = arm.body.kind
+    && let [stmt] = block.stmts
+    && let hir::StmtKind::Expr(e) = stmt.kind
+    && let hir::ExprKind::Match(_, [_, some_arm], _) = e.kind
+    && let hir::PatKind::Struct(_, [field], _) = some_arm.pat.kind
+    {
+        Some((field.pat, arg))
+    } else {
+        None
+    }
+}
+
+fn extract_iterator_next_call<'tcx>(
+    cx: &LateContext<'_>,
+    expr: &Expr<'tcx>,
+) -> Option<&'tcx Expr<'tcx>> {
+    // This won't work for `Iterator::next(iter)`, is this an issue?
+    if let hir::ExprKind::MethodCall(_, recv, _, _) = expr.kind
+    && cx.typeck_results().type_dependent_def_id(expr.hir_id) == cx.tcx.lang_items().next_fn()
+    {
+        Some(recv)
+    } else {
+        return None
+    }
+}
+
+fn suggest_question_mark<'tcx>(
+    cx: &LateContext<'tcx>,
+    adt: ty::AdtDef<'tcx>,
+    substs: &List<ty::GenericArg<'tcx>>,
+    span: Span,
+) -> bool {
+    let Some(body_id) = cx.enclosing_body else { return false };
+    let Some(into_iterator_did) = cx.tcx.get_diagnostic_item(sym::IntoIterator) else { return false };
+
+    if !cx.tcx.is_diagnostic_item(sym::Result, adt.did()) {
+        return false;
+    }
+
+    // Check that the function/closure/constant we are in has a `Result` type.
+    // Otherwise suggesting using `?` may not be a good idea.
+    {
+        let ty = cx.typeck_results().expr_ty(&cx.tcx.hir().body(body_id).value);
+        let ty::Adt(ret_adt, ..) = ty.kind() else { return false };
+        if !cx.tcx.is_diagnostic_item(sym::Result, ret_adt.did()) {
+            return false;
+        }
+    }
+
+    let ty = substs.type_at(0);
+    let infcx = cx.tcx.infer_ctxt().build();
+    let mut fulfill_cx = <dyn TraitEngine<'_>>::new(infcx.tcx);
+
+    let cause = ObligationCause::new(
+        span,
+        body_id.hir_id,
+        rustc_infer::traits::ObligationCauseCode::MiscObligation,
+    );
+    fulfill_cx.register_bound(
+        &infcx,
+        ty::ParamEnv::empty(),
+        // Erase any region vids from the type, which may not be resolved
+        infcx.tcx.erase_regions(ty),
+        into_iterator_did,
+        cause,
+    );
+
+    // Select all, including ambiguous predicates
+    let errors = fulfill_cx.select_all_or_error(&infcx);
+
+    errors.is_empty()
+}

compiler/rustc_lint/src/lib.rs

@@ -52,6 +52,7 @@ mod early;
 mod enum_intrinsics_non_enums;
 mod errors;
 mod expect;
+mod for_loops_over_fallibles;
 pub mod hidden_unicode_codepoints;
 mod internal;
 mod late;
@@ -86,6 +87,7 @@ use rustc_span::Span;
 use array_into_iter::ArrayIntoIter;
 use builtin::*;
 use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums;
+use for_loops_over_fallibles::*;
 use hidden_unicode_codepoints::*;
 use internal::*;
 use let_underscore::*;
@@ -188,6 +190,7 @@ macro_rules! late_lint_mod_passes {
         $macro!(
             $args,
             [
+                ForLoopsOverFallibles: ForLoopsOverFallibles,
                 HardwiredLints: HardwiredLints,
                 ImproperCTypesDeclarations: ImproperCTypesDeclarations,
                 ImproperCTypesDefinitions: ImproperCTypesDefinitions,

compiler/rustc_lint/src/nonstandard_style.rs

@@ -187,7 +187,7 @@ impl EarlyLintPass for NonCamelCaseTypes {
     }
 
     fn check_trait_item(&mut self, cx: &EarlyContext<'_>, it: &ast::AssocItem) {
-        if let ast::AssocItemKind::TyAlias(..) = it.kind {
+        if let ast::AssocItemKind::Type(..) = it.kind {
             self.check_case(cx, "associated type", &it.ident);
         }
     }