Rollup merge of rust-lang#129840 - GrigorenkoPV:elided-named-lifetimes-suggestion, r=cjgillot

compiler-errors · web-flow · commit 41a20dcb87bc · 2024-09-07T14:21:20.000+03:00
Implement suggestions for `elided_named_lifetimes` A follow-up to rust-lang#129207, as per rust-lang#129207 (comment). r? cjgillot I will probably squash this a bit, but later. `@rustbot` label +A-lint
diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl
@@ -255,6 +255,7 @@ lint_duplicate_matcher_binding = duplicate matcher binding
 lint_elided_named_lifetime = elided lifetime has a name
     .label_elided = this elided lifetime gets resolved as `{$name}`
     .label_named = lifetime `{$name}` declared here
+    .suggestion = consider specifying it explicitly
 
 lint_enum_intrinsics_mem_discriminant =
     the return value of `mem::discriminant` is unspecified when called with a non-enum type
diff --git a/compiler/rustc_lint/src/context/diagnostics.rs b/compiler/rustc_lint/src/context/diagnostics.rs
@@ -8,13 +8,13 @@ use rustc_errors::{
     elided_lifetime_in_path_suggestion, Applicability, Diag, DiagArgValue, LintDiagnostic,
 };
 use rustc_middle::middle::stability;
-use rustc_session::lint::BuiltinLintDiag;
+use rustc_session::lint::{BuiltinLintDiag, ElidedLifetimeResolution};
 use rustc_session::Session;
 use rustc_span::symbol::kw;
 use rustc_span::BytePos;
 use tracing::debug;
 
-use crate::lints;
+use crate::lints::{self, ElidedNamedLifetime};
 
 mod check_cfg;
 
@@ -442,15 +442,14 @@ pub(super) fn decorate_lint(sess: &Session, diagnostic: BuiltinLintDiag, diag: &
         BuiltinLintDiag::UnexpectedBuiltinCfg { cfg, cfg_name, controlled_by } => {
             lints::UnexpectedBuiltinCfg { cfg, cfg_name, controlled_by }.decorate_lint(diag)
         }
-        BuiltinLintDiag::ElidedIsStatic { elided } => {
-            lints::ElidedNamedLifetime { elided, name: kw::StaticLifetime, named_declaration: None }
-                .decorate_lint(diag)
-        }
-        BuiltinLintDiag::ElidedIsParam { elided, param: (param_name, param_span) } => {
-            lints::ElidedNamedLifetime {
-                elided,
-                name: param_name,
-                named_declaration: Some(param_span),
+        BuiltinLintDiag::ElidedNamedLifetimes { elided: (span, kind), resolution } => {
+            match resolution {
+                ElidedLifetimeResolution::Static => {
+                    ElidedNamedLifetime { span, kind, name: kw::StaticLifetime, declaration: None }
+                }
+                ElidedLifetimeResolution::Param(name, declaration) => {
+                    ElidedNamedLifetime { span, kind, name, declaration: Some(declaration) }
+                }
             }
             .decorate_lint(diag)
         }
diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs
@@ -9,7 +9,7 @@ use rustc_errors::{
 };
 use rustc_hir::def::Namespace;
 use rustc_hir::def_id::DefId;
-use rustc_hir::{self as hir};
+use rustc_hir::{self as hir, MissingLifetimeKind};
 use rustc_macros::{LintDiagnostic, Subdiagnostic};
 use rustc_middle::ty::inhabitedness::InhabitedPredicate;
 use rustc_middle::ty::{Clause, PolyExistentialTraitRef, Ty, TyCtxt};
@@ -2623,14 +2623,56 @@ pub(crate) struct ElidedLifetimesInPaths {
     pub subdiag: ElidedLifetimeInPathSubdiag,
 }
 
-#[derive(LintDiagnostic)]
-#[diag(lint_elided_named_lifetime)]
 pub(crate) struct ElidedNamedLifetime {
-    #[label(lint_label_elided)]
-    pub elided: Span,
+    pub span: Span,
+    pub kind: MissingLifetimeKind,
     pub name: Symbol,
-    #[label(lint_label_named)]
-    pub named_declaration: Option<Span>,
+    pub declaration: Option<Span>,
+}
+
+impl<G: EmissionGuarantee> LintDiagnostic<'_, G> for ElidedNamedLifetime {
+    fn decorate_lint(self, diag: &mut rustc_errors::Diag<'_, G>) {
+        let Self { span, kind, name, declaration } = self;
+        diag.primary_message(fluent::lint_elided_named_lifetime);
+        diag.arg("name", name);
+        diag.span_label(span, fluent::lint_label_elided);
+        if let Some(declaration) = declaration {
+            diag.span_label(declaration, fluent::lint_label_named);
+        }
+        // FIXME(GrigorenkoPV): this `if` and `return` should be removed,
+        //  but currently this lint's suggestions can conflict with those of `clippy::needless_lifetimes`:
+        //  https://github.com/rust-lang/rust/pull/129840#issuecomment-2323349119
+        // HACK: `'static` suggestions will never sonflict, emit only those for now.
+        if name != rustc_span::symbol::kw::StaticLifetime {
+            return;
+        }
+        match kind {
+            MissingLifetimeKind::Underscore => diag.span_suggestion_verbose(
+                span,
+                fluent::lint_suggestion,
+                format!("{name}"),
+                Applicability::MachineApplicable,
+            ),
+            MissingLifetimeKind::Ampersand => diag.span_suggestion_verbose(
+                span.shrink_to_hi(),
+                fluent::lint_suggestion,
+                format!("{name} "),
+                Applicability::MachineApplicable,
+            ),
+            MissingLifetimeKind::Comma => diag.span_suggestion_verbose(
+                span.shrink_to_hi(),
+                fluent::lint_suggestion,
+                format!("{name}, "),
+                Applicability::MachineApplicable,
+            ),
+            MissingLifetimeKind::Brackets => diag.span_suggestion_verbose(
+                span.shrink_to_hi(),
+                fluent::lint_suggestion,
+                format!("<{name}>"),
+                Applicability::MachineApplicable,
+            ),
+        };
+    }
 }
 
 #[derive(LintDiagnostic)]
diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs
@@ -10,7 +10,7 @@ use rustc_data_structures::stable_hasher::{
 };
 use rustc_error_messages::{DiagMessage, MultiSpan};
 use rustc_hir::def::Namespace;
-use rustc_hir::{HashStableContext, HirId};
+use rustc_hir::{HashStableContext, HirId, MissingLifetimeKind};
 use rustc_macros::{Decodable, Encodable, HashStable_Generic};
 use rustc_span::edition::Edition;
 use rustc_span::symbol::{Ident, MacroRulesNormalizedIdent};
@@ -556,6 +556,12 @@ pub enum DeprecatedSinceKind {
     InVersion(String),
 }
 
+#[derive(Debug)]
+pub enum ElidedLifetimeResolution {
+    Static,
+    Param(Symbol, Span),
+}
+
 // This could be a closure, but then implementing derive trait
 // becomes hacky (and it gets allocated).
 #[derive(Debug)]
@@ -568,12 +574,9 @@ pub enum BuiltinLintDiag {
     },
     MacroExpandedMacroExportsAccessedByAbsolutePaths(Span),
     ElidedLifetimesInPaths(usize, Span, bool, Span),
-    ElidedIsStatic {
-        elided: Span,
-    },
-    ElidedIsParam {
-        elided: Span,
-        param: (Symbol, Span),
+    ElidedNamedLifetimes {
+        elided: (Span, MissingLifetimeKind),
+        resolution: ElidedLifetimeResolution,
     },
     UnknownCrateTypes {
         span: Span,
diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs
@@ -2062,7 +2062,10 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
                                 lint::builtin::ELIDED_NAMED_LIFETIMES,
                                 missing.id_for_lint,
                                 missing.span,
-                                BuiltinLintDiag::ElidedIsStatic { elided: missing.span },
+                                BuiltinLintDiag::ElidedNamedLifetimes {
+                                    elided: (missing.span, missing.kind),
+                                    resolution: lint::ElidedLifetimeResolution::Static,
+                                },
                             );
                         }
                     }
@@ -2072,9 +2075,12 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
                             lint::builtin::ELIDED_NAMED_LIFETIMES,
                             missing.id_for_lint,
                             missing.span,
-                            BuiltinLintDiag::ElidedIsParam {
-                                elided: missing.span,
-                                param: (tcx.item_name(param.into()), tcx.source_span(param)),
+                            BuiltinLintDiag::ElidedNamedLifetimes {
+                                elided: (missing.span, missing.kind),
+                                resolution: lint::ElidedLifetimeResolution::Param(
+                                    tcx.item_name(param.into()),
+                                    tcx.source_span(param),
+                                ),
                             },
                         );
                     }
diff --git a/tests/ui/lint/elided-named-lifetimes/example-from-issue48686.stderr b/tests/ui/lint/elided-named-lifetimes/example-from-issue48686.stderr
@@ -9,6 +9,10 @@ note: the lint level is defined here
    |
 LL | #![deny(elided_named_lifetimes)]
    |         ^^^^^^^^^^^^^^^^^^^^^^
+help: consider specifying it explicitly
+   |
+LL |     pub fn get_mut(&'static self, x: &mut u8) -> &'static mut u8 {
+   |                                                   +++++++
 
 error: aborting due to 1 previous error
 
diff --git a/tests/ui/lint/elided-named-lifetimes/not-tied-to-crate.stderr b/tests/ui/lint/elided-named-lifetimes/not-tied-to-crate.stderr
@@ -9,6 +9,10 @@ note: the lint level is defined here
    |
 LL | #[warn(elided_named_lifetimes)]
    |        ^^^^^^^^^^^^^^^^^^^^^^
+help: consider specifying it explicitly
+   |
+LL |     fn bar(x: &'static u8) -> &'static u8 {
+   |                                +++++++
 
 error: elided lifetime has a name
   --> $DIR/not-tied-to-crate.rs:11:31
@@ -21,6 +25,10 @@ note: the lint level is defined here
    |
 LL |     #[deny(elided_named_lifetimes)]
    |            ^^^^^^^^^^^^^^^^^^^^^^
+help: consider specifying it explicitly
+   |
+LL |     fn baz(x: &'static u8) -> &'static u8 {
+   |                                +++++++
 
 error: aborting due to 1 previous error; 1 warning emitted
 
diff --git a/tests/ui/lint/elided-named-lifetimes/static.stderr b/tests/ui/lint/elided-named-lifetimes/static.stderr
@@ -9,24 +9,43 @@ note: the lint level is defined here
    |
 LL | #![deny(elided_named_lifetimes)]
    |         ^^^^^^^^^^^^^^^^^^^^^^
+help: consider specifying it explicitly
+   |
+LL | fn ampersand(x: &'static u8) -> &'static u8 {
+   |                                  +++++++
 
 error: elided lifetime has a name
   --> $DIR/static.rs:23:32
    |
 LL | fn brackets(x: &'static u8) -> Brackets {
    |                                ^^^^^^^^ this elided lifetime gets resolved as `'static`
+   |
+help: consider specifying it explicitly
+   |
+LL | fn brackets(x: &'static u8) -> Brackets<'static> {
+   |                                        +++++++++
 
 error: elided lifetime has a name
   --> $DIR/static.rs:30:34
    |
 LL | fn comma(x: &'static u8) -> Comma<u8> {
    |                                  ^ this elided lifetime gets resolved as `'static`
+   |
+help: consider specifying it explicitly
+   |
+LL | fn comma(x: &'static u8) -> Comma<'static, u8> {
+   |                                   ++++++++
 
 error: elided lifetime has a name
   --> $DIR/static.rs:35:35
    |
 LL | fn underscore(x: &'static u8) -> &'_ u8 {
    |                                   ^^ this elided lifetime gets resolved as `'static`
+   |
+help: consider specifying it explicitly
+   |
+LL | fn underscore(x: &'static u8) -> &'static u8 {
+   |                                   ~~~~~~~
 
 error: aborting due to 4 previous errors
 

Original file line number	Diff line number	Diff line change
`@@ -9,6 +9,10 @@ note: the lint level is defined here`
`9`	`9`	`\|`
`10`	`10`	`LL \| #![deny(elided_named_lifetimes)]`
`11`	`11`	`\| ^^^^^^^^^^^^^^^^^^^^^^`
	`12`	`+help: consider specifying it explicitly`
	`13`	`+ \|`
	`14`	`+LL \| pub fn get_mut(&'static self, x: &mut u8) -> &'static mut u8 {`
	`15`	`+ \| +++++++`
`12`	`16`
`13`	`17`	`error: aborting due to 1 previous error`
`14`	`18`