Skip to content
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

#[expect(...)] attribute not working for clippy::unsafe_derive_deserialize #12802

Closed
B14CK313 opened this issue May 15, 2024 · 0 comments · Fixed by #12804
Closed

#[expect(...)] attribute not working for clippy::unsafe_derive_deserialize #12802

B14CK313 opened this issue May 15, 2024 · 0 comments · Fixed by #12804
Labels
C-bug Category: Clippy is not doing the correct thing

Comments

@B14CK313
Copy link
Contributor

B14CK313 commented May 15, 2024

Summary

The #[expect(clippy::unsafe_derive_deserialize)] attribute (enabled by the lint_reasons feature) doesn't suppress the warning about deriving serde::Deserialize on a struct with an unsafe fn.
At the same time the expect attribute gives a warning about the lint expectation being unfulfilled.

At first glance this looked like the same bug as rust#119358 but in that case the warning got suppressed, which isn't the case here.

If this issue is not caused by clippy, I am happy to copy this issue over to the rust repo.

Reproducer

I tried this code:

#![feature(lint_reasons)]
#![warn(clippy::unsafe_derive_deserialize)]

#[expect(clippy::unsafe_derive_deserialize)]
#[derive(serde::Deserialize)]
struct Foo {}

impl Foo {
    #[allow(dead_code)]
    unsafe fn unsafe_fn(&self) {
        todo!()
    }
}

Playground

I expected to see this happen:
No warning emitted.

Instead, this happened:
Two warnings get emitted:

  • warning: you are deriving `serde::Deserialize` on a type that has methods using `unsafe`
  • warning: this lint expectation is unfulfilled

Version

rustc: 1.80.0-nightly (2024-05-14 8387315ab3c26a57a1f5)
clippy: 0.1.80 (2024-05-14 8387315)

Additional Labels

No response

@B14CK313 B14CK313 added the C-bug Category: Clippy is not doing the correct thing label May 15, 2024
B14CK313 added a commit to B14CK313/rust-clippy that referenced this issue May 15, 2024
…lize`

This commit adds fulfilling expectations to `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, expectations are checked and
fulfilled if found. In that case actually emitting the lint is skipped.

fixes: rust-lang#12802

changelog: fulfill expectations in `check_unsafe_derive_deserialize`
B14CK313 added a commit to B14CK313/rust-clippy that referenced this issue May 15, 2024
…lize`

This commit adds fulfilling expectations to `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, expectations are checked and
fulfilled if found. In that case actually emitting the lint is skipped.

fixes: rust-lang#12802

changelog: fulfill expectations in `check_unsafe_derive_deserialize`
bors added a commit that referenced this issue May 21, 2024
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`
bors added a commit that referenced this issue May 21, 2024
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: fulfill expectations in [`unsafe_derive_deserialize`]
@bors bors closed this as completed in fc2f703 May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant