[librsvg: 7/43] Convert Color, FontSizeSpec, LetterSpacingSpec to CssParseError

[Federico Mena Quintero <federico src gnome org>]

commit 03203c3f28140e943004c03a18705749f6595854
Author: Federico Mena Quintero <federico gnome org>
Date:   Thu Dec 19 20:04:10 2019 -0600

    Convert Color, FontSizeSpec, LetterSpacingSpec to CssParseError
    
    These are the ones which used newtype_parse from make_property!(); now
    they use a new newtype_parse_to_parse_error macro syntax.
    
    This also changes all the newtype properties which use those types:
    Color, FloodColor, FontSize, LetterSpacing, LightingColor, StopColor,

 rsvg_internals/src/color.rs           | 34 ++++++++++++++++-------------
 rsvg_internals/src/font_props.rs      | 40 +++++++++++++++++------------------
 rsvg_internals/src/properties.rs      | 12 +++++------
 rsvg_internals/src/property_defs.rs   |  8 +++----
 rsvg_internals/src/property_macros.rs | 27 +++++++++++++++++++----
 5 files changed, 72 insertions(+), 49 deletions(-)
---
diff --git a/rsvg_internals/src/color.rs b/rsvg_internals/src/color.rs
index b1d8003a..45e914ff 100644
--- a/rsvg_internals/src/color.rs
+++ b/rsvg_internals/src/color.rs
@@ -4,26 +4,30 @@ use cssparser::{self, Parser};
 use libc;
 
 use crate::error::*;
-use crate::parsers::Parse;
+use crate::parsers::ParseToParseError;
 use crate::util::utf8_cstr;
 
 pub use cssparser::Color;
 
-impl Parse for cssparser::Color {
-    fn parse(parser: &mut Parser<'_, '_>) -> Result<cssparser::Color, ValueErrorKind> {
-        cssparser::Color::parse(parser)
-            .map_err(|_| ValueErrorKind::parse_error("invalid syntax for color"))
+impl ParseToParseError for cssparser::Color {
+    fn parse_to_parse_error<'i>(
+        parser: &mut Parser<'i, '_>,
+    ) -> Result<cssparser::Color, CssParseError<'i>> {
+        Ok(cssparser::Color::parse(parser)?)
     }
 }
 
-impl Parse for cssparser::RGBA {
-    fn parse(parser: &mut Parser<'_, '_>) -> Result<cssparser::RGBA, ValueErrorKind> {
-        match cssparser::Color::parse(parser) {
-            Ok(cssparser::Color::RGBA(rgba)) => Ok(rgba),
-            Ok(cssparser::Color::CurrentColor) => Err(ValueErrorKind::Value(
+impl ParseToParseError for cssparser::RGBA {
+    fn parse_to_parse_error<'i>(
+        parser: &mut Parser<'i, '_>,
+    ) -> Result<cssparser::RGBA, CssParseError<'i>> {
+        let loc = parser.current_source_location();
+
+        match cssparser::Color::parse_to_parse_error(parser)? {
+            cssparser::Color::RGBA(rgba) => Ok(rgba),
+            cssparser::Color::CurrentColor => Err(loc.new_custom_error(ValueErrorKind::Value(
                 "currentColor is not allowed here".to_string(),
-            )),
-            _ => Err(ValueErrorKind::parse_error("invalid syntax for color")),
+            ))),
         }
     }
 }
@@ -66,8 +70,8 @@ pub fn rgba_to_argb(rgba: cssparser::RGBA) -> u32 {
         | u32::from(rgba.blue)
 }
 
-impl From<Result<Option<cssparser::Color>, ValueErrorKind>> for ColorSpec {
-    fn from(result: Result<Option<cssparser::Color>, ValueErrorKind>) -> ColorSpec {
+impl<'i> From<Result<Option<cssparser::Color>, CssParseError<'i>>> for ColorSpec {
+    fn from(result: Result<Option<cssparser::Color>, CssParseError<'i>>) -> ColorSpec {
         match result {
             Ok(None) => ColorSpec {
                 kind: ColorKind::Inherit,
@@ -102,7 +106,7 @@ pub extern "C" fn rsvg_css_parse_color(string: *const libc::c_char) -> ColorSpec
             argb: 0,
         }
     } else {
-        ColorSpec::from(<Color as Parse>::parse_str(s).map(Some))
+        ColorSpec::from(<Color as ParseToParseError>::parse_str_to_parse_error(s).map(Some))
     }
 }
 
diff --git a/rsvg_internals/src/font_props.rs b/rsvg_internals/src/font_props.rs
index 71bdfdb9..2115471f 100644
--- a/rsvg_internals/src/font_props.rs
+++ b/rsvg_internals/src/font_props.rs
@@ -5,7 +5,7 @@ use cssparser::{BasicParseError, Parser};
 use crate::drawing_ctx::ViewParams;
 use crate::error::*;
 use crate::length::*;
-use crate::parsers::Parse;
+use crate::parsers::{Parse, ParseToParseError};
 use crate::properties::ComputedValues;
 
 // https://www.w3.org/TR/2008/REC-CSS2-20080411/fonts.html#propdef-font-size
@@ -60,13 +60,15 @@ impl FontSizeSpec {
     }
 }
 
-impl Parse for FontSizeSpec {
-    fn parse(parser: &mut Parser<'_, '_>) -> Result<FontSizeSpec, crate::error::ValueErrorKind> {
+impl ParseToParseError for FontSizeSpec {
+    fn parse_to_parse_error<'i>(
+        parser: &mut Parser<'i, '_>,
+    ) -> Result<FontSizeSpec, CssParseError<'i>> {
         parser
             .try_parse(|p| Length::<Both>::parse(p))
             .and_then(|l| Ok(FontSizeSpec::Value(l)))
             .or_else(|_| {
-                parse_identifiers!(
+                Ok(parse_identifiers!(
                     parser,
                     "smaller" => FontSizeSpec::Smaller,
                     "larger" => FontSizeSpec::Larger,
@@ -77,9 +79,8 @@ impl Parse for FontSizeSpec {
                     "large" => FontSizeSpec::Large,
                     "x-large" => FontSizeSpec::XLarge,
                     "xx-large" => FontSizeSpec::XXLarge,
-                )
+                )?)
             })
-            .map_err(|_| ValueErrorKind::parse_error("parse error"))
     }
 }
 
@@ -102,7 +103,7 @@ pub enum FontWeightSpec {
 }
 
 impl Parse for FontWeightSpec {
-    fn parse(parser: &mut Parser<'_, '_>) -> Result<FontWeightSpec, crate::error::ValueErrorKind> {
+    fn parse(parser: &mut Parser<'_, '_>) -> Result<FontWeightSpec, ValueErrorKind> {
         parser
             .try_parse(|p| {
                 parse_identifiers!(
@@ -163,20 +164,19 @@ impl LetterSpacingSpec {
     }
 }
 
-impl Parse for LetterSpacingSpec {
-    fn parse(
-        parser: &mut Parser<'_, '_>,
-    ) -> Result<LetterSpacingSpec, crate::error::ValueErrorKind> {
+impl ParseToParseError for LetterSpacingSpec {
+    fn parse_to_parse_error<'i>(
+        parser: &mut Parser<'i, '_>,
+    ) -> Result<LetterSpacingSpec, CssParseError<'i>> {
         parser
             .try_parse(|p| Length::<Horizontal>::parse(p))
             .and_then(|l| Ok(LetterSpacingSpec::Value(l)))
             .or_else(|_| {
-                parse_identifiers!(
+                Ok(parse_identifiers!(
                     parser,
                     "normal" => LetterSpacingSpec::Normal,
-                )
+                )?)
             })
-            .map_err(|_| ValueErrorKind::parse_error("parse error"))
     }
 }
 
@@ -216,7 +216,7 @@ mod tests {
 
     #[test]
     fn detects_invalid_invalid_font_size() {
-        assert!(is_parse_error(&FontSizeSpec::parse_str("furlong")));
+        assert!(FontSizeSpec::parse_str_to_parse_error("furlong").is_err());
     }
 
     #[test]
@@ -246,11 +246,11 @@ mod tests {
     #[test]
     fn parses_letter_spacing() {
         assert_eq!(
-            <LetterSpacingSpec as Parse>::parse_str("normal"),
+            <LetterSpacingSpec as ParseToParseError>::parse_str_to_parse_error("normal"),
             Ok(LetterSpacingSpec::Normal)
         );
         assert_eq!(
-            <LetterSpacingSpec as Parse>::parse_str("10em"),
+            <LetterSpacingSpec as ParseToParseError>::parse_str_to_parse_error("10em"),
             Ok(LetterSpacingSpec::Value(Length::<Horizontal>::new(
                 10.0,
                 LengthUnit::Em,
@@ -261,14 +261,14 @@ mod tests {
     #[test]
     fn computes_letter_spacing() {
         assert_eq!(
-            <LetterSpacingSpec as Parse>::parse_str("normal").map(|s| s.compute()),
+            <LetterSpacingSpec as ParseToParseError>::parse_str_to_parse_error("normal").map(|s| 
s.compute()),
             Ok(LetterSpacingSpec::Value(Length::<Horizontal>::new(
                 0.0,
                 LengthUnit::Px,
             )))
         );
         assert_eq!(
-            <LetterSpacingSpec as Parse>::parse_str("10em").map(|s| s.compute()),
+            <LetterSpacingSpec as ParseToParseError>::parse_str_to_parse_error("10em").map(|s| s.compute()),
             Ok(LetterSpacingSpec::Value(Length::<Horizontal>::new(
                 10.0,
                 LengthUnit::Em,
@@ -278,7 +278,7 @@ mod tests {
 
     #[test]
     fn detects_invalid_invalid_letter_spacing() {
-        assert!(is_parse_error(&LetterSpacingSpec::parse_str("furlong")));
+        assert!(LetterSpacingSpec::parse_str_to_parse_error("furlong").is_err());
     }
 
     #[test]
diff --git a/rsvg_internals/src/properties.rs b/rsvg_internals/src/properties.rs
index aaec6e15..085a5ac1 100644
--- a/rsvg_internals/src/properties.rs
+++ b/rsvg_internals/src/properties.rs
@@ -240,7 +240,7 @@ pub fn parse_property<'i>(prop_name: &QualName, input: &mut Parser<'i, '_>, acce
             Ok(ParsedProperty::ClipRule(parse_input_to_parse_error(input)?)),
 
         expanded_name!(svg "color") =>
-            Ok(ParsedProperty::Color(parse_input(input)?)),
+            Ok(ParsedProperty::Color(parse_input_to_parse_error(input)?)),
 
         expanded_name!(svg "color-interpolation-filters") =>
             Ok(ParsedProperty::ColorInterpolationFilters(parse_input_to_parse_error(input)?)),
@@ -267,7 +267,7 @@ pub fn parse_property<'i>(prop_name: &QualName, input: &mut Parser<'i, '_>, acce
             Ok(ParsedProperty::Filter(parse_input(input)?)),
 
         expanded_name!(svg "flood-color") =>
-            Ok(ParsedProperty::FloodColor(parse_input(input)?)),
+            Ok(ParsedProperty::FloodColor(parse_input_to_parse_error(input)?)),
 
         expanded_name!(svg "flood-opacity") =>
             Ok(ParsedProperty::FloodOpacity(parse_input(input)?)),
@@ -276,7 +276,7 @@ pub fn parse_property<'i>(prop_name: &QualName, input: &mut Parser<'i, '_>, acce
             Ok(ParsedProperty::FontFamily(parse_input(input)?)),
 
         expanded_name!(svg "font-size") =>
-            Ok(ParsedProperty::FontSize(parse_input(input)?)),
+            Ok(ParsedProperty::FontSize(parse_input_to_parse_error(input)?)),
 
         expanded_name!(svg "font-stretch") =>
             Ok(ParsedProperty::FontStretch(parse_input_to_parse_error(input)?)),
@@ -291,10 +291,10 @@ pub fn parse_property<'i>(prop_name: &QualName, input: &mut Parser<'i, '_>, acce
             Ok(ParsedProperty::FontWeight(parse_input(input)?)),
 
         expanded_name!(svg "letter-spacing") =>
-            Ok(ParsedProperty::LetterSpacing(parse_input(input)?)),
+            Ok(ParsedProperty::LetterSpacing(parse_input_to_parse_error(input)?)),
 
         expanded_name!(svg "lighting-color") =>
-            Ok(ParsedProperty::LightingColor(parse_input(input)?)),
+            Ok(ParsedProperty::LightingColor(parse_input_to_parse_error(input)?)),
 
         expanded_name!(svg "marker") => {
             if accept_shorthands {
@@ -326,7 +326,7 @@ pub fn parse_property<'i>(prop_name: &QualName, input: &mut Parser<'i, '_>, acce
             Ok(ParsedProperty::ShapeRendering(parse_input_to_parse_error(input)?)),
 
         expanded_name!(svg "stop-color") =>
-            Ok(ParsedProperty::StopColor(parse_input(input)?)),
+            Ok(ParsedProperty::StopColor(parse_input_to_parse_error(input)?)),
 
         expanded_name!(svg "stop-opacity") =>
             Ok(ParsedProperty::StopOpacity(parse_input(input)?)),
diff --git a/rsvg_internals/src/property_defs.rs b/rsvg_internals/src/property_defs.rs
index 7b1e0693..871ee83d 100644
--- a/rsvg_internals/src/property_defs.rs
+++ b/rsvg_internals/src/property_defs.rs
@@ -93,7 +93,7 @@ make_property!(
     // surrounding text.
     default: cssparser::RGBA::new(0, 0, 0, 0xff),
     inherits_automatically: true,
-    newtype_parse: cssparser::RGBA,
+    newtype_parse_to_parse_error: cssparser::RGBA,
 );
 
 // https://www.w3.org/TR/SVG11/painting.html#ColorInterpolationProperty
@@ -205,7 +205,7 @@ make_property!(
     FloodColor,
     default: cssparser::Color::RGBA(cssparser::RGBA::new(0, 0, 0, 0)),
     inherits_automatically: false,
-    newtype_parse: cssparser::Color,
+    newtype_parse_to_parse_error: cssparser::Color,
 );
 
 // https://www.w3.org/TR/SVG/filters.html#FloodOpacityProperty
@@ -325,7 +325,7 @@ make_property!(
     LightingColor,
     default: cssparser::Color::RGBA(cssparser::RGBA::new(255, 255, 255, 255)),
     inherits_automatically: false,
-    newtype_parse: cssparser::Color,
+    newtype_parse_to_parse_error: cssparser::Color,
 );
 
 make_property!(
@@ -415,7 +415,7 @@ make_property!(
     StopColor,
     default: cssparser::Color::RGBA(cssparser::RGBA::new(0, 0, 0, 255)),
     inherits_automatically: false,
-    newtype_parse: cssparser::Color,
+    newtype_parse_to_parse_error: cssparser::Color,
 );
 
 // https://www.w3.org/TR/SVG/pservers.html#StopOpacityProperty
diff --git a/rsvg_internals/src/property_macros.rs b/rsvg_internals/src/property_macros.rs
index 4bffee1e..dcbd9626 100644
--- a/rsvg_internals/src/property_macros.rs
+++ b/rsvg_internals/src/property_macros.rs
@@ -74,6 +74,25 @@ macro_rules! make_property {
         }
     };
 
+    ($computed_values_type: ty,
+     $name: ident,
+     default: $default: expr,
+     inherits_automatically: $inherits_automatically: expr,
+     newtype_parse_to_parse_error: $type: ty,
+    ) => {
+        #[derive(Debug, Clone, PartialEq)]
+        pub struct $name(pub $type);
+
+        impl_default!($name, $name($default));
+        impl_property!($computed_values_type, $name, $inherits_automatically);
+
+        impl crate::parsers::ParseToParseError for $name {
+            fn parse_to_parse_error<'i>(parser: &mut ::cssparser::Parser<'i, '_>) -> Result<$name, 
crate::error::CssParseError<'i>> {
+                Ok($name(<$type as crate::parsers::ParseToParseError>::parse_to_parse_error(parser)?))
+            }
+        }
+    };
+
     ($computed_values_type: ty,
      $name: ident,
      default: $default: expr,
@@ -87,9 +106,9 @@ macro_rules! make_property {
 
         $prop
 
-        impl crate::parsers::Parse for $name {
-            fn parse(parser: &mut ::cssparser::Parser<'_, '_>) -> Result<$name, 
crate::error::ValueErrorKind> {
-                Ok($name(<$type as crate::parsers::Parse>::parse(parser)?))
+        impl crate::parsers::ParseToParseError for $name {
+            fn parse_to_parse_error<'i>(parser: &mut ::cssparser::Parser<'i, '_>) -> Result<$name, 
crate::error::CssParseError<'i>> {
+                Ok($name(<$type as crate::parsers::ParseToParseError>::parse_to_parse_error(parser)?))
             }
         }
     };
@@ -224,7 +243,7 @@ mod tests {
         }
 
         let color = RGBA::new(1, 1, 1, 1);
-        let a = <AddColor as Parse>::parse_str("#02030405").unwrap();
+        let a = <AddColor as ParseToParseError>::parse_str_to_parse_error("#02030405").unwrap();
         let b = a.compute(&color);
 
         assert_eq!(b, AddColor(RGBA::new(3, 4, 5, 6)));