Skip to content
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

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

Draft
wants to merge 1 commit 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
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -720,7 +720,6 @@ classDiagram
class Comment {
}

RuleSet <|-- DeclarationBlock: inheritance
Renderable <|-- CSSListItem: inheritance
Commentable <|-- CSSListItem: inheritance
Positionable <|.. RuleSet: realization
Expand Down Expand Up @@ -752,6 +751,8 @@ classDiagram
AtRule <|.. KeyFrame: realization
CSSBlockList <|-- AtRuleBlockList: inheritance
AtRule <|.. AtRuleBlockList: realization
Positionable <|.. DeclarationBlock: realization
CSSListItem <|.. DeclarationBlock: realization
CSSFunction <|-- Color: inheritance
PrimitiveValue <|-- URL: inheritance
RuleValueList <|-- CalcRuleValueList: inheritance
Expand Down Expand Up @@ -781,6 +782,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
18 changes: 12 additions & 6 deletions config/phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -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

-
Expand Down
6 changes: 4 additions & 2 deletions src/CSSList/CSSBlockList.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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);
}
Expand Down
91 changes: 87 additions & 4 deletions src/RuleSet/DeclarationBlock.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,31 +4,55 @@

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;
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 CSSListItem, Positionable
{
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 +91,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 +161,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
*
Expand All @@ -158,7 +241,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 @@ -23,10 +23,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, Positionable
class RuleSet implements CSSListItem, Positionable
{
use CommentContainer;
use Position;
Expand Down Expand Up @@ -250,6 +249,14 @@ public function removeRule($searchPattern): void
}
}

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

protected function renderRules(OutputFormat $outputFormat): string
{
$result = '';
Expand Down
24 changes: 13 additions & 11 deletions tests/ParserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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]);
}

/**
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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) . '`.');
}
Expand All @@ -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) . '`.'
Expand Down Expand Up @@ -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();
Expand All @@ -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);
Expand All @@ -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();

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

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

Expand All @@ -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();
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
Loading