Rollup merge of #154592 - reddevilmidzy:lifetime, r=chenyukang

Fix `mismatched_lifetime_syntaxes` suggestions for hidden path lifetimes

close: https://github.com/rust-lang/rust/issues/154493

r? shepmaster

cc: @chenyukang

compiler/rustc_lint/src/lifetime_syntax.rs

@@ -387,9 +387,7 @@ fn emit_mismatch_diagnostic<'tcx>(
         build_mismatch_suggestion(info.lifetime.ident.as_str(), &suggest_change_to_explicit_bound)
     });
 
-    let is_bound_static = bound_lifetime.is_some_and(|info| info.lifetime.is_static());
-
-    tracing::debug!(?bound_lifetime, ?explicit_bound_suggestion, ?is_bound_static);
+    tracing::debug!(?bound_lifetime, ?explicit_bound_suggestion);
 
     let should_suggest_mixed =
         // Do we have a mixed case?
@@ -397,16 +395,16 @@ fn emit_mismatch_diagnostic<'tcx>(
         // Is there anything to change?
         (!suggest_change_to_mixed_implicit.is_empty() ||
          !suggest_change_to_mixed_explicit_anonymous.is_empty()) &&
-        // If we have `'static`, we don't want to remove it.
-        !is_bound_static;
+        // If we have a named lifetime, prefer consistent naming.
+        bound_lifetime.is_none();
 
     let mixed_suggestion = should_suggest_mixed.then(|| {
         let implicit_suggestions = make_implicit_suggestions(&suggest_change_to_mixed_implicit);
 
-        let explicit_anonymous_suggestions = suggest_change_to_mixed_explicit_anonymous
-            .iter()
-            .map(|info| info.suggestion("'_"))
-            .collect();
+        let explicit_anonymous_suggestions = build_mismatch_suggestions_for_lifetime(
+            "'_",
+            &suggest_change_to_mixed_explicit_anonymous,
+        );
 
         lints::MismatchedLifetimeSyntaxesSuggestion::Mixed {
             implicit_suggestions,
@@ -426,8 +424,8 @@ fn emit_mismatch_diagnostic<'tcx>(
         !suggest_change_to_implicit.is_empty() &&
         // We never want to hide the lifetime in a path (or similar).
         allow_suggesting_implicit &&
-        // If we have `'static`, we don't want to remove it.
-        !is_bound_static;
+        // If we have a named lifetime, prefer consistent naming.
+        bound_lifetime.is_none();
 
     let implicit_suggestion = should_suggest_implicit.then(|| {
         let suggestions = make_implicit_suggestions(&suggest_change_to_implicit);
@@ -448,8 +446,10 @@ fn emit_mismatch_diagnostic<'tcx>(
     let should_suggest_explicit_anonymous =
         // Is there anything to change?
         !suggest_change_to_explicit_anonymous.is_empty() &&
-        // If we have `'static`, we don't want to remove it.
-        !is_bound_static;
+        // If we already have a mixed suggestion, avoid overlapping alternatives.
+        mixed_suggestion.is_none() &&
+        // If we have a named lifetime, prefer consistent naming.
+        bound_lifetime.is_none();
 
     let explicit_anonymous_suggestion = should_suggest_explicit_anonymous
         .then(|| build_mismatch_suggestion("'_", &suggest_change_to_explicit_anonymous));
@@ -483,7 +483,7 @@ fn build_mismatch_suggestion(
 ) -> lints::MismatchedLifetimeSyntaxesSuggestion {
     let lifetime_name = lifetime_name.to_owned();
 
-    let suggestions = infos.iter().map(|info| info.suggestion(&lifetime_name)).collect();
+    let suggestions = build_mismatch_suggestions_for_lifetime(&lifetime_name, infos);
 
     lints::MismatchedLifetimeSyntaxesSuggestion::Explicit {
         lifetime_name,
@@ -492,6 +492,57 @@ fn build_mismatch_suggestion(
     }
 }
 
+fn build_mismatch_suggestions_for_lifetime(
+    lifetime_name: &str,
+    infos: &[&Info<'_>],
+) -> Vec<(Span, String)> {
+    use hir::{AngleBrackets, LifetimeSource, LifetimeSyntax};
+
+    #[derive(Clone, Copy, PartialEq, Eq, Hash)]
+    enum PathSuggestionKind {
+        Missing,
+        Empty,
+        Full,
+    }
+
+    let mut suggestions = Vec::new();
+    let mut path_counts: FxIndexMap<(hir::HirId, PathSuggestionKind), (Span, usize)> =
+        FxIndexMap::default();
+
+    for info in infos {
+        let lifetime = info.lifetime;
+        if matches!(lifetime.syntax, LifetimeSyntax::Implicit) {
+            if let LifetimeSource::Path { angle_brackets } = lifetime.source {
+                let (span, kind) = match angle_brackets {
+                    AngleBrackets::Missing => {
+                        (lifetime.ident.span.shrink_to_hi(), PathSuggestionKind::Missing)
+                    }
+                    AngleBrackets::Empty => (lifetime.ident.span, PathSuggestionKind::Empty),
+                    AngleBrackets::Full => (lifetime.ident.span, PathSuggestionKind::Full),
+                };
+                let entry = path_counts.entry((info.ty.hir_id, kind)).or_insert((span, 0));
+                entry.1 += 1;
+                continue;
+            }
+        }
+        suggestions.push(info.suggestion(lifetime_name));
+    }
+
+    for ((_ty_hir_id, kind), (span, count)) in path_counts {
+        let repeated = std::iter::repeat(lifetime_name).take(count).collect::<Vec<_>>().join(", ");
+
+        let suggestion = match kind {
+            PathSuggestionKind::Missing => format!("<{repeated}>"),
+            PathSuggestionKind::Empty => repeated,
+            PathSuggestionKind::Full => format!("{repeated}, "),
+        };
+
+        suggestions.push((span, suggestion));
+    }
+
+    suggestions
+}
+
 #[derive(Debug)]
 struct Info<'tcx> {
     lifetime: &'tcx hir::Lifetime,

compiler/rustc_lint/src/lints.rs

@@ -2,6 +2,7 @@
 
 use std::num::NonZero;
 
+use rustc_data_structures::fx::FxIndexMap;
 use rustc_errors::codes::*;
 use rustc_errors::formatting::DiagMessageAddArg;
 use rustc_errors::{
@@ -3231,8 +3232,23 @@ fn into_diag(self, dcx: DiagCtxtHandle<'a>, level: Level) -> Diag<'a, G> {
             diag.span_label(s, msg!("the lifetime is named here"));
         }
 
+        let mut hidden_output_counts: FxIndexMap<Span, usize> = FxIndexMap::default();
         for s in self.outputs.hidden {
-            diag.span_label(s, msg!("the same lifetime is hidden here"));
+            *hidden_output_counts.entry(s).or_insert(0) += 1;
+        }
+        for (span, count) in hidden_output_counts {
+            let label = msg!(
+                "the same {$count ->
+                    [one] lifetime
+                    *[other] lifetimes
+                } {$count ->
+                    [one] is
+                    *[other] are
+                } hidden here"
+            )
+            .arg("count", count)
+            .format();
+            diag.span_label(span, label);
         }
         for s in self.outputs.elided {
             diag.span_label(s, msg!("the same lifetime is elided here"));

tests/ui/lifetimes/mismatched-lifetime-syntaxes-details/macro.fixed

@@ -0,0 +1,19 @@
+//! Regression test for <https://github.com/rust-lang/rust/issues/154493>
+//@ run-rustfix
+#![deny(mismatched_lifetime_syntaxes)]
+#![allow(unused)]
+
+struct Pair<'a, 'b>(&'a u8, &'b u8);
+
+macro_rules! repeated {
+    ($($pair:ident),+ ; $middle:ty) => {
+        ($($pair<'a, 'a>),+, $middle, $($pair<'a, 'a>),+)
+        //~^ ERROR hiding or eliding a lifetime that's named elsewhere is confusing
+    };
+}
+
+fn repeated_macro<'a>(x: &'a u8) -> repeated!(Pair, Pair; &'a u8) {
+    (Pair(x, x), Pair(x, x), x, Pair(x, x), Pair(x, x))
+}
+
+fn main() {}

tests/ui/lifetimes/mismatched-lifetime-syntaxes-details/macro.rs

@@ -0,0 +1,19 @@
+//! Regression test for <https://github.com/rust-lang/rust/issues/154493>
+//@ run-rustfix
+#![deny(mismatched_lifetime_syntaxes)]
+#![allow(unused)]
+
+struct Pair<'a, 'b>(&'a u8, &'b u8);
+
+macro_rules! repeated {
+    ($($pair:ident),+ ; $middle:ty) => {
+        ($($pair),+, $middle, $($pair),+)
+        //~^ ERROR hiding or eliding a lifetime that's named elsewhere is confusing
+    };
+}
+
+fn repeated_macro<'a>(x: &'a u8) -> repeated!(Pair, Pair; &'_ u8) {
+    (Pair(x, x), Pair(x, x), x, Pair(x, x), Pair(x, x))
+}
+
+fn main() {}

tests/ui/lifetimes/mismatched-lifetime-syntaxes-details/macro.stderr

@@ -0,0 +1,33 @@
+error: hiding or eliding a lifetime that's named elsewhere is confusing
+  --> $DIR/macro.rs:10:12
+   |
+LL |         ($($pair),+, $middle, $($pair),+)
+   |            ^^^^^                ^^^^^ the same lifetimes are hidden here
+   |            |
+   |            the same lifetimes are hidden here
+...
+LL | fn repeated_macro<'a>(x: &'a u8) -> repeated!(Pair, Pair; &'_ u8) {
+   |                           --        -----------------------^^----
+   |                           |         |                      |
+   |                           |         |                      the same lifetime is elided here
+   |                           |         in this macro invocation
+   |                           the lifetime is named here
+   |
+   = help: the same lifetime is referred to in inconsistent ways, making the signature confusing
+note: the lint level is defined here
+  --> $DIR/macro.rs:3:9
+   |
+LL | #![deny(mismatched_lifetime_syntaxes)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = note: this error originates in the macro `repeated` (in Nightly builds, run with -Z macro-backtrace for more info)
+help: consistently use `'a`
+   |
+LL ~         ($($pair<'a, 'a>),+, $middle, $($pair<'a, 'a>),+)
+LL |
+...
+LL |
+LL ~ fn repeated_macro<'a>(x: &'a u8) -> repeated!(Pair, Pair; &'a u8) {
+   |
+
+error: aborting due to 1 previous error
+

tests/ui/lifetimes/mismatched-lifetime-syntaxes-details/path-count.fixed

@@ -0,0 +1,69 @@
+//! Regression test for <https://github.com/rust-lang/rust/issues/154493>
+//@ run-rustfix
+#![deny(mismatched_lifetime_syntaxes)]
+#![allow(unused)]
+
+struct Foo<'a, 'b> {
+    ptr1: &'a str,
+    ptr2: &'b str,
+}
+
+// with generic
+struct Bar<'a, 'b, T> {
+    ptr1: &'a T,
+    ptr2: &'b T,
+}
+
+struct Wrapper<T>(T);
+
+fn missing(_: &str) -> Foo<'_, '_> {
+    //~^ ERROR: hiding a lifetime that's elided elsewhere is confusing
+    todo!()
+}
+
+fn missing_double(_: &str) -> (Foo<'_, '_>, Foo<'_, '_>) {
+    //~^ ERROR: hiding a lifetime that's elided elsewhere is confusing
+    todo!()
+}
+
+fn empty(_: &str) -> Foo<'_, '_> {
+    //~^ ERROR: hiding a lifetime that's elided elsewhere is confusing
+    todo!()
+}
+
+fn wrapper_missing(_: &str) -> Wrapper<Foo<'_, '_>> {
+    //~^ ERROR: hiding a lifetime that's elided elsewhere is confusing
+    todo!()
+}
+
+fn missing_generic(_: &str) -> Bar<'_, '_, u8> {
+    //~^ ERROR: hiding a lifetime that's elided elsewhere is confusing
+    todo!()
+}
+
+fn named_missing<'a>(_: &'a u8) -> Foo<'a, 'a> {
+    //~^ ERROR: hiding a lifetime that's named elsewhere is confusing
+    todo!()
+}
+
+fn named_empty<'a>(_: &'a u8) -> Foo<'a, 'a> {
+    //~^ ERROR: hiding a lifetime that's named elsewhere is confusing
+    todo!()
+}
+
+fn static_missing(_: &'static u8) -> Foo<'static, 'static> {
+    //~^ ERROR: hiding a lifetime that's named elsewhere is confusing
+    todo!()
+}
+
+fn static_empty(_: &'static u8) -> Foo<'static, 'static> {
+    //~^ ERROR: hiding a lifetime that's named elsewhere is confusing
+    todo!()
+}
+
+fn static_missing_generic(_: &'static str) -> Bar<'static, 'static, u8> {
+    //~^ ERROR: hiding a lifetime that's named elsewhere is confusing
+    todo!()
+}
+
+fn main() {}

tests/ui/lifetimes/mismatched-lifetime-syntaxes-details/path-count.rs

@@ -0,0 +1,69 @@
+//! Regression test for <https://github.com/rust-lang/rust/issues/154493>
+//@ run-rustfix
+#![deny(mismatched_lifetime_syntaxes)]
+#![allow(unused)]
+
+struct Foo<'a, 'b> {
+    ptr1: &'a str,
+    ptr2: &'b str,
+}
+
+// with generic
+struct Bar<'a, 'b, T> {
+    ptr1: &'a T,
+    ptr2: &'b T,
+}
+
+struct Wrapper<T>(T);
+
+fn missing(_: &str) -> Foo {
+    //~^ ERROR: hiding a lifetime that's elided elsewhere is confusing
+    todo!()
+}
+
+fn missing_double(_: &str) -> (Foo, Foo) {
+    //~^ ERROR: hiding a lifetime that's elided elsewhere is confusing
+    todo!()
+}
+
+fn empty(_: &str) -> Foo<> {
+    //~^ ERROR: hiding a lifetime that's elided elsewhere is confusing
+    todo!()
+}
+
+fn wrapper_missing(_: &str) -> Wrapper<Foo> {
+    //~^ ERROR: hiding a lifetime that's elided elsewhere is confusing
+    todo!()
+}
+
+fn missing_generic(_: &str) -> Bar<u8> {
+    //~^ ERROR: hiding a lifetime that's elided elsewhere is confusing
+    todo!()
+}
+
+fn named_missing<'a>(_: &'a u8) -> Foo {
+    //~^ ERROR: hiding a lifetime that's named elsewhere is confusing
+    todo!()
+}
+
+fn named_empty<'a>(_: &'a u8) -> Foo<> {
+    //~^ ERROR: hiding a lifetime that's named elsewhere is confusing
+    todo!()
+}
+
+fn static_missing(_: &'static u8) -> Foo {
+    //~^ ERROR: hiding a lifetime that's named elsewhere is confusing
+    todo!()
+}
+
+fn static_empty(_: &'static u8) -> Foo<> {
+    //~^ ERROR: hiding a lifetime that's named elsewhere is confusing
+    todo!()
+}
+
+fn static_missing_generic(_: &'static str) -> Bar<u8> {
+    //~^ ERROR: hiding a lifetime that's named elsewhere is confusing
+    todo!()
+}
+
+fn main() {}

tests/ui/lifetimes/mismatched-lifetime-syntaxes-details/path-count.stderr

@@ -0,0 +1,148 @@
+error: hiding a lifetime that's elided elsewhere is confusing
+  --> $DIR/path-count.rs:19:15
+   |
+LL | fn missing(_: &str) -> Foo {
+   |               ^^^^     ^^^ the same lifetimes are hidden here
+   |               |
+   |               the lifetime is elided here
+   |
+   = help: the same lifetime is referred to in inconsistent ways, making the signature confusing
+note: the lint level is defined here
+  --> $DIR/path-count.rs:3:9
+   |
+LL | #![deny(mismatched_lifetime_syntaxes)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: use `'_` for type paths
+   |
+LL | fn missing(_: &str) -> Foo<'_, '_> {
+   |                           ++++++++
+
+error: hiding a lifetime that's elided elsewhere is confusing
+  --> $DIR/path-count.rs:24:22
+   |
+LL | fn missing_double(_: &str) -> (Foo, Foo) {
+   |                      ^^^^      ^^^  ^^^ the same lifetimes are hidden here
+   |                      |         |
+   |                      |         the same lifetimes are hidden here
+   |                      the lifetime is elided here
+   |
+   = help: the same lifetime is referred to in inconsistent ways, making the signature confusing
+help: use `'_` for type paths
+   |
+LL | fn missing_double(_: &str) -> (Foo<'_, '_>, Foo<'_, '_>) {
+   |                                   ++++++++     ++++++++
+
+error: hiding a lifetime that's elided elsewhere is confusing
+  --> $DIR/path-count.rs:29:13
+   |
+LL | fn empty(_: &str) -> Foo<> {
+   |             ^^^^     ^^^^^ the same lifetimes are hidden here
+   |             |
+   |             the lifetime is elided here
+   |
+   = help: the same lifetime is referred to in inconsistent ways, making the signature confusing
+help: use `'_` for type paths
+   |
+LL | fn empty(_: &str) -> Foo<'_, '_> {
+   |                          ++++++
+
+error: hiding a lifetime that's elided elsewhere is confusing
+  --> $DIR/path-count.rs:34:23
+   |
+LL | fn wrapper_missing(_: &str) -> Wrapper<Foo> {
+   |                       ^^^^             ^^^ the same lifetimes are hidden here
+   |                       |
+   |                       the lifetime is elided here
+   |
+   = help: the same lifetime is referred to in inconsistent ways, making the signature confusing
+help: use `'_` for type paths
+   |
+LL | fn wrapper_missing(_: &str) -> Wrapper<Foo<'_, '_>> {
+   |                                           ++++++++
+
+error: hiding a lifetime that's elided elsewhere is confusing
+  --> $DIR/path-count.rs:39:23
+   |
+LL | fn missing_generic(_: &str) -> Bar<u8> {
+   |                       ^^^^     ^^^^^^^ the same lifetimes are hidden here
+   |                       |
+   |                       the lifetime is elided here
+   |
+   = help: the same lifetime is referred to in inconsistent ways, making the signature confusing
+help: use `'_` for type paths
+   |
+LL | fn missing_generic(_: &str) -> Bar<'_, '_, u8> {
+   |                                    +++++++
+
+error: hiding a lifetime that's named elsewhere is confusing
+  --> $DIR/path-count.rs:44:36
+   |
+LL | fn named_missing<'a>(_: &'a u8) -> Foo {
+   |                          --        ^^^ the same lifetimes are hidden here
+   |                          |
+   |                          the lifetime is named here
+   |
+   = help: the same lifetime is referred to in inconsistent ways, making the signature confusing
+help: consistently use `'a`
+   |
+LL | fn named_missing<'a>(_: &'a u8) -> Foo<'a, 'a> {
+   |                                       ++++++++
+
+error: hiding a lifetime that's named elsewhere is confusing
+  --> $DIR/path-count.rs:49:34
+   |
+LL | fn named_empty<'a>(_: &'a u8) -> Foo<> {
+   |                        --        ^^^^^ the same lifetimes are hidden here
+   |                        |
+   |                        the lifetime is named here
+   |
+   = help: the same lifetime is referred to in inconsistent ways, making the signature confusing
+help: consistently use `'a`
+   |
+LL | fn named_empty<'a>(_: &'a u8) -> Foo<'a, 'a> {
+   |                                      ++++++
+
+error: hiding a lifetime that's named elsewhere is confusing
+  --> $DIR/path-count.rs:54:38
+   |
+LL | fn static_missing(_: &'static u8) -> Foo {
+   |                       -------        ^^^ the same lifetimes are hidden here
+   |                       |
+   |                       the lifetime is named here
+   |
+   = help: the same lifetime is referred to in inconsistent ways, making the signature confusing
+help: consistently use `'static`
+   |
+LL | fn static_missing(_: &'static u8) -> Foo<'static, 'static> {
+   |                                         ++++++++++++++++++
+
+error: hiding a lifetime that's named elsewhere is confusing
+  --> $DIR/path-count.rs:59:36
+   |
+LL | fn static_empty(_: &'static u8) -> Foo<> {
+   |                     -------        ^^^^^ the same lifetimes are hidden here
+   |                     |
+   |                     the lifetime is named here
+   |
+   = help: the same lifetime is referred to in inconsistent ways, making the signature confusing
+help: consistently use `'static`
+   |
+LL | fn static_empty(_: &'static u8) -> Foo<'static, 'static> {
+   |                                        ++++++++++++++++
+
+error: hiding a lifetime that's named elsewhere is confusing
+  --> $DIR/path-count.rs:64:47
+   |
+LL | fn static_missing_generic(_: &'static str) -> Bar<u8> {
+   |                               -------         ^^^^^^^ the same lifetimes are hidden here
+   |                               |
+   |                               the lifetime is named here
+   |
+   = help: the same lifetime is referred to in inconsistent ways, making the signature confusing
+help: consistently use `'static`
+   |
+LL | fn static_missing_generic(_: &'static str) -> Bar<'static, 'static, u8> {
+   |                                                   +++++++++++++++++
+
+error: aborting due to 10 previous errors
+