Skip to content

Commit 8aae8a4

Browse files
ada4aada4a
committed
create let-chain
1 parent f35c203 commit 8aae8a4

File tree

1 file changed

+106
-116
lines changed

1 file changed

+106
-116
lines changed

‎clippy_lints/src/non_canonical_impls.rs‎

Lines changed: 106 additions & 116 deletions
Original file line numberDiff line numberDiff line change
@@ -114,136 +114,126 @@ declare_lint_pass!(NonCanonicalImpls => [NON_CANONICAL_CLONE_IMPL, NON_CANONICAL
114114
impl LateLintPass<'_> for NonCanonicalImpls {
115115
#[expect(clippy::too_many_lines)]
116116
fn check_impl_item<'tcx>(&mut self, cx: &LateContext<'tcx>, impl_item: &ImplItem<'tcx>) {
117-
let Node::Item(item) = cx.tcx.parent_hir_node(impl_item.hir_id()) else {
118-
return;
119-
};
120-
let Some(trait_impl) = cx.tcx.impl_trait_ref(item.owner_id).map(EarlyBinder::skip_binder) else {
121-
return;
122-
};
123-
if cx.tcx.is_automatically_derived(item.owner_id.to_def_id()) {
124-
return;
125-
}
126-
let ImplItemKind::Fn(_, impl_item_id) = cx.tcx.hir_impl_item(impl_item.impl_item_id()).kind else {
127-
return;
128-
};
129-
let body = cx.tcx.hir_body(impl_item_id);
130-
let ExprKind::Block(block, ..) = body.value.kind else {
131-
return;
132-
};
133-
if block.span.in_external_macro(cx.sess().source_map()) || is_from_proc_macro(cx, impl_item) {
134-
return;
135-
}
136-
137-
let trait_name = cx.tcx.get_diagnostic_name(trait_impl.def_id);
138-
if trait_name == Some(sym::Clone)
139-
&& let Some(copy_def_id) = cx.tcx.get_diagnostic_item(sym::Copy)
140-
&& implements_trait(cx, trait_impl.self_ty(), copy_def_id, &[])
117+
if let Node::Item(item) = cx.tcx.parent_hir_node(impl_item.hir_id())
118+
&& let Some(trait_impl) = cx.tcx.impl_trait_ref(item.owner_id).map(EarlyBinder::skip_binder)
119+
&& !cx.tcx.is_automatically_derived(item.owner_id.to_def_id())
120+
&& let ImplItemKind::Fn(_, impl_item_id) = cx.tcx.hir_impl_item(impl_item.impl_item_id()).kind
121+
&& let body = cx.tcx.hir_body(impl_item_id)
122+
&& let ExprKind::Block(block, ..) = body.value.kind
123+
&& !block.span.in_external_macro(cx.sess().source_map())
124+
&& !is_from_proc_macro(cx, impl_item)
141125
{
142-
if impl_item.ident.name == sym::clone {
143-
if block.stmts.is_empty()
144-
&& let Some(expr) = block.expr
145-
&& let ExprKind::Unary(UnOp::Deref, deref) = expr.kind
146-
&& let ExprKind::Path(qpath) = deref.kind
147-
&& last_path_segment(&qpath).ident.name == kw::SelfLower
148-
{
149-
} else {
126+
let trait_name = cx.tcx.get_diagnostic_name(trait_impl.def_id);
127+
if trait_name == Some(sym::Clone)
128+
&& let Some(copy_def_id) = cx.tcx.get_diagnostic_item(sym::Copy)
129+
&& implements_trait(cx, trait_impl.self_ty(), copy_def_id, &[])
130+
{
131+
if impl_item.ident.name == sym::clone {
132+
if block.stmts.is_empty()
133+
&& let Some(expr) = block.expr
134+
&& let ExprKind::Unary(UnOp::Deref, deref) = expr.kind
135+
&& let ExprKind::Path(qpath) = deref.kind
136+
&& last_path_segment(&qpath).ident.name == kw::SelfLower
137+
{
138+
} else {
139+
span_lint_and_sugg(
140+
cx,
141+
NON_CANONICAL_CLONE_IMPL,
142+
block.span,
143+
"non-canonical implementation of `clone` on a `Copy` type",
144+
"change this to",
145+
"{ *self }".to_owned(),
146+
Applicability::MaybeIncorrect,
147+
);
148+
149+
return;
150+
}
151+
}
152+
153+
if impl_item.ident.name == sym::clone_from {
150154
span_lint_and_sugg(
151155
cx,
152156
NON_CANONICAL_CLONE_IMPL,
153-
block.span,
154-
"non-canonical implementation of `clone` on a `Copy` type",
155-
"change this to",
156-
"{ *self }".to_owned(),
157+
impl_item.span,
158+
"unnecessary implementation of `clone_from` on a `Copy` type",
159+
"remove it",
160+
String::new(),
157161
Applicability::MaybeIncorrect,
158162
);
163+
}
164+
} else if trait_name == Some(sym::PartialOrd)
165+
&& impl_item.ident.name == sym::partial_cmp
166+
&& let Some(ord_def_id) = cx.tcx.get_diagnostic_item(sym::Ord)
167+
&& implements_trait(cx, trait_impl.self_ty(), ord_def_id, &[])
168+
{
169+
// If the `cmp` call likely needs to be fully qualified in the suggestion
170+
// (like `std::cmp::Ord::cmp`). It's unfortunate we must put this here but we can't
171+
// access `cmp_expr` in the suggestion without major changes, as we lint in `else`.
172+
let mut needs_fully_qualified = false;
159173

174+
if block.stmts.is_empty()
175+
&& let Some(expr) = block.expr
176+
&& expr_is_cmp(cx, expr, impl_item, &mut needs_fully_qualified)
177+
{
178+
return;
179+
}
180+
// Fix #12683, allow [`needless_return`] here
181+
else if block.expr.is_none()
182+
&& let Some(stmt) = block.stmts.first()
183+
&& let rustc_hir::StmtKind::Semi(Expr {
184+
kind: ExprKind::Ret(Some(ret)),
185+
..
186+
}) = stmt.kind
187+
&& expr_is_cmp(cx, ret, impl_item, &mut needs_fully_qualified)
188+
{
189+
return;
190+
}
191+
// If `Self` and `Rhs` are not the same type, bail. This makes creating a valid
192+
// suggestion tons more complex.
193+
else if let [lhs, rhs, ..] = trait_impl.args.as_slice()
194+
&& lhs != rhs
195+
{
160196
return;
161197
}
162-
}
163198

164-
if impl_item.ident.name == sym::clone_from {
165-
span_lint_and_sugg(
199+
span_lint_and_then(
166200
cx,
167-
NON_CANONICAL_CLONE_IMPL,
168-
impl_item.span,
169-
"unnecessary implementation of `clone_from` on a `Copy` type",
170-
"remove it",
171-
String::new(),
172-
Applicability::MaybeIncorrect,
173-
);
174-
}
175-
} else if trait_name == Some(sym::PartialOrd)
176-
&& impl_item.ident.name == sym::partial_cmp
177-
&& let Some(ord_def_id) = cx.tcx.get_diagnostic_item(sym::Ord)
178-
&& implements_trait(cx, trait_impl.self_ty(), ord_def_id, &[])
179-
{
180-
// If the `cmp` call likely needs to be fully qualified in the suggestion
181-
// (like `std::cmp::Ord::cmp`). It's unfortunate we must put this here but we can't
182-
// access `cmp_expr` in the suggestion without major changes, as we lint in `else`.
183-
let mut needs_fully_qualified = false;
184-
185-
if block.stmts.is_empty()
186-
&& let Some(expr) = block.expr
187-
&& expr_is_cmp(cx, expr, impl_item, &mut needs_fully_qualified)
188-
{
189-
return;
190-
}
191-
// Fix #12683, allow [`needless_return`] here
192-
else if block.expr.is_none()
193-
&& let Some(stmt) = block.stmts.first()
194-
&& let rustc_hir::StmtKind::Semi(Expr {
195-
kind: ExprKind::Ret(Some(ret)),
196-
..
197-
}) = stmt.kind
198-
&& expr_is_cmp(cx, ret, impl_item, &mut needs_fully_qualified)
199-
{
200-
return;
201-
}
202-
// If `Self` and `Rhs` are not the same type, bail. This makes creating a valid
203-
// suggestion tons more complex.
204-
else if let [lhs, rhs, ..] = trait_impl.args.as_slice()
205-
&& lhs != rhs
206-
{
207-
return;
208-
}
209-
210-
span_lint_and_then(
211-
cx,
212-
NON_CANONICAL_PARTIAL_ORD_IMPL,
213-
item.span,
214-
"non-canonical implementation of `partial_cmp` on an `Ord` type",
215-
|diag| {
216-
let [_, other] = body.params else {
217-
return;
218-
};
219-
let Some(std_or_core) = std_or_core(cx) else {
220-
return;
221-
};
201+
NON_CANONICAL_PARTIAL_ORD_IMPL,
202+
item.span,
203+
"non-canonical implementation of `partial_cmp` on an `Ord` type",
204+
|diag| {
205+
let [_, other] = body.params else {
206+
return;
207+
};
208+
let Some(std_or_core) = std_or_core(cx) else {
209+
return;
210+
};
222211

223-
let suggs = match (other.pat.simple_ident(), needs_fully_qualified) {
224-
(Some(other_ident), true) => vec![(
225-
block.span,
226-
format!("{{ Some({std_or_core}::cmp::Ord::cmp(self, {})) }}", other_ident.name),
227-
)],
228-
(Some(other_ident), false) => {
229-
vec![(block.span, format!("{{ Some(self.cmp({})) }}", other_ident.name))]
230-
},
231-
(None, true) => vec![
232-
(
212+
let suggs = match (other.pat.simple_ident(), needs_fully_qualified) {
213+
(Some(other_ident), true) => vec![(
233214
block.span,
234-
format!("{{ Some({std_or_core}::cmp::Ord::cmp(self, other)) }}"),
235-
),
236-
(other.pat.span, "other".to_owned()),
237-
],
238-
(None, false) => vec![
239-
(block.span, "{ Some(self.cmp(other)) }".to_owned()),
240-
(other.pat.span, "other".to_owned()),
241-
],
242-
};
215+
format!("{{ Some({std_or_core}::cmp::Ord::cmp(self, {})) }}", other_ident.name),
216+
)],
217+
(Some(other_ident), false) => {
218+
vec![(block.span, format!("{{ Some(self.cmp({})) }}", other_ident.name))]
219+
},
220+
(None, true) => vec![
221+
(
222+
block.span,
223+
format!("{{ Some({std_or_core}::cmp::Ord::cmp(self, other)) }}"),
224+
),
225+
(other.pat.span, "other".to_owned()),
226+
],
227+
(None, false) => vec![
228+
(block.span, "{ Some(self.cmp(other)) }".to_owned()),
229+
(other.pat.span, "other".to_owned()),
230+
],
231+
};
243232

244-
diag.multipart_suggestion("change this to", suggs, Applicability::Unspecified);
245-
},
246-
);
233+
diag.multipart_suggestion("change this to", suggs, Applicability::Unspecified);
234+
},
235+
);
236+
}
247237
}
248238
}
249239
}

0 commit comments

Comments
 (0)