Serde: The serde crate's derive feature interacts poorly with 2018-style macro imports

[sfackler]

Say I have a crate that derives Serialize for one type, and has to manually implement it for another. I believe the "standard" way of doing this would be

use serde_derive::Serialize;
use serde::{Serialize, Serializer};

#[derive(Serialize)]
struct Foo(u32);

struct Bar(u32);

impl Serialize for Bar {
    fn serialize<S>(&self, s: S) -> Result<S::Ok, S::Error>
    where
        S: Serializer,
    {
        self.0.to_string().serialize(s)
    }
}

This works fine in isolation, but if this crate is used alongside some other crate that activates serde's derive feature, it will stop compiling:

error[E0252]: the name `Serialize` is defined multiple times                                                                                                            
 --> src/lib.rs:2:13                                                                                                                                                    
  |                                                                                                                                                                     
1 | use serde_derive::Serialize;                                                                                                                                        
  |     ----------------------- previous import of the macro `Serialize` here                                                                                           
2 | use serde::{Serialize, Serializer};                                                                                                                                 
  |             ^^^^^^^^^ `Serialize` reimported here                                                                                                                   
  |                                                                                                                                                                     
  = note: `Serialize` must be defined only once in the macro namespace of this module                                                                                   
help: you can use `as` to change the binding name of the import                                                                                                         
  |                                                                                                                                                                     
2 | use serde::{Serialize as OtherSerialize, Serializer};                                                                                                               
  |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^                                                                                                                             

error: aborting due to previous error                                                                                                                                   

For more information about this error, try `rustc --explain E0252`. 

The crate can be made to compile in both scenarios by importing Serialize and Serializer from serde::ser rather than serde, but it seems like an easy thing to forget, and you're not going to notice until it breaks someone downstream that's trying to use your crate.

What do you think the right way of avoiding this is? Should the recommended approach change from depending on serde_derive to using the derive feature directly?

[dtolnay]

I am on board with your proposed solution -- let's update all documentation and examples to recommend depending on the derive macros re-exported by serde rather than using serde_derive.

[hcpl]

This issue made me think about the impact path-imported macros has on re-exported macros and other items with the exact same name. Previously, they were in completely separate namespaces, so that was fine. Now re-exporting macros will silently re-assign a new item under that name (you could say now the macro namespace silently leaks into the other items namespace).

Which means the pattern where:

• there are related proc-macro and "other items defining" crates;
• the latter re-exports items from the former;
• the proc-macro crate is not disallowed/banned for usage by the author(s);

does not seamlessly work anymore in all newer Rust versions because of the edge case this issue presents.

As a result, now that using the serde_derive is dangerous in upstream crates, should it be deprecated? This will surely create a huge impact on the whole serde ecosystem, but prioritizing buildability is the way to go, in my opinion.

Deprecating will have an added benefit of being future-compatible with procedural macros that are defined in the same crate as non-procedural macro items.

If deprecating serde_derive right now is not viable, we could fix equivalent issues in major crates that use Serde in the beginning. This will raise awareness about this particular pattern with proc-macro crates and more crates will get updated to not follow it. And only when it feels OK to do so, deprecate.

[dtolnay]

Instead of deprecating serde_derive I would prefer if multiple imports of the same item did not conflict with each other. In this case serde::Serialize (the macro) and serde_derive::Serialize resolve to the same item, after re-exports are considered. It should be fine for the compiler to accept having both of those imports because together they only import a single macro -- thus invocations of that macro can be resolved without an ambiguity. Will see if I can find an existing issue for this.

[hcpl]

To be fair, use serde::Serialize; meaning:

• Serialize trait when derive feature is disabled;
• Serialize trait _and_ Serialize macro when derive feature is enabled;

is quite surprising without knowing the context, both because a single use can import 2 items at once and because a use can add an item of a different type to the import list depending on a #[cfg] --- with non-macros the compiler would complain. But macros were always special in one way or another. so _shrug_.

[vorner]

I find the thing surprising in the opposite direction too, somehow. I usually enable the derive feature instead of depending separately on serde-derive (that seems more comfortable). Anyway, if I import as use serde::Deserialize, I get the macro, but if I do use serde::de::{Deserialize, Deserializer};, the macro is no longer available.

[dtolnay]

I updated all example code to use serde's derive feature rather than depending directly on serde_derive.