CSSDataTypeParser consume() function (facebook#48986)

Summary: This adds a new `consume()` function to data type parsers which passes a raw parser. This can be used for types which are compounds of other data types, where we may want to accept more than the first token. This will be used for shadow parsing, but also fixes a hypothetical future bug with ratios. E.g. `calc(foo) / calc(bar)` may be a valid ratio, not starting with a token. We instead just want to try to parse a number data type from the stream. The form of parsing a preserved token + rest is removed, with the assumption that anything parsing more than a single token should use compound parsing. Differential Revision: D68735370
NickGerleman · Jan 28, 2025 · 7fd1a7d · 7fd1a7d
1 parent 69c4da0
commit 7fd1a7d
Show file tree

Hide file tree

Showing 4 changed files with 106 additions and 82 deletions.
diff --git a/packages/react-native/ReactCommon/react/renderer/css/CSSDataType.h b/packages/react-native/ReactCommon/react/renderer/css/CSSDataType.h
@@ -48,29 +48,27 @@ concept CSSSimpleBlockSink =
  * concrete representation.
  */
 template <typename T, typename ReturnT = std::any>
-concept CSSPreservedTokenSink =
-    requires(const CSSPreservedToken& token, CSSSyntaxParser& parser) {
-      {
-        T::consumePreservedToken(token, parser)
-      } -> std::convertible_to<ReturnT>;
-    };
+concept CSSPreservedTokenSink = requires(const CSSPreservedToken& token) {
+  { T::consumePreservedToken(token) } -> std::convertible_to<ReturnT>;
+};
 
 /**
- * Accepts a CSS preserved token and may parse it into a concrete
- * representation.
+ * Accepts a raw syntax parser, to be able to parse compounded values
  */
 template <typename T, typename ReturnT = std::any>
-concept CSSSimplePreservedTokenSink = requires(const CSSPreservedToken& token) {
-  { T::consumePreservedToken(token) } -> std::convertible_to<ReturnT>;
+concept CSSParserSink = requires(CSSSyntaxParser& parser) {
+  { T::consume(parser) } -> std::convertible_to<ReturnT>;
 };
 
 /**
  * Represents a valid specialization of CSSDataTypeParser
  */
 template <typename T, typename ReturnT = std::any>
-concept CSSValidDataTypeParser = CSSFunctionBlockSink<T, ReturnT> ||
-    CSSSimpleBlockSink<T, ReturnT> || CSSPreservedTokenSink<T, ReturnT> ||
-    CSSSimplePreservedTokenSink<T, ReturnT>;
+concept CSSValidDataTypeParser =
+    ((CSSFunctionBlockSink<T, ReturnT> || CSSSimpleBlockSink<T, ReturnT> ||
+      CSSPreservedTokenSink<T, ReturnT>) &&
+     !CSSParserSink<T, ReturnT>) ||
+    CSSParserSink<T, ReturnT>;
 
 /**
  * Concrete representation for a CSS data type, or keywords

diff --git a/packages/react-native/ReactCommon/react/renderer/css/CSSRatio.h b/packages/react-native/ReactCommon/react/renderer/css/CSSRatio.h
@@ -37,23 +37,26 @@ struct CSSRatio {
 
 template <>
 struct CSSDataTypeParser<CSSRatio> {
-  static constexpr auto consumePreservedToken(
-      const CSSPreservedToken& token,
-      CSSSyntaxParser& parser) -> std::optional<CSSRatio> {
+  static constexpr auto consume(CSSSyntaxParser& parser)
+      -> std::optional<CSSRatio> {
     // <ratio> = <number [0,∞]> [ / <number [0,∞]> ]?
     // https://www.w3.org/TR/css-values-4/#ratio
-    if (token.numericValue() >= 0) {
-      float numerator = token.numericValue();
+    auto numerator = parseNextCSSValue<CSSNumber>(parser);
+    if (!std::holds_alternative<CSSNumber>(numerator)) {
+      return {};
+    }
 
+    auto numeratorValue = std::get<CSSNumber>(numerator).value;
+    if (numeratorValue >= 0) {
       auto denominator =
           peekNextCSSValue<CSSNumber>(parser, CSSDelimiter::Solidus);
       if (std::holds_alternative<CSSNumber>(denominator) &&
           std::get<CSSNumber>(denominator).value >= 0) {
         parseNextCSSValue<CSSNumber>(parser, CSSDelimiter::Solidus);
-        return CSSRatio{numerator, std::get<CSSNumber>(denominator).value};
+        return CSSRatio{numeratorValue, std::get<CSSNumber>(denominator).value};
       }
 
-      return CSSRatio{numerator, 1.0f};
+      return CSSRatio{numeratorValue, 1.0f};
     }
 
     return {};

diff --git a/packages/react-native/ReactCommon/react/renderer/css/CSSSyntaxParser.h b/packages/react-native/ReactCommon/react/renderer/css/CSSSyntaxParser.h
@@ -173,6 +173,59 @@ class CSSSyntaxParser {
     }
   }
 
+  /**
+   * Consume a delimiter, returning false if a required delimiter is not found.
+   */
+  constexpr bool consumeDelimiter(CSSDelimiter delimiter) {
+    if (delimiter == CSSDelimiter::None) {
+      return true;
+    }
+
+    bool hasWhiteSpace = peek().type() == CSSTokenType::WhiteSpace;
+    consumeWhitespace();
+
+    switch (delimiter) {
+      case CSSDelimiter::Comma:
+        if (peek().type() == CSSTokenType::Comma) {
+          consumeToken();
+          consumeWhitespace();
+          return true;
+        }
+        return false;
+      case CSSDelimiter::Whitespace:
+        return hasWhiteSpace;
+      case CSSDelimiter::OptionalWhitespace:
+        return true;
+      case CSSDelimiter::CommaOrWhitespace:
+        if (peek().type() == CSSTokenType::Comma) {
+          consumeToken();
+          consumeWhitespace();
+          return true;
+        }
+        return hasWhiteSpace;
+      case CSSDelimiter::Solidus:
+        if (peek().type() == CSSTokenType::Delim &&
+            peek().stringValue() == "/") {
+          consumeToken();
+          consumeWhitespace();
+          return true;
+        }
+        return false;
+      case CSSDelimiter::SolidusOrWhitespace:
+        if (peek().type() == CSSTokenType::Delim &&
+            peek().stringValue() == "/") {
+          consumeToken();
+          consumeWhitespace();
+          return true;
+        }
+        return hasWhiteSpace;
+      case CSSDelimiter::None:
+        return true;
+    }
+
+    return false;
+  }
+
  private:
   constexpr CSSSyntaxParser(CSSSyntaxParser& parser, CSSTokenType terminator)
       : tokenizer_{parser.tokenizer_},
@@ -207,7 +260,7 @@ struct CSSComponentValueVisitorDispatcher {
       CSSDelimiter delimiter,
       const VisitorsT&... visitors) {
     auto originalParser = parser;
-    if (!consumeDelimiter(delimiter)) {
+    if (!parser.consumeDelimiter(delimiter)) {
       parser = originalParser;
       return {};
     }
@@ -252,59 +305,6 @@ struct CSSComponentValueVisitorDispatcher {
     return ReturnT{};
   }
 
-  /**
-   * Consume a delimiter, returning false if a required delimiter is not found.
-   */
-  constexpr bool consumeDelimiter(CSSDelimiter delimiter) {
-    if (delimiter == CSSDelimiter::None) {
-      return true;
-    }
-
-    bool hasWhiteSpace = parser.peek().type() == CSSTokenType::WhiteSpace;
-    parser.consumeWhitespace();
-
-    switch (delimiter) {
-      case CSSDelimiter::Comma:
-        if (parser.peek().type() == CSSTokenType::Comma) {
-          parser.consumeToken();
-          parser.consumeWhitespace();
-          return true;
-        }
-        return false;
-      case CSSDelimiter::Whitespace:
-        return hasWhiteSpace;
-      case CSSDelimiter::OptionalWhitespace:
-        return true;
-      case CSSDelimiter::CommaOrWhitespace:
-        if (parser.peek().type() == CSSTokenType::Comma) {
-          parser.consumeToken();
-          parser.consumeWhitespace();
-          return true;
-        }
-        return hasWhiteSpace;
-      case CSSDelimiter::Solidus:
-        if (parser.peek().type() == CSSTokenType::Delim &&
-            parser.peek().stringValue() == "/") {
-          parser.consumeToken();
-          parser.consumeWhitespace();
-          return true;
-        }
-        return false;
-      case CSSDelimiter::SolidusOrWhitespace:
-        if (parser.peek().type() == CSSTokenType::Delim &&
-            parser.peek().stringValue() == "/") {
-          parser.consumeToken();
-          parser.consumeWhitespace();
-          return true;
-        }
-        return hasWhiteSpace;
-      case CSSDelimiter::None:
-        return true;
-    }
-
-    return false;
-  }
-
   constexpr std::optional<ReturnT> visitFunction(
       const CSSComponentValueVisitor<ReturnT> auto& visitor,
       const CSSComponentValueVisitor<ReturnT> auto&... rest) {

diff --git a/packages/react-native/ReactCommon/react/renderer/css/CSSValueParser.h b/packages/react-native/ReactCommon/react/renderer/css/CSSValueParser.h
@@ -35,6 +35,14 @@ class CSSValueParser {
       CSSDelimiter delimeter = CSSDelimiter::None) {
     using ReturnT = std::variant<std::monostate, AllowedTypesT...>;
 
+    auto consumedValue =
+        tryConsumeParser<ReturnT, CSSDataTypeParser<AllowedTypesT>...>(
+            delimeter);
+
+    if (!std::holds_alternative<std::monostate>(consumedValue)) {
+      return consumedValue;
+    }
+
     return parser_.consumeComponentValue<ReturnT>(
         delimeter,
         [&](const CSSPreservedToken& token) {
@@ -75,14 +83,6 @@ class CSSValueParser {
       CSSValidDataTypeParser... RestParserT>
   constexpr ReturnT tryConsumePreservedToken(const CSSPreservedToken& token) {
     if constexpr (CSSPreservedTokenSink<ParserT>) {
-      auto currentParser = parser_;
-      if (auto ret = ParserT::consumePreservedToken(token, parser_)) {
-        return *ret;
-      }
-      parser_ = currentParser;
-    }
-
-    if constexpr (CSSSimplePreservedTokenSink<ParserT>) {
       if (auto ret = ParserT::consumePreservedToken(token)) {
         return *ret;
       }
@@ -141,6 +141,29 @@ class CSSValueParser {
     return tryConsumeFunctionBlock<ReturnT, RestParserT...>(func, blockParser);
   }
 
+  template <typename ReturnT>
+  constexpr ReturnT tryConsumeParser(CSSDelimiter /*delimeter*/) {
+    return {};
+  }
+
+  template <
+      typename ReturnT,
+      CSSValidDataTypeParser ParserT,
+      CSSValidDataTypeParser... RestParserT>
+  constexpr ReturnT tryConsumeParser(CSSDelimiter delimeter) {
+    if constexpr (CSSParserSink<ParserT>) {
+      auto currentParser = parser_;
+      if (parser_.consumeDelimiter(delimeter)) {
+        if (auto ret = ParserT::consume(parser_)) {
+          return *ret;
+        }
+      }
+      parser_ = currentParser;
+    }
+
+    return tryConsumeParser<ReturnT, RestParserT...>(delimeter);
+  }
+
   CSSSyntaxParser& parser_;
 };