Skip to content

add #[allow(deprecated)] to derive implementations #2879

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rcrisanti
Copy link

Allow deprecated in the Serialize/Deserialize derive implementations. This allows you to deprecate structs, enums, struct fields, or enum variants and not get compiler warnings/errors about use of deprecated thing.

Resolves #2195

@KarstenB
Copy link

KarstenB commented Jan 9, 2025

I just encountered the same issue and was wondering how that could be implemented. While I am perfectly fine with the given PR, I thought about a more generic way. Like:

#[derive(Serialize, Deserialize)]
#[deprecated]
#[serde(allow = deprecated)]
struct bla {}

@rcrisanti
Copy link
Author

@KarstenB that is definitely doable if that is the desired behavior. Personally I'm not sure I can think of a scenario where I do want to see these messages, so I'd at least be in favor of making the default not showing them, with some sort of option to turn them back on.

(For some justification of this, in rust itself they hide deprecation messages in derives, so in my opinion this would put serde more inline with expected behavior.)

@oli-obk
Copy link
Member

oli-obk commented Jan 20, 2025

(For some justification of this, in rust itself they hide deprecation messages in derives, so in my opinion this would put serde more inline with expected behavior.)

in theory that check should work for serde automatically, but making that happen at the proc macro level causes lots of problems inside other diagnostics: #2847 (comment)

@rcrisanti
Copy link
Author

@oli-obk is there anything you're waiting to see from me on this? From my perspective it is ready for review

@oli-obk
Copy link
Member

oli-obk commented Mar 26, 2025

No... this is mostly me not wanting to add workarounds that are necessary because of other workarounds. w

@oli-obk
Copy link
Member

oli-obk commented Mar 26, 2025

Considering the low cost to maintenance this PR adds, gonna merge it as is

@oli-obk oli-obk added this pull request to the merge queue Mar 26, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Mar 26, 2025
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

I don't like this workaround because it silently suppresses legitimate deprecations on parts of the generated impl that come from user-specified attributes. Example:

use serde::Deserializer;
use serde_derive::Deserialize;

#[derive(Deserialize)]
pub struct Struct {
    #[serde(deserialize_with = "deprecated_with")]
    pub field: i32,
}

#[deprecated]
fn deprecated_with<'de, D>(_deserializer: D) -> Result<i32, D::Error>
where
    D: Deserializer<'de>,
{
    unimplemented!()
}
warning: use of deprecated function `deprecated_with`
 --> src/main.rs:6:32
  |
6 |     #[serde(deserialize_with = "deprecated_with")]
  |                                ^^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(deprecated)]` on by default

Please put allow(deprecated) if and only if the input data structure has either #[deprecated] or #[allow(deprecated)].

Allow deprecated in the `Serialize`/`Deserialize`
derive implementations. This allows you to
deprecate structs, enums, struct fields, or enum
variants and not get compiler warnings/errors
about use of deprecated thing. We only do this
if `#[deprecated]` or `#[allow(deprecated)]` exist
on the root object or the variants of the root
object (if it is an enum).

Resolves serde-rs#2195
@rcrisanti rcrisanti force-pushed the fix/silence-deprecation-warnings-derive branch from e4238d4 to ae38b27 Compare May 30, 2025 19:02
@rcrisanti
Copy link
Author

I finally got some time to look into this and made some adjustments based on @dtolnay's suggestions. Now we only add the #[allow(deprecated)] if the struct/enum has #[allow(deprecated)] or #[deprecated] or if one of its variants do (if it is an enum). Let me know if there's anything else you'd want to see on this!

@rcrisanti rcrisanti requested a review from dtolnay May 30, 2025 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

#[derive(Serialize) and #[derive(Deserialize)] on deprecated types emit deprecation warnings
4 participants