-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fulfill expectations in check_unsafe_derive_deserialize
#12804
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Manishearth (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should also ideally have a test to make sure that #[expect]
now actually works (in tests/ui/unsafe_derive_serialize.rs
).
The utility function `clippy_utils::fulfill_or_allowed` is not used because using it would require to move the check for allowed after the check iterating over all inherent impls of the type, doing possibly unnecessary work. Instead, `is_lint_allowed` is called as before, but additionally, once certain that the lint should be emitted, `span_lint_hir_and_then` is called instead of `span_lint_and_help` to also fulfill expectations. fixes: rust-lang#12802 changelog: fulfill expectations in `check_unsafe_derive_deserialize`
Adding `#![feature(lint_reasons)]` to the top of the file also changed the line numbers in the expected error output.
check_unsafe_derive_deserialize
check_unsafe_derive_deserialize
r? @y21 just got back from my trip and have a large backlog |
Looks good, thanks! @bors r+ If you're interested in a similar followup PR, there's another lint in this file ( |
This comment was marked as resolved.
This comment was marked as resolved.
fulfill expectations in `check_unsafe_derive_deserialize` The utility function `clippy_utils::fulfill_or_allowed` is not used because using it would require to move the check for allowed after the check iterating over all inherent impls of the type, doing possibly unnecessary work. Instead, `is_lint_allowed` is called as before, but additionally, once certain that the lint should be emitted, `span_lint_hir_and_then` is called instead of `span_lint_and_help` to also fulfill expectations. Note: as this is my first contribution, please feel free to nitpick or request changes. I am happy to adjust the implementation. fixes: #12802 changelog [`unsafe_derive_deserialize`]: fulfill expectations in `check_unsafe_derive_deserialize`
💔 Test failed - checks-action_test |
@bors retry |
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
fulfill expectations in `check_partial_eq_without_eq` This is a followup to #12804, fixing a similar issue for `derive_partial_eq_without_eq` by using `span_lint_hir_and_then` instead of `span_lint_and_sugg`. Additionally tests for both `#[allow(clippy::derive_partial_eq_without_eq)]` and `#[expect(clippy::derive_partial_eq_without_eq)]` are added. changelog:[`derive_partial_eq_without_eq`]: fulfill expectations
fulfill expectations in `check_unsafe_derive_deserialize` The utility function `clippy_utils::fulfill_or_allowed` is not used because using it would require to move the check for allowed after the check iterating over all inherent impls of the type, doing possibly unnecessary work. Instead, `is_lint_allowed` is called as before, but additionally, once certain that the lint should be emitted, `span_lint_hir_and_then` is called instead of `span_lint_and_help` to also fulfill expectations. Note: as this is my first contribution, please feel free to nitpick or request changes. I am happy to adjust the implementation. fixes: rust-lang#12802 changelog: fulfill expectations in [`unsafe_derive_deserialize`]
The utility function
clippy_utils::fulfill_or_allowed
is not used because using it would require to move the check for allowed after the check iterating over all inherent impls of the type, doing possibly unnecessary work.Instead,
is_lint_allowed
is called as before, but additionally, once certain that the lint should be emitted,span_lint_hir_and_then
is called instead ofspan_lint_and_help
to also fulfill expectations.Note: as this is my first contribution, please feel free to nitpick or request changes. I am happy to adjust the implementation.
fixes: #12802
changelog: fulfill expectations in [
unsafe_derive_deserialize
]