use other instead of self

rust-lang · Jun 15, 2023 · c5c1a77 · c5c1a77
1 parent 041d07e
commit c5c1a77
Show file tree

Hide file tree

Showing 12 changed files with 81 additions and 128 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4833,6 +4833,7 @@ Released 2018-09-13
 [`imprecise_flops`]: https://rust-lang.github.io/rust-clippy/master/index.html#imprecise_flops
 [`inconsistent_digit_grouping`]: https://rust-lang.github.io/rust-clippy/master/index.html#inconsistent_digit_grouping
 [`inconsistent_struct_constructor`]: https://rust-lang.github.io/rust-clippy/master/index.html#inconsistent_struct_constructor
+[`incorrect_partial_ord_impl_on_ord_type`]: https://rust-lang.github.io/rust-clippy/master/index.html#incorrect_partial_ord_impl_on_ord_type
 [`index_refutable_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#index_refutable_slice
 [`indexing_slicing`]: https://rust-lang.github.io/rust-clippy/master/index.html#indexing_slicing
 [`ineffective_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#ineffective_bit_mask
@@ -5017,7 +5018,6 @@ Released 2018-09-13
 [`needless_option_as_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_option_as_deref
 [`needless_option_take`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_option_take
 [`needless_parens_on_range_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_parens_on_range_literals
-[`needless_partial_ord_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_partial_ord_impl
 [`needless_pass_by_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_value
 [`needless_question_mark`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_question_mark
 [`needless_range_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop

diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
@@ -206,6 +206,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
  crate::implicit_saturating_add::IMPLICIT_SATURATING_ADD_INFO,
  crate::implicit_saturating_sub::IMPLICIT_SATURATING_SUB_INFO,
  crate::inconsistent_struct_constructor::INCONSISTENT_STRUCT_CONSTRUCTOR_INFO,
+ crate::incorrect_impls::INCORRECT_PARTIAL_ORD_IMPL_ON_ORD_TYPE_INFO,
  crate::index_refutable_slice::INDEX_REFUTABLE_SLICE_INFO,
  crate::indexing_slicing::INDEXING_SLICING_INFO,
  crate::indexing_slicing::OUT_OF_BOUNDS_INDEXING_INFO,
@@ -463,7 +464,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
  crate::needless_else::NEEDLESS_ELSE_INFO,
  crate::needless_for_each::NEEDLESS_FOR_EACH_INFO,
  crate::needless_if::NEEDLESS_IF_INFO,
- crate::needless_impls::NEEDLESS_PARTIAL_ORD_IMPL_INFO,
  crate::needless_late_init::NEEDLESS_LATE_INIT_INFO,
  crate::needless_parens_on_range_literals::NEEDLESS_PARENS_ON_RANGE_LITERALS_INFO,
  crate::needless_pass_by_value::NEEDLESS_PASS_BY_VALUE_INFO,

diff --git a/clippy_lints/src/needless_impls.rs → clippy_lints/src/incorrect_impls.rs b/clippy_lints/src/needless_impls.rs → clippy_lints/src/incorrect_impls.rs
@@ -55,13 +55,13 @@ declare_clippy_lint! {
  /// }
  /// ```
  #[clippy::version = "1.72.0"]
- pub NEEDLESS_PARTIAL_ORD_IMPL,
+ pub INCORRECT_PARTIAL_ORD_IMPL_ON_ORD_TYPE,
  correctness,
  "manual implementation of `PartialOrd` when `Ord` is already implemented"
 }
-declare_lint_pass!(NeedlessImpls => [NEEDLESS_PARTIAL_ORD_IMPL]);
+declare_lint_pass!(IncorrectImpls => [INCORRECT_PARTIAL_ORD_IMPL_ON_ORD_TYPE]);
 
-impl LateLintPass<'_> for NeedlessImpls {
+impl LateLintPass<'_> for IncorrectImpls {
  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 {
@@ -114,13 +114,22 @@ impl LateLintPass<'_> for NeedlessImpls {
  && 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
+ // suggestion tons more complex.
+ if let Some(lhs) = trait_impl.substs.get(0)
+ && let Some(rhs) = trait_impl.substs.get(1)
+ && lhs != rhs
+ {
+ return;
+ }
+
  span_lint_and_then(
  cx,
- NEEDLESS_PARTIAL_ORD_IMPL,
+ INCORRECT_PARTIAL_ORD_IMPL_ON_ORD_TYPE,
  item.span,
- "manual implementation of `PartialOrd` when `Ord` is already implemented",
+ "incorrect implementation of `partial_cmp` on an `Ord` type",
  |diag| {
- let (help, app) = if let Some(other) = body.params.get(0)
+ let (help, app) = if let Some(other) = body.params.get(1)
  && let PatKind::Binding(_, _, other_ident, ..) = other.pat.kind
  {
  (

diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -150,6 +150,7 @@ mod implicit_return;
 mod implicit_saturating_add;
 mod implicit_saturating_sub;
 mod inconsistent_struct_constructor;
+mod incorrect_impls;
 mod index_refutable_slice;
 mod indexing_slicing;
 mod infinite_iter;
@@ -226,7 +227,6 @@ mod needless_continue;
 mod needless_else;
 mod needless_for_each;
 mod needless_if;
-mod needless_impls;
 mod needless_late_init;
 mod needless_parens_on_range_literals;
 mod needless_pass_by_value;
@@ -1048,6 +1048,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
  let stack_size_threshold = conf.stack_size_threshold;
  store.register_late_pass(move |_| Box::new(large_stack_frames::LargeStackFrames::new(stack_size_threshold)));
  store.register_late_pass(|_| Box::new(single_range_in_vec_init::SingleRangeInVecInit));
+ store.register_late_pass(|_| Box::new(incorrect_impls::IncorrectImpls));
  // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

diff --git a/tests/ui/bool_comparison.fixed b/tests/ui/bool_comparison.fixed
@@ -2,7 +2,7 @@
 
 #![allow(clippy::needless_if)]
 #![warn(clippy::bool_comparison)]
-#![allow(clippy::needless_partial_ord_impl)]
+#![allow(clippy::incorrect_partial_ord_impl_on_ord_type)]
 
 fn main() {
  let x = true;

diff --git a/tests/ui/bool_comparison.rs b/tests/ui/bool_comparison.rs
@@ -2,7 +2,7 @@
 
 #![allow(clippy::needless_if)]
 #![warn(clippy::bool_comparison)]
-#![allow(clippy::needless_partial_ord_impl)]
+#![allow(clippy::incorrect_partial_ord_impl_on_ord_type)]
 
 fn main() {
  let x = true;

diff --git a/tests/ui/derive.rs b/tests/ui/derive.rs
@@ -1,4 +1,4 @@
-#![allow(clippy::needless_partial_ord_impl, dead_code)]
+#![allow(clippy::incorrect_partial_ord_impl_on_ord_type, dead_code)]
 #![warn(clippy::expl_impl_clone_on_copy)]
 
 #[derive(Copy)]

diff --git a/tests/ui/derive_ord_xor_partial_ord.rs b/tests/ui/derive_ord_xor_partial_ord.rs
@@ -1,6 +1,6 @@
 #![warn(clippy::derive_ord_xor_partial_ord)]
 #![allow(clippy::unnecessary_wraps)]
-#![allow(clippy::needless_partial_ord_impl)]
+#![allow(clippy::incorrect_partial_ord_impl_on_ord_type)]
 
 use std::cmp::Ordering;
 

diff --git a/tests/ui/needless_partial_ord_impl.rs → ...incorrect_partial_ord_impl_on_ord_type.rs b/tests/ui/needless_partial_ord_impl.rs → ...incorrect_partial_ord_impl_on_ord_type.rs
@@ -1,4 +1,3 @@
-//@run-rustfix
 #![allow(unused)]
 #![no_main]
 
@@ -51,7 +50,7 @@ impl Ord for C {
 
 impl PartialOrd for C {
  fn partial_cmp(&self, _: &Self) -> Option<Ordering> {
- todo!();
+ todo!(); // don't run rustfix, or else this will cause it to fail to compile
  }
 }
 
@@ -87,3 +86,32 @@ impl<A: Ord + PartialOrd> PartialOrd for Uwu<A> {
  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!();
+ }
+}
diff --git a/tests/ui/incorrect_partial_ord_impl_on_ord_type.stderr b/tests/ui/incorrect_partial_ord_impl_on_ord_type.stderr
@@ -0,0 +1,28 @@
+error: incorrect implementation of `partial_cmp` on an `Ord` type
+ --> $DIR/incorrect_partial_ord_impl_on_ord_type.rs:17:1
+ |
+LL | / impl PartialOrd for A {
+LL | | fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+ | | _____________________________________________________________-
+LL | || todo!();
+LL | || }
+ | ||_____- help: change this to: `{ Some(self.cmp(other)) }`
+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
+ |
+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 | | }
+ | |__^
+
+error: aborting due to 2 previous errors
+
diff --git a/tests/ui/needless_partial_ord_impl.fixed b/tests/ui/needless_partial_ord_impl.fixed
diff --git a/tests/ui/needless_partial_ord_impl.stderr b/tests/ui/needless_partial_ord_impl.stderr