Warn against redundant use<...>
This commit is contained in:
parent
f3fb727b08
commit
1529c661e4
10 changed files with 276 additions and 118 deletions
|
@ -1,9 +1,11 @@
|
|||
use rustc_data_structures::fx::FxIndexSet;
|
||||
use rustc_data_structures::unord::UnordSet;
|
||||
use rustc_errors::{Applicability, LintDiagnostic};
|
||||
use rustc_hir as hir;
|
||||
use rustc_hir::def::DefKind;
|
||||
use rustc_hir::def_id::{DefId, LocalDefId};
|
||||
use rustc_hir::intravisit;
|
||||
use rustc_macros::LintDiagnostic;
|
||||
use rustc_middle::middle::resolve_bound_vars::ResolvedArg;
|
||||
use rustc_middle::ty::{
|
||||
self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor,
|
||||
};
|
||||
|
@ -14,10 +16,12 @@ use rustc_span::{BytePos, Span};
|
|||
use crate::fluent_generated as fluent;
|
||||
use crate::{LateContext, LateLintPass};
|
||||
|
||||
// TODO: feature gate these too
|
||||
|
||||
declare_lint! {
|
||||
/// UwU
|
||||
pub IMPL_TRAIT_OVERCAPTURES,
|
||||
Warn,
|
||||
Allow,
|
||||
"will capture more lifetimes than possibly intended in edition 2024",
|
||||
@future_incompatible = FutureIncompatibleInfo {
|
||||
reason: FutureIncompatibilityReason::EditionSemanticsChange(Edition::Edition2024),
|
||||
|
@ -25,66 +29,64 @@ declare_lint! {
|
|||
};
|
||||
}
|
||||
|
||||
declare_lint! {
|
||||
/// UwU
|
||||
pub IMPL_TRAIT_REDUNDANT_CAPTURES,
|
||||
Warn,
|
||||
"uwu 2"
|
||||
}
|
||||
|
||||
declare_lint_pass!(
|
||||
/// Lint for opaque types that will begin capturing in-scope but unmentioned lifetimes
|
||||
/// in edition 2024.
|
||||
ImplTraitOvercaptures => [IMPL_TRAIT_OVERCAPTURES]
|
||||
ImplTraitOvercaptures => [IMPL_TRAIT_OVERCAPTURES, IMPL_TRAIT_REDUNDANT_CAPTURES]
|
||||
);
|
||||
|
||||
impl<'tcx> LateLintPass<'tcx> for ImplTraitOvercaptures {
|
||||
fn check_fn(
|
||||
&mut self,
|
||||
cx: &LateContext<'tcx>,
|
||||
_: intravisit::FnKind<'tcx>,
|
||||
_: &'tcx hir::FnDecl<'tcx>,
|
||||
_: &'tcx hir::Body<'tcx>,
|
||||
_: Span,
|
||||
parent_def_id: LocalDefId,
|
||||
) {
|
||||
match cx.tcx.def_kind(parent_def_id) {
|
||||
DefKind::AssocFn => {
|
||||
// RPITITs already capture all lifetimes in scope, so skip them.
|
||||
if matches!(
|
||||
cx.tcx.def_kind(cx.tcx.local_parent(parent_def_id)),
|
||||
DefKind::Trait | DefKind::Impl { of_trait: true }
|
||||
) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
DefKind::Fn => {
|
||||
// All free functions need to check for overcaptures.
|
||||
}
|
||||
DefKind::Closure => return,
|
||||
kind => {
|
||||
unreachable!(
|
||||
"expected function item, found {}",
|
||||
kind.descr(parent_def_id.to_def_id())
|
||||
)
|
||||
}
|
||||
fn check_item(&mut self, cx: &LateContext<'tcx>, it: &'tcx hir::Item<'tcx>) {
|
||||
match &it.kind {
|
||||
hir::ItemKind::Fn(..) => check_fn(cx.tcx, it.owner_id.def_id),
|
||||
_ => {}
|
||||
}
|
||||
|
||||
let sig = cx.tcx.fn_sig(parent_def_id).instantiate_identity();
|
||||
|
||||
let mut in_scope_parameters = FxIndexSet::default();
|
||||
// Populate the in_scope_parameters list first with all of the generics in scope
|
||||
let mut current_def_id = Some(parent_def_id.to_def_id());
|
||||
while let Some(def_id) = current_def_id {
|
||||
let generics = cx.tcx.generics_of(def_id);
|
||||
for param in &generics.params {
|
||||
in_scope_parameters.insert(param.def_id);
|
||||
}
|
||||
current_def_id = generics.parent;
|
||||
}
|
||||
|
||||
// Then visit the signature to walk through all the binders (incl. the late-bound
|
||||
// vars on the function itself, which we need to count too).
|
||||
sig.visit_with(&mut VisitOpaqueTypes {
|
||||
tcx: cx.tcx,
|
||||
parent_def_id,
|
||||
in_scope_parameters,
|
||||
seen: Default::default(),
|
||||
});
|
||||
}
|
||||
|
||||
fn check_impl_item(&mut self, cx: &LateContext<'tcx>, it: &'tcx hir::ImplItem<'tcx>) {
|
||||
match &it.kind {
|
||||
hir::ImplItemKind::Fn(_, _) => check_fn(cx.tcx, it.owner_id.def_id),
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
fn check_trait_item(&mut self, cx: &LateContext<'tcx>, it: &'tcx hir::TraitItem<'tcx>) {
|
||||
match &it.kind {
|
||||
hir::TraitItemKind::Fn(_, _) => check_fn(cx.tcx, it.owner_id.def_id),
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn check_fn(tcx: TyCtxt<'_>, parent_def_id: LocalDefId) {
|
||||
let sig = tcx.fn_sig(parent_def_id).instantiate_identity();
|
||||
|
||||
let mut in_scope_parameters = FxIndexSet::default();
|
||||
// Populate the in_scope_parameters list first with all of the generics in scope
|
||||
let mut current_def_id = Some(parent_def_id.to_def_id());
|
||||
while let Some(def_id) = current_def_id {
|
||||
let generics = tcx.generics_of(def_id);
|
||||
for param in &generics.own_params {
|
||||
in_scope_parameters.insert(param.def_id);
|
||||
}
|
||||
current_def_id = generics.parent;
|
||||
}
|
||||
|
||||
// Then visit the signature to walk through all the binders (incl. the late-bound
|
||||
// vars on the function itself, which we need to count too).
|
||||
sig.visit_with(&mut VisitOpaqueTypes {
|
||||
tcx,
|
||||
parent_def_id,
|
||||
in_scope_parameters,
|
||||
seen: Default::default(),
|
||||
});
|
||||
}
|
||||
|
||||
struct VisitOpaqueTypes<'tcx> {
|
||||
|
@ -109,14 +111,11 @@ impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for VisitOpaqueTypes<'tcx> {
|
|||
let unique = self.in_scope_parameters.insert(def_id);
|
||||
assert!(unique);
|
||||
}
|
||||
ty::BoundVariableKind::Ty(_) => {
|
||||
todo!("we don't support late-bound type params in `impl Trait`")
|
||||
}
|
||||
ty::BoundVariableKind::Region(..) => {
|
||||
unreachable!("all AST-derived bound regions should have a name")
|
||||
}
|
||||
ty::BoundVariableKind::Const => {
|
||||
unreachable!("non-lifetime binder consts are not allowed")
|
||||
_ => {
|
||||
self.tcx.dcx().span_delayed_bug(
|
||||
self.tcx.def_span(self.parent_def_id),
|
||||
format!("unsupported bound variable kind: {arg:?}"),
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -131,11 +130,19 @@ impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for VisitOpaqueTypes<'tcx> {
|
|||
}
|
||||
|
||||
fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result {
|
||||
if !t.has_opaque_types() {
|
||||
if !t.has_aliases() {
|
||||
return;
|
||||
}
|
||||
|
||||
if let ty::Alias(ty::Opaque, opaque_ty) = *t.kind()
|
||||
if let ty::Alias(ty::Projection, opaque_ty) = *t.kind()
|
||||
&& self.tcx.is_impl_trait_in_trait(opaque_ty.def_id)
|
||||
{
|
||||
// visit the opaque of the RPITIT
|
||||
self.tcx
|
||||
.type_of(opaque_ty.def_id)
|
||||
.instantiate(self.tcx, opaque_ty.args)
|
||||
.visit_with(self)
|
||||
} else if let ty::Alias(ty::Opaque, opaque_ty) = *t.kind()
|
||||
&& let Some(opaque_def_id) = opaque_ty.def_id.as_local()
|
||||
// Don't recurse infinitely on an opaque
|
||||
&& self.seen.insert(opaque_def_id)
|
||||
|
@ -144,8 +151,6 @@ impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for VisitOpaqueTypes<'tcx> {
|
|||
self.tcx.hir_node_by_def_id(opaque_def_id).expect_item().expect_opaque_ty()
|
||||
&& let hir::OpaqueTyOrigin::FnReturn(parent_def_id) = opaque.origin
|
||||
&& parent_def_id == self.parent_def_id
|
||||
// And if the opaque doesn't already have `use<>` syntax on it...
|
||||
&& opaque.precise_capturing_args.is_none()
|
||||
{
|
||||
// Compute the set of args that are captured by the opaque...
|
||||
let mut captured = FxIndexSet::default();
|
||||
|
@ -178,9 +183,16 @@ impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for VisitOpaqueTypes<'tcx> {
|
|||
.map(|def_id| self.tcx.def_span(def_id))
|
||||
.collect();
|
||||
|
||||
if !uncaptured_spans.is_empty() {
|
||||
let opaque_span = self.tcx.def_span(opaque_def_id);
|
||||
let opaque_span = self.tcx.def_span(opaque_def_id);
|
||||
let new_capture_rules =
|
||||
opaque_span.at_least_rust_2024() || self.tcx.features().lifetime_capture_rules_2024;
|
||||
|
||||
// If we have uncaptured args, and if the opaque doesn't already have
|
||||
// `use<>` syntax on it, and we're < edition 2024, then warn the user.
|
||||
if !new_capture_rules
|
||||
&& opaque.precise_capturing_args.is_none()
|
||||
&& !uncaptured_spans.is_empty()
|
||||
{
|
||||
let suggestion = if let Ok(snippet) =
|
||||
self.tcx.sess.source_map().span_to_snippet(opaque_span)
|
||||
&& snippet.starts_with("impl ")
|
||||
|
@ -207,11 +219,11 @@ impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for VisitOpaqueTypes<'tcx> {
|
|||
None
|
||||
};
|
||||
|
||||
self.tcx.emit_node_lint(
|
||||
self.tcx.emit_node_span_lint(
|
||||
IMPL_TRAIT_OVERCAPTURES,
|
||||
self.tcx.local_def_id_to_hir_id(opaque_def_id),
|
||||
opaque_span,
|
||||
ImplTraitOvercapturesLint {
|
||||
opaque_span,
|
||||
self_ty: t,
|
||||
num_captured: uncaptured_spans.len(),
|
||||
uncaptured_spans,
|
||||
|
@ -219,6 +231,60 @@ impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for VisitOpaqueTypes<'tcx> {
|
|||
},
|
||||
);
|
||||
}
|
||||
// Otherwise, if we are edition 2024, have `use<>` syntax, and
|
||||
// have no uncaptured args, then we should warn to the user that
|
||||
// it's redundant to capture all args explicitly.
|
||||
else if new_capture_rules
|
||||
&& let Some((captured_args, capturing_span)) = opaque.precise_capturing_args
|
||||
{
|
||||
let mut explicitly_captured = UnordSet::default();
|
||||
for arg in captured_args {
|
||||
match self.tcx.named_bound_var(arg.hir_id()) {
|
||||
Some(
|
||||
ResolvedArg::EarlyBound(def_id) | ResolvedArg::LateBound(_, _, def_id),
|
||||
) => {
|
||||
if self.tcx.def_kind(self.tcx.parent(def_id)) == DefKind::OpaqueTy {
|
||||
let (ty::ReEarlyParam(ty::EarlyParamRegion { def_id, .. })
|
||||
| ty::ReLateParam(ty::LateParamRegion {
|
||||
bound_region: ty::BoundRegionKind::BrNamed(def_id, _),
|
||||
..
|
||||
})) = self
|
||||
.tcx
|
||||
.map_opaque_lifetime_to_parent_lifetime(def_id.expect_local())
|
||||
.kind()
|
||||
else {
|
||||
span_bug!(
|
||||
self.tcx.def_span(def_id),
|
||||
"variable should have been duplicated from a parent"
|
||||
);
|
||||
};
|
||||
explicitly_captured.insert(def_id);
|
||||
} else {
|
||||
explicitly_captured.insert(def_id);
|
||||
}
|
||||
}
|
||||
_ => {
|
||||
self.tcx.dcx().span_delayed_bug(
|
||||
self.tcx().hir().span(arg.hir_id()),
|
||||
"no valid for captured arg",
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if self
|
||||
.in_scope_parameters
|
||||
.iter()
|
||||
.all(|def_id| explicitly_captured.contains(def_id))
|
||||
{
|
||||
self.tcx.emit_node_span_lint(
|
||||
IMPL_TRAIT_REDUNDANT_CAPTURES,
|
||||
self.tcx.local_def_id_to_hir_id(opaque_def_id),
|
||||
opaque_span,
|
||||
ImplTraitRedundantCapturesLint { capturing_span },
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Walk into the bounds of the opaque, too, since we want to get nested opaques
|
||||
// in this lint as well. Interestingly, one place that I expect this lint to fire
|
||||
|
@ -236,7 +302,6 @@ impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for VisitOpaqueTypes<'tcx> {
|
|||
}
|
||||
|
||||
struct ImplTraitOvercapturesLint<'tcx> {
|
||||
opaque_span: Span,
|
||||
uncaptured_spans: Vec<Span>,
|
||||
self_ty: Ty<'tcx>,
|
||||
num_captured: usize,
|
||||
|
@ -247,7 +312,6 @@ impl<'a> LintDiagnostic<'a, ()> for ImplTraitOvercapturesLint<'_> {
|
|||
fn decorate_lint<'b>(self, diag: &'b mut rustc_errors::Diag<'a, ()>) {
|
||||
diag.arg("self_ty", self.self_ty.to_string())
|
||||
.arg("num_captured", self.num_captured)
|
||||
.span(self.opaque_span)
|
||||
.span_note(self.uncaptured_spans, fluent::lint_note)
|
||||
.note(fluent::lint_note2);
|
||||
if let Some((suggestion, span)) = self.suggestion {
|
||||
|
@ -265,6 +329,13 @@ impl<'a> LintDiagnostic<'a, ()> for ImplTraitOvercapturesLint<'_> {
|
|||
}
|
||||
}
|
||||
|
||||
#[derive(LintDiagnostic)]
|
||||
#[diag(lint_impl_trait_redundant_captures)]
|
||||
struct ImplTraitRedundantCapturesLint {
|
||||
#[suggestion(lint_suggestion, code = "", applicability = "machine-applicable")]
|
||||
capturing_span: Span,
|
||||
}
|
||||
|
||||
fn extract_def_id_from_arg<'tcx>(
|
||||
tcx: TyCtxt<'tcx>,
|
||||
generics: &'tcx ty::Generics,
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue