Skip to content

Commit

Permalink
Rename incorrect_impls to manual_impls, move them to warn by default
Browse files Browse the repository at this point in the history
  • Loading branch information
Alexendoo committed Aug 20, 2023
1 parent d9e6aac commit 4f40fc4
Show file tree
Hide file tree
Showing 22 changed files with 155 additions and 123 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5065,6 +5065,7 @@ Released 2018-09-13
[`manual_async_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_async_fn
[`manual_bits`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_bits
[`manual_clamp`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_clamp
[`manual_clone_impl_on_copy_type`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_clone_impl_on_copy_type
[`manual_filter`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_filter
[`manual_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_filter_map
[`manual_find`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_find
Expand All @@ -5081,6 +5082,7 @@ Released 2018-09-13
[`manual_next_back`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_next_back
[`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive
[`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or
[`manual_partial_ord_impl_on_ord_type`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_partial_ord_impl_on_ord_type
[`manual_range_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains
[`manual_range_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_patterns
[`manual_rem_euclid`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_rem_euclid
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,6 @@ 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_CLONE_IMPL_ON_COPY_TYPE_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,
Expand Down Expand Up @@ -279,6 +277,8 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::manual_clamp::MANUAL_CLAMP_INFO,
crate::manual_float_methods::MANUAL_IS_FINITE_INFO,
crate::manual_float_methods::MANUAL_IS_INFINITE_INFO,
crate::manual_impls::MANUAL_CLONE_IMPL_ON_COPY_TYPE_INFO,
crate::manual_impls::MANUAL_PARTIAL_ORD_IMPL_ON_ORD_TYPE_INFO,
crate::manual_is_ascii_check::MANUAL_IS_ASCII_CHECK_INFO,
crate::manual_let_else::MANUAL_LET_ELSE_INFO,
crate::manual_main_separator_str::MANUAL_MAIN_SEPARATOR_STR_INFO,
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@ 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;
Expand Down Expand Up @@ -188,6 +187,7 @@ mod manual_async_fn;
mod manual_bits;
mod manual_clamp;
mod manual_float_methods;
mod manual_impls;
mod manual_is_ascii_check;
mod manual_let_else;
mod manual_main_separator_str;
Expand Down Expand Up @@ -1066,7 +1066,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
avoid_breaking_exported_api,
))
});
store.register_late_pass(|_| Box::new(incorrect_impls::IncorrectImpls));
store.register_late_pass(|_| Box::new(manual_impls::ManualImpls));
store.register_late_pass(move |_| {
Box::new(single_call_fn::SingleCallFn {
avoid_breaking_exported_api,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ declare_clippy_lint! {
/// Checks for manual implementations of `Clone` when `Copy` is already implemented.
///
/// ### Why is this bad?
/// If both `Clone` and `Copy` are implemented, they must agree. This is done by dereferencing
/// `self` in `Clone`'s implementation. Anything else is incorrect.
/// If both `Clone` and `Copy` are implemented, they must agree. This can done by dereferencing
/// `self` in `Clone`'s implementation, which will avoid any possibility of the implementations
/// becoming out of sync.
///
/// ### Example
/// ```rust,ignore
Expand Down Expand Up @@ -47,8 +48,8 @@ declare_clippy_lint! {
/// impl Copy for A {}
/// ```
#[clippy::version = "1.72.0"]
pub INCORRECT_CLONE_IMPL_ON_COPY_TYPE,
correctness,
pub MANUAL_CLONE_IMPL_ON_COPY_TYPE,
complexity,
"manual implementation of `Clone` on a `Copy` type"
}
declare_clippy_lint! {
Expand All @@ -62,11 +63,8 @@ declare_clippy_lint! {
/// introduce an error upon refactoring.
///
/// ### Known issues
/// Code that calls the `.into()` method instead will be flagged as incorrect, despite `.into()`
/// wrapping it in `Some`.
///
/// ### Limitations
/// Will not lint if `Self` and `Rhs` do not have the same type.
/// Code that calls the `.into()` method instead will be flagged, despite `.into()` wrapping it
/// in `Some`.
///
/// ### Example
/// ```rust
Expand Down Expand Up @@ -108,13 +106,13 @@ declare_clippy_lint! {
/// }
/// ```
#[clippy::version = "1.72.0"]
pub INCORRECT_PARTIAL_ORD_IMPL_ON_ORD_TYPE,
correctness,
pub MANUAL_PARTIAL_ORD_IMPL_ON_ORD_TYPE,
complexity,
"manual implementation of `PartialOrd` when `Ord` is already implemented"
}
declare_lint_pass!(IncorrectImpls => [INCORRECT_CLONE_IMPL_ON_COPY_TYPE, INCORRECT_PARTIAL_ORD_IMPL_ON_ORD_TYPE]);
declare_lint_pass!(ManualImpls => [MANUAL_CLONE_IMPL_ON_COPY_TYPE, MANUAL_PARTIAL_ORD_IMPL_ON_ORD_TYPE]);

impl LateLintPass<'_> for IncorrectImpls {
impl LateLintPass<'_> for ManualImpls {
#[expect(clippy::too_many_lines)]
fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &ImplItem<'_>) {
let Some(Node::Item(item)) = get_parent_node(cx.tcx, impl_item.hir_id()) else {
Expand Down Expand Up @@ -152,27 +150,31 @@ impl LateLintPass<'_> for IncorrectImpls {
&& 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,
INCORRECT_CLONE_IMPL_ON_COPY_TYPE,
block.span,
"incorrect implementation of `clone` on a `Copy` type",
"change this to",
"{ *self }".to_owned(),
Applicability::MaybeIncorrect,
);

return;
{
// body is already `*self`
return
}

span_lint_and_then(
cx,
MANUAL_CLONE_IMPL_ON_COPY_TYPE,
block.span,
"manual implementation of `clone` on a `Copy` type",
|diag| {
diag.span_suggestion(block.span, "change this to", "{ *self }", Applicability::MaybeIncorrect);
diag.note("this ensures the requirement that `Copy` and `Clone` are consistent is satisfied");
}
);

return;
}

if impl_item.ident.name == sym::clone_from {
span_lint_and_sugg(
cx,
INCORRECT_CLONE_IMPL_ON_COPY_TYPE,
MANUAL_CLONE_IMPL_ON_COPY_TYPE,
impl_item.span,
"incorrect implementation of `clone_from` on a `Copy` type",
"unneeded implementation of `clone_from` on a `Copy` type",
"remove it",
String::new(),
Applicability::MaybeIncorrect,
Expand Down Expand Up @@ -218,9 +220,9 @@ impl LateLintPass<'_> for IncorrectImpls {

span_lint_and_then(
cx,
INCORRECT_PARTIAL_ORD_IMPL_ON_ORD_TYPE,
MANUAL_PARTIAL_ORD_IMPL_ON_ORD_TYPE,
item.span,
"incorrect implementation of `partial_cmp` on an `Ord` type",
"manual implementation of `partial_cmp` on an `Ord` type",
|diag| {
let [_, other] = body.params else {
return;
Expand Down Expand Up @@ -255,6 +257,7 @@ impl LateLintPass<'_> for IncorrectImpls {
suggs,
Applicability::Unspecified,
);
diag.note("this ensures the requirement that `Ord` and `PartialOrd` are consistent is satisfied");
}
);
}
Expand Down
4 changes: 3 additions & 1 deletion clippy_lints/src/renamed_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ pub static RENAMED_LINTS: &[(&str, &str)] = &[
("clippy::eval_order_dependence", "clippy::mixed_read_write_in_expression"),
("clippy::identity_conversion", "clippy::useless_conversion"),
("clippy::if_let_some_result", "clippy::match_result_ok"),
("clippy::incorrect_clone_impl_on_copy_type", "clippy::manual_clone_impl_on_copy_type"),
("clippy::incorrect_partial_ord_impl_on_ord_type", "clippy::manual_partial_ord_impl_on_ord_type"),
("clippy::integer_arithmetic", "clippy::arithmetic_side_effects"),
("clippy::logic_bug", "clippy::overly_complex_bool_expr"),
("clippy::new_without_default_derive", "clippy::new_without_default"),
Expand All @@ -38,12 +40,12 @@ pub static RENAMED_LINTS: &[(&str, &str)] = &[
("clippy::drop_bounds", "drop_bounds"),
("clippy::drop_copy", "dropping_copy_types"),
("clippy::drop_ref", "dropping_references"),
("clippy::fn_null_check", "useless_ptr_null_checks"),
("clippy::for_loop_over_option", "for_loops_over_fallibles"),
("clippy::for_loop_over_result", "for_loops_over_fallibles"),
("clippy::for_loops_over_fallibles", "for_loops_over_fallibles"),
("clippy::forget_copy", "forgetting_copy_types"),
("clippy::forget_ref", "forgetting_references"),
("clippy::fn_null_check", "useless_ptr_null_checks"),
("clippy::into_iter_on_array", "array_into_iter"),
("clippy::invalid_atomic_ordering", "invalid_atomic_ordering"),
("clippy::invalid_ref", "invalid_value"),
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/bool_comparison.fixed
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![allow(clippy::needless_if)]
#![warn(clippy::bool_comparison)]
#![allow(clippy::incorrect_partial_ord_impl_on_ord_type)]
#![allow(clippy::manual_partial_ord_impl_on_ord_type)]

fn main() {
let x = true;
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/bool_comparison.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![allow(clippy::needless_if)]
#![warn(clippy::bool_comparison)]
#![allow(clippy::incorrect_partial_ord_impl_on_ord_type)]
#![allow(clippy::manual_partial_ord_impl_on_ord_type)]

fn main() {
let x = true;
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/clone_on_copy_impl.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![allow(clippy::incorrect_clone_impl_on_copy_type)]
#![allow(clippy::manual_clone_impl_on_copy_type)]

use std::fmt;
use std::marker::PhantomData;
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/derive.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![allow(
clippy::incorrect_clone_impl_on_copy_type,
clippy::incorrect_partial_ord_impl_on_ord_type,
clippy::manual_clone_impl_on_copy_type,
clippy::manual_partial_ord_impl_on_ord_type,
dead_code
)]
#![warn(clippy::expl_impl_clone_on_copy)]
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/derive_ord_xor_partial_ord.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![warn(clippy::derive_ord_xor_partial_ord)]
#![allow(clippy::unnecessary_wraps)]
#![allow(clippy::incorrect_partial_ord_impl_on_ord_type)]
#![allow(clippy::manual_partial_ord_impl_on_ord_type)]

use std::cmp::Ordering;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: incorrect implementation of `partial_cmp` on an `Ord` type
error: manual implementation of `partial_cmp` on an `Ord` type
--> $DIR/incorrect_partial_ord_impl_on_ord_type_fully_qual.rs:23:1
|
LL | / impl PartialOrd for A {
Expand All @@ -12,9 +12,9 @@ LL | || }
LL | | }
| |__^
|
= note: `#[deny(clippy::incorrect_partial_ord_impl_on_ord_type)]` on by default
= note: `-D clippy::manual-partial-ord-impl-on-ord-type` implied by `-D warnings`

error: incorrect implementation of `partial_cmp` on an `Ord` type
error: manual implementation of `partial_cmp` on an `Ord` type
--> $DIR/incorrect_partial_ord_impl_on_ord_type_fully_qual.rs:46:1
|
LL | / impl PartialOrd for B {
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,34 +1,37 @@
error: incorrect implementation of `clone` on a `Copy` type
--> $DIR/incorrect_clone_impl_on_copy_type.rs:9:29
error: manual implementation of `clone` on a `Copy` type
--> $DIR/manual_clone_impl_on_copy_type.rs:9:29
|
LL | fn clone(&self) -> Self {
| _____________________________^
LL | | Self(self.0)
LL | | }
| |_____^ help: change this to: `{ *self }`
|
= note: `#[deny(clippy::incorrect_clone_impl_on_copy_type)]` on by default
= note: this ensures the requirement that `Copy` and `Clone` are consistent is satisfied
= note: `-D clippy::manual-clone-impl-on-copy-type` implied by `-D warnings`

error: incorrect implementation of `clone_from` on a `Copy` type
--> $DIR/incorrect_clone_impl_on_copy_type.rs:13:5
error: unneeded implementation of `clone_from` on a `Copy` type
--> $DIR/manual_clone_impl_on_copy_type.rs:13:5
|
LL | / fn clone_from(&mut self, source: &Self) {
LL | | source.clone();
LL | | *self = source.clone();
LL | | }
| |_____^ help: remove it

error: incorrect implementation of `clone` on a `Copy` type
--> $DIR/incorrect_clone_impl_on_copy_type.rs:80:29
error: manual implementation of `clone` on a `Copy` type
--> $DIR/manual_clone_impl_on_copy_type.rs:80:29
|
LL | fn clone(&self) -> Self {
| _____________________________^
LL | | Self(self.0)
LL | | }
| |_____^ help: change this to: `{ *self }`
|
= note: this ensures the requirement that `Copy` and `Clone` are consistent is satisfied

error: incorrect implementation of `clone_from` on a `Copy` type
--> $DIR/incorrect_clone_impl_on_copy_type.rs:84:5
error: unneeded implementation of `clone_from` on a `Copy` type
--> $DIR/manual_clone_impl_on_copy_type.rs:84:5
|
LL | / fn clone_from(&mut self, source: &Self) {
LL | | source.clone();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: incorrect implementation of `partial_cmp` on an `Ord` type
--> $DIR/incorrect_partial_ord_impl_on_ord_type.rs:16:1
error: manual implementation of `partial_cmp` on an `Ord` type
--> $DIR/manual_partial_ord_impl_on_ord_type.rs:16:1
|
LL | / impl PartialOrd for A {
LL | | fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
Expand All @@ -10,10 +10,11 @@ LL | || }
LL | | }
| |__^
|
= note: `#[deny(clippy::incorrect_partial_ord_impl_on_ord_type)]` on by default
= note: this ensures the requirement that `Ord` and `PartialOrd` are consistent is satisfied
= note: `-D clippy::manual-partial-ord-impl-on-ord-type` implied by `-D warnings`

error: incorrect implementation of `partial_cmp` on an `Ord` type
--> $DIR/incorrect_partial_ord_impl_on_ord_type.rs:50:1
error: manual implementation of `partial_cmp` on an `Ord` type
--> $DIR/manual_partial_ord_impl_on_ord_type.rs:50:1
|
LL | / impl PartialOrd for C {
LL | | fn partial_cmp(&self, _: &Self) -> Option<Ordering> {
Expand All @@ -22,6 +23,7 @@ LL | | }
LL | | }
| |_^
|
= note: this ensures the requirement that `Ord` and `PartialOrd` are consistent is satisfied
help: change this to
|
LL | fn partial_cmp(&self, other: &Self) -> Option<Ordering> { Some(self.cmp(other)) }
Expand Down
8 changes: 6 additions & 2 deletions tests/ui/rename.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
#![allow(clippy::mixed_read_write_in_expression)]
#![allow(clippy::useless_conversion)]
#![allow(clippy::match_result_ok)]
#![allow(clippy::manual_clone_impl_on_copy_type)]
#![allow(clippy::manual_partial_ord_impl_on_ord_type)]
#![allow(clippy::arithmetic_side_effects)]
#![allow(clippy::overly_complex_bool_expr)]
#![allow(clippy::new_without_default)]
Expand All @@ -33,10 +35,10 @@
#![allow(drop_bounds)]
#![allow(dropping_copy_types)]
#![allow(dropping_references)]
#![allow(useless_ptr_null_checks)]
#![allow(for_loops_over_fallibles)]
#![allow(forgetting_copy_types)]
#![allow(forgetting_references)]
#![allow(useless_ptr_null_checks)]
#![allow(array_into_iter)]
#![allow(invalid_atomic_ordering)]
#![allow(invalid_value)]
Expand All @@ -62,6 +64,8 @@
#![warn(clippy::mixed_read_write_in_expression)]
#![warn(clippy::useless_conversion)]
#![warn(clippy::match_result_ok)]
#![warn(clippy::manual_clone_impl_on_copy_type)]
#![warn(clippy::manual_partial_ord_impl_on_ord_type)]
#![warn(clippy::arithmetic_side_effects)]
#![warn(clippy::overly_complex_bool_expr)]
#![warn(clippy::new_without_default)]
Expand All @@ -85,12 +89,12 @@
#![warn(drop_bounds)]
#![warn(dropping_copy_types)]
#![warn(dropping_references)]
#![warn(useless_ptr_null_checks)]
#![warn(for_loops_over_fallibles)]
#![warn(for_loops_over_fallibles)]
#![warn(for_loops_over_fallibles)]
#![warn(forgetting_copy_types)]
#![warn(forgetting_references)]
#![warn(useless_ptr_null_checks)]
#![warn(array_into_iter)]
#![warn(invalid_atomic_ordering)]
#![warn(invalid_value)]
Expand Down
Loading

0 comments on commit 4f40fc4

Please sign in to comment.