Bug 1609256 - Centralize calc function parsing. r=boris

So that extending it to support other math functions like min / max / etc is
simpler.

There should be no behavior change with this patch, though I added a comment to
some places where we don't do calc() clamping correctly (though other browsers
don't either so...).

Differential Revision: https://phabricator.services.mozilla.com/D59939

--HG--
extra : moz-landing-system : lando

servo/components/style/values/specified/angle.rs

@@ -208,7 +208,9 @@ impl Angle {
         input: &mut Parser<'i, 't>,
         allow_unitless_zero: AllowUnitlessZeroAngle,
     ) -> Result<Self, ParseError<'i>> {
+        let location = input.current_source_location();
         let t = input.next()?;
+        let allow_unitless_zero = matches!(allow_unitless_zero, AllowUnitlessZeroAngle::Yes);
         match *t {
             Token::Dimension {
                 value, ref unit, ..
@@ -221,15 +223,12 @@ impl Angle {
                     },
                 }
             },
-            Token::Number { value, .. } if value == 0. => match allow_unitless_zero {
-                AllowUnitlessZeroAngle::Yes => Ok(Angle::zero()),
-                AllowUnitlessZeroAngle::No => {
-                    let t = t.clone();
-                    Err(input.new_unexpected_token_error(t))
-                },
+            Token::Function(ref name) => {
+                let function = CalcNode::math_function(name, location)?;
+                CalcNode::parse_angle(context, input, function)
             },
-            Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => {
-                return input.parse_nested_block(|i| CalcNode::parse_angle(context, i));
+            Token::Number { value, .. } if value == 0. && allow_unitless_zero => {
+                Ok(Angle::zero())
             },
             ref t => {
                 let t = t.clone();

servo/components/style/values/specified/calc.rs

@@ -12,11 +12,19 @@ use crate::values::specified::length::ViewportPercentageLength;
 use crate::values::specified::length::{AbsoluteLength, FontRelativeLength, NoCalcLength};
 use crate::values::specified::{Angle, Time};
 use crate::values::{CSSFloat, CSSInteger};
-use cssparser::{AngleOrNumber, NumberOrPercentage, Parser, Token};
+use cssparser::{AngleOrNumber, CowRcStr, NumberOrPercentage, Parser, Token};
 use std::fmt::{self, Write};
 use style_traits::values::specified::AllowedNumericType;
 use style_traits::{CssWriter, ParseError, SpecifiedValueInfo, StyleParseErrorKind, ToCss};
 
+/// The name of the mathematical function that we're parsing.
+#[derive(Debug, Copy, Clone)]
+pub enum MathFunction {
+    /// `calc()`
+    Calc,
+    // FIXME: min() / max() / clamp
+}
+
 /// A node inside a `Calc` expression's AST.
 #[derive(Clone, Debug)]
 pub enum CalcNode {
@@ -204,10 +212,13 @@ impl CalcNode {
                 Ok(CalcNode::Percentage(unit_value))
             },
             (&Token::ParenthesisBlock, _) => {
-                input.parse_nested_block(|i| CalcNode::parse(context, i, expected_unit))
+                input.parse_nested_block(|input| {
+                    CalcNode::parse_argument(context, input, expected_unit)
+                })
             },
-            (&Token::Function(ref name), _) if name.eq_ignore_ascii_case("calc") => {
-                input.parse_nested_block(|i| CalcNode::parse(context, i, expected_unit))
+            (&Token::Function(ref name), _) => {
+                let function = CalcNode::math_function(name, location)?;
+                CalcNode::parse(context, input, function, expected_unit)
             },
             (t, _) => Err(location.new_unexpected_token_error(t.clone())),
         }
@@ -217,6 +228,20 @@ impl CalcNode {
     ///
     /// This is in charge of parsing, for example, `2 + 3 * 100%`.
     fn parse<'i, 't>(
+        context: &ParserContext,
+        input: &mut Parser<'i, 't>,
+        _function: MathFunction,
+        expected_unit: CalcUnit,
+    ) -> Result<Self, ParseError<'i>> {
+        // TODO: Do something different based on the function name. In
+        // particular, for non-calc function we need to take a list of
+        // comma-separated arguments and such.
+        input.parse_nested_block(|input| {
+            Self::parse_argument(context, input, expected_unit)
+        })
+    }
+
+    fn parse_argument<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
         expected_unit: CalcUnit,
@@ -526,12 +551,26 @@ impl CalcNode {
         })
     }
 
+    /// Given a function name, and the location from where the token came from,
+    /// return a mathematical function corresponding to that name or an error.
+    #[inline]
+    pub fn math_function<'i>(
+        name: &CowRcStr<'i>,
+        location: cssparser::SourceLocation,
+    ) -> Result<MathFunction, ParseError<'i>> {
+        if !name.eq_ignore_ascii_case("calc") {
+            return Err(location.new_unexpected_token_error(Token::Function(name.clone())));
+        }
+        Ok(MathFunction::Calc)
+    }
+
     /// Convenience parsing function for integers.
     pub fn parse_integer<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
+        function: MathFunction,
     ) -> Result<CSSInteger, ParseError<'i>> {
-        Self::parse_number(context, input).map(|n| n.round() as CSSInteger)
+        Self::parse_number(context, input, function).map(|n| n.round() as CSSInteger)
     }
 
     /// Convenience parsing function for `<length> | <percentage>`.
@@ -539,8 +578,9 @@ impl CalcNode {
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
         clamping_mode: AllowedNumericType,
+        function: MathFunction,
     ) -> Result<CalcLengthPercentage, ParseError<'i>> {
-        Self::parse(context, input, CalcUnit::LengthPercentage)?
+        Self::parse(context, input, function, CalcUnit::LengthPercentage)?
             .to_length_or_percentage(clamping_mode)
             .map_err(|()| input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
     }
@@ -549,8 +589,9 @@ impl CalcNode {
     pub fn parse_percentage<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
+        function: MathFunction,
     ) -> Result<CSSFloat, ParseError<'i>> {
-        Self::parse(context, input, CalcUnit::Percentage)?
+        Self::parse(context, input, function, CalcUnit::Percentage)?
             .to_percentage()
             .map_err(|()| input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
     }
@@ -560,8 +601,9 @@ impl CalcNode {
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
         clamping_mode: AllowedNumericType,
+        function: MathFunction,
     ) -> Result<CalcLengthPercentage, ParseError<'i>> {
-        Self::parse(context, input, CalcUnit::Length)?
+        Self::parse(context, input, function, CalcUnit::Length)?
             .to_length_or_percentage(clamping_mode)
             .map_err(|()| input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
     }
@@ -570,8 +612,9 @@ impl CalcNode {
     pub fn parse_number<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
+        function: MathFunction,
     ) -> Result<CSSFloat, ParseError<'i>> {
-        Self::parse(context, input, CalcUnit::Number)?
+        Self::parse(context, input, function, CalcUnit::Number)?
             .to_number()
             .map_err(|()| input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
     }
@@ -580,8 +623,9 @@ impl CalcNode {
     pub fn parse_angle<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
+        function: MathFunction,
     ) -> Result<Angle, ParseError<'i>> {
-        Self::parse(context, input, CalcUnit::Angle)?
+        Self::parse(context, input, function, CalcUnit::Angle)?
             .to_angle()
             .map_err(|()| input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
     }
@@ -590,8 +634,9 @@ impl CalcNode {
     pub fn parse_time<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
+        function: MathFunction,
     ) -> Result<Time, ParseError<'i>> {
-        Self::parse(context, input, CalcUnit::Time)?
+        Self::parse(context, input, function, CalcUnit::Time)?
             .to_time()
             .map_err(|()| input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
     }
@@ -600,8 +645,9 @@ impl CalcNode {
     pub fn parse_number_or_percentage<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
+        function: MathFunction,
     ) -> Result<NumberOrPercentage, ParseError<'i>> {
-        let node = Self::parse(context, input, CalcUnit::Percentage)?;
+        let node = Self::parse(context, input, function, CalcUnit::Percentage)?;
 
         if let Ok(value) = node.to_number() {
             return Ok(NumberOrPercentage::Number { value });
@@ -617,8 +663,9 @@ impl CalcNode {
     pub fn parse_angle_or_number<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
+        function: MathFunction,
     ) -> Result<AngleOrNumber, ParseError<'i>> {
-        let node = Self::parse(context, input, CalcUnit::Angle)?;
+        let node = Self::parse(context, input, function, CalcUnit::Angle)?;
 
         if let Ok(angle) = node.to_angle() {
             let degrees = angle.degrees();

servo/components/style/values/specified/color.rs

@@ -298,8 +298,9 @@ impl<'a, 'b: 'a, 'i: 'a> ::cssparser::ColorComponentParser<'i> for ColorComponen
                 Ok(AngleOrNumber::Angle { degrees })
             },
             Token::Number { value, .. } => Ok(AngleOrNumber::Number { value }),
-            Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => {
-                input.parse_nested_block(|i| CalcNode::parse_angle_or_number(self.0, i))
+            Token::Function(ref name) => {
+                let function = CalcNode::math_function(name, location)?;
+                CalcNode::parse_angle_or_number(self.0, input, function)
             },
             t => return Err(location.new_unexpected_token_error(t)),
         }
@@ -323,15 +324,16 @@ impl<'a, 'b: 'a, 'i: 'a> ::cssparser::ColorComponentParser<'i> for ColorComponen
     ) -> Result<NumberOrPercentage, ParseError<'i>> {
         let location = input.current_source_location();
 
-        match input.next()?.clone() {
+        match *input.next()? {
             Token::Number { value, .. } => Ok(NumberOrPercentage::Number { value }),
             Token::Percentage { unit_value, .. } => {
                 Ok(NumberOrPercentage::Percentage { unit_value })
             },
-            Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => {
-                input.parse_nested_block(|i| CalcNode::parse_number_or_percentage(self.0, i))
+            Token::Function(ref name) => {
+                let function = CalcNode::math_function(name, location)?;
+                CalcNode::parse_number_or_percentage(self.0, input, function)
             },
-            t => return Err(location.new_unexpected_token_error(t)),
+            ref t => return Err(location.new_unexpected_token_error(t.clone())),
         }
     }
 }

servo/components/style/values/specified/length.rs

@@ -599,11 +599,11 @@ impl Length {
                     value,
                 ))))
             },
-            Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => input
-                .parse_nested_block(|input| {
-                    CalcNode::parse_length(context, input, num_context)
-                        .map(|calc| Length::Calc(Box::new(calc)))
-                }),
+            Token::Function(ref name) => {
+                let function = CalcNode::math_function(name, location)?;
+                let calc = CalcNode::parse_length(context, input, num_context, function)?;
+                Ok(Length::Calc(Box::new(calc)))
+            },
             ref token => return Err(location.new_unexpected_token_error(token.clone())),
         }
     }
@@ -822,10 +822,9 @@ impl LengthPercentage {
                     return Ok(LengthPercentage::Length(NoCalcLength::from_px(value)));
                 }
             },
-            Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => {
-                let calc = input.parse_nested_block(|i| {
-                    CalcNode::parse_length_or_percentage(context, i, num_context)
-                })?;
+            Token::Function(ref name) => {
+                let function = CalcNode::math_function(name, location)?;
+                let calc = CalcNode::parse_length_or_percentage(context, input, num_context, function)?;
                 Ok(LengthPercentage::Calc(Box::new(calc)))
             },
             _ => return Err(location.new_unexpected_token_error(token.clone())),

servo/components/style/values/specified/mod.rs

@@ -144,8 +144,9 @@ fn parse_number_with_clamping_mode<'i, 't>(
                 calc_clamping_mode: None,
             })
         },
-        Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => {
-            let result = input.parse_nested_block(|i| CalcNode::parse_number(context, i))?;
+        Token::Function(ref name) => {
+            let function = CalcNode::math_function(name, location)?;
+            let result = CalcNode::parse_number(context, input, function)?;
             Ok(Number {
                 value: result.min(f32::MAX).max(f32::MIN),
                 calc_clamping_mode: Some(clamping_mode),
@@ -543,8 +544,9 @@ impl Parse for Integer {
             Token::Number {
                 int_value: Some(v), ..
             } => Ok(Integer::new(v)),
-            Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => {
-                let result = input.parse_nested_block(|i| CalcNode::parse_integer(context, i))?;
+            Token::Function(ref name) => {
+                let function = CalcNode::math_function(name, location)?;
+                let result = CalcNode::parse_integer(context, input, function)?;
                 Ok(Integer::from_calc(result))
             },
             ref t => Err(location.new_unexpected_token_error(t.clone())),
@@ -559,16 +561,16 @@ impl Integer {
         input: &mut Parser<'i, 't>,
         min: i32,
     ) -> Result<Integer, ParseError<'i>> {
-        match Integer::parse(context, input) {
-            // FIXME(emilio): The spec asks us to avoid rejecting it at parse
-            // time except until computed value time.
-            //
-            // It's not totally clear it's worth it though, and no other browser
-            // does this.
-            Ok(value) if value.value() >= min => Ok(value),
-            Ok(_value) => Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)),
-            Err(e) => Err(e),
+        let value = Integer::parse(context, input)?;
+        // FIXME(emilio): The spec asks us to avoid rejecting it at parse
+        // time except until computed value time.
+        //
+        // It's not totally clear it's worth it though, and no other browser
+        // does this.
+        if value.value() < min {
+            return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
         }
+        Ok(value)
     }
 
     /// Parse a non-negative integer.

servo/components/style/values/specified/percentage.rs

@@ -117,14 +117,14 @@ impl Percentage {
             {
                 Ok(Percentage::new(unit_value))
             },
-            Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => {
-                let result =
-                    input.parse_nested_block(|i| CalcNode::parse_percentage(context, i))?;
+            Token::Function(ref name) => {
+                let function = CalcNode::math_function(name, location)?;
+                let value = CalcNode::parse_percentage(context, input, function)?;
 
                 // TODO(emilio): -moz-image-rect is the only thing that uses
                 // the clamping mode... I guess we could disallow it...
                 Ok(Percentage {
-                    value: result,
+                    value,
                     calc_clamping_mode: Some(num_context),
                 })
             },

servo/components/style/values/specified/time.rs

@@ -69,7 +69,7 @@ impl Time {
     /// Returns a `Time` value from a CSS `calc()` expression.
     pub fn from_calc(seconds: CSSFloat) -> Self {
         Time {
-            seconds: seconds,
+            seconds,
             unit: TimeUnit::Second,
             was_calc: true,
         }
@@ -95,11 +95,20 @@ impl Time {
                 Time::parse_dimension(value, unit, /* from_calc = */ false)
                     .map_err(|()| location.new_custom_error(StyleParseErrorKind::UnspecifiedError))
             },
-            Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => {
-                match input.parse_nested_block(|i| CalcNode::parse_time(context, i)) {
-                    Ok(time) if clamping_mode.is_ok(ParsingMode::DEFAULT, time.seconds) => Ok(time),
-                    _ => Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)),
+            Token::Function(ref name) => {
+                let function = CalcNode::math_function(name, location)?;
+                let time = CalcNode::parse_time(context, input, function)?;
+
+                // FIXME(emilio): Rejecting calc() at parse time is wrong,
+                // was_calc should probably be replaced by calc_clamping_mode or
+                // something like we do for numbers, or we should do the
+                // clamping here instead (simpler, but technically incorrect,
+                // though still more correct than this!).
+                if !clamping_mode.is_ok(ParsingMode::DEFAULT, time.seconds) {
+                    return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
                 }
+
+                Ok(time)
             },
             ref t => return Err(location.new_unexpected_token_error(t.clone())),
         }