[TASK] Use delegation for DeclarationBlock -> RuleSet

JakeQZ · JakeQZ · commit 4d7831a98325 · 2025-03-31T23:51:42.000+01:00
... rather than inheritance. This will allow `DeclarationBlock` to instead extend `CSSBlockList` in order to support [CSS nesting](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_nesting). This is a slightly-breaking change, since now `CSSBlockList::getAllRuleSets()` will include the `RuleSet` property of the `DeclarationBlock` instead of the `DeclarationBlock` itself. Part of #1170.
diff --git a/config/phpstan-baseline.neon b/config/phpstan-baseline.neon
@@ -48,22 +48,28 @@ parameters:
 			count: 1
 			path: ../src/RuleSet/DeclarationBlock.php
 
+		-
+			message: '#^Parameters should have "Sabberworm\\CSS\\Rule\\Rule" types as the only types passed to this method$#'
+			identifier: typePerfect.narrowPublicClassMethodParamType
+			count: 1
+			path: ../src/RuleSet/DeclarationBlock.php
+
 		-
 			message: '#^Parameters should have "string" types as the only types passed to this method$#'
 			identifier: typePerfect.narrowPublicClassMethodParamType
 			count: 1
 			path: ../src/RuleSet/DeclarationBlock.php
 
 		-
-			message: '#^Only booleans are allowed in a negated boolean, string\|null given\.$#'
-			identifier: booleanNot.exprNotBoolean
+			message: '#^Parameters should have "string\|null" types as the only types passed to this method$#'
+			identifier: typePerfect.narrowPublicClassMethodParamType
 			count: 2
-			path: ../src/RuleSet/RuleSet.php
+			path: ../src/RuleSet/DeclarationBlock.php
 
 		-
-			message: '#^Parameters should have "string" types as the only types passed to this method$#'
-			identifier: typePerfect.narrowPublicClassMethodParamType
-			count: 1
+			message: '#^Only booleans are allowed in a negated boolean, string\|null given\.$#'
+			identifier: booleanNot.exprNotBoolean
+			count: 2
 			path: ../src/RuleSet/RuleSet.php
 
 		-
diff --git a/src/CSSList/CSSBlockList.php b/src/CSSList/CSSBlockList.php
@@ -68,14 +68,16 @@ public function getAllRuleSets(): array
                 $result[] = $item;
             } elseif ($item instanceof CSSBlockList) {
                 $result = \array_merge($result, $item->getAllRuleSets());
+            } elseif ($item instanceof DeclarationBlock) {
+                $result[] = $item->getRuleSet();
             }
         }
 
         return $result;
     }
 
     /**
-     * @param CSSList|Rule|RuleSet|Value $element
+     * @param CSSList|DeclarationBlock|Rule|RuleSet|Value $element
      * @param list<Value> $result
      */
     protected function allValues(
@@ -88,7 +90,7 @@ protected function allValues(
             foreach ($element->getContents() as $content) {
                 $this->allValues($content, $result, $searchString, $searchInFunctionArguments);
             }
-        } elseif ($element instanceof RuleSet) {
+        } elseif ($element instanceof RuleSet || $element instanceof DeclarationBlock) {
             foreach ($element->getRules($searchString) as $rule) {
                 $this->allValues($rule, $result, $searchString, $searchInFunctionArguments);
             }
diff --git a/src/RuleSet/DeclarationBlock.php b/src/RuleSet/DeclarationBlock.php
@@ -4,7 +4,9 @@
 
 namespace Sabberworm\CSS\RuleSet;
 
+use Sabberworm\CSS\Comment\CommentContainer;
 use Sabberworm\CSS\CSSList\CSSList;
+use Sabberworm\CSS\CSSList\CSSListItem;
 use Sabberworm\CSS\CSSList\KeyFrame;
 use Sabberworm\CSS\OutputFormat;
 use Sabberworm\CSS\Parsing\OutputException;
@@ -13,22 +15,47 @@
 use Sabberworm\CSS\Parsing\UnexpectedTokenException;
 use Sabberworm\CSS\Property\KeyframeSelector;
 use Sabberworm\CSS\Property\Selector;
+use Sabberworm\CSS\Renderable;
+use Sabberworm\CSS\Rule\Rule;
 
 /**
- * This class represents a `RuleSet` constrained by a `Selector`.
+ * This class includes a `RuleSet` constrained by a `Selector`.
  *
  * It contains an array of selector objects (comma-separated in the CSS) as well as the rules to be applied to the
  * matching elements.
  *
  * Declaration blocks usually appear directly inside a `Document` or another `CSSList` (mostly a `MediaQuery`).
+ *
+ * Note that `CSSListItem` extends both `Commentable` and `Renderable`, so those interfaces must also be implemented.
  */
-class DeclarationBlock extends RuleSet
+class DeclarationBlock implements CSSListItem
 {
+    use CommentContainer;
+
     /**
      * @var array<Selector|string>
      */
     private $selectors = [];
 
+    /**
+     * @var RuleSet
+     */
+    private $ruleSet;
+
+    /**
+     * @var int<0, max>
+     */
+    private $lineNumber;
+
+    /**
+     * @param int<0, max> $lineNumber
+     */
+    public function __construct(int $lineNumber = 0)
+    {
+        $this->lineNumber = $lineNumber;
+        $this->ruleSet = new RuleSet($lineNumber);
+    }
+
     /**
      * @throws UnexpectedTokenException
      * @throws UnexpectedEOFException
@@ -67,10 +94,20 @@ public static function parse(ParserState $parserState, ?CSSList $list = null): ?
             }
         }
         $result->setComments($comments);
-        RuleSet::parseRuleSet($parserState, $result);
+
+        RuleSet::parseRuleSet($parserState, $result->ruleSet);
+
         return $result;
     }
 
+    /**
+     * @return int<0, max>
+     */
+    public function getLineNo(): int
+    {
+        return $this->lineNumber;
+    }
+
     /**
      * @param array<Selector|string>|string $selectors
      *
@@ -135,6 +172,63 @@ public function getSelectors(): array
         return $this->selectors;
     }
 
+    public function getRuleSet(): RuleSet
+    {
+        return $this->ruleSet;
+    }
+
+    /**
+     * @see RuleSet::addRule()
+     */
+    public function addRule(Rule $ruleToAdd, ?Rule $sibling = null): void
+    {
+        $this->ruleSet->addRule($ruleToAdd, $sibling);
+    }
+
+    /**
+     * @see RuleSet::getRules()
+     *
+     * @param Rule|string|null $searchPattern
+     *
+     * @return array<int<0, max>, Rule>
+     */
+    public function getRules($searchPattern = null): array
+    {
+        return $this->ruleSet->getRules($searchPattern);
+    }
+
+    /**
+     * @see RuleSet::setRules()
+     *
+     * @param array<Rule> $rules
+     */
+    public function setRules(array $rules): void
+    {
+        $this->ruleSet->setRules($rules);
+    }
+
+    /**
+     * @see RuleSet::getRulesAssoc()
+     *
+     * @param Rule|string|null $searchPattern
+     *
+     * @return array<string, Rule>
+     */
+    public function getRulesAssoc($searchPattern = null): array
+    {
+        return $this->ruleSet->getRulesAssoc($searchPattern);
+    }
+
+    /**
+     * @see RuleSet::removeRule()
+     *
+     * @param Rule|string|null $searchPattern
+     */
+    public function removeRule($searchPattern): void
+    {
+        $this->ruleSet->removeRule($searchPattern);
+    }
+
     /**
      * @return non-empty-string
      *
@@ -155,7 +249,7 @@ public function render(OutputFormat $outputFormat): string
         );
         $result .= $outputFormat->getContentAfterDeclarationBlockSelectors();
         $result .= $formatter->spaceBeforeOpeningBrace() . '{';
-        $result .= $this->renderRules($outputFormat);
+        $result .= $this->ruleSet->render($outputFormat);
         $result .= '}';
         $result .= $outputFormat->getContentAfterDeclarationBlock();
 
diff --git a/src/RuleSet/RuleSet.php b/src/RuleSet/RuleSet.php
@@ -21,10 +21,9 @@
  * If you want to manipulate a `RuleSet`, use the methods `addRule(Rule $rule)`, `getRules()` and `removeRule($rule)`
  * (which accepts either a `Rule` or a rule name; optionally suffixed by a dash to remove all related rules).
  *
- * Note that `CSSListItem` extends both `Commentable` and `Renderable`,
- * so those interfaces must also be implemented by concrete subclasses.
+ * Note that `CSSListItem` extends both `Commentable` and `Renderable`, so those interfaces must also be implemented.
  */
-abstract class RuleSet implements CSSListItem
+class RuleSet implements CSSListItem
 {
     use CommentContainer;
 
@@ -262,6 +261,14 @@ public function removeRule($searchPattern): void
         }
     }
 
+    /**
+     * @internal
+     */
+    public function render(OutputFormat $outputFormat): string
+    {
+        return $this->renderRules($outputFormat);
+    }
+
     protected function renderRules(OutputFormat $outputFormat): string
     {
         $result = '';
diff --git a/tests/ParserTest.php b/tests/ParserTest.php
@@ -37,7 +37,7 @@ final class ParserTest extends TestCase
     /**
      * @test
      */
-    public function parseForOneRuleSetReturnsDocumentWithOneRuleSet(): void
+    public function parseForOneDeclarationBlockReturnsDocumentWithOneDeclarationBlock(): void
     {
         $css = '.thing { left: 10px; }';
         $parser = new Parser($css);
@@ -48,7 +48,7 @@ public function parseForOneRuleSetReturnsDocumentWithOneRuleSet(): void
 
         $cssList = $document->getContents();
         self::assertCount(1, $cssList);
-        self::assertInstanceOf(RuleSet::class, $cssList[0]);
+        self::assertInstanceOf(DeclarationBlock::class, $cssList[0]);
     }
 
     /**
@@ -928,9 +928,9 @@ public function missingPropertyValueStrict(): void
     public function missingPropertyValueLenient(): void
     {
         $parsed = self::parsedStructureForFile('missing-property-value', Settings::create()->withLenientParsing(true));
-        $rulesets = $parsed->getAllRuleSets();
-        self::assertCount(1, $rulesets);
-        $block = $rulesets[0];
+        $declarationBlocks = $parsed->getAllDeclarationBlocks();
+        self::assertCount(1, $declarationBlocks);
+        $block = $declarationBlocks[0];
         self::assertInstanceOf(DeclarationBlock::class, $block);
         self::assertEquals([new Selector('div')], $block->getSelectors());
         $rules = $block->getRules();
@@ -985,6 +985,7 @@ public function lineNumbersParsing(): void
                 && !$contentItem instanceof CSSNamespace
                 && !$contentItem instanceof Import
                 && !$contentItem instanceof RuleSet
+                && !$contentItem instanceof DeclarationBlock
             ) {
                 self::fail('Content item is not of an expected type.  It\'s a `' . \get_class($contentItem) . '`.');
             }
@@ -994,6 +995,7 @@ public function lineNumbersParsing(): void
                     if (
                         !$block instanceof CSSList
                         && !$block instanceof RuleSet
+                        && !$block instanceof DeclarationBlock
                     ) {
                         self::fail(
                             'KeyFrame content item is not of an expected type.  It\'s a `' . \get_class($block) . '`.'
@@ -1076,7 +1078,7 @@ public function commentExtracting(): void
         // $this->assertSame("* Number 5 *", $fooBarBlockComments[1]->getComment());
 
         // Declaration rules.
-        self::assertInstanceOf(RuleSet::class, $fooBarBlock);
+        self::assertInstanceOf(DeclarationBlock::class, $fooBarBlock);
         $fooBarRules = $fooBarBlock->getRules();
         $fooBarRule = $fooBarRules[0];
         $fooBarRuleComments = $fooBarRule->getComments();
@@ -1097,7 +1099,7 @@ public function commentExtracting(): void
         self::assertSame('* Number 10 *', $fooBarComments[0]->getComment());
 
         // Media -> declaration -> rule.
-        self::assertInstanceOf(RuleSet::class, $mediaRules[0]);
+        self::assertInstanceOf(DeclarationBlock::class, $mediaRules[0]);
         $fooBarRules = $mediaRules[0]->getRules();
         $fooBarChildComments = $fooBarRules[0]->getComments();
         self::assertCount(1, $fooBarChildComments);
@@ -1113,7 +1115,7 @@ public function flatCommentExtractingOneComment(): void
         $document = $parser->parse();
 
         $contents = $document->getContents();
-        self::assertInstanceOf(RuleSet::class, $contents[0]);
+        self::assertInstanceOf(DeclarationBlock::class, $contents[0]);
         $divRules = $contents[0]->getRules();
         $comments = $divRules[0]->getComments();
 
@@ -1130,7 +1132,7 @@ public function flatCommentExtractingTwoConjoinedCommentsForOneRule(): void
         $document = $parser->parse();
 
         $contents = $document->getContents();
-        self::assertInstanceOf(RuleSet::class, $contents[0]);
+        self::assertInstanceOf(DeclarationBlock::class, $contents[0]);
         $divRules = $contents[0]->getRules();
         $comments = $divRules[0]->getComments();
 
@@ -1148,7 +1150,7 @@ public function flatCommentExtractingTwoSpaceSeparatedCommentsForOneRule(): void
         $document = $parser->parse();
 
         $contents = $document->getContents();
-        self::assertInstanceOf(RuleSet::class, $contents[0]);
+        self::assertInstanceOf(DeclarationBlock::class, $contents[0]);
         $divRules = $contents[0]->getRules();
         $comments = $divRules[0]->getComments();
 
@@ -1166,7 +1168,7 @@ public function flatCommentExtractingCommentsForTwoRules(): void
         $document = $parser->parse();
 
         $contents = $document->getContents();
-        self::assertInstanceOf(RuleSet::class, $contents[0]);
+        self::assertInstanceOf(DeclarationBlock::class, $contents[0]);
         $divRules = $contents[0]->getRules();
         $rule1Comments = $divRules[0]->getComments();
         $rule2Comments = $divRules[1]->getComments();
diff --git a/tests/RuleSet/DeclarationBlockTest.php b/tests/RuleSet/DeclarationBlockTest.php
@@ -8,13 +8,12 @@
 use Sabberworm\CSS\OutputFormat;
 use Sabberworm\CSS\Parser;
 use Sabberworm\CSS\Rule\Rule;
-use Sabberworm\CSS\RuleSet\RuleSet;
+use Sabberworm\CSS\RuleSet\DeclarationBlock;
 use Sabberworm\CSS\Settings as ParserSettings;
 use Sabberworm\CSS\Value\Size;
 
 /**
  * @covers \Sabberworm\CSS\RuleSet\DeclarationBlock
- * @covers \Sabberworm\CSS\RuleSet\RuleSet
  */
 final class DeclarationBlockTest extends TestCase
 {
@@ -31,7 +30,7 @@ public function overrideRules(): void
         $contents = $document->getContents();
         $wrapper = $contents[0];
 
-        self::assertInstanceOf(RuleSet::class, $wrapper);
+        self::assertInstanceOf(DeclarationBlock::class, $wrapper);
         self::assertCount(2, $wrapper->getRules());
         $wrapper->setRules([$rule]);
 
@@ -52,7 +51,7 @@ public function ruleInsertion(): void
         $contents = $document->getContents();
         $wrapper = $contents[0];
 
-        self::assertInstanceOf(RuleSet::class, $wrapper);
+        self::assertInstanceOf(DeclarationBlock::class, $wrapper);
 
         $leftRules = $wrapper->getRules('left');
         self::assertCount(1, $leftRules);
diff --git a/tests/Unit/CSSList/CSSBlockListTest.php b/tests/Unit/CSSList/CSSBlockListTest.php

Original file line number	Diff line number	Diff line change
`@@ -68,14 +68,16 @@ public function getAllRuleSets(): array`
`68`	`68`	`$result[] = $item;`
`69`	`69`	`} elseif ($item instanceof CSSBlockList) {`
`70`	`70`	`$result = \array_merge($result, $item->getAllRuleSets());`
	`71`	`+ } elseif ($item instanceof DeclarationBlock) {`
	`72`	`+ $result[] = $item->getRuleSet();`
`71`	`73`	`}`
`72`	`74`	`}`
`73`	`75`
`74`	`76`	`return $result;`
`75`	`77`	`}`
`76`	`78`
`77`	`79`	`/**`
`78`		`- * @param CSSList\|Rule\|RuleSet\|Value $element`
	`80`	`+ * @param CSSList\|DeclarationBlock\|Rule\|RuleSet\|Value $element`
`79`	`81`	`* @param list<Value> $result`
`80`	`82`	`*/`
`81`	`83`	`protected function allValues(`
`@@ -88,7 +90,7 @@ protected function allValues(`
`88`	`90`	`foreach ($element->getContents() as $content) {`
`89`	`91`	`$this->allValues($content, $result, $searchString, $searchInFunctionArguments);`
`90`	`92`	`}`
`91`		`- } elseif ($element instanceof RuleSet) {`
	`93`	`+ } elseif ($element instanceof RuleSet \|\| $element instanceof DeclarationBlock) {`
`92`	`94`	`foreach ($element->getRules($searchString) as $rule) {`
`93`	`95`	`$this->allValues($rule, $result, $searchString, $searchInFunctionArguments);`
`94`	`96`	`}`
Original file line number	Diff line number	Diff line change
`@@ -21,10 +21,9 @@`
`21`	`21`	* If you want to manipulate a `RuleSet`, use the methods `addRule(Rule $rule)`, `getRules()` and `removeRule($rule)`
`22`	`22`	* (which accepts either a `Rule` or a rule name; optionally suffixed by a dash to remove all related rules).
`23`	`23`	`*`
`24`		- * Note that `CSSListItem` extends both `Commentable` and `Renderable`,
`25`		`- * so those interfaces must also be implemented by concrete subclasses.`
	`24`	+ * Note that `CSSListItem` extends both `Commentable` and `Renderable`, so those interfaces must also be implemented.
`26`	`25`	`*/`
`27`		`-abstract class RuleSet implements CSSListItem`
	`26`	`+class RuleSet implements CSSListItem`
`28`	`27`	`{`
`29`	`28`	`use CommentContainer;`
`30`	`29`
`@@ -262,6 +261,14 @@ public function removeRule($searchPattern): void`
`262`	`261`	`}`
`263`	`262`	`}`
`264`	`263`
	`264`	`+ /**`
	`265`	`+ * @internal`
	`266`	`+ */`
	`267`	`+ public function render(OutputFormat $outputFormat): string`
	`268`	`+ {`
	`269`	`+ return $this->renderRules($outputFormat);`
	`270`	`+ }`
	`271`	`+`
`265`	`272`	`protected function renderRules(OutputFormat $outputFormat): string`
`266`	`273`	`{`
`267`	`274`	`$result = '';`