Skip to content

[TASK] Add (and use) a CSSListItem type #1212

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

Merged
merged 2 commits into from
Mar 25, 2025
Merged
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
12 changes: 6 additions & 6 deletions config/phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ parameters:
count: 1
path: ../src/CSSList/CSSList.php

-
message: '#^Parameters should have "Sabberworm\\CSS\\CSSList\\CSSListItem\|array" types as the only types passed to this method$#'
identifier: typePerfect.narrowPublicClassMethodParamType
count: 1
path: ../src/CSSList/CSSList.php

-
message: '#^Short ternary operator is not allowed\. Use null coalesce operator if applicable or consider using long ternary\.$#'
identifier: ternary.shortNotAllowed
Expand Down Expand Up @@ -54,12 +60,6 @@ parameters:
count: 2
path: ../src/RuleSet/RuleSet.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/RuleSet.php

-
message: '#^Parameters should have "string" types as the only types passed to this method$#'
identifier: typePerfect.narrowPublicClassMethodParamType
Expand Down
42 changes: 18 additions & 24 deletions src/CSSList/CSSList.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,11 @@
* `RuleSet`s as well as other `CSSList` objects.
*
* It can also contain `Import` and `Charset` objects stemming from at-rules.
*
* Note that `CSSListItem` extends both `Commentable` and `Renderable`,
* so those interfaces must also be implemented by concrete subclasses.
*/
abstract class CSSList implements Renderable, Commentable
abstract class CSSList implements CSSListItem
{
/**
* @var list<Comment>
Expand All @@ -41,7 +44,7 @@ abstract class CSSList implements Renderable, Commentable
protected $comments = [];

/**
* @var array<int<0, max>, RuleSet|CSSList|Import|Charset>
* @var array<int<0, max>, CSSListItem>
*
* @internal since 8.8.0
*/
Expand Down Expand Up @@ -105,7 +108,7 @@ public static function parseList(ParserState $parserState, CSSList $list): void
}

/**
* @return AtRuleBlockList|KeyFrame|Charset|CSSNamespace|Import|AtRuleSet|DeclarationBlock|false|null
* @return CSSListItem|false|null
* If `null` is returned, it means the end of the list has been reached.
* If `false` is returned, it means an invalid item has been encountered,
* but parsing of the next item should proceed.
Expand Down Expand Up @@ -156,13 +159,11 @@ private static function parseListItem(ParserState $parserState, CSSList $list)
}

/**
* @return AtRuleBlockList|KeyFrame|Charset|CSSNamespace|Import|AtRuleSet|null
*
* @throws SourceException
* @throws UnexpectedTokenException
* @throws UnexpectedEOFException
*/
private static function parseAtRule(ParserState $parserState)
private static function parseAtRule(ParserState $parserState): ?CSSListItem
{
$parserState->consume('@');
$identifier = $parserState->parseIdentifier();
Expand Down Expand Up @@ -265,28 +266,24 @@ public function getLineNo(): int

/**
* Prepends an item to the list of contents.
*
* @param RuleSet|CSSList|Import|Charset $item
*/
public function prepend($item): void
public function prepend(CSSListItem $item): void
{
\array_unshift($this->contents, $item);
}

/**
* Appends an item to the list of contents.
*
* @param RuleSet|CSSList|Import|Charset $item
*/
public function append($item): void
public function append(CSSListItem $item): void
{
$this->contents[] = $item;
}

/**
* Splices the list of contents.
*
* @param array<int, RuleSet|CSSList|Import|Charset> $replacement
* @param array<int, CSSListItem> $replacement
*/
public function splice(int $offset, ?int $length = null, ?array $replacement = null): void
{
Expand All @@ -296,11 +293,8 @@ public function splice(int $offset, ?int $length = null, ?array $replacement = n
/**
* Inserts an item in the CSS list before its sibling. If the desired sibling cannot be found,
* the item is appended at the end.
*
* @param RuleSet|CSSList|Import|Charset $item
* @param RuleSet|CSSList|Import|Charset $sibling
*/
public function insertBefore($item, $sibling): void
public function insertBefore(CSSListItem $item, CSSListItem $sibling): void
{
if (\in_array($sibling, $this->contents, true)) {
$this->replace($sibling, [$item, $sibling]);
Expand All @@ -312,13 +306,13 @@ public function insertBefore($item, $sibling): void
/**
* Removes an item from the CSS list.
*
* @param RuleSet|Import|Charset|CSSList $itemToRemove
* @param CSSListItem $itemToRemove
* May be a `RuleSet` (most likely a `DeclarationBlock`), an `Import`,
* a `Charset` or another `CSSList` (most likely a `MediaQuery`)
*
* @return bool whether the item was removed
*/
public function remove($itemToRemove): bool
public function remove(CSSListItem $itemToRemove): bool
{
$key = \array_search($itemToRemove, $this->contents, true);
if ($key !== false) {
Expand All @@ -332,12 +326,12 @@ public function remove($itemToRemove): bool
/**
* Replaces an item from the CSS list.
*
* @param RuleSet|Import|Charset|CSSList $oldItem
* @param CSSListItem $oldItem
* May be a `RuleSet` (most likely a `DeclarationBlock`), an `Import`, a `Charset`
* or another `CSSList` (most likely a `MediaQuery`)
* @param RuleSet|Import|Charset|CSSList|array<RuleSet|Import|Charset|CSSList> $newItem
* @param CSSListItem|array<CSSListItem> $newItem
*/
public function replace($oldItem, $newItem): bool
public function replace(CSSListItem $oldItem, $newItem): bool
{
$key = \array_search($oldItem, $this->contents, true);
if ($key !== false) {
Expand All @@ -353,7 +347,7 @@ public function replace($oldItem, $newItem): bool
}

/**
* @param array<int, RuleSet|Import|Charset|CSSList> $contents
* @param array<int, CSSListItem> $contents
*/
public function setContents(array $contents): void
{
Expand Down Expand Up @@ -444,7 +438,7 @@ abstract public function isRootList(): bool;
/**
* Returns the stored items.
*
* @return array<int<0, max>, RuleSet|Import|Charset|CSSList>
* @return array<int<0, max>, CSSListItem>
*/
public function getContents(): array
{
Expand Down
18 changes: 18 additions & 0 deletions src/CSSList/CSSListItem.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php

declare(strict_types=1);

namespace Sabberworm\CSS\CSSList;

use Sabberworm\CSS\Comment\Commentable;
use Sabberworm\CSS\Renderable;

/**
* Represents anything that can be in the `$contents` of a `CSSList`.
*
* The interface does not define any methods to implement.
* It's purpose is to allow a single type to be specified for `CSSList::$contents` and manipulation methods thereof.
* It extends `Commentable` and `Renderable` because all `CSSListItem`s are both.
* This allows implementations to call methods from those interfaces without any additional type checks.
*/
interface CSSListItem extends Commentable, Renderable {}
7 changes: 6 additions & 1 deletion src/Property/AtRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,14 @@
namespace Sabberworm\CSS\Property;

use Sabberworm\CSS\Comment\Commentable;
use Sabberworm\CSS\CSSList\CSSListItem;
use Sabberworm\CSS\Renderable;

interface AtRule extends Renderable, Commentable
/**
* Note that `CSSListItem` extends both `Commentable` and `Renderable`,
* so concrete classes implementing this interface must also implement those.
*/
interface AtRule extends CSSListItem
{
/**
* Since there are more set rules than block rules,
Expand Down
6 changes: 5 additions & 1 deletion src/RuleSet/RuleSet.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use Sabberworm\CSS\Comment\Comment;
use Sabberworm\CSS\Comment\Commentable;
use Sabberworm\CSS\CSSList\CSSListItem;
use Sabberworm\CSS\OutputFormat;
use Sabberworm\CSS\Parsing\ParserState;
use Sabberworm\CSS\Parsing\UnexpectedEOFException;
Expand All @@ -21,8 +22,11 @@
*
* 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.
*/
abstract class RuleSet implements Renderable, Commentable
abstract class RuleSet implements CSSListItem
{
/**
* the rules in this rule set, using the property name as the key,
Expand Down
3 changes: 3 additions & 0 deletions tests/CSSList/AtRuleBlockListTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace Sabberworm\CSS\Tests\CSSList;

use PHPUnit\Framework\TestCase;
use Sabberworm\CSS\CSSList\AtRuleBlockList;
use Sabberworm\CSS\Parser;
use Sabberworm\CSS\Settings;

Expand Down Expand Up @@ -60,6 +61,7 @@ public function parsesRuleNameOfMediaQueries(string $css): void
$contents = (new Parser($css))->parse()->getContents();
$atRuleBlockList = $contents[0];

self::assertInstanceOf(AtRuleBlockList::class, $atRuleBlockList);
self::assertSame('media', $atRuleBlockList->atRuleName());
}

Expand All @@ -73,6 +75,7 @@ public function parsesArgumentsOfMediaQueries(string $css): void
$contents = (new Parser($css))->parse()->getContents();
$atRuleBlockList = $contents[0];

self::assertInstanceOf(AtRuleBlockList::class, $atRuleBlockList);
self::assertSame('(min-width: 768px)', $atRuleBlockList->atRuleArgs());
}

Expand Down
36 changes: 36 additions & 0 deletions tests/ParserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
namespace Sabberworm\CSS\Tests;

use PHPUnit\Framework\TestCase;
use Sabberworm\CSS\Comment\Commentable;
use Sabberworm\CSS\CSSList\CSSList;
use Sabberworm\CSS\CSSList\Document;
use Sabberworm\CSS\CSSList\KeyFrame;
use Sabberworm\CSS\OutputFormat;
Expand Down Expand Up @@ -973,9 +975,30 @@ public function lineNumbersParsing(): void

$actual = [];
foreach ($document->getContents() as $contentItem) {
// PHPStan can see what `assertInstanceOf()` does,
// but does not understand `LogicalOr` with multiple `IsIntanceOf` constraints.
// So a more explicit type check is required.
// TODO: tidy this up when an interface with `getLineNo()` is added.
if (
!$contentItem instanceof Charset &&
!$contentItem instanceof CSSList &&
!$contentItem instanceof CSSNamespace &&
!$contentItem instanceof Import &&
!$contentItem instanceof RuleSet
) {
self::fail('Content item is not of an expected type. It\'s a `' . \get_class($contentItem) . '`.');
}
$actual[$contentItem->getLineNo()] = [\get_class($contentItem)];
if ($contentItem instanceof KeyFrame) {
foreach ($contentItem->getContents() as $block) {
if (
!$block instanceof CSSList &&
!$block instanceof RuleSet
) {
self::fail(
'KeyFrame content item is not of an expected type. It\'s a `' . \get_class($block) . '`.'
);
}
$actual[$contentItem->getLineNo()][] = $block->getLineNo();
}
}
Expand Down Expand Up @@ -1037,37 +1060,44 @@ public function commentExtracting(): void
$nodes = $document->getContents();

// Import property.
self::assertInstanceOf(Commentable::class, $nodes[0]);
$importComments = $nodes[0]->getComments();
self::assertCount(2, $importComments);
self::assertSame("*\n * Comments\n ", $importComments[0]->getComment());
self::assertSame(' Hell ', $importComments[1]->getComment());

// Declaration block.
$fooBarBlock = $nodes[1];
self::assertInstanceOf(Commentable::class, $fooBarBlock);
$fooBarBlockComments = $fooBarBlock->getComments();
// TODO Support comments in selectors.
// $this->assertCount(2, $fooBarBlockComments);
// $this->assertSame("* Number 4 *", $fooBarBlockComments[0]->getComment());
// $this->assertSame("* Number 5 *", $fooBarBlockComments[1]->getComment());

// Declaration rules.
self::assertInstanceOf(RuleSet::class, $fooBarBlock);
$fooBarRules = $fooBarBlock->getRules();
$fooBarRule = $fooBarRules[0];
$fooBarRuleComments = $fooBarRule->getComments();
self::assertCount(1, $fooBarRuleComments);
self::assertSame(' Number 6 ', $fooBarRuleComments[0]->getComment());

// Media property.
self::assertInstanceOf(Commentable::class, $nodes[2]);
$mediaComments = $nodes[2]->getComments();
self::assertCount(0, $mediaComments);

// Media children.
self::assertInstanceOf(CSSList::class, $nodes[2]);
$mediaRules = $nodes[2]->getContents();
self::assertInstanceOf(Commentable::class, $mediaRules[0]);
$fooBarComments = $mediaRules[0]->getComments();
self::assertCount(1, $fooBarComments);
self::assertSame('* Number 10 *', $fooBarComments[0]->getComment());

// Media -> declaration -> rule.
self::assertInstanceOf(RuleSet::class, $mediaRules[0]);
$fooBarRules = $mediaRules[0]->getRules();
$fooBarChildComments = $fooBarRules[0]->getComments();
self::assertCount(1, $fooBarChildComments);
Expand All @@ -1083,6 +1113,7 @@ public function flatCommentExtractingOneComment(): void
$document = $parser->parse();

$contents = $document->getContents();
self::assertInstanceOf(RuleSet::class, $contents[0]);
$divRules = $contents[0]->getRules();
$comments = $divRules[0]->getComments();

Expand All @@ -1099,6 +1130,7 @@ public function flatCommentExtractingTwoConjoinedCommentsForOneRule(): void
$document = $parser->parse();

$contents = $document->getContents();
self::assertInstanceOf(RuleSet::class, $contents[0]);
$divRules = $contents[0]->getRules();
$comments = $divRules[0]->getComments();

Expand All @@ -1116,6 +1148,7 @@ public function flatCommentExtractingTwoSpaceSeparatedCommentsForOneRule(): void
$document = $parser->parse();

$contents = $document->getContents();
self::assertInstanceOf(RuleSet::class, $contents[0]);
$divRules = $contents[0]->getRules();
$comments = $divRules[0]->getComments();

Expand All @@ -1133,6 +1166,7 @@ public function flatCommentExtractingCommentsForTwoRules(): void
$document = $parser->parse();

$contents = $document->getContents();
self::assertInstanceOf(RuleSet::class, $contents[0]);
$divRules = $contents[0]->getRules();
$rule1Comments = $divRules[0]->getComments();
$rule2Comments = $divRules[1]->getComments();
Expand All @@ -1151,6 +1185,7 @@ public function topLevelCommentExtracting(): void
$parser = new Parser('/*Find Me!*/div {left:10px; text-align:left;}');
$document = $parser->parse();
$contents = $document->getContents();
self::assertInstanceOf(Commentable::class, $contents[0]);
$comments = $contents[0]->getComments();
self::assertCount(1, $comments);
self::assertSame('Find Me!', $comments[0]->getComment());
Expand Down Expand Up @@ -1216,6 +1251,7 @@ public function escapedSpecialCaseTokens(): void
{
$document = self::parsedStructureForFile('escaped-tokens');
$contents = $document->getContents();
self::assertInstanceOf(RuleSet::class, $contents[0]);
$rules = $contents[0]->getRules();
$urlRule = $rules[0];
$calcRule = $rules[1];
Expand Down
Loading