Skip to content

Conversation

samueltardieu
Copy link
Member

@samueltardieu samueltardieu commented Sep 18, 2025

Out of the 5 lints in the derive directory, only two of them (unsafe_derive_deserialize and derive_partial_eq_without_eq) posted the lint on the ADT node. This fixes the situation for the three other lints:

  • derived_hash_with_manual_eq
  • derive_ord_xor_partial_ord
  • expl_impl_clone_on_copy

This allows #[expect] to be used on the ADT to silence the lint.

Also, this makes expl_impl_clone_on_copy properly use an "help" message instead of a "note" one when suggesting a fix.

Fixes #15708
Fixes #13356

changelog: [derived_hash_with_manual_eq]: honor the use of #[expect] on the type to which the lint applies
changelog: [derive_ord_xor_partial_ord]: honor the use of #[expect] on the type to which the lint applies
changelog: [expl_impl_clone_on_copy]: honor the use of #[expect] on the type to which the lint applies

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Sep 18, 2025
@rustbot
Copy link
Collaborator

rustbot commented Sep 18, 2025

r? @blyxyas

rustbot has assigned @blyxyas.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Copy link

github-actions bot commented Sep 18, 2025

Lintcheck changes for d84505b

Lint Added Removed Changed
clippy::expl_impl_clone_on_copy 0 0 326
clippy::missing_trait_methods 0 0 165
clippy::multiple_crate_versions 0 10 1

This comment will be updated if you push new changes

@ada4a
Copy link
Contributor

ada4a commented Sep 19, 2025

Oh, I was trying to do basically this for #13356, which complains about the particular case of derived_hash_with_manual_eq, so that issue can be closed as well.

Speaking of that lint, you have a small typo in its name -- derive instead of derived. Although the other lints use derive, and one could argue that that sounds more natural -- maybe this lint should be renamed?

@samueltardieu
Copy link
Member Author

Oh, I was trying to do basically this for #13356, which complains about the particular case of derived_hash_with_manual_eq, so that issue can be closed as well.

Speaking of that lint, you have a small typo in its name -- derive instead of derived. Although the other lints use derive, and one could argue that that sounds more natural -- maybe this lint should be renamed?

I fixed the typo, thanks. Yeah, it should probably be renamed, but not in this PR though. I'll note that #13356 will be closed as well.

}

fn main() {}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

}

fn main() {}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

…and here

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Sep 19, 2025
Out of the 5 lints in the `derive` directory, only two of them
(`unsafe_derive_deserialize` and `derive_partial_eq_without_eq`) posted
the lint on the ADT node. This fixes the situation for the three other
lints:

- `derive_hash_with_manual_eq`
- `derive_ord_xor_partial_ord`
- `expl_impl_clone_on_copy`

This allows `#[expect]` to be used on the ADT to silence the lint.

Also, this makes `expl_impl_clone_on_copy` properly use an "help" message
instead of a "note" one when suggesting a fix.
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Sep 21, 2025
Copy link
Member

@blyxyas blyxyas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the bugfix!

View changes since this review

@blyxyas blyxyas added this pull request to the merge queue Sep 21, 2025
Merged via the queue into rust-lang:master with commit d99cf5c Sep 21, 2025
11 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Sep 21, 2025
@samueltardieu samueltardieu deleted the issues/15708 branch September 21, 2025 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants