Fix: the serialize feature should also enable serde/derive

[loyd]

In master/last release serialize feature implicitly depends on serde/derive.

Just add resolve = "2" to Cargo.toml to see the following:

error[E0433]: failed to resolve: could not find `Deserialize` in `serde`
  --> src/name.rs:19:45
   |
19 | #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
   |                                             ^^^^^^^^^^^ could not find `Deserialize` in `serde`

error[E0433]: failed to resolve: could not find `Serialize` in `serde`
  --> src/name.rs:19:65
   |
19 | #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
   |                                                                 ^^^^^^^^^ could not find `Serialize` in `serde`

error[E0433]: failed to resolve: could not find `Deserialize` in `serde`
   --> src/name.rs:136:45
    |
136 | #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
    |                                             ^^^^^^^^^^^ could not find `Deserialize` in `serde`

error[E0433]: failed to resolve: could not find `Serialize` in `serde`
   --> src/name.rs:136:65
    |
136 | #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
    |                                                                 ^^^^^^^^^ could not find `Serialize` in `serde`

error[E0433]: failed to resolve: could not find `Deserialize` in `serde`
   --> src/name.rs:186:45
    |
186 | #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
    |                                             ^^^^^^^^^^^ could not find `Deserialize` in `serde`

error[E0433]: failed to resolve: could not find `Serialize` in `serde`
   --> src/name.rs:186:65
    |
186 | #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
    |                                                                 ^^^^^^^^^ could not find `Serialize` in `serde`

error[E0433]: failed to resolve: could not find `Deserialize` in `serde`
   --> src/name.rs:228:45
    |
228 | #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
    |                                             ^^^^^^^^^^^ could not find `Deserialize` in `serde`

error[E0433]: failed to resolve: could not find `Serialize` in `serde`
   --> src/name.rs:228:65
    |
228 | #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
    |                                                                 ^^^^^^^^^ could not find `Serialize` in `serde`

[Mingun]

I would like instead derive Serialize/Deserialize only if serde/derive feature was activated in a downstream crate

[loyd]

@Mingun, how is it possible? Also, I think it's the wrong behavior. A downstream crate can use impl Serialize directly and wants to use quick-xml. The need to explicitly provide a feature is exposing implementation details. The library can easily switch to impl Serialize instead of deriving and it should not change features.

[codecov-commenter]

Codecov Report

Merging #472 (d921a58) into master (f8b292b) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #472      +/-   ##
==========================================
- Coverage   51.01%   51.00%   -0.02%     
==========================================
  Files          30       30              
  Lines       13103    13103              
==========================================
- Hits         6685     6683       -2     
- Misses       6418     6420       +2     

Flag	Coverage Δ
unittests	51.00% <ø> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/reader/parser.rs	98.57% <0.00%> (-0.72%)	⬇️
src/reader/buffered_reader.rs	85.09% <0.00%> (-0.49%)	⬇️
src/de/map.rs	71.62% <0.00%> (-0.46%)	⬇️
src/de/mod.rs	82.02% <0.00%> (+0.08%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[Mingun]

I don't known what steps exactly need to be done to achieve that. That needs an investigation. It is possible to enable feature of another crate, and since Rust 1.60 it is possible to enable feature, but does not automatically depend on that crate using "package-name?/feature-name" syntax. Maybe that may help. If not, that need to introduce another feature in quick-xml, something like "serializable-types" (better name welcomed) and enable serialization of QName & Co. only when this feature is enabled (instead of serialize).

A downstream crate can use impl Serialize directly and wants to use quick-xml. The need to explicitly provide a feature is exposing implementation details. The library can easily switch to impl Serialize instead of deriving and it should not change features.

If downstream crate implement traits manually, it won't needed derive. If it at the same time does not need to serialize QName, it shouldn't need the serde/derive in quick-xml

[loyd]

Ok, just fix it any way you want

[loyd]

Btw, a usual name for the feature is serde, because deserialization is also enabled.

[Mingun]

Btw, a usual name for the feature is serde, because deserialization is also enabled.

You're totally right and I even was thinking about rename that feature, but after that PR I think that this could be wrong. Usually feature named serde means, that crate has some types that implements Serialize and Deserialize and enabling that feature will enable them.

But in quick-xml serialize feature has other meaning: it enables XML (de)serializer. From that point of view enable Serialize and Deserialize for QName under that feature is wrong

[loyd]

You're right. Btw, another way is to use a separate crate (e.g. rmp-serde).