mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

refactor

Browse Source
This commit is contained in:
Catherine 2023-06-30 15:16:56 -05:00
parent 844afbfeba
commit a5dfb68491
5 changed files with 174 additions and 50 deletions

78
clippy_lints/src/incorrect_impls.rs

@ -4,13 +4,12 @@ use clippy_utils::{
ty::implements_trait,
};
use rustc_errors::Applicability;
use rustc_hir::{def::Res, Expr, ExprKind, ImplItem, ImplItemKind, ItemKind, LangItem, Node, PatKind, UnOp};
use rustc_hir::{ExprKind, ImplItem, ImplItemKind, Node, UnOp};
use rustc_hir::{def::Res, Expr, ExprKind, ImplItem, ImplItemKind, ItemKind, LangItem, Node, UnOp};
use rustc_hir_analysis::hir_ty_to_ty;
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::EarlyBinder;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::{sym, symbol};
use std::borrow::Cow;
use rustc_span::{sym, symbol::kw};
declare_clippy_lint! {
/// ### What it does
@ -61,31 +60,39 @@ declare_clippy_lint! {
/// wrapping the result of `cmp` in `Some` for `partial_cmp`. Not doing this may silently
/// introduce an error upon refactoring.
///
/// ### Limitations
/// Will not lint if `Self` and `Rhs` do not have the same type.
///
/// ### Example
/// ```rust,ignore
/// ```rust
/// # use std::cmp::Ordering;
/// #[derive(Eq, PartialEq)]
/// struct A(u32);
///
/// impl Ord for A {
/// fn cmp(&self, other: &Self) -> Ordering {
/// todo!();
/// // ...
/// # todo!();
/// }
/// }
///
/// impl PartialOrd for A {
/// fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
/// todo!();
/// // ...
/// # todo!();
/// }
/// }
/// ```
/// Use instead:
/// ```rust,ignore
/// ```rust
/// # use std::cmp::Ordering;
/// #[derive(Eq, PartialEq)]
/// struct A(u32);
///
/// impl Ord for A {
/// fn cmp(&self, other: &Self) -> Ordering {
/// todo!();
/// // ...
/// # todo!();
/// }
/// }
///
@ -105,17 +112,18 @@ declare_lint_pass!(IncorrectImpls => [INCORRECT_CLONE_IMPL_ON_COPY_TYPE, INCORRE
impl LateLintPass<'_> for IncorrectImpls {
#[expect(clippy::too_many_lines)]
fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &ImplItem<'_>) {
let node = get_parent_node(cx.tcx, impl_item.hir_id());
let Some(Node::Item(item)) = node else {
let Some(Node::Item(item)) = get_parent_node(cx.tcx, impl_item.hir_id()) else {
return;
};
let Some(trait_impl) = cx.tcx.impl_trait_ref(item.owner_id).map(EarlyBinder::skip_binder) else {
return;
};
let trait_impl_def_id = trait_impl.def_id;
if cx.tcx.is_automatically_derived(item.owner_id.to_def_id()) {
return;
}
let ItemKind::Impl(imp) = item.kind else {
return;
};
let ImplItemKind::Fn(_, impl_item_id) = cx.tcx.hir().impl_item(impl_item.impl_item_id()).kind else {
return;
};
@ -124,7 +132,7 @@ impl LateLintPass<'_> for IncorrectImpls {
return;
};
if cx.tcx.is_diagnostic_item(sym::Clone, trait_impl_def_id)
if cx.tcx.is_diagnostic_item(sym::Clone, trait_impl.def_id)
&& let Some(copy_def_id) = cx.tcx.get_diagnostic_item(sym::Copy)
&& implements_trait(
cx,
@ -136,9 +144,9 @@ impl LateLintPass<'_> for IncorrectImpls {
if impl_item.ident.name == sym::clone {
if block.stmts.is_empty()
&& let Some(expr) = block.expr
&& let ExprKind::Unary(UnOp::Deref, inner) = expr.kind
&& let ExprKind::Path(qpath) = inner.kind
&& last_path_segment(&qpath).ident.name == symbol::kw::SelfLower
&& let ExprKind::Unary(UnOp::Deref, deref) = expr.kind
&& let ExprKind::Path(qpath) = deref.kind
&& last_path_segment(&qpath).ident.name == kw::SelfLower
{} else {
span_lint_and_sugg(
cx,
@ -160,7 +168,7 @@ impl LateLintPass<'_> for IncorrectImpls {
INCORRECT_CLONE_IMPL_ON_COPY_TYPE,
impl_item.span,
"incorrect implementation of `clone_from` on a `Copy` type",
"remove this",
"remove it",
String::new(),
Applicability::MaybeIncorrect,
);
@ -169,7 +177,7 @@ impl LateLintPass<'_> for IncorrectImpls {
}
}
if cx.tcx.is_diagnostic_item(sym::PartialOrd, trait_impl_def_id)
if cx.tcx.is_diagnostic_item(sym::PartialOrd, trait_impl.def_id)
&& impl_item.ident.name == sym::partial_cmp
&& let Some(ord_def_id) = cx
.tcx
@ -198,12 +206,9 @@ impl LateLintPass<'_> for IncorrectImpls {
&& cmp_path.ident.name == sym::cmp
&& let Res::Local(..) = path_res(cx, other_expr)
{} else {
// If lhs and rhs are not the same type, bail. This makes creating a valid
// If `Self` and `Rhs` are not the same type, bail. This makes creating a valid
// suggestion tons more complex.
if let Some(lhs) = trait_impl.substs.get(0)
&& let Some(rhs) = trait_impl.substs.get(1)
&& lhs != rhs
{
if let [lhs, rhs, ..] = trait_impl.substs.as_slice() && lhs != rhs {
return;
}
@ -213,22 +218,23 @@ impl LateLintPass<'_> for IncorrectImpls {
item.span,
"incorrect implementation of `partial_cmp` on an `Ord` type",
|diag| {
let (help, app) = if let Some(other) = body.params.get(1)
&& let PatKind::Binding(_, _, other_ident, ..) = other.pat.kind
{
(
Cow::Owned(format!("{{ Some(self.cmp({})) }}", other_ident.name)),
Applicability::Unspecified,
)
} else {
(Cow::Borrowed("{ Some(self.cmp(...)) }"), Applicability::HasPlaceholders)
let [_, other] = body.params else {
return;
};
diag.span_suggestion(
block.span,
let suggs = if let Some(other_ident) = other.pat.simple_ident() {
vec![(block.span, format!("{{ Some(self.cmp({})) }}", other_ident.name))]
} else {
vec![
(block.span, "{ Some(self.cmp(other)) }".to_owned()),
(other.pat.span, "other".to_owned()),
]
};
diag.multipart_suggestion(
"change this to",
help,
app,
suggs,
Applicability::Unspecified,
);
}
);

4
tests/ui/incorrect_clone_impl_on_copy_type.stderr

@ -16,7 +16,7 @@ LL | / fn clone_from(&mut self, source: &Self) {
LL | | source.clone();
LL | | *self = source.clone();
LL | | }
| |_____^ help: remove this
| |_____^ help: remove it
error: incorrect implementation of `clone` on a `Copy` type
--> $DIR/incorrect_clone_impl_on_copy_type.rs:81:29
@ -34,7 +34,7 @@ LL | / fn clone_from(&mut self, source: &Self) {
LL | | source.clone();
LL | | *self = source.clone();
LL | | }
| |_____^ help: remove this
| |_____^ help: remove it
error: aborting due to 4 previous errors

114
tests/ui/incorrect_partial_ord_impl_on_ord_type.fixed Normal file

@ -0,0 +1,114 @@
//@run-rustfix
#![allow(unused)]
#![no_main]
use std::cmp::Ordering;
// lint
#[derive(Eq, PartialEq)]
struct A(u32);
impl Ord for A {
fn cmp(&self, other: &Self) -> Ordering {
todo!();
}
}
impl PartialOrd for A {
fn partial_cmp(&self, other: &Self) -> Option<Ordering> { Some(self.cmp(other)) }
}
// do not lint
#[derive(Eq, PartialEq)]
struct B(u32);
impl Ord for B {
fn cmp(&self, other: &Self) -> Ordering {
todo!();
}
}
impl PartialOrd for B {
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
Some(self.cmp(other))
}
}
// lint, and give `_` a name
#[derive(Eq, PartialEq)]
struct C(u32);
impl Ord for C {
fn cmp(&self, other: &Self) -> Ordering {
todo!();
}
}
impl PartialOrd for C {
fn partial_cmp(&self, other: &Self) -> Option<Ordering> { Some(self.cmp(other)) }
}
// do not lint derived
#[derive(Eq, Ord, PartialEq, PartialOrd)]
struct D(u32);
// do not lint if ord is not manually implemented
#[derive(Eq, PartialEq)]
struct E(u32);
impl PartialOrd for E {
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
todo!();
}
}
// do not lint since ord has more restrictive bounds
#[derive(Eq, PartialEq)]
struct Uwu<A>(A);
impl<A: std::fmt::Debug + Ord + PartialOrd> Ord for Uwu<A> {
fn cmp(&self, other: &Self) -> Ordering {
todo!();
}
}
impl<A: Ord + PartialOrd> PartialOrd for Uwu<A> {
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
todo!();
}
}
// do not lint since `Rhs` is not `Self`
#[derive(Eq, PartialEq)]
struct F(u32);
impl Ord for F {
fn cmp(&self, other: &Self) -> Ordering {
todo!();
}
}
impl PartialOrd for F {
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
Some(self.cmp(other))
}
}
impl PartialEq<u32> for F {
fn eq(&self, other: &u32) -> bool {
todo!();
}
}
impl PartialOrd<u32> for F {
fn partial_cmp(&self, other: &u32) -> Option<Ordering> {
todo!();
}
}

5
tests/ui/incorrect_partial_ord_impl_on_ord_type.rs

@ -1,3 +1,4 @@
//@run-rustfix
#![allow(unused)]
#![no_main]
@ -37,7 +38,7 @@ impl PartialOrd for B {
}
}
// lint, but we can't give a suggestion since &Self is not named
// lint, and give `_` a name
#[derive(Eq, PartialEq)]
struct C(u32);
@ -50,7 +51,7 @@ impl Ord for C {
impl PartialOrd for C {
fn partial_cmp(&self, _: &Self) -> Option<Ordering> {
todo!(); // don't run rustfix, or else this will cause it to fail to compile
todo!();
}
}

23
tests/ui/incorrect_partial_ord_impl_on_ord_type.stderr

@ -1,5 +1,5 @@
error: incorrect implementation of `partial_cmp` on an `Ord` type
--> $DIR/incorrect_partial_ord_impl_on_ord_type.rs:17:1
--> $DIR/incorrect_partial_ord_impl_on_ord_type.rs:18:1
|
LL | / impl PartialOrd for A {
LL | | fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
@ -13,16 +13,19 @@ LL | | }
= note: `#[deny(clippy::incorrect_partial_ord_impl_on_ord_type)]` on by default
error: incorrect implementation of `partial_cmp` on an `Ord` type
--> $DIR/incorrect_partial_ord_impl_on_ord_type.rs:51:1
--> $DIR/incorrect_partial_ord_impl_on_ord_type.rs:52:1
|
LL | / impl PartialOrd for C {
LL | | fn partial_cmp(&self, _: &Self) -> Option<Ordering> {
| | _________________________________________________________-
LL | || todo!(); // don't run rustfix, or else this will cause it to fail to compile
LL | || }
| ||_____- help: change this to: `{ Some(self.cmp(...)) }`
LL | | }
| |__^
LL | / impl PartialOrd for C {
LL | | fn partial_cmp(&self, _: &Self) -> Option<Ordering> {
LL | | todo!();
LL | | }
LL | | }
| |_^
|
help: change this to
|
LL | fn partial_cmp(&self, other: &Self) -> Option<Ordering> { Some(self.cmp(other)) }
| ~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~
error: aborting due to 2 previous errors