Rollup merge of #71235 - estebank:lt-sugg-2, r=ecstatic-morse

Tweak `'static` suggestion code

Fix #71196.

src/librustc_ast_lowering/path.rs

@@ -273,7 +273,10 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             .next();
         if !generic_args.parenthesized && !has_lifetimes {
             generic_args.args = self
-                .elided_path_lifetimes(path_span, expected_lifetimes)
+                .elided_path_lifetimes(
+                    first_generic_span.map(|s| s.shrink_to_lo()).unwrap_or(segment.ident.span),
+                    expected_lifetimes,
+                )
                 .map(GenericArg::Lifetime)
                 .chain(generic_args.args.into_iter())
                 .collect();

src/librustc_resolve/late/diagnostics.rs

@@ -1034,101 +1034,125 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
         lifetime_names: &FxHashSet<ast::Ident>,
         params: &[ElisionFailureInfo],
     ) {
-        if count > 1 {
-            err.span_label(span, format!("expected {} lifetime parameters", count));
-        } else {
-            let snippet = self.tcx.sess.source_map().span_to_snippet(span).ok();
-            let suggest_existing = |err: &mut DiagnosticBuilder<'_>, sugg| {
-                err.span_suggestion(
-                    span,
-                    "consider using the named lifetime",
-                    sugg,
-                    Applicability::MaybeIncorrect,
-                );
-            };
-            let suggest_new = |err: &mut DiagnosticBuilder<'_>, sugg: &str| {
-                err.span_label(span, "expected named lifetime parameter");
+        let snippet = self.tcx.sess.source_map().span_to_snippet(span).ok();
 
-                for missing in self.missing_named_lifetime_spots.iter().rev() {
-                    let mut introduce_suggestion = vec![];
-                    let msg;
-                    let should_break;
-                    introduce_suggestion.push(match missing {
-                        MissingLifetimeSpot::Generics(generics) => {
-                            msg = "consider introducing a named lifetime parameter".to_string();
-                            should_break = true;
-                            if let Some(param) = generics.params.iter().find(|p| match p.kind {
-                                hir::GenericParamKind::Type {
-                                    synthetic: Some(hir::SyntheticTyParamKind::ImplTrait),
-                                    ..
-                                } => false,
-                                _ => true,
-                            }) {
-                                (param.span.shrink_to_lo(), "'a, ".to_string())
-                            } else {
-                                (generics.span, "<'a>".to_string())
-                            }
-                        }
-                        MissingLifetimeSpot::HigherRanked { span, span_type } => {
-                            msg = format!(
-                                "consider making the {} lifetime-generic with a new `'a` lifetime",
-                                span_type.descr(),
-                            );
-                            should_break = false;
-                            err.note(
-                                "for more information on higher-ranked polymorphism, visit \
-                             https://doc.rust-lang.org/nomicon/hrtb.html",
-                            );
-                            (*span, span_type.suggestion("'a"))
-                        }
-                    });
-                    for param in params {
-                        if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(param.span)
-                        {
-                            if snippet.starts_with('&') && !snippet.starts_with("&'") {
-                                introduce_suggestion
-                                    .push((param.span, format!("&'a {}", &snippet[1..])));
-                            } else if snippet.starts_with("&'_ ") {
-                                introduce_suggestion
-                                    .push((param.span, format!("&'a {}", &snippet[4..])));
-                            }
+        err.span_label(
+            span,
+            &format!(
+                "expected {} lifetime parameter{}",
+                if count == 1 { "named".to_string() } else { count.to_string() },
+                pluralize!(count)
+            ),
+        );
+
+        let suggest_existing = |err: &mut DiagnosticBuilder<'_>, sugg| {
+            err.span_suggestion_verbose(
+                span,
+                &format!("consider using the `{}` lifetime", lifetime_names.iter().next().unwrap()),
+                sugg,
+                Applicability::MaybeIncorrect,
+            );
+        };
+        let suggest_new = |err: &mut DiagnosticBuilder<'_>, sugg: &str| {
+            for missing in self.missing_named_lifetime_spots.iter().rev() {
+                let mut introduce_suggestion = vec![];
+                let msg;
+                let should_break;
+                introduce_suggestion.push(match missing {
+                    MissingLifetimeSpot::Generics(generics) => {
+                        msg = "consider introducing a named lifetime parameter".to_string();
+                        should_break = true;
+                        if let Some(param) = generics.params.iter().find(|p| match p.kind {
+                            hir::GenericParamKind::Type {
+                                synthetic: Some(hir::SyntheticTyParamKind::ImplTrait),
+                                ..
+                            } => false,
+                            _ => true,
+                        }) {
+                            (param.span.shrink_to_lo(), "'a, ".to_string())
+                        } else {
+                            (generics.span, "<'a>".to_string())
                         }
                     }
-                    introduce_suggestion.push((span, sugg.to_string()));
-                    err.multipart_suggestion(
-                        &msg,
-                        introduce_suggestion,
-                        Applicability::MaybeIncorrect,
-                    );
-                    if should_break {
-                        break;
+                    MissingLifetimeSpot::HigherRanked { span, span_type } => {
+                        msg = format!(
+                            "consider making the {} lifetime-generic with a new `'a` lifetime",
+                            span_type.descr(),
+                        );
+                        should_break = false;
+                        err.note(
+                            "for more information on higher-ranked polymorphism, visit \
+                            https://doc.rust-lang.org/nomicon/hrtb.html",
+                        );
+                        (*span, span_type.suggestion("'a"))
+                    }
+                });
+                for param in params {
+                    if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(param.span) {
+                        if snippet.starts_with('&') && !snippet.starts_with("&'") {
+                            introduce_suggestion
+                                .push((param.span, format!("&'a {}", &snippet[1..])));
+                        } else if snippet.starts_with("&'_ ") {
+                            introduce_suggestion
+                                .push((param.span, format!("&'a {}", &snippet[4..])));
+                        }
                     }
                 }
-            };
-
-            match (lifetime_names.len(), lifetime_names.iter().next(), snippet.as_deref()) {
-                (1, Some(name), Some("&")) => {
-                    suggest_existing(err, format!("&{} ", name));
-                }
-                (1, Some(name), Some("'_")) => {
-                    suggest_existing(err, name.to_string());
-                }
-                (1, Some(name), Some(snippet)) if !snippet.ends_with('>') => {
-                    suggest_existing(err, format!("{}<{}>", snippet, name));
-                }
-                (0, _, Some("&")) => {
-                    suggest_new(err, "&'a ");
-                }
-                (0, _, Some("'_")) => {
-                    suggest_new(err, "'a");
-                }
-                (0, _, Some(snippet)) if !snippet.ends_with('>') => {
-                    suggest_new(err, &format!("{}<'a>", snippet));
+                introduce_suggestion.push((span, sugg.to_string()));
+                err.multipart_suggestion(&msg, introduce_suggestion, Applicability::MaybeIncorrect);
+                if should_break {
+                    break;
                 }
-                _ => {
-                    err.span_label(span, "expected lifetime parameter");
+            }
+        };
+
+        match (lifetime_names.len(), lifetime_names.iter().next(), snippet.as_deref()) {
+            (1, Some(name), Some("&")) => {
+                suggest_existing(err, format!("&{} ", name));
+            }
+            (1, Some(name), Some("'_")) => {
+                suggest_existing(err, name.to_string());
+            }
+            (1, Some(name), Some("")) => {
+                suggest_existing(err, format!("{}, ", name).repeat(count));
+            }
+            (1, Some(name), Some(snippet)) if !snippet.ends_with('>') => {
+                suggest_existing(
+                    err,
+                    format!(
+                        "{}<{}>",
+                        snippet,
+                        std::iter::repeat(name.to_string())
+                            .take(count)
+                            .collect::<Vec<_>>()
+                            .join(", ")
+                    ),
+                );
+            }
+            (0, _, Some("&")) if count == 1 => {
+                suggest_new(err, "&'a ");
+            }
+            (0, _, Some("'_")) if count == 1 => {
+                suggest_new(err, "'a");
+            }
+            (0, _, Some(snippet)) if !snippet.ends_with('>') && count == 1 => {
+                suggest_new(err, &format!("{}<'a>", snippet));
+            }
+            (n, ..) if n > 1 => {
+                let spans: Vec<Span> = lifetime_names.iter().map(|lt| lt.span).collect();
+                err.span_note(spans, "these named lifetimes are available to use");
+                if Some("") == snippet.as_deref() {
+                    // This happens when we have `Foo<T>` where we point at the space before `T`,
+                    // but this can be confusing so we give a suggestion with placeholders.
+                    err.span_suggestion_verbose(
+                        span,
+                        "consider using one of the available lifetimes here",
+                        "'lifetime, ".repeat(count),
+                        Applicability::HasPlaceholders,
+                    );
                 }
             }
+            _ => {}
         }
     }
 }

src/librustc_resolve/late/lifetimes.rs

@@ -2393,52 +2393,28 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
         };
 
         let mut err = self.report_missing_lifetime_specifiers(span, lifetime_refs.len());
-        let mut add_label = true;
 
         if let Some(params) = error {
-            if lifetime_refs.len() == 1 {
-                add_label = add_label && self.report_elision_failure(&mut err, params, span);
+            // If there's no lifetime available, suggest `'static`.
+            if self.report_elision_failure(&mut err, params) && lifetime_names.is_empty() {
+                lifetime_names.insert(ast::Ident::from_str("'static"));
             }
         }
-        if add_label {
-            self.add_missing_lifetime_specifiers_label(
-                &mut err,
-                span,
-                lifetime_refs.len(),
-                &lifetime_names,
-                error.map(|p| &p[..]).unwrap_or(&[]),
-            );
-        }
-
+        self.add_missing_lifetime_specifiers_label(
+            &mut err,
+            span,
+            lifetime_refs.len(),
+            &lifetime_names,
+            error.map(|p| &p[..]).unwrap_or(&[]),
+        );
         err.emit();
     }
 
-    fn suggest_lifetime(&self, db: &mut DiagnosticBuilder<'_>, span: Span, msg: &str) -> bool {
-        match self.tcx.sess.source_map().span_to_snippet(span) {
-            Ok(ref snippet) => {
-                let (sugg, applicability) = if snippet == "&" {
-                    ("&'static ".to_owned(), Applicability::MachineApplicable)
-                } else if snippet == "'_" {
-                    ("'static".to_owned(), Applicability::MachineApplicable)
-                } else {
-                    (format!("{} + 'static", snippet), Applicability::MaybeIncorrect)
-                };
-                db.span_suggestion(span, msg, sugg, applicability);
-                false
-            }
-            Err(_) => {
-                db.help(msg);
-                true
-            }
-        }
-    }
-
     fn report_elision_failure(
         &mut self,
         db: &mut DiagnosticBuilder<'_>,
         params: &[ElisionFailureInfo],
-        span: Span,
-    ) -> bool {
+    ) -> bool /* add `'static` lifetime to lifetime list */ {
         let mut m = String::new();
         let len = params.len();
 
@@ -2487,29 +2463,28 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
                 "this function's return type contains a borrowed value, \
                  but there is no value for it to be borrowed from",
             );
-            self.suggest_lifetime(db, span, "consider giving it a 'static lifetime")
+            true
         } else if elided_len == 0 {
             db.help(
                 "this function's return type contains a borrowed value with \
                  an elided lifetime, but the lifetime cannot be derived from \
                  the arguments",
             );
-            let msg = "consider giving it an explicit bounded or 'static lifetime";
-            self.suggest_lifetime(db, span, msg)
+            true
         } else if elided_len == 1 {
             db.help(&format!(
                 "this function's return type contains a borrowed value, \
                  but the signature does not say which {} it is borrowed from",
                 m
             ));
-            true
+            false
         } else {
             db.help(&format!(
                 "this function's return type contains a borrowed value, \
                  but the signature does not say whether it is borrowed from {}",
                 m
             ));
-            true
+            false
         }
     }
 

src/librustc_span/lib.rs

@@ -657,7 +657,8 @@ impl MultiSpan {
         MultiSpan { primary_spans: vec![primary_span], span_labels: vec![] }
     }
 
-    pub fn from_spans(vec: Vec<Span>) -> MultiSpan {
+    pub fn from_spans(mut vec: Vec<Span>) -> MultiSpan {
+        vec.sort();
         MultiSpan { primary_spans: vec, span_labels: vec![] }
     }
 

src/librustc_typeck/astconv.rs

@@ -1017,18 +1017,8 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
 
         self.prohibit_generics(trait_ref.path.segments.split_last().unwrap().1);
 
-        let path_span = if let [segment] = &trait_ref.path.segments[..] {
-            // FIXME: `trait_ref.path.span` can point to a full path with multiple
-            // segments, even though `trait_ref.path.segments` is of length `1`. Work
-            // around that bug here, even though it should be fixed elsewhere.
-            // This would otherwise cause an invalid suggestion. For an example, look at
-            // `src/test/ui/issues/issue-28344.rs`.
-            segment.ident.span
-        } else {
-            trait_ref.path.span
-        };
         let (substs, assoc_bindings, arg_count_correct) = self.create_substs_for_ast_trait_ref(
-            path_span,
+            trait_ref.path.span,
             trait_def_id,
             self_ty,
             trait_ref.path.segments.last().unwrap(),
@@ -1729,6 +1719,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
                     self.ast_region_to_region(lifetime, None)
                 } else {
                     self.re_infer(None, span).unwrap_or_else(|| {
+                        // FIXME: these can be redundant with E0106, but not always.
                         struct_span_err!(
                             tcx.sess,
                             span,

src/test/ui/associated-types/bound-lifetime-in-binding-only.elision.stderr

@@ -2,9 +2,13 @@ error[E0106]: missing lifetime specifier
   --> $DIR/bound-lifetime-in-binding-only.rs:52:23
    |
 LL | fn elision<T: Fn() -> &i32>() {
-   |                       ^ help: consider giving it a 'static lifetime: `&'static`
+   |                       ^ expected named lifetime parameter
    |
    = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
+help: consider using the `'static` lifetime
+   |
+LL | fn elision<T: Fn() -> &'static i32>() {
+   |                       ^^^^^^^^
 
 error: aborting due to previous error
 

src/test/ui/associated-types/bound-lifetime-in-return-only.elision.stderr

@@ -2,9 +2,13 @@ error[E0106]: missing lifetime specifier
   --> $DIR/bound-lifetime-in-return-only.rs:34:23
    |
 LL | fn elision(_: fn() -> &i32) {
-   |                       ^ help: consider giving it a 'static lifetime: `&'static`
+   |                       ^ expected named lifetime parameter
    |
    = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
+help: consider using the `'static` lifetime
+   |
+LL | fn elision(_: fn() -> &'static i32) {
+   |                       ^^^^^^^^
 
 error: aborting due to previous error