From bdd6ac00321638280fe2dad3a110db080a217bcd Mon Sep 17 00:00:00 2001
From: Jake Hotson <jake@qzdesign.co.uk>
Date: Sat, 17 Apr 2021 16:06:18 +0100
Subject: [PATCH] Changes suggested in review

---
 src/CssInliner.php            | 22 +++++++++----------
 src/Utilities/CssDocument.php | 40 ++++++++++++++++-------------------
 2 files changed, 29 insertions(+), 33 deletions(-)

diff --git a/src/CssInliner.php b/src/CssInliner.php
index 78c2e720..59161c10 100644
--- a/src/CssInliner.php
+++ b/src/CssInliner.php
@@ -179,10 +179,10 @@ public function inlineCss(string $css = ''): self
         if ($this->isStyleBlocksParsingEnabled) {
             $combinedCss .= $this->getCssFromAllStyleNodes();
         }
-        $cssDocument = new CssDocument($combinedCss);
+        $parsedCss = new CssDocument($combinedCss);
 
         $excludedNodes = $this->getNodesToExclude();
-        $cssRules = $this->collateCssRules($cssDocument);
+        $cssRules = $this->collateCssRules($parsedCss);
         $cssSelectorConverter = $this->getCssSelectorConverter();
         foreach ($cssRules['inlinable'] as $cssRule) {
             try {
@@ -210,7 +210,7 @@ public function inlineCss(string $css = ''): self
         $this->removeImportantAnnotationFromAllInlineStyles();
 
         $this->determineMatchingUninlinableCssRules($cssRules['uninlinable']);
-        $this->copyUninlinableCssToStyleNode($cssDocument);
+        $this->copyUninlinableCssToStyleNode($parsedCss);
 
         return $this;
     }
@@ -552,7 +552,7 @@ private function getCssSelectorConverter(): CssSelectorConverter
     /**
      * Collates the individual rules from a `CssDocument` object.
      *
-     * @param CssDocument $cssDocument
+     * @param CssDocument $parsedCss
      *
      * @return array<string, array<array-key, array{
      *           media: string,
@@ -573,9 +573,9 @@ private function getCssSelectorConverter(): CssSelectorConverter
      *           e.g., `color: red; height: 4px;`);
      *         - "line" (the line number, e.g. 42).
      */
-    private function collateCssRules(CssDocument $cssDocument): array
+    private function collateCssRules(CssDocument $parsedCss): array
     {
-        $matches = $cssDocument->getStyleRulesData(\array_keys($this->allowedMediaTypes));
+        $matches = $parsedCss->getStyleRulesData(\array_keys($this->allowedMediaTypes));
 
         $cssRules = [
             'inlinable' => [],
@@ -1072,15 +1072,15 @@ private function removeUnsupportedOfTypePseudoClasses(string $selectorPart): str
      * Applies `$this->matchingUninlinableCssRules` to `$this->domDocument` by placing them as CSS in a `<style>`
      * element.
      *
-     * @param CssDocument $cssDocument
+     * @param CssDocument $parsedCss
      *        This may contain any `@import` or `@font-face` rules that should precede the CSS placed in the `<style>`
-     *        element.  If there are no unlinlinable CSS rules to copy there, a `<style>` element will be created
-     *        containing only the applicable at-rules from `$cssDocument`.  If there are none, and there are also no
+     *        element.  If there are no uninlinable CSS rules to copy there, a `<style>` element will be created
+     *        containing only the applicable at-rules from `$parsedCss`.  If there are none, and there are also no
      *        unlinlinable CSS rules, an empty `<style>` element will not be created.
      */
-    private function copyUninlinableCssToStyleNode(CssDocument $cssDocument): void
+    private function copyUninlinableCssToStyleNode(CssDocument $parsedCss): void
     {
-        $css = $cssDocument->renderNonConditionalAtRules();
+        $css = $parsedCss->renderNonConditionalAtRules();
 
         // avoid including unneeded class dependency if there are no rules
         if ($this->getMatchingUninlinableCssRules() !== []) {
diff --git a/src/Utilities/CssDocument.php b/src/Utilities/CssDocument.php
index ec5cf41f..8d336ca8 100644
--- a/src/Utilities/CssDocument.php
+++ b/src/Utilities/CssDocument.php
@@ -12,6 +12,11 @@
  */
 class CssDocument
 {
+    /**
+     * @var string
+     */
+    private const AT_CHARSET_OR_IMPORT_RULE_PATTERN = '/^\\s*+(@((?i)import(?-i)|charset)\\s[^;]++;\\s*+)/';
+
     /**
      * This regular expression pattern will match any nested at-rule apart from
      * {@link https://developer.mozilla.org/en-US/docs/Web/CSS/At-rule#conditional_group_rules conditional group rules},
@@ -25,9 +30,14 @@ class CssDocument
      *
      * @var string
      */
-    private const NON_CONDITIONAL_AT_RULE_MATCHER
+    private const NON_CONDITIONAL_AT_RULE_PATTERN
         = '/(@(?!media\\b)[\\w\\-]++)[^\\{]*+(\\{[^\\{\\}]*+(?:(?2)[^\\{\\}]*+)*+\\})\\s*+/i';
 
+    /**
+     * @var string
+     */
+    private const MEDIA_RULE_BODY_MATCHER = '[^{]*+{(?:[^{}]*+{.*})?\\s*+}\\s*+';
+
     /**
      * Includes regular style rules, and style rules within conditional group rules such as `@media`.
      *
@@ -134,13 +144,7 @@ private function extractImportAndCharsetRules(string $css): array
         $possiblyModifiedCss = $css;
         $importRules = '';
 
-        while (
-            \preg_match(
-                '/^\\s*+(@((?i)import(?-i)|charset)\\s[^;]++;\\s*+)/',
-                $possiblyModifiedCss,
-                $matches
-            )
-        ) {
+        while (\preg_match(self::AT_CHARSET_OR_IMPORT_RULE_PATTERN, $possiblyModifiedCss, $matches)) {
             [$fullMatch, $atRuleAndFollowingWhitespace, $atRuleName] = $matches;
 
             if (\strtolower($atRuleName) === 'import') {
@@ -172,25 +176,19 @@ private function extractImportAndCharsetRules(string $css): array
     private function extractNonConditionalAtRules(string $css): array
     {
         $possiblyModifiedCss = $css;
-        $atRules = '';
+        $atRules = [];
 
-        while (
-            \preg_match(
-                self::NON_CONDITIONAL_AT_RULE_MATCHER,
-                $possiblyModifiedCss,
-                $matches
-            )
-        ) {
+        while (\preg_match(self::NON_CONDITIONAL_AT_RULE_PATTERN, $possiblyModifiedCss, $matches)) {
             [$fullMatch, $atRuleName] = $matches;
 
             if ($this->isValidAtRule($atRuleName, $fullMatch)) {
-                $atRules .= $fullMatch;
+                $atRules[] = $fullMatch;
             }
 
             $possiblyModifiedCss = \str_replace($fullMatch, '', $possiblyModifiedCss);
         }
 
-        return [$possiblyModifiedCss, $atRules];
+        return [$possiblyModifiedCss, \implode('', $atRules)];
     }
 
     /**
@@ -247,10 +245,8 @@ private function splitCssAndMediaQuery(array $allowedMediaTypes): array
             $mediaTypesExpression = '|' . \implode('|', $allowedMediaTypes);
         }
 
-        $mediaRuleBodyMatcher = '[^{]*+{(?:[^{}]*+{.*})?\\s*+}\\s*+';
-
         $cssSplitForAllowedMediaTypes = \preg_split(
-            '#(@media\\s++(?:only\\s++)?+(?:(?=[{(])' . $mediaTypesExpression . ')' . $mediaRuleBodyMatcher
+            '#(@media\\s++(?:only\\s++)?+(?:(?=[{(])' . $mediaTypesExpression . ')' . self::MEDIA_RULE_BODY_MATCHER
             . ')#misU',
             $this->styleRules,
             -1,
@@ -260,7 +256,7 @@ private function splitCssAndMediaQuery(array $allowedMediaTypes): array
         // filter the CSS outside/between allowed @media rules
         $cssCleaningMatchers = [
             'import/charset directives' => '/\\s*+@(?:import|charset)\\s[^;]++;/i',
-            'remaining media enclosures' => '/\\s*+@media\\s' . $mediaRuleBodyMatcher . '/isU',
+            'remaining media enclosures' => '/\\s*+@media\\s' . self::MEDIA_RULE_BODY_MATCHER . '/isU',
         ];
 
         $splitCss = [];