Implement elided_named_lifetimes lint

This commit is contained in:
Pavel Grigorenko 2024-08-15 18:05:59 +03:00
parent 0d634185df
commit 5d04472461
10 changed files with 154 additions and 20 deletions

View File

@ -837,7 +837,7 @@ fn lifetime_res_to_generic_param(
(hir::ParamName::Fresh, hir::LifetimeParamKind::Elided(kind))
}
LifetimeRes::Static | LifetimeRes::Error => return None,
LifetimeRes::Static { .. } | LifetimeRes::Error => return None,
res => panic!(
"Unexpected lifetime resolution {:?} for {:?} at {:?}",
res, ident, ident.span
@ -1656,7 +1656,7 @@ fn lower_opaque_inner(
}
// Opaques do not capture `'static`
LifetimeRes::Static | LifetimeRes::Error => {
LifetimeRes::Static { .. } | LifetimeRes::Error => {
continue;
}
@ -2069,7 +2069,7 @@ fn new_named_lifetime_with_res(
hir::LifetimeName::Param(param)
}
LifetimeRes::Infer => hir::LifetimeName::Infer,
LifetimeRes::Static => hir::LifetimeName::Static,
LifetimeRes::Static { .. } => hir::LifetimeName::Static,
LifetimeRes::Error => hir::LifetimeName::Error,
res => panic!(
"Unexpected lifetime resolution {:?} for {:?} at {:?}",

View File

@ -27,7 +27,7 @@ fn record_lifetime_use(&mut self, lifetime: Lifetime) {
self.collected_lifetimes.insert(lifetime);
}
}
LifetimeRes::Static | LifetimeRes::Error => {
LifetimeRes::Static { .. } | LifetimeRes::Error => {
self.collected_lifetimes.insert(lifetime);
}
LifetimeRes::Infer => {}

View File

@ -863,8 +863,13 @@ pub enum LifetimeRes {
/// This variant is used for anonymous lifetimes that we did not resolve during
/// late resolution. Those lifetimes will be inferred by typechecking.
Infer,
/// Explicit `'static` lifetime.
Static,
/// `'static` lifetime.
Static {
/// We do not want to emit `elided_named_lifetimes`
/// when we are inside of a const item or a static,
/// because it would get too annoying.
suppress_elision_warning: bool,
},
/// Resolution failure.
Error,
/// HACK: This is used to recover the NodeId of an elided lifetime.

View File

@ -252,6 +252,10 @@ lint_duplicate_macro_attribute =
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
lint_enum_intrinsics_mem_discriminant =
the return value of `mem::discriminant` is unspecified when called with a non-enum type
.note = the argument to `discriminant` should be a reference to an enum, but it was passed a reference to a `{$ty_param}`, which is not an enum

View File

@ -10,6 +10,7 @@
use rustc_middle::middle::stability;
use rustc_session::lint::BuiltinLintDiag;
use rustc_session::Session;
use rustc_span::symbol::kw;
use rustc_span::BytePos;
use tracing::debug;
@ -441,5 +442,17 @@ 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),
}
.decorate_lint(diag)
}
}
}

View File

@ -2611,6 +2611,16 @@ 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 name: Symbol,
#[label(lint_label_named)]
pub named_declaration: Option<Span>,
}
#[derive(LintDiagnostic)]
#[diag(lint_invalid_crate_type_value)]
pub(crate) struct UnknownCrateTypes {

View File

@ -42,6 +42,7 @@
DUPLICATE_MACRO_ATTRIBUTES,
ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT,
ELIDED_LIFETIMES_IN_PATHS,
ELIDED_NAMED_LIFETIMES,
EXPLICIT_BUILTIN_CFGS_IN_FLAGS,
EXPORTED_PRIVATE_DEPENDENCIES,
FFI_UNWIND_CALLS,
@ -1862,6 +1863,38 @@
"hidden lifetime parameters in types are deprecated"
}
declare_lint! {
/// The `elided_named_lifetimes` lint detects when an elided
/// lifetime ends up being a named lifetime, such as `'static`
/// or some lifetime parameter `'a`.
///
/// ### Example
///
/// ```rust,compile_fail
/// #![deny(elided_named_lifetimes)]
/// struct Foo;
/// impl Foo {
/// pub fn get_mut(&'static self, x: &mut u8) -> &mut u8 {
/// unsafe { &mut *(x as *mut _) }
/// }
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// Lifetime elision is quite useful, because it frees you from having
/// to give each lifetime its own name, but sometimes it can produce
/// somewhat surprising resolutions. In safe code, it is mostly okay,
/// because the borrow checker prevents any unsoundness, so the worst
/// case scenario is you get a confusing error message in some other place.
/// But with `unsafe` code, such unexpected resolutions may lead to unsound code.
pub ELIDED_NAMED_LIFETIMES,
Warn,
"detects when an elided lifetime gets resolved to be `'static` or some named parameter"
}
declare_lint! {
/// The `bare_trait_objects` lint suggests using `dyn Trait` for trait
/// objects.

View File

@ -589,6 +589,13 @@ pub enum BuiltinLintDiag {
},
MacroExpandedMacroExportsAccessedByAbsolutePaths(Span),
ElidedLifetimesInPaths(usize, Span, bool, Span),
ElidedIsStatic {
elided: Span,
},
ElidedIsParam {
elided: Span,
param: (Symbol, Span),
},
UnknownCrateTypes {
span: Span,
candidate: Option<Symbol>,

View File

@ -1557,14 +1557,14 @@ fn resolve_lifetime(&mut self, lifetime: &'ast Lifetime, use_ctxt: visit::Lifeti
if ident.name == kw::StaticLifetime {
self.record_lifetime_res(
lifetime.id,
LifetimeRes::Static,
LifetimeRes::Static { suppress_elision_warning: false },
LifetimeElisionCandidate::Named,
);
return;
}
if ident.name == kw::UnderscoreLifetime {
return self.resolve_anonymous_lifetime(lifetime, false);
return self.resolve_anonymous_lifetime(lifetime, lifetime.id, false);
}
let mut lifetime_rib_iter = self.lifetime_ribs.iter().rev();
@ -1667,13 +1667,23 @@ fn resolve_lifetime(&mut self, lifetime: &'ast Lifetime, use_ctxt: visit::Lifeti
}
#[instrument(level = "debug", skip(self))]
fn resolve_anonymous_lifetime(&mut self, lifetime: &Lifetime, elided: bool) {
fn resolve_anonymous_lifetime(
&mut self,
lifetime: &Lifetime,
id_for_lint: NodeId,
elided: bool,
) {
debug_assert_eq!(lifetime.ident.name, kw::UnderscoreLifetime);
let kind =
if elided { MissingLifetimeKind::Ampersand } else { MissingLifetimeKind::Underscore };
let missing_lifetime =
MissingLifetime { id: lifetime.id, span: lifetime.ident.span, kind, count: 1 };
let missing_lifetime = MissingLifetime {
id: lifetime.id,
span: lifetime.ident.span,
kind,
count: 1,
id_for_lint,
};
let elision_candidate = LifetimeElisionCandidate::Missing(missing_lifetime);
for (i, rib) in self.lifetime_ribs.iter().enumerate().rev() {
debug!(?rib.kind);
@ -1697,7 +1707,8 @@ fn resolve_anonymous_lifetime(&mut self, lifetime: &Lifetime, elided: bool) {
if lifetimes_in_scope.is_empty() {
self.record_lifetime_res(
lifetime.id,
LifetimeRes::Static,
// We are inside a const item, so do not warn.
LifetimeRes::Static { suppress_elision_warning: true },
elision_candidate,
);
return;
@ -1800,7 +1811,7 @@ fn resolve_elided_lifetime(&mut self, anchor_id: NodeId, span: Span) {
LifetimeRes::ElidedAnchor { start: id, end: NodeId::from_u32(id.as_u32() + 1) },
LifetimeElisionCandidate::Ignore,
);
self.resolve_anonymous_lifetime(&lt, true);
self.resolve_anonymous_lifetime(&lt, anchor_id, true);
}
#[instrument(level = "debug", skip(self))]
@ -1916,6 +1927,7 @@ fn resolve_elided_lifetimes_in_path(
};
let missing_lifetime = MissingLifetime {
id: node_ids.start,
id_for_lint: segment_id,
span: elided_lifetime_span,
kind,
count: expected_lifetimes,
@ -2039,8 +2051,44 @@ fn record_lifetime_res(
if let Some(prev_res) = self.r.lifetimes_res_map.insert(id, res) {
panic!("lifetime {id:?} resolved multiple times ({prev_res:?} before, {res:?} now)")
}
match candidate {
LifetimeElisionCandidate::Missing(missing @ MissingLifetime { .. }) => {
debug_assert_eq!(id, missing.id);
match res {
LifetimeRes::Static { suppress_elision_warning } => {
if !suppress_elision_warning {
self.r.lint_buffer.buffer_lint(
lint::builtin::ELIDED_NAMED_LIFETIMES,
missing.id_for_lint,
missing.span,
BuiltinLintDiag::ElidedIsStatic { elided: missing.span },
);
}
}
LifetimeRes::Param { param, binder: _ } => {
let tcx = self.r.tcx();
self.r.lint_buffer.buffer_lint(
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)),
},
);
}
LifetimeRes::Fresh { .. }
| LifetimeRes::Infer
| LifetimeRes::Error
| LifetimeRes::ElidedAnchor { .. } => {}
}
}
LifetimeElisionCandidate::Ignore | LifetimeElisionCandidate::Named => {}
}
match res {
LifetimeRes::Param { .. } | LifetimeRes::Fresh { .. } | LifetimeRes::Static => {
LifetimeRes::Param { .. } | LifetimeRes::Fresh { .. } | LifetimeRes::Static { .. } => {
if let Some(ref mut candidates) = self.lifetime_elision_candidates {
candidates.push((res, candidate));
}
@ -2558,9 +2606,14 @@ fn resolve_item(&mut self, item: &'ast Item) {
ItemKind::Static(box ast::StaticItem { ref ty, ref expr, .. }) => {
self.with_static_rib(def_kind, |this| {
this.with_lifetime_rib(LifetimeRibKind::Elided(LifetimeRes::Static), |this| {
this.visit_ty(ty);
});
this.with_lifetime_rib(
LifetimeRibKind::Elided(LifetimeRes::Static {
suppress_elision_warning: true,
}),
|this| {
this.visit_ty(ty);
},
);
if let Some(expr) = expr {
// We already forbid generic params because of the above item rib,
// so it doesn't matter whether this is a trivial constant.
@ -2589,7 +2642,9 @@ fn resolve_item(&mut self, item: &'ast Item) {
this.visit_generics(generics);
this.with_lifetime_rib(
LifetimeRibKind::Elided(LifetimeRes::Static),
LifetimeRibKind::Elided(LifetimeRes::Static {
suppress_elision_warning: true,
}),
|this| this.visit_ty(ty),
);

View File

@ -102,6 +102,13 @@ fn import_candidate_to_enum_paths(suggestion: &ImportSuggestion) -> (String, Str
pub(super) struct MissingLifetime {
/// Used to overwrite the resolution with the suggestion, to avoid cascading errors.
pub id: NodeId,
/// As we cannot yet emit lints in this crate and have to buffer them instead,
/// we need to associate each lint with some `NodeId`,
/// however for some `MissingLifetime`s their `NodeId`s are "fake",
/// in a sense that they are temporary and not get preserved down the line,
/// which means that the lints for those nodes will not get emitted.
/// To combat this, we can try to use some other `NodeId`s as a fallback option.
pub id_for_lint: NodeId,
/// Where to suggest adding the lifetime.
pub span: Span,
/// How the lifetime was introduced, to have the correct space and comma.
@ -3028,7 +3035,7 @@ fn add_missing_lifetime_specifiers_label(
maybe_static = true;
in_scope_lifetimes = vec![(
Ident::with_dummy_span(kw::StaticLifetime),
(DUMMY_NODE_ID, LifetimeRes::Static),
(DUMMY_NODE_ID, LifetimeRes::Static { suppress_elision_warning: false }),
)];
}
} else if elided_len == 0 {
@ -3040,7 +3047,7 @@ fn add_missing_lifetime_specifiers_label(
maybe_static = true;
in_scope_lifetimes = vec![(
Ident::with_dummy_span(kw::StaticLifetime),
(DUMMY_NODE_ID, LifetimeRes::Static),
(DUMMY_NODE_ID, LifetimeRes::Static { suppress_elision_warning: false }),
)];
}
} else if num_params == 1 {