Skip to content

Commit 60aeef0

Browse files
committed
heavily refactor + bump clippy::version
1 parent c10fc0f commit 60aeef0

File tree

1 file changed

+59
-76
lines changed

1 file changed

+59
-76
lines changed

clippy_lints/src/manual_partial_ord_and_ord_impl.rs

Lines changed: 59 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
1-
use clippy_utils::diagnostics::span_lint_and_then;
2-
use clippy_utils::{match_qpath, path_res, ty::implements_trait};
1+
use clippy_utils::{diagnostics::span_lint_and_then, get_parent_node, match_qpath, path_res, ty::implements_trait};
32
use rustc_errors::Applicability;
4-
use rustc_hir::def::Res;
5-
use rustc_hir::{Expr, ExprKind, Impl, ImplItemKind, Item, ItemKind, PatKind};
3+
use rustc_hir::{def::Res, Expr, ExprKind, ImplItem, ImplItemKind, ItemKind, Node, PatKind};
64
use rustc_hir_analysis::hir_ty_to_ty;
75
use rustc_lint::{LateContext, LateLintPass};
8-
use rustc_middle::ty::{EarlyBinder, TraitRef};
6+
use rustc_middle::ty::EarlyBinder;
97
use rustc_session::{declare_lint_pass, declare_tool_lint};
108
use rustc_span::sym;
119

@@ -53,86 +51,71 @@ declare_clippy_lint! {
5351
/// }
5452
/// }
5553
/// ```
56-
#[clippy::version = "1.71.0"]
54+
#[clippy::version = "1.72.0"]
5755
pub MANUAL_PARTIAL_ORD_AND_ORD_IMPL,
5856
correctness,
5957
"manual implementation of `PartialOrd` when `Ord` is already implemented"
6058
}
6159
declare_lint_pass!(ManualPartialOrdAndOrdImpl => [MANUAL_PARTIAL_ORD_AND_ORD_IMPL]);
6260

6361
impl LateLintPass<'_> for ManualPartialOrdAndOrdImpl {
64-
fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
65-
if_chain! {
66-
if let ItemKind::Impl(imp) = &item.kind;
67-
if let Some(impl_trait_ref) = cx.tcx.impl_trait_ref(item.owner_id).map(EarlyBinder::skip_binder);
68-
let partial_ord_def_id = impl_trait_ref.def_id;
69-
if cx.tcx.is_diagnostic_item(sym::PartialOrd, partial_ord_def_id);
70-
if !cx.tcx.is_automatically_derived(item.owner_id.to_def_id());
71-
then {
72-
lint_impl_body(cx, imp, item, impl_trait_ref);
73-
}
74-
}
75-
}
76-
}
77-
78-
fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, imp: &Impl<'_>, item: &Item<'_>, impl_trait_ref: TraitRef<'tcx>) {
79-
for imp_item in imp.items {
80-
if_chain! {
81-
if imp_item.ident.name == sym::partial_cmp;
82-
if let ImplItemKind::Fn(_, id) = cx.tcx.hir().impl_item(imp_item.id).kind;
83-
then {
84-
let body = cx.tcx.hir().body(id);
85-
let ord_def_id = cx
86-
.tcx
87-
.diagnostic_items(impl_trait_ref.def_id.krate)
88-
.name_to_id
89-
.get(&sym::Ord)
90-
.unwrap();
91-
if let ExprKind::Block(block, ..) = body.value.kind && implements_trait(
92-
cx,
93-
hir_ty_to_ty(cx.tcx, imp.self_ty),
94-
*ord_def_id,
95-
impl_trait_ref.substs,
96-
)
97-
{
98-
if_chain! {
99-
if block.stmts.is_empty();
100-
if let Some(expr) = block.expr;
101-
if let ExprKind::Call(Expr { kind: ExprKind::Path(some_path), ..}, [cmp_expr]) = expr.kind;
102-
// TODO: We can likely use the `option_some_variant` lang item instead, but this may be tricky
103-
// due to the fact that `expr` is the constructor, not `option_some_variant` itself.
104-
if match_qpath(some_path, &["Some"]);
105-
if let ExprKind::MethodCall(cmp_path, _, [other_expr], ..) = cmp_expr.kind;
106-
if cmp_path.ident.name == sym::cmp;
107-
if let Res::Local(..) = path_res(cx, other_expr);
108-
then {}
109-
else {
110-
span_lint_and_then(
111-
cx,
112-
MANUAL_PARTIAL_ORD_AND_ORD_IMPL,
113-
item.span,
114-
"manual implementation of `PartialOrd` when `Ord` is already implemented",
115-
// TODO: Is this how this should be done? Should we instead suggest
116-
// changing the entire block, including the name
117-
// of (by default) other if we can't find it?
118-
|diag| {
119-
if let Some(param) = body.params.get(1)
120-
&& let PatKind::Binding(_, _, param_ident, ..) = param.pat.kind
121-
{
122-
diag.span_suggestion(
123-
block.span,
124-
"change this to",
125-
format!("{{ Some(self.cmp({})) }}", param_ident.name),
126-
Applicability::Unspecified,
127-
);
128-
} else {
129-
diag.help("return the value of `self.cmp` wrapped in `Some` instead");
130-
};
131-
}
62+
fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &ImplItem<'_>) {
63+
let node = get_parent_node(cx.tcx, impl_item.hir_id());
64+
if let Some(Node::Item(item)) = node
65+
&& let ItemKind::Impl(imp) = item.kind
66+
&& let Some(partial_ord_trait) = cx.tcx.impl_trait_ref(item.owner_id).map(EarlyBinder::skip_binder)
67+
&& let partial_ord_def_id = partial_ord_trait.def_id
68+
&& cx.tcx.is_diagnostic_item(sym::PartialOrd, partial_ord_def_id)
69+
&& !cx.tcx.is_automatically_derived(item.owner_id.to_def_id())
70+
&& impl_item.ident.name == sym::partial_cmp
71+
&& let ImplItemKind::Fn(_, partial_cmp_id) = cx.tcx.hir().impl_item(impl_item.impl_item_id()).kind
72+
&& let body = cx.tcx.hir().body(partial_cmp_id)
73+
&& let ord_def_id = cx
74+
.tcx
75+
.diagnostic_items(partial_ord_trait.def_id.krate)
76+
.name_to_id
77+
.get(&sym::Ord)
78+
.unwrap()
79+
&& let ExprKind::Block(block, ..) = body.value.kind && implements_trait(
80+
cx,
81+
hir_ty_to_ty(cx.tcx, imp.self_ty),
82+
*ord_def_id,
83+
partial_ord_trait.substs,
84+
)
85+
{
86+
if block.stmts.is_empty()
87+
&& let Some(expr) = block.expr
88+
&& let ExprKind::Call(Expr { kind: ExprKind::Path(some_path), .. }, [cmp_expr]) = expr.kind
89+
// TODO: We can likely use the `option_some_variant` lang item instead, but this may be tricky
90+
// due to the fact that `expr` is the constructor, not `option_some_variant` itself.
91+
&& match_qpath(some_path, &["Some"])
92+
&& let ExprKind::MethodCall(cmp_path, _, [other_expr], ..) = cmp_expr.kind
93+
&& cmp_path.ident.name == sym::cmp
94+
&& let Res::Local(..) = path_res(cx, other_expr)
95+
{} else {
96+
span_lint_and_then(
97+
cx,
98+
MANUAL_PARTIAL_ORD_AND_ORD_IMPL,
99+
item.span,
100+
"manual implementation of `PartialOrd` when `Ord` is already implemented",
101+
// TODO: Is this how this should be done? Should we instead suggest
102+
// changing the entire block, including the name
103+
// of (by default) other if we can't find it?
104+
|diag| {
105+
if let Some(param) = body.params.get(1)
106+
&& let PatKind::Binding(_, _, param_ident, ..) = param.pat.kind
107+
{
108+
diag.span_suggestion(
109+
block.span,
110+
"change this to",
111+
format!("{{ Some(self.cmp({})) }}", param_ident.name),
112+
Applicability::Unspecified,
132113
);
133-
}
114+
} else {
115+
diag.help("return the value of `self.cmp` wrapped in `Some` instead");
116+
};
134117
}
135-
}
118+
);
136119
}
137120
}
138121
}

0 commit comments

Comments
 (0)