Skip to content

Commit af88b73

Browse files
estebankrylev
estebank
authored and
rylev
committed
Clean up code rightward drift
1 parent 59577b8 commit af88b73

File tree

1 file changed

+79
-65
lines changed

1 file changed

+79
-65
lines changed

‎compiler/rustc_lint/src/noop_method_call.rs

Lines changed: 79 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -38,73 +38,87 @@ declare_lint_pass!(NoopMethodCall => [NOOP_METHOD_CALL]);
3838

3939
impl<'tcx> LateLintPass<'tcx> for NoopMethodCall {
4040
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
41-
// We only care about method calls
42-
if let ExprKind::MethodCall(call, _, elements, _) = expr.kind {
43-
// Get the `DefId` only when dealing with an `AssocFn`
44-
if let Some((DefKind::AssocFn, did)) =
45-
cx.typeck_results().type_dependent_def(expr.hir_id)
46-
{
47-
// Check that we're dealing with a trait method
48-
if let Some(trait_id) = cx.tcx.trait_of_item(did) {
49-
// Check we're dealing with one of the traits we care about
50-
if ![sym::Clone, sym::Deref, sym::Borrow]
41+
// We only care about method calls.
42+
let (call, elements) = match expr.kind {
43+
ExprKind::MethodCall(call, _, elements, _) => (call, elements),
44+
_ => return,
45+
};
46+
// We only care about method calls corresponding to the `Clone`, `Deref` and `Borrow`
47+
// traits and ignore any other method call.
48+
let (trait_id, did) = match cx.typeck_results().type_dependent_def(expr.hir_id) {
49+
// Verify we are dealing with a method/associated function.
50+
Some((DefKind::AssocFn, did)) => match cx.tcx.trait_of_item(did) {
51+
// Check that we're dealing with a trait method for one of the traits we care about.
52+
Some(trait_id)
53+
if [sym::Clone, sym::Deref, sym::Borrow]
5154
.iter()
52-
.any(|s| cx.tcx.is_diagnostic_item(*s, trait_id))
53-
{
54-
return;
55-
}
56-
57-
let substs = cx.typeck_results().node_substs(expr.hir_id);
58-
// We can't resolve on types that require monomorphization,
59-
// so check that we don't need to perfom substitution
60-
if !substs.needs_subst() {
61-
let param_env = cx.tcx.param_env(trait_id);
62-
// Resolve the trait method instance
63-
if let Ok(Some(i)) = ty::Instance::resolve(cx.tcx, param_env, did, substs) {
64-
// Check that it implements the noop diagnostic
65-
for (s, peel_ref) in [
66-
(sym::noop_method_borrow, true),
67-
(sym::noop_method_clone, false),
68-
(sym::noop_method_deref, true),
69-
]
70-
.iter()
71-
{
72-
if cx.tcx.is_diagnostic_item(*s, i.def_id()) {
73-
let method = &call.ident.name;
74-
let receiver = &elements[0];
75-
let receiver_ty = cx.typeck_results().expr_ty(receiver);
76-
let receiver_ty = match receiver_ty.kind() {
77-
// Remove one borrow from the receiver as all the trait methods
78-
// we care about here have a `&self` receiver.
79-
ty::Ref(_, ty, _) if *peel_ref => ty,
80-
_ => receiver_ty,
81-
};
82-
let expr_ty = cx.typeck_results().expr_ty_adjusted(expr);
83-
if receiver_ty != expr_ty {
84-
return;
85-
}
86-
let expr_span = expr.span;
87-
let note = format!(
88-
"the type `{:?}` which `{}` is being called on is the same as \
89-
the type returned from `{}`, so the method call does not do \
90-
anything and can be removed",
91-
receiver_ty, method, method,
92-
);
93-
94-
let span = expr_span.with_lo(receiver.span.hi());
95-
cx.struct_span_lint(NOOP_METHOD_CALL, span, |lint| {
96-
let method = &call.ident.name;
97-
let message = format!("call to `.{}()` on a reference in this situation does nothing", &method);
98-
lint.build(&message)
99-
.span_label(span, "unnecessary method call")
100-
.note(&note)
101-
.emit()
102-
});
103-
}
104-
}
105-
}
106-
}
55+
.any(|s| cx.tcx.is_diagnostic_item(*s, trait_id)) =>
56+
{
57+
(trait_id, did)
10758
}
59+
_ => return,
60+
},
61+
_ => return,
62+
};
63+
let substs = cx.typeck_results().node_substs(expr.hir_id);
64+
if substs.needs_subst() {
65+
// We can't resolve on types that require monomorphization, so we don't handle them if
66+
// we need to perfom substitution.
67+
return;
68+
}
69+
let param_env = cx.tcx.param_env(trait_id);
70+
// Resolve the trait method instance.
71+
let i = match ty::Instance::resolve(cx.tcx, param_env, did, substs) {
72+
Ok(Some(i)) => i,
73+
_ => return,
74+
};
75+
// (Re)check that it implements the noop diagnostic.
76+
for (s, peel_ref) in [
77+
(sym::noop_method_borrow, true),
78+
(sym::noop_method_clone, false),
79+
(sym::noop_method_deref, true),
80+
]
81+
.iter()
82+
{
83+
if cx.tcx.is_diagnostic_item(*s, i.def_id()) {
84+
let method = &call.ident.name;
85+
let receiver = &elements[0];
86+
let receiver_ty = cx.typeck_results().expr_ty(receiver);
87+
let receiver_ty = match receiver_ty.kind() {
88+
// Remove one borrow from the receiver if appropriate to positively verify that
89+
// the receiver `&self` type and the return type are the same, depending on the
90+
// involved trait being checked.
91+
ty::Ref(_, ty, _) if *peel_ref => ty,
92+
// When it comes to `Clone` we need to check the `receiver_ty` directly.
93+
// FIXME: we must come up with a better strategy for this.
94+
_ => receiver_ty,
95+
};
96+
let expr_ty = cx.typeck_results().expr_ty_adjusted(expr);
97+
if receiver_ty != expr_ty {
98+
// This lint will only trigger if the receiver type and resulting expression \
99+
// type are the same, implying that the method call is unnecessary.
100+
return;
101+
}
102+
let expr_span = expr.span;
103+
let note = format!(
104+
"the type `{:?}` which `{}` is being called on is the same as \
105+
the type returned from `{}`, so the method call does not do \
106+
anything and can be removed",
107+
receiver_ty, method, method,
108+
);
109+
110+
let span = expr_span.with_lo(receiver.span.hi());
111+
cx.struct_span_lint(NOOP_METHOD_CALL, span, |lint| {
112+
let method = &call.ident.name;
113+
let message = format!(
114+
"call to `.{}()` on a reference in this situation does nothing",
115+
&method,
116+
);
117+
lint.build(&message)
118+
.span_label(span, "unnecessary method call")
119+
.note(&note)
120+
.emit()
121+
});
108122
}
109123
}
110124
}

0 commit comments

Comments
 (0)