Auto merge of #521 - servo:serde, r=SimonSapin

Some serde improvements

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-url/521)
<!-- Reviewable:end -->

Cargo.toml

@@ -40,7 +40,7 @@ bencher = "0.1"
 idna = { version = "0.2.0", path = "./idna" }
 matches = "0.1"
 percent-encoding = { version = "2.0.0", path = "./percent_encoding" }
-serde = {version = "1.0", optional = true}
+serde = {version = "1.0", optional = true, features = ["derive"]}
 
 [[bench]]
 name = "parse_url"

src/host.rs

@@ -9,57 +9,21 @@
 use idna;
 use parser::{ParseError, ParseResult};
 use percent_encoding::{percent_decode, utf8_percent_encode, CONTROLS};
+#[cfg(feature = "serde")]
+use serde::{Deserialize, Serialize};
 use std::cmp;
 use std::fmt::{self, Formatter};
 use std::net::{Ipv4Addr, Ipv6Addr};
 
+#[cfg_attr(feature = "serde", derive(Deserialize, Serialize))]
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
-pub enum HostInternal {
+pub(crate) enum HostInternal {
     None,
     Domain,
     Ipv4(Ipv4Addr),
     Ipv6(Ipv6Addr),
 }
 
-#[cfg(feature = "serde")]
-impl ::serde::Serialize for HostInternal {
-    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-    where
-        S: ::serde::Serializer,
-    {
-        // This doesn’t use `derive` because that involves
-        // large dependencies (that take a long time to build), and
-        // either Macros 1.1 which are not stable yet or a cumbersome build script.
-        //
-        // Implementing `Serializer` correctly for an enum is tricky,
-        // so let’s use existing enums that already do.
-        use std::net::IpAddr;
-        match *self {
-            HostInternal::None => None,
-            HostInternal::Domain => Some(None),
-            HostInternal::Ipv4(addr) => Some(Some(IpAddr::V4(addr))),
-            HostInternal::Ipv6(addr) => Some(Some(IpAddr::V6(addr))),
-        }
-        .serialize(serializer)
-    }
-}
-
-#[cfg(feature = "serde")]
-impl<'de> ::serde::Deserialize<'de> for HostInternal {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    where
-        D: ::serde::Deserializer<'de>,
-    {
-        use std::net::IpAddr;
-        Ok(match ::serde::Deserialize::deserialize(deserializer)? {
-            None => HostInternal::None,
-            Some(None) => HostInternal::Domain,
-            Some(Some(IpAddr::V4(addr))) => HostInternal::Ipv4(addr),
-            Some(Some(IpAddr::V6(addr))) => HostInternal::Ipv6(addr),
-        })
-    }
-}
-
 impl<S> From<Host<S>> for HostInternal {
     fn from(host: Host<S>) -> HostInternal {
         match host {
@@ -71,6 +35,7 @@ impl<S> From<Host<S>> for HostInternal {
 }
 
 /// The host name of an URL.
+#[cfg_attr(feature = "serde", derive(Deserialize, Serialize))]
 #[derive(Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)]
 pub enum Host<S = String> {
     /// A DNS domain name, as '.' dot-separated labels.
@@ -92,37 +57,6 @@ pub enum Host<S = String> {
     Ipv6(Ipv6Addr),
 }
 
-#[cfg(feature = "serde")]
-impl<S: ::serde::Serialize> ::serde::Serialize for Host<S> {
-    fn serialize<R>(&self, serializer: R) -> Result<R::Ok, R::Error>
-    where
-        R: ::serde::Serializer,
-    {
-        use std::net::IpAddr;
-        match *self {
-            Host::Domain(ref s) => Ok(s),
-            Host::Ipv4(addr) => Err(IpAddr::V4(addr)),
-            Host::Ipv6(addr) => Err(IpAddr::V6(addr)),
-        }
-        .serialize(serializer)
-    }
-}
-
-#[cfg(feature = "serde")]
-impl<'de, S: ::serde::Deserialize<'de>> ::serde::Deserialize<'de> for Host<S> {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    where
-        D: ::serde::Deserializer<'de>,
-    {
-        use std::net::IpAddr;
-        Ok(match ::serde::Deserialize::deserialize(deserializer)? {
-            Ok(s) => Host::Domain(s),
-            Err(IpAddr::V4(addr)) => Host::Ipv4(addr),
-            Err(IpAddr::V6(addr)) => Host::Ipv6(addr),
-        })
-    }
-}
-
 impl<'a> Host<&'a str> {
     /// Return a copy of `self` that owns an allocated `String` but does not borrow an `&Url`.
     pub fn to_owned(&self) -> Host<String> {

src/lib.rs

@@ -2281,11 +2281,28 @@ impl<'de> serde::Deserialize<'de> for Url {
     where
         D: serde::Deserializer<'de>,
     {
-        use serde::de::{Error, Unexpected};
-        let string_representation: String = serde::Deserialize::deserialize(deserializer)?;
-        Url::parse(&string_representation).map_err(|err| {
-            Error::invalid_value(Unexpected::Str(&string_representation), &err.description())
-        })
+        use serde::de::{Error, Unexpected, Visitor};
+
+        struct UrlVisitor;
+
+        impl<'de> Visitor<'de> for UrlVisitor {
+            type Value = Url;
+
+            fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
+                formatter.write_str("a string representing an URL")
+            }
+
+            fn visit_str<E>(self, s: &str) -> Result<Self::Value, E>
+            where
+                E: Error,
+            {
+                Url::parse(s).map_err(|err| {
+                    Error::invalid_value(Unexpected::Str(s), &err.description())
+                })
+            }
+        }
+
+        deserializer.deserialize_str(UrlVisitor)
     }
 }
 

src/parser.rs

@@ -972,7 +972,7 @@ impl<'a> Parser<'a> {
         Ok((host, input))
     }
 
-    pub fn parse_file_host<'i>(
+    pub(crate) fn parse_file_host<'i>(
         &mut self,
         input: Input<'i>,
     ) -> ParseResult<(bool, HostInternal, Input<'i>)> {