diff --git a/compiler/rustc_lint/src/lifetime_syntax.rs b/compiler/rustc_lint/src/lifetime_syntax.rs index 0cac91c23408..6c65673acd0d 100644 --- a/compiler/rustc_lint/src/lifetime_syntax.rs +++ b/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::>().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, diff --git a/tests/ui/lifetimes/mismatched-lifetime-syntaxes-details/macro.fixed b/tests/ui/lifetimes/mismatched-lifetime-syntaxes-details/macro.fixed new file mode 100644 index 000000000000..cd5cda6d27a9 --- /dev/null +++ b/tests/ui/lifetimes/mismatched-lifetime-syntaxes-details/macro.fixed @@ -0,0 +1,19 @@ +//! Regression test for +//@ 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() {} diff --git a/tests/ui/lifetimes/mismatched-lifetime-syntaxes-details/macro.rs b/tests/ui/lifetimes/mismatched-lifetime-syntaxes-details/macro.rs new file mode 100644 index 000000000000..8e8b6d9d7e09 --- /dev/null +++ b/tests/ui/lifetimes/mismatched-lifetime-syntaxes-details/macro.rs @@ -0,0 +1,19 @@ +//! Regression test for +//@ 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() {} diff --git a/tests/ui/lifetimes/mismatched-lifetime-syntaxes-details/macro.stderr b/tests/ui/lifetimes/mismatched-lifetime-syntaxes-details/macro.stderr new file mode 100644 index 000000000000..a13db05ceacb --- /dev/null +++ b/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 lifetime is 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 + diff --git a/tests/ui/lifetimes/mismatched-lifetime-syntaxes-details/path-count.fixed b/tests/ui/lifetimes/mismatched-lifetime-syntaxes-details/path-count.fixed new file mode 100644 index 000000000000..5689eed81dde --- /dev/null +++ b/tests/ui/lifetimes/mismatched-lifetime-syntaxes-details/path-count.fixed @@ -0,0 +1,69 @@ +//! Regression test for +//@ 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); + +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> { + //~^ 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() {} diff --git a/tests/ui/lifetimes/mismatched-lifetime-syntaxes-details/path-count.rs b/tests/ui/lifetimes/mismatched-lifetime-syntaxes-details/path-count.rs new file mode 100644 index 000000000000..3c12600e07a5 --- /dev/null +++ b/tests/ui/lifetimes/mismatched-lifetime-syntaxes-details/path-count.rs @@ -0,0 +1,69 @@ +//! Regression test for +//@ 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); + +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 { + //~^ ERROR: hiding a lifetime that's elided elsewhere is confusing + todo!() +} + +fn missing_generic(_: &str) -> Bar { + //~^ 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 { + //~^ ERROR: hiding a lifetime that's named elsewhere is confusing + todo!() +} + +fn main() {} diff --git a/tests/ui/lifetimes/mismatched-lifetime-syntaxes-details/path-count.stderr b/tests/ui/lifetimes/mismatched-lifetime-syntaxes-details/path-count.stderr new file mode 100644 index 000000000000..dbec4fee767b --- /dev/null +++ b/tests/ui/lifetimes/mismatched-lifetime-syntaxes-details/path-count.stderr @@ -0,0 +1,169 @@ +error: hiding a lifetime that's elided elsewhere is confusing + --> $DIR/path-count.rs:19:15 + | +LL | fn missing(_: &str) -> Foo { + | ^^^^ ^^^ + | | | + | | the same lifetime is hidden here + | | the same lifetime is 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 lifetime is hidden here + | | | the same lifetime is hidden here + | | the same lifetime is hidden here + | | the same lifetime is 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 lifetime is hidden here + | | the same lifetime is 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 { + | ^^^^ ^^^ + | | | + | | the same lifetime is hidden here + | | the same lifetime is 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> { + | ++++++++ + +error: hiding a lifetime that's elided elsewhere is confusing + --> $DIR/path-count.rs:39:23 + | +LL | fn missing_generic(_: &str) -> Bar { + | ^^^^ ^^^^^^^ + | | | + | | the same lifetime is hidden here + | | the same lifetime is 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 lifetime is hidden here + | | the same lifetime is 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 lifetime is hidden here + | | the same lifetime is 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 lifetime is hidden here + | | the same lifetime is 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 lifetime is hidden here + | | the same lifetime is 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 { + | ------- ^^^^^^^ + | | | + | | the same lifetime is hidden here + | | the same lifetime is 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 +