Skip to content

[TASK] Use delegation for DeclarationBlock -> RuleSet #1194

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,6 @@ classDiagram
class Comment {
}

RuleSet <|-- DeclarationBlock: inheritance
Renderable <|-- CSSElement: inheritance
Renderable <|-- CSSListItem: inheritance
Commentable <|-- CSSListItem: inheritance
Expand Down Expand Up @@ -762,6 +761,9 @@ classDiagram
AtRule <|.. KeyFrame: realization
CSSBlockList <|-- AtRuleBlockList: inheritance
AtRule <|.. AtRuleBlockList: realization
Positionable <|.. DeclarationBlock: realization
CSSElement <|.. DeclarationBlock: realization
CSSListItem <|.. DeclarationBlock: realization
CSSFunction <|-- Color: inheritance
PrimitiveValue <|-- URL: inheritance
RuleValueList <|-- CalcRuleValueList: inheritance
Expand Down Expand Up @@ -791,6 +793,7 @@ classDiagram
Charset --> "*" Comment : comments
Charset --> "1" CSSString : charset
DeclarationBlock --> "*" Selector : selectors
DeclarationBlock --> "*" RuleSet : ruleSet
Import --> "*" Comment : comments
OutputFormat --> "1" OutputFormat : nextLevelFormat
OutputFormat --> "1" OutputFormatter : outputFormatter
Expand Down
2 changes: 2 additions & 0 deletions src/CSSList/CSSBlockList.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ public function getAllRuleSets(): array
$result[] = $item;
} elseif ($item instanceof CSSBlockList) {
$result = \array_merge($result, $item->getAllRuleSets());
} elseif ($item instanceof DeclarationBlock) {
$result[] = $item->getRuleSet();
}
}

Expand Down
102 changes: 98 additions & 4 deletions src/RuleSet/DeclarationBlock.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,31 +4,56 @@

namespace Sabberworm\CSS\RuleSet;

use Sabberworm\CSS\Comment\CommentContainer;
use Sabberworm\CSS\CSSElement;
use Sabberworm\CSS\CSSList\CSSList;
use Sabberworm\CSS\CSSList\CSSListItem;
use Sabberworm\CSS\CSSList\KeyFrame;
use Sabberworm\CSS\OutputFormat;
use Sabberworm\CSS\Parsing\OutputException;
use Sabberworm\CSS\Parsing\ParserState;
use Sabberworm\CSS\Parsing\UnexpectedEOFException;
use Sabberworm\CSS\Parsing\UnexpectedTokenException;
use Sabberworm\CSS\Position\Position;
use Sabberworm\CSS\Position\Positionable;
use Sabberworm\CSS\Property\KeyframeSelector;
use Sabberworm\CSS\Property\Selector;
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 CSSElement, CSSListItem, Positionable, RuleContainer
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also should have tests that the two classes implement the interface.

{
use CommentContainer;
use Position;

/**
* @var array<Selector|string>
*/
private $selectors = [];

/**
* @var RuleSet
*/
private $ruleSet;

/**
* @param int<0, max> $lineNumber
*/
public function __construct(int $lineNumber = 0)
{
$this->setPosition($lineNumber);
$this->ruleSet = new RuleSet($lineNumber);
}

/**
* @throws UnexpectedTokenException
* @throws UnexpectedEOFException
Expand Down Expand Up @@ -67,7 +92,9 @@ public static function parse(ParserState $parserState, ?CSSList $list = null): ?
}
}
$result->setComments($comments);
RuleSet::parseRuleSet($parserState, $result);

RuleSet::parseRuleSet($parserState, $result->ruleSet);

return $result;
}

Expand Down Expand Up @@ -135,6 +162,73 @@ 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()
*
* @return array<int<0, max>, Rule>
*/
public function getRules(?string $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()
*
* @return array<string, Rule>
*/
public function getRulesAssoc(?string $searchPattern = null): array
{
return $this->ruleSet->getRulesAssoc($searchPattern);
}

/**
* @see RuleSet::removeRule()
*/
public function removeRule(Rule $ruleToRemove): void
{
$this->ruleSet->removeRule($ruleToRemove);
}

/**
* @see RuleSet::removeMatchingRules()
*/
public function removeMatchingRules(string $searchPattern): void
{
$this->ruleSet->removeMatchingRules($searchPattern);
}

/**
* @see RuleSet::removeAllRules()
*/
public function removeAllRules(): void
{
$this->ruleSet->removeAllRules();
}

/**
* @return non-empty-string
*
Expand All @@ -158,7 +252,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();

Expand Down
13 changes: 10 additions & 3 deletions src/RuleSet/RuleSet.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,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 CSSElement, CSSListItem, Positionable, RuleContainer
class RuleSet implements CSSElement, CSSListItem, Positionable, RuleContainer
{
use CommentContainer;
use Position;
Expand Down Expand Up @@ -264,6 +263,14 @@ public function removeAllRules(): void
$this->rules = [];
}

/**
* @internal
*/
public function render(OutputFormat $outputFormat): string
{
return $this->renderRules($outputFormat);
}

protected function renderRules(OutputFormat $outputFormat): string
{
$result = '';
Expand Down
22 changes: 11 additions & 11 deletions tests/ParserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ final class ParserTest extends TestCase
/**
* @test
*/
public function parseForOneRuleSetReturnsDocumentWithOneRuleSet(): void
public function parseForOneDeclarationBlockReturnsDocumentWithOneDeclarationBlock(): void
{
$css = '.thing { left: 10px; }';
$parser = new Parser($css);
Expand All @@ -49,7 +49,7 @@ public function parseForOneRuleSetReturnsDocumentWithOneRuleSet(): void

$cssList = $document->getContents();
self::assertCount(1, $cssList);
self::assertInstanceOf(RuleSet::class, $cssList[0]);
self::assertInstanceOf(DeclarationBlock::class, $cssList[0]);
}

/**
Expand Down Expand Up @@ -929,9 +929,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();
Expand Down Expand Up @@ -1058,7 +1058,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();
Expand All @@ -1079,7 +1079,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);
Expand All @@ -1095,7 +1095,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();

Expand All @@ -1112,7 +1112,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();

Expand All @@ -1130,7 +1130,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();

Expand All @@ -1148,7 +1148,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();
Expand Down
7 changes: 3 additions & 4 deletions tests/RuleSet/DeclarationBlockTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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]);

Expand All @@ -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);
Expand Down
12 changes: 6 additions & 6 deletions tests/Unit/CSSList/CSSBlockListTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ public function getAllRuleSetsWhenNoContentSetReturnsEmptyArray(): void
/**
* @test
*/
public function getAllRuleSetsReturnsOneDeclarationBlockDirectlySetAsContent(): void
public function getAllRuleSetsReturnsRuleSetFromOneDeclarationBlockDirectlySetAsContent(): void
{
$subject = new ConcreteCSSBlockList();

Expand All @@ -166,7 +166,7 @@ public function getAllRuleSetsReturnsOneDeclarationBlockDirectlySetAsContent():

$result = $subject->getAllRuleSets();

self::assertSame([$declarationBlock], $result);
self::assertSame([$declarationBlock->getRuleSet()], $result);
}

/**
Expand All @@ -187,7 +187,7 @@ public function getAllRuleSetsReturnsOneAtRuleSetDirectlySetAsContent(): void
/**
* @test
*/
public function getAllRuleSetsReturnsMultipleDeclarationBlocksDirectlySetAsContents(): void
public function getAllRuleSetsReturnsRuleSetsFromMultipleDeclarationBlocksDirectlySetAsContents(): void
{
$subject = new ConcreteCSSBlockList();

Expand All @@ -197,7 +197,7 @@ public function getAllRuleSetsReturnsMultipleDeclarationBlocksDirectlySetAsConte

$result = $subject->getAllRuleSets();

self::assertSame([$declarationBlock1, $declarationBlock2], $result);
self::assertSame([$declarationBlock1->getRuleSet(), $declarationBlock2->getRuleSet()], $result);
}

/**
Expand All @@ -219,7 +219,7 @@ public function getAllRuleSetsReturnsMultipleAtRuleSetsDirectlySetAsContents():
/**
* @test
*/
public function getAllRuleSetsReturnsDeclarationBlocksWithinAtRuleBlockList(): void
public function getAllRuleSetsReturnsRuleSetsFromDeclarationBlocksWithinAtRuleBlockList(): void
{
$subject = new ConcreteCSSBlockList();

Expand All @@ -230,7 +230,7 @@ public function getAllRuleSetsReturnsDeclarationBlocksWithinAtRuleBlockList(): v

$result = $subject->getAllRuleSets();

self::assertSame([$declarationBlock], $result);
self::assertSame([$declarationBlock->getRuleSet()], $result);
}

/**
Expand Down