Split elided_lifetime_in_paths into tied and untied

Types that contain a reference can be confusing when lifetime elision occurs: ```rust // Confusing fn foo(_: &u8) -> Bar { todo!() } // Less confusing fn foo(_: &u8) -> Bar<'_> { todo!() } ``` However, the previous lint did not distinguish when these types were not "tying" lifetimes across the function inputs / outputs: ```rust // Maybe a little confusing fn foo(_: Bar) {} // More explicit but noisier with less obvious value fn foo(_: Bar<'_>) {} ``` We now report different lints for each case, hopefully paving the way to marking the first case (when lifetimes are tied together) as warn-by-default. Additionally, when multiple errors occur in the same function during the tied case, they are coalesced into one error. There is also some help text pointing out where the lifetime source is.
rust-lang · Aug 9, 2024 · 6f3941c · 6f3941c
1 parent 3fe5dee
commit 6f3941c
Show file tree

Hide file tree

Showing 17 changed files with 752 additions and 40 deletions.
diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs
@@ -1971,16 +1971,24 @@ pub fn elided_lifetime_in_path_suggestion(
     insertion_span: Span,
 ) -> ElidedLifetimeInPathSubdiag {
     let expected = ExpectedLifetimeParameter { span: path_span, count: n };
+    let indicate = indicate_anonymous_lifetime(source_map, n, incl_angl_brckt, insertion_span);
+    ElidedLifetimeInPathSubdiag { expected, indicate }
+}
+
+pub fn indicate_anonymous_lifetime(
+    source_map: &SourceMap,
+    n: usize,
+    incl_angl_brckt: bool,
+    insertion_span: Span,
+) -> Option<IndicateAnonymousLifetime> {
     // Do not try to suggest anything if generated by a proc-macro.
-    let indicate = source_map.is_span_accessible(insertion_span).then(|| {
+    source_map.is_span_accessible(insertion_span).then(|| {
         let anon_lts = vec!["'_"; n].join(", ");
         let suggestion =
             if incl_angl_brckt { format!("<{anon_lts}>") } else { format!("{anon_lts}, ") };
 
         IndicateAnonymousLifetime { span: insertion_span.shrink_to_hi(), count: n, suggestion }
-    });
-
-    ElidedLifetimeInPathSubdiag { expected, indicate }
+    })
 }
 
 pub fn report_ambiguity_error<'a, G: EmissionGuarantee>(

diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl
@@ -290,6 +290,8 @@ lint_hidden_glob_reexport = private item shadows public glob re-export
 
 lint_hidden_lifetime_parameters = hidden lifetime parameters in types are deprecated
 
+lint_hidden_lifetime_parameters_tied_source = the lifetime comes from here
+
 lint_hidden_unicode_codepoints = unicode codepoint changing visible direction of text present in {$label}
     .label = this {$label} contains {$count ->
         [one] an invisible

diff --git a/compiler/rustc_lint/src/context/diagnostics.rs b/compiler/rustc_lint/src/context/diagnostics.rs
@@ -5,7 +5,8 @@ use std::borrow::Cow;
 
 use rustc_ast::util::unicode::TEXT_FLOW_CONTROL_CHARS;
 use rustc_errors::{
-    elided_lifetime_in_path_suggestion, Applicability, Diag, DiagArgValue, LintDiagnostic,
+    elided_lifetime_in_path_suggestion, indicate_anonymous_lifetime, Applicability, Diag,
+    DiagArgValue, ExpectedLifetimeParameter, LintDiagnostic,
 };
 use rustc_middle::middle::stability;
 use rustc_session::lint::BuiltinLintDiag;
@@ -82,6 +83,27 @@ pub(super) fn decorate_lint(sess: &Session, diagnostic: BuiltinLintDiag, diag: &
             }
             .decorate_lint(diag);
         }
+
+        BuiltinLintDiag::ElidedLifetimesInPathsTied { elided_lifetime_source, suggestions } => {
+            let elided_lifetime_source =
+                elided_lifetime_source.map(|span| lints::ElidedLifetimesInPathsTiedSource { span });
+
+            let expected = suggestions
+                .iter()
+                .map(|&(span, count, _)| ExpectedLifetimeParameter { span, count })
+                .collect();
+
+            let suggestions = suggestions
+                .into_iter()
+                .flat_map(|(span, n, incl_angl_brckt)| {
+                    indicate_anonymous_lifetime(sess.source_map(), n, incl_angl_brckt, span)
+                })
+                .collect();
+
+            lints::ElidedLifetimesInPathsTied { expected, suggestions, elided_lifetime_source }
+                .decorate_lint(diag)
+        }
+
         BuiltinLintDiag::UnknownCrateTypes { span, candidate } => {
             let sugg = candidate.map(|candidate| lints::UnknownCrateTypesSub { span, candidate });
             lints::UnknownCrateTypes { sugg }.decorate_lint(diag);

diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs
@@ -308,7 +308,8 @@ fn register_builtins(store: &mut LintStore) {
         BARE_TRAIT_OBJECTS,
         UNUSED_EXTERN_CRATES,
         ELLIPSIS_INCLUSIVE_RANGE_PATTERNS,
-        ELIDED_LIFETIMES_IN_PATHS,
+        ELIDED_LIFETIMES_IN_PATHS_TIED,
+        ELIDED_LIFETIMES_IN_PATHS_UNTIED,
         EXPLICIT_OUTLIVES_REQUIREMENTS,
         // FIXME(#52665, #47816) not always applicable and not all
         // macros are ready for this yet.
@@ -328,9 +329,14 @@ fn register_builtins(store: &mut LintStore) {
 
     add_lint_group!("deprecated_safe", DEPRECATED_SAFE_2024);
 
+    add_lint_group!(
+        "elided_lifetimes_in_paths",
+        ELIDED_LIFETIMES_IN_PATHS_TIED,
+        ELIDED_LIFETIMES_IN_PATHS_UNTIED,
+    );
+
     // Register renamed and removed lints.
     store.register_renamed("single_use_lifetime", "single_use_lifetimes");
-    store.register_renamed("elided_lifetime_in_path", "elided_lifetimes_in_paths");
     store.register_renamed("bare_trait_object", "bare_trait_objects");
     store.register_renamed("unstable_name_collision", "unstable_name_collisions");
     store.register_renamed("unused_doc_comment", "unused_doc_comments");
@@ -344,6 +350,9 @@ fn register_builtins(store: &mut LintStore) {
     store.register_renamed("unused_tuple_struct_fields", "dead_code");
     store.register_renamed("static_mut_ref", "static_mut_refs");
 
+    // Register renamed lint groups
+    store.register_renamed_group("elided_lifetime_in_path", "elided_lifetimes_in_paths");
+
     // These were moved to tool lints, but rustc still sees them when compiling normally, before
     // tool lints are registered, so `check_tool_name_for_backwards_compat` doesn't work. Use
     // `register_removed` explicitly.

diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs
@@ -5,7 +5,8 @@ use std::num::NonZero;
 use rustc_errors::codes::*;
 use rustc_errors::{
     Applicability, Diag, DiagArgValue, DiagMessage, DiagStyledString, ElidedLifetimeInPathSubdiag,
-    EmissionGuarantee, LintDiagnostic, MultiSpan, SubdiagMessageOp, Subdiagnostic, SuggestionStyle,
+    EmissionGuarantee, ExpectedLifetimeParameter, IndicateAnonymousLifetime, LintDiagnostic,
+    MultiSpan, SubdiagMessageOp, Subdiagnostic, SuggestionStyle,
 };
 use rustc_hir::def::Namespace;
 use rustc_hir::def_id::DefId;
@@ -2611,6 +2612,26 @@ pub struct ElidedLifetimesInPaths {
     pub subdiag: ElidedLifetimeInPathSubdiag,
 }
 
+#[derive(LintDiagnostic)]
+#[diag(lint_hidden_lifetime_parameters)] // deliberately the same translation
+pub struct ElidedLifetimesInPathsTied {
+    #[subdiagnostic]
+    pub expected: Vec<ExpectedLifetimeParameter>,
+
+    #[subdiagnostic]
+    pub suggestions: Vec<IndicateAnonymousLifetime>,
+
+    #[subdiagnostic]
+    pub elided_lifetime_source: Option<ElidedLifetimesInPathsTiedSource>,
+}
+
+#[derive(Subdiagnostic)]
+#[label(lint_hidden_lifetime_parameters_tied_source)]
+pub struct ElidedLifetimesInPathsTiedSource {
+    #[primary_span]
+    pub span: Span,
+}
+
 #[derive(LintDiagnostic)]
 #[diag(lint_invalid_crate_type_value)]
 pub struct UnknownCrateTypes {

diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs
@@ -41,7 +41,8 @@ declare_lint_pass! {
         DEPRECATED_WHERE_CLAUSE_LOCATION,
         DUPLICATE_MACRO_ATTRIBUTES,
         ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT,
-        ELIDED_LIFETIMES_IN_PATHS,
+        ELIDED_LIFETIMES_IN_PATHS_TIED,
+        ELIDED_LIFETIMES_IN_PATHS_UNTIED,
         EXPLICIT_BUILTIN_CFGS_IN_FLAGS,
         EXPORTED_PRIVATE_DEPENDENCIES,
         FFI_UNWIND_CALLS,
@@ -1829,19 +1830,21 @@ declare_lint! {
 }
 
 declare_lint! {
-    /// The `elided_lifetimes_in_paths` lint detects the use of hidden
-    /// lifetime parameters.
+    /// The `elided_lifetimes_in_paths_tied` lint detects the use of
+    /// hidden lifetime parameters when those lifetime parameters tie
+    /// an input lifetime parameter to an output lifetime parameter.
     ///
     /// ### Example
     ///
     /// ```rust,compile_fail
-    /// #![deny(elided_lifetimes_in_paths)]
+    /// #![deny(elided_lifetimes_in_paths_tied)]
     /// #![deny(warnings)]
     /// struct Foo<'a> {
     ///     x: &'a u32
     /// }
     ///
-    /// fn foo(x: &Foo) {
+    /// fn foo(x: Foo) -> &u32 {
+    ///     &x.0
     /// }
     /// ```
     ///
@@ -1858,11 +1861,50 @@ declare_lint! {
     /// may require a significant transition for old code.
     ///
     /// [placeholder lifetime]: https://doc.rust-lang.org/reference/lifetime-elision.html#lifetime-elision-in-functions
-    pub ELIDED_LIFETIMES_IN_PATHS,
+    pub ELIDED_LIFETIMES_IN_PATHS_TIED,
     Allow,
     "hidden lifetime parameters in types are deprecated"
 }
 
+declare_lint! {
+    /// The `elided_lifetimes_in_paths_untied` lint detects the use of
+    /// hidden lifetime parameters when those lifetime parameters do
+    /// not tie an input lifetime parameter to an output lifetime
+    /// parameter.
+    ///
+    /// ### Example
+    ///
+    /// ```rust,compile_fail
+    /// #![deny(elided_lifetimes_in_paths_untied)]
+    /// #![deny(warnings)]
+    /// struct Foo<'a> {
+    ///     x: &'a u32
+    /// }
+    ///
+    /// fn foo(x: Foo) -> u32 {
+    ///     x.0
+    /// }
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// Elided lifetime parameters can make it difficult to see at a glance
+    /// that borrowing is occurring. This lint ensures that lifetime
+    /// parameters are always explicitly stated, even if it is the `'_`
+    /// [placeholder lifetime].
+    ///
+    /// This lint is "allow" by default because it has some known issues, and
+    /// may require a significant transition for old code.
+    ///
+    /// [placeholder lifetime]: https://doc.rust-lang.org/reference/lifetime-elision.html#lifetime-elision-in-functions
+    pub ELIDED_LIFETIMES_IN_PATHS_UNTIED,
+    Allow,
+    "hidden lifetime parameters in types make it hard to tell when borrowing is happening",
+    crate_level_only
+}
+
 declare_lint! {
     /// The `bare_trait_objects` lint suggests using `dyn Trait` for trait
     /// objects.

diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs
@@ -585,6 +585,12 @@ pub enum BuiltinLintDiag {
     },
     MacroExpandedMacroExportsAccessedByAbsolutePaths(Span),
     ElidedLifetimesInPaths(usize, Span, bool, Span),
+
+    ElidedLifetimesInPathsTied {
+        elided_lifetime_source: Option<Span>,
+        suggestions: Vec<(Span, usize, bool)>,
+    },
+
     UnknownCrateTypes {
         span: Span,
         candidate: Option<Symbol>,