Skip to content

Commit

Permalink
Clean up code rightward drift
Browse files Browse the repository at this point in the history
  • Loading branch information
@estebank @rylev
estebank authored and rylev committed Mar 3, 2021
1 parent e48670c commit 055db16
Showing 1 changed file with 79 additions and 65 deletions.
144 changes: 79 additions & 65 deletions compiler/rustc_lint/src/noop_method_call.rs
View file
Expand Up @@ -38,73 +38,87 @@ declare_lint_pass!(NoopMethodCall => [NOOP_METHOD_CALL]);

impl<'tcx> LateLintPass<'tcx> for NoopMethodCall {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
// We only care about method calls
if let ExprKind::MethodCall(call, _, elements, _) = expr.kind {
// Get the `DefId` only when dealing with an `AssocFn`
if let Some((DefKind::AssocFn, did)) =
cx.typeck_results().type_dependent_def(expr.hir_id)
{
// Check that we're dealing with a trait method
if let Some(trait_id) = cx.tcx.trait_of_item(did) {
// Check we're dealing with one of the traits we care about
if ![sym::Clone, sym::Deref, sym::Borrow]
// We only care about method calls.
let (call, elements) = match expr.kind {
ExprKind::MethodCall(call, _, elements, _) => (call, elements),
_ => return,
};
// We only care about method calls corresponding to the `Clone`, `Deref` and `Borrow`
// traits and ignore any other method call.
let (trait_id, did) = match cx.typeck_results().type_dependent_def(expr.hir_id) {
// Verify we are dealing with a method/associated function.
Some((DefKind::AssocFn, did)) => match cx.tcx.trait_of_item(did) {
// Check that we're dealing with a trait method for one of the traits we care about.
Some(trait_id)
if [sym::Clone, sym::Deref, sym::Borrow]
.iter()
.any(|s| cx.tcx.is_diagnostic_item(*s, trait_id))
{
return;
}

let substs = cx.typeck_results().node_substs(expr.hir_id);
// We can't resolve on types that require monomorphization,
// so check that we don't need to perfom substitution
if !substs.needs_subst() {
let param_env = cx.tcx.param_env(trait_id);
// Resolve the trait method instance
if let Ok(Some(i)) = ty::Instance::resolve(cx.tcx, param_env, did, substs) {
// Check that it implements the noop diagnostic
for (s, peel_ref) in [
(sym::noop_method_borrow, true),
(sym::noop_method_clone, false),
(sym::noop_method_deref, true),
]
.iter()
{
if cx.tcx.is_diagnostic_item(*s, i.def_id()) {
let method = &call.ident.name;
let receiver = &elements[0];
let receiver_ty = cx.typeck_results().expr_ty(receiver);
let receiver_ty = match receiver_ty.kind() {
// Remove one borrow from the receiver as all the trait methods
// we care about here have a `&self` receiver.
ty::Ref(_, ty, _) if *peel_ref => ty,
_ => receiver_ty,
};
let expr_ty = cx.typeck_results().expr_ty_adjusted(expr);
if receiver_ty != expr_ty {
return;
}
let expr_span = expr.span;
let note = format!(
"the type `{:?}` which `{}` is being called on is the same as \
the type returned from `{}`, so the method call does not do \
anything and can be removed",
receiver_ty, method, method,
);

let span = expr_span.with_lo(receiver.span.hi());
cx.struct_span_lint(NOOP_METHOD_CALL, span, |lint| {
let method = &call.ident.name;
let message = format!("call to `.{}()` on a reference in this situation does nothing", &method);
lint.build(&message)
.span_label(span, "unnecessary method call")
.note(&note)
.emit()
});
}
}
}
}
.any(|s| cx.tcx.is_diagnostic_item(*s, trait_id)) =>
{
(trait_id, did)
}
_ => return,
},
_ => return,
};
let substs = cx.typeck_results().node_substs(expr.hir_id);
if substs.needs_subst() {
// We can't resolve on types that require monomorphization, so we don't handle them if
// we need to perfom substitution.
return;
}
let param_env = cx.tcx.param_env(trait_id);
// Resolve the trait method instance.
let i = match ty::Instance::resolve(cx.tcx, param_env, did, substs) {
Ok(Some(i)) => i,
_ => return,
};
// (Re)check that it implements the noop diagnostic.
for (s, peel_ref) in [
(sym::noop_method_borrow, true),
(sym::noop_method_clone, false),
(sym::noop_method_deref, true),
]
.iter()
{
if cx.tcx.is_diagnostic_item(*s, i.def_id()) {
let method = &call.ident.name;
let receiver = &elements[0];
let receiver_ty = cx.typeck_results().expr_ty(receiver);
let receiver_ty = match receiver_ty.kind() {
// Remove one borrow from the receiver if appropriate to positively verify that
// the receiver `&self` type and the return type are the same, depending on the
// involved trait being checked.
ty::Ref(_, ty, _) if *peel_ref => ty,
// When it comes to `Clone` we need to check the `receiver_ty` directly.
// FIXME: we must come up with a better strategy for this.
_ => receiver_ty,
};
let expr_ty = cx.typeck_results().expr_ty_adjusted(expr);
if receiver_ty != expr_ty {
// This lint will only trigger if the receiver type and resulting expression \
// type are the same, implying that the method call is unnecessary.
return;
}
let expr_span = expr.span;
let note = format!(
"the type `{:?}` which `{}` is being called on is the same as \
the type returned from `{}`, so the method call does not do \
anything and can be removed",
receiver_ty, method, method,
);

let span = expr_span.with_lo(receiver.span.hi());
cx.struct_span_lint(NOOP_METHOD_CALL, span, |lint| {
let method = &call.ident.name;
let message = format!(
"call to `.{}()` on a reference in this situation does nothing",
&method,
);
lint.build(&message)
.span_label(span, "unnecessary method call")
.note(&note)
.emit()
});
}
}
}
Expand Down

0 comments on commit 055db16

Please sign in to comment.