[librsvg: 6/10] Remove the Href type

[Federico Mena Quintero <federico src gnome org>]

commit 7c29d398db80082fe1e1647ae56164a1c057adda
Author: Paolo Borelli <pborelli gnome org>
Date:   Sat Dec 12 16:01:12 2020 +0100

    Remove the Href type
    
    It is not used anymore. Also cleanup the corresponding error types.

 src/error.rs        | 38 ++++++-----------------
 src/handle.rs       |  2 +-
 src/iri.rs          |  2 +-
 src/paint_server.rs |  4 +--
 src/url_resolver.rs | 88 +++++++++--------------------------------------------
 src/xml/mod.rs      |  2 +-
 6 files changed, 30 insertions(+), 106 deletions(-)
---
diff --git a/src/error.rs b/src/error.rs
index e2a53c50..25f0e056 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -89,10 +89,8 @@ impl fmt::Display for ElementError {
 /// Errors returned when looking up a resource by URL reference.
 #[derive(Debug, Clone)]
 pub enum DefsLookupErrorKind {
-    /// Error when parsing an [`Href`].
-    ///
-    /// [`Href`]: allowed_url/enum.Href.html
-    HrefError(HrefError),
+    /// Error when parsing the id to lookup.
+    InvalidId,
 
     /// Used when the public API tries to look up an external URL, which is not allowed.
     ///
@@ -111,7 +109,7 @@ pub enum DefsLookupErrorKind {
 impl fmt::Display for DefsLookupErrorKind {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match *self {
-            DefsLookupErrorKind::HrefError(_) => write!(f, "invalid URL"),
+            DefsLookupErrorKind::InvalidId => write!(f, "invalid id"),
             DefsLookupErrorKind::CannotLookupExternalReferences => {
                 write!(f, "cannot lookup references to elements in external files")
             }
@@ -282,7 +280,7 @@ impl<'i, O> AttributeResultExt<O> for Result<O, ParseError<'i>> {
 #[derive(Debug, Clone)]
 pub enum AllowedUrlError {
     /// parsing error from `Url::parse()`
-    HrefParseError(url::ParseError),
+    UrlParseError(url::ParseError),
 
     /// A base file/uri was not set
     BaseRequired,
@@ -310,7 +308,7 @@ pub enum AllowedUrlError {
 impl fmt::Display for AllowedUrlError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match *self {
-            AllowedUrlError::HrefParseError(e) => write!(f, "href parse error: {}", e),
+            AllowedUrlError::UrlParseError(e) => write!(f, "URL parse error: {}", e),
             AllowedUrlError::BaseRequired => write!(f, "base required"),
             AllowedUrlError::DifferentURISchemes => write!(f, "different URI schemes"),
             AllowedUrlError::DisallowedScheme => write!(f, "disallowed scheme"),
@@ -324,32 +322,16 @@ impl fmt::Display for AllowedUrlError {
     }
 }
 
-/// Errors returned when creating an `Href` out of a string
+/// Errors returned when creating a `Fragment` out of a string
 #[derive(Debug, Clone)]
-pub enum HrefError {
-    /// The href is an invalid URI or has empty components.
-    ParseError,
-
-    /// A fragment identifier ("`#foo`") is not allowed here
-    ///
-    /// For example, the SVG `<image>` element only allows referencing
-    /// resources without fragment identifiers like
-    /// `xlink:href="foo.png"`.
-    FragmentForbidden,
-
-    /// A fragment identifier ("`#foo`") was required but not found.  For example,
-    /// the SVG `<use>` element requires one, as in `<use xlink:href="foo.svg#bar">`.
+pub enum FragmentError {
     FragmentRequired,
 }
 
-impl From<HrefError> for ValueErrorKind {
-    fn from(e: HrefError) -> ValueErrorKind {
+impl From<FragmentError> for ValueErrorKind {
+    fn from(e: FragmentError) -> ValueErrorKind {
         match e {
-            HrefError::ParseError => ValueErrorKind::parse_error("url parse error"),
-            HrefError::FragmentForbidden => {
-                ValueErrorKind::value_error("fragment identifier not allowed")
-            }
-            HrefError::FragmentRequired => {
+            FragmentError::FragmentRequired => {
                 ValueErrorKind::value_error("fragment identifier required")
             }
         }
diff --git a/src/handle.rs b/src/handle.rs
index b20e700c..e65333cb 100644
--- a/src/handle.rs
+++ b/src/handle.rs
@@ -192,7 +192,7 @@ impl Handle {
     }
 
     fn lookup_node(&self, id: &str) -> Result<Node, DefsLookupErrorKind> {
-        let fragment = Fragment::parse(&id).map_err(DefsLookupErrorKind::HrefError)?;
+        let fragment = Fragment::parse(&id).map_err(|_| DefsLookupErrorKind::InvalidId)?;
 
         // The public APIs to get geometries of individual elements, or to render
         // them, should only allow referencing elements within the main handle's
diff --git a/src/iri.rs b/src/iri.rs
index c461b078..d6de265e 100644
--- a/src/iri.rs
+++ b/src/iri.rs
@@ -11,7 +11,7 @@ use crate::url_resolver::Fragment;
 /// This is not to be used for values which don't come from properties.
 /// For example, the `xlink:href` attribute in the `<image>` element
 /// does not take a funciri value (which looks like `url(...)`), but rather
-/// it takes a plain URL.  Use the `Href` type in that case.
+/// it takes a plain URL.
 #[derive(Debug, Clone, PartialEq)]
 pub enum IRI {
     None,
diff --git a/src/paint_server.rs b/src/paint_server.rs
index 8390bce2..6dba1bbe 100644
--- a/src/paint_server.rs
+++ b/src/paint_server.rs
@@ -7,7 +7,7 @@ use crate::document::AcquiredNodes;
 use crate::drawing_ctx::DrawingCtx;
 use crate::element::Element;
 use crate::error::{
-    AcquireError, HrefError, ImplementationLimit, ParseError, RenderingError, ValueErrorKind,
+    AcquireError, FragmentError, ImplementationLimit, ParseError, RenderingError, ValueErrorKind,
 };
 use crate::gradient::{ResolvedGradient, UserSpaceGradient};
 use crate::node::NodeBorrow;
@@ -65,7 +65,7 @@ impl Parse for PaintServer {
 
             Ok(PaintServer::Iri {
                 iri: Fragment::parse(&url)
-                    .map_err(|e: HrefError| -> ValueErrorKind { e.into() })
+                    .map_err(|e: FragmentError| -> ValueErrorKind { e.into() })
                     .map_err(|e| loc.new_custom_error(e))?,
                 alternate,
             })
diff --git a/src/url_resolver.rs b/src/url_resolver.rs
index 52eae3f0..305037ef 100644
--- a/src/url_resolver.rs
+++ b/src/url_resolver.rs
@@ -6,7 +6,7 @@ use std::ops::Deref;
 use std::path::{Path, PathBuf};
 use url::Url;
 
-use crate::error::{AllowedUrlError, HrefError};
+use crate::error::{AllowedUrlError, FragmentError};
 
 /// Currently only contains the base URL.
 ///
@@ -33,7 +33,7 @@ impl UrlResolver {
         let url = Url::options()
             .base_url(self.base_url.as_ref())
             .parse(href)
-            .map_err(AllowedUrlError::HrefParseError)?;
+            .map_err(AllowedUrlError::UrlParseError)?;
 
         // Allow loads of data: from any location
         if url.scheme() == "data" {
@@ -127,34 +127,28 @@ fn canonicalize<P: AsRef<Path>>(path: P) -> Result<PathBuf, io::Error> {
     Ok(path.as_ref().to_path_buf())
 }
 
-/// Parsed result of an href from an SVG or CSS file
-///
-/// Sometimes in SVG element references (e.g. the `href` in the `<feImage>` element) we
-/// must decide between referencing an external file, or using a plain fragment identifier
-/// like `href="#foo"` as a reference to an SVG element in the same file as the one being
-/// processed.  This enum makes that distinction.
-#[derive(Debug, PartialEq)]
-pub enum Href {
-    PlainUrl(String),
-    WithFragment(Fragment),
-}
-
 /// Optional URI, mandatory fragment id
 #[derive(Debug, PartialEq, Clone)]
 pub struct Fragment(Option<String>, String);
 
 impl Fragment {
     // Outside of testing, we don't want code creating Fragments by hand;
-    // they should get them from Href.
+    // they are obtained by parsing a href string.
     #[cfg(test)]
     pub fn new(uri: Option<String>, fragment: String) -> Fragment {
         Fragment(uri, fragment)
     }
 
-    pub fn parse(href: &str) -> Result<Fragment, HrefError> {
-        match Href::parse(&href)? {
-            Href::PlainUrl(_) => Err(HrefError::FragmentRequired),
-            Href::WithFragment(f) => Ok(f),
+    pub fn parse(href: &str) -> Result<Fragment, FragmentError> {
+        let (uri, fragment) = match href.rfind('#') {
+            None => (Some(href), None),
+            Some(p) if p == 0 => (None, Some(&href[1..])),
+            Some(p) => (Some(&href[..p]), Some(&href[(p + 1)..])),
+        };
+
+        match (uri, fragment) {
+            (u, Some(f)) if !f.is_empty() => Ok(Fragment(u.map(String::from), String::from(f))),
+            _ => Err(FragmentError::FragmentRequired),
         }
     }
 
@@ -173,35 +167,6 @@ impl fmt::Display for Fragment {
     }
 }
 
-impl Href {
-    /// Parses a string into an Href, or returns an error
-    ///
-    /// An href can come from an `xlink:href` attribute in an SVG
-    /// element.  This function determines if the provided href is a
-    /// plain absolute or relative URL ("`foo.png`"), or one with a
-    /// fragment identifier ("`foo.svg#bar`").
-    pub fn parse(href: &str) -> Result<Href, HrefError> {
-        let (uri, fragment) = match href.rfind('#') {
-            None => (Some(href), None),
-            Some(p) if p == 0 => (None, Some(&href[1..])),
-            Some(p) => (Some(&href[..p]), Some(&href[(p + 1)..])),
-        };
-
-        match (uri, fragment) {
-            (None, Some(f)) if f.is_empty() => Err(HrefError::ParseError),
-            (None, Some(f)) => Ok(Href::WithFragment(Fragment(None, f.to_string()))),
-            (Some(u), _) if u.is_empty() => Err(HrefError::ParseError),
-            (Some(u), None) => Ok(Href::PlainUrl(u.to_string())),
-            (Some(_u), Some(f)) if f.is_empty() => Err(HrefError::ParseError),
-            (Some(u), Some(f)) => Ok(Href::WithFragment(Fragment(
-                Some(u.to_string()),
-                f.to_string(),
-            ))),
-            (_, _) => Err(HrefError::ParseError),
-        }
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -211,7 +176,7 @@ mod tests {
         let url_resolver = UrlResolver::new(None);
         assert!(matches!(
             url_resolver.resolve_href("foo.svg"),
-            Err(AllowedUrlError::HrefParseError(
+            Err(AllowedUrlError::UrlParseError(
                 url::ParseError::RelativeUrlWithoutBase
             ))
         ));
@@ -300,29 +265,6 @@ mod tests {
         ));
     }
 
-    #[test]
-    fn parses_href() {
-        assert_eq!(
-            Href::parse("uri").unwrap(),
-            Href::PlainUrl("uri".to_string())
-        );
-        assert_eq!(
-            Href::parse("#fragment").unwrap(),
-            Href::WithFragment(Fragment::new(None, "fragment".to_string()))
-        );
-        assert_eq!(
-            Href::parse("uri#fragment").unwrap(),
-            Href::WithFragment(Fragment::new(
-                Some("uri".to_string()),
-                "fragment".to_string()
-            ))
-        );
-
-        assert!(matches!(Href::parse(""), Err(HrefError::ParseError)));
-        assert!(matches!(Href::parse("#"), Err(HrefError::ParseError)));
-        assert!(matches!(Href::parse("uri#"), Err(HrefError::ParseError)));
-    }
-
     #[test]
     fn parses_fragment() {
         assert_eq!(
@@ -337,7 +279,7 @@ mod tests {
 
         assert!(matches!(
             Fragment::parse("uri"),
-            Err(HrefError::FragmentRequired)
+            Err(FragmentError::FragmentRequired)
         ));
     }
 }
diff --git a/src/xml/mod.rs b/src/xml/mod.rs
index 20f07409..6c619cc8 100644
--- a/src/xml/mod.rs
+++ b/src/xml/mod.rs
@@ -491,7 +491,7 @@ impl XmlState {
                 .unwrap()
                 .resolve_href(href)
                 .map_err(|e| {
-                    // FIXME: should AlloweUrlError::HrefParseError be a fatal error,
+                    // FIXME: should AlloweUrlError::UrlParseError be a fatal error,
                     // not a resource error?
                     rsvg_log!("could not acquire \"{}\": {}", href, e);
                     AcquireError::ResourceError