Fix incorrect suggestion for undeclared hrtb lifetimes in where clauses.

fixes #122714
rust-lang · bors · May 21, 2024 · May 9, 2024 · Mar 27, 2024 · May 19, 2024
commit 4ebbb5f048797598e22916532b94517285492f03
diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs
@@ -17,6 +17,7 @@ use rustc_ast::{
 };
 use rustc_ast_pretty::pprust::where_bound_predicate_to_string;
 use rustc_data_structures::fx::FxHashSet;
+use rustc_data_structures::fx::FxIndexSet;
 use rustc_errors::{
     codes::*, pluralize, struct_span_code_err, Applicability, Diag, ErrorGuaranteed, MultiSpan,
     SuggestionStyle,
@@ -31,7 +32,7 @@ use rustc_span::edit_distance::find_best_match_for_name;
 use rustc_span::edition::Edition;
 use rustc_span::hygiene::MacroKind;
 use rustc_span::symbol::{kw, sym, Ident, Symbol};
-use rustc_span::Span;
+use rustc_span::{Span, DUMMY_SP};
 
 use rustc_middle::ty;
 
@@ -2714,8 +2715,17 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
         self.suggest_introducing_lifetime(
             &mut err,
             Some(lifetime_ref.ident.name.as_str()),
-            |err, _, span, message, suggestion| {
-                err.span_suggestion(span, message, suggestion, Applicability::MaybeIncorrect);
+            |err, _, span, message, suggestion, span_suggs| {
+                err.multipart_suggestion_with_style(
+                    message,
+                    std::iter::once((span, suggestion)).chain(span_suggs.clone()).collect(),
+                    Applicability::MaybeIncorrect,
+                    if span_suggs.is_empty() {
+                        SuggestionStyle::ShowCode
+                    } else {
+                        SuggestionStyle::ShowAlways
+                    },
+                );
                 true
             },
         );
@@ -2726,13 +2736,20 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
         &self,
         err: &mut Diag<'_>,
         name: Option<&str>,
-        suggest: impl Fn(&mut Diag<'_>, bool, Span, Cow<'static, str>, String) -> bool,
+        suggest: impl Fn(
+            &mut Diag<'_>,
+            bool,
+            Span,
+            Cow<'static, str>,
+            String,
+            Vec<(Span, String)>,
+        ) -> bool,
     ) {
         let mut suggest_note = true;
         for rib in self.lifetime_ribs.iter().rev() {
             let mut should_continue = true;
             match rib.kind {
-                LifetimeRibKind::Generics { binder: _, span, kind } => {
+                LifetimeRibKind::Generics { binder, span, kind } => {
                     // Avoid suggesting placing lifetime parameters on constant items unless the relevant
                     // feature is enabled. Suggest the parent item as a possible location if applicable.
                     if let LifetimeBinderKind::ConstItem = kind
@@ -2761,11 +2778,53 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
                             | LifetimeBinderKind::PolyTrait
                             | LifetimeBinderKind::WhereBound
                     );
+
+                    let mut rm_inner_binders: FxIndexSet<Span> = Default::default();
                     let (span, sugg) = if span.is_empty() {
+                        let mut binder_idents: FxIndexSet<Ident> = Default::default();
+                        binder_idents.insert(Ident::from_str(name.unwrap_or("'a")));
+
+                        // We need to special case binders in the following situation:
+                        // Change `T: for<'a> Trait<T> + 'b` to `for<'a, 'b> T: Trait<T> + 'b`
+                        // T: for<'a> Trait<T> + 'b
+                        //    ^^^^^^^  remove existing inner binder `for<'a>`
+                        // for<'a, 'b> T: Trait<T> + 'b
+                        // ^^^^^^^^^^^  suggest outer binder `for<'a, 'b>`
+                        if let LifetimeBinderKind::WhereBound = kind
+                            && let Some(ast::WherePredicate::BoundPredicate(
+                                ast::WhereBoundPredicate { bounded_ty, bounds, .. },
+                            )) = self.diag_metadata.current_where_predicate
+                            && bounded_ty.id == binder
+                        {
+                            for bound in bounds {
+                                if let ast::GenericBound::Trait(poly_trait_ref, _) = bound
+                                    && let span = poly_trait_ref
+                                        .span
+                                        .with_hi(poly_trait_ref.trait_ref.path.span.lo())
+                                    && !span.is_empty()
+                                {
+                                    rm_inner_binders.insert(span);
+                                    poly_trait_ref.bound_generic_params.iter().for_each(|v| {
+                                        binder_idents.insert(v.ident);
+                                    });
+                                }
+                            }
+                        }
+
+                        let binders_sugg = binder_idents.into_iter().enumerate().fold(
+                            "".to_string(),
+                            |mut binders, (i, x)| {
+                                if i != 0 {
+                                    binders += ", ";
+                                }
+                                binders += x.as_str();
+                                binders
+                            },
+                        );
                         let sugg = format!(
                             "{}<{}>{}",
                             if higher_ranked { "for" } else { "" },
-                            name.unwrap_or("'a"),
+                            binders_sugg,
                             if higher_ranked { " " } else { "" },
                         );
                         (span, sugg)
@@ -2780,24 +2839,39 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
                         let sugg = format!("{}, ", name.unwrap_or("'a"));
                         (span, sugg)
                     };
+
                     if higher_ranked {
                         let message = Cow::from(format!(
                             "consider making the {} lifetime-generic with a new `{}` lifetime",
                             kind.descr(),
                             name.unwrap_or("'a"),
                         ));
-                        should_continue = suggest(err, true, span, message, sugg);
+                        should_continue = suggest(
+                            err,
+                            true,
+                            span,
+                            message,
+                            sugg,
+                            if !rm_inner_binders.is_empty() {
+                                rm_inner_binders
+                                    .into_iter()
+                                    .map(|v| (v, "".to_string()))
+                                    .collect::<Vec<_>>()
+                            } else {
+                                vec![]
+                            },
+                        );
                         err.note_once(
                             "for more information on higher-ranked polymorphism, visit \
                              https://doc.rust-lang.org/nomicon/hrtb.html",
                         );
                     } else if let Some(name) = name {
                         let message =
                             Cow::from(format!("consider introducing lifetime `{name}` here"));
-                        should_continue = suggest(err, false, span, message, sugg);
+                        should_continue = suggest(err, false, span, message, sugg, vec![]);
                     } else {
                         let message = Cow::from("consider introducing a named lifetime parameter");
-                        should_continue = suggest(err, false, span, message, sugg);
+                        should_continue = suggest(err, false, span, message, sugg, vec![]);
                     }
                 }
                 LifetimeRibKind::Item | LifetimeRibKind::ConstParamTy => break,
@@ -3033,11 +3107,11 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
                 self.suggest_introducing_lifetime(
                     err,
                     None,
-                    |err, higher_ranked, span, message, intro_sugg| {
+                    |err, higher_ranked, span, message, intro_sugg, _| {
                         err.multipart_suggestion_verbose(
                             message,
                             std::iter::once((span, intro_sugg))
-                                .chain(spans_suggs.iter().cloned())
+                                .chain(spans_suggs.clone())
                                 .collect(),
                             Applicability::MaybeIncorrect,
                         );
@@ -3161,11 +3235,11 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
                             self.suggest_introducing_lifetime(
                                 err,
                                 None,
-                                |err, higher_ranked, span, message, intro_sugg| {
+                                |err, higher_ranked, span, message, intro_sugg, _| {
                                     err.multipart_suggestion_verbose(
                                         message,
                                         std::iter::once((span, intro_sugg))
-                                            .chain(spans_suggs.iter().cloned())
+                                            .chain(spans_suggs.clone())
                                             .collect(),
                                         Applicability::MaybeIncorrect,
                                     );
@@ -3309,7 +3383,6 @@ fn mk_where_bound_predicate(
     poly_trait_ref: &ast::PolyTraitRef,
     ty: &Ty,
 ) -> Option<ast::WhereBoundPredicate> {
-    use rustc_span::DUMMY_SP;
     let modified_segments = {
         let mut segments = path.segments.clone();
         let [preceding @ .., second_last, last] = segments.as_mut_slice() else {

diff --git a/tests/ui/generics/generic-higher-ranked-lifetime-issue-122714.rs b/tests/ui/generics/generic-higher-ranked-lifetime-issue-122714.rs
@@ -0,0 +1,28 @@
+#![allow(dead_code)]
+
+trait Trait1<T>
+  where T: for<'a> Trait1<T> + 'b { } //~ ERROR use of undeclared lifetime name `'b`
+
+trait Trait2<T>
+where
+    T: B<'b> + for<'a> A<'a>, //~ ERROR use of undeclared lifetime name `'b`
+{
+}
+
+trait Trait3<T>
+where
+    T: B<'b> + for<'a> A<'a> + 'c {}
+    //~^ ERROR use of undeclared lifetime name `'b`
+    //~| ERROR use of undeclared lifetime name `'c`
+
+trait Trait4<T>
+where
+    T: for<'a> A<'a> + 'x + for<'b> B<'b>, //~ ERROR use of undeclared lifetime name `'x`
+{
+}
+
+trait A<'a> {}
+trait B<'a> {}
+
+
+fn main() {}
diff --git a/tests/ui/generics/generic-higher-ranked-lifetime-issue-122714.stderr b/tests/ui/generics/generic-higher-ranked-lifetime-issue-122714.stderr
@@ -0,0 +1,92 @@
+error[E0261]: use of undeclared lifetime name `'b`
+  --> $DIR/generic-higher-ranked-lifetime-issue-122714.rs:4:32
+   |
+LL |   where T: for<'a> Trait1<T> + 'b { }
+   |                                ^^ undeclared lifetime
+   |
+   = note: for more information on higher-ranked polymorphism, visit https://doc.rust-lang.org/nomicon/hrtb.html
+help: consider making the bound lifetime-generic with a new `'b` lifetime
+   |
+LL -   where T: for<'a> Trait1<T> + 'b { }
+LL +   where for<'b, 'a> T: Trait1<T> + 'b { }
+   |
+help: consider introducing lifetime `'b` here
+   |
+LL | trait Trait1<'b, T>
+   |              +++
+
+error[E0261]: use of undeclared lifetime name `'b`
+  --> $DIR/generic-higher-ranked-lifetime-issue-122714.rs:8:10
+   |
+LL |     T: B<'b> + for<'a> A<'a>,
+   |          ^^ undeclared lifetime
+   |
+help: consider making the bound lifetime-generic with a new `'b` lifetime
+   |
+LL |     T: for<'b> B<'b> + for<'a> A<'a>,
+   |        +++++++
+help: consider making the bound lifetime-generic with a new `'b` lifetime
+   |
+LL -     T: B<'b> + for<'a> A<'a>,
+LL +     for<'b, 'a> T: B<'b> + A<'a>,
+   |
+help: consider introducing lifetime `'b` here
+   |
+LL | trait Trait2<'b, T>
+   |              +++
+
+error[E0261]: use of undeclared lifetime name `'b`
+  --> $DIR/generic-higher-ranked-lifetime-issue-122714.rs:14:10
+   |
+LL |     T: B<'b> + for<'a> A<'a> + 'c {}
+   |          ^^ undeclared lifetime
+   |
+help: consider making the bound lifetime-generic with a new `'b` lifetime
+   |
+LL |     T: for<'b> B<'b> + for<'a> A<'a> + 'c {}
+   |        +++++++
+help: consider making the bound lifetime-generic with a new `'b` lifetime
+   |
+LL -     T: B<'b> + for<'a> A<'a> + 'c {}
+LL +     for<'b, 'a> T: B<'b> + A<'a> + 'c {}
+   |
+help: consider introducing lifetime `'b` here
+   |
+LL | trait Trait3<'b, T>
+   |              +++
+
+error[E0261]: use of undeclared lifetime name `'c`
+  --> $DIR/generic-higher-ranked-lifetime-issue-122714.rs:14:32
+   |
+LL |     T: B<'b> + for<'a> A<'a> + 'c {}
+   |                                ^^ undeclared lifetime
+   |
+help: consider making the bound lifetime-generic with a new `'c` lifetime
+   |
+LL -     T: B<'b> + for<'a> A<'a> + 'c {}
+LL +     for<'c, 'a> T: B<'b> + A<'a> + 'c {}
+   |
+help: consider introducing lifetime `'c` here
+   |
+LL | trait Trait3<'c, T>
+   |              +++
+
+error[E0261]: use of undeclared lifetime name `'x`
+  --> $DIR/generic-higher-ranked-lifetime-issue-122714.rs:20:24
+   |
+LL |     T: for<'a> A<'a> + 'x + for<'b> B<'b>,
+   |                        ^^ undeclared lifetime
+   |
+help: consider making the bound lifetime-generic with a new `'x` lifetime
+   |
+LL -     T: for<'a> A<'a> + 'x + for<'b> B<'b>,
+LL +     for<'x, 'a, 'b> T: A<'a> + 'x + B<'b>,
+   |
+help: consider introducing lifetime `'x` here
+   |
+LL | trait Trait4<'x, T>
+   |              +++
+
+error: aborting due to 5 previous errors
+
+For more information about this error, try `rustc --explain E0261`.