Skip to content
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ Please also have a look at our

### Fixed

- Support attribute selectors with values containing commas in
`DeclarationBlock::setSelectors()` (#1419)
- Allow `removeDeclarationBlockBySelector()` to be order-insensitve (#1406)
- Fix parsing of `calc` expressions when a newline immediately precedes or
follows a `+` or `-` operator (#1399)
Expand Down
23 changes: 21 additions & 2 deletions src/RuleSet/DeclarationBlock.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use Sabberworm\CSS\Property\KeyframeSelector;
use Sabberworm\CSS\Property\Selector;
use Sabberworm\CSS\Rule\Rule;
use Sabberworm\CSS\Settings;

/**
* This class represents a `RuleSet` constrained by a `Selector`.
Expand Down Expand Up @@ -98,7 +99,25 @@ public function setSelectors($selectors, ?CSSList $list = null): void
if (\is_array($selectors)) {
$selectorsToSet = $selectors;
} else {
$selectorsToSet = \explode(',', $selectors);
// A string of comma-separated selectors requires parsing.
// Parse as if it's the opening part of a rule.
try {
$parserState = new ParserState($selectors . '{', Settings::create());
$selectorsToSet = self::parseSelectors($parserState);
$parserState->consume('{'); // throw exception if this is not next
if (!$parserState->isEnd()) {
throw new UnexpectedTokenException('EOF', 'more');
}
} catch (UnexpectedTokenException $exception) {
// The exception message from parsing may refer to the faux `{` block start token,
// which would be confusing.
// Rethrow with a more useful message, that also includes the selector(s) string that was passed.
throw new UnexpectedTokenException(
'Selector(s) string is not valid.',
$selectors,
'custom'
);
}
}

// Convert all items to a `Selector` if not already
Expand Down Expand Up @@ -261,7 +280,7 @@ public function render(OutputFormat $outputFormat): string
*
* @throws UnexpectedTokenException
*/
private static function parseSelectors(ParserState $parserState, array &$comments): array
private static function parseSelectors(ParserState $parserState, array &$comments = []): array
{
$selectors = [];
$selectorParts = [];
Expand Down
70 changes: 70 additions & 0 deletions tests/Unit/RuleSet/DeclarationBlockTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use Sabberworm\CSS\CSSElement;
use Sabberworm\CSS\CSSList\CSSListItem;
use Sabberworm\CSS\Parsing\ParserState;
use Sabberworm\CSS\Parsing\UnexpectedTokenException;
use Sabberworm\CSS\Position\Positionable;
use Sabberworm\CSS\Property\Selector;
use Sabberworm\CSS\Rule\Rule;
Expand Down Expand Up @@ -293,4 +294,73 @@ public function setSelectorsIgnoresKeys(): void

self::assertSame([0, 1], \array_keys($result));
}

/**
* @test
*
* @param non-empty-string $selector
*
* @dataProvider provideSelector
*/
public function setSelectorsSetsSingleSelectorProvidedAsString(string $selector): void
{
$subject = new DeclarationBlock();

$subject->setSelectors($selector);

$result = $subject->getSelectors();
self::assertSame([$selector], self::getSelectorsAsStrings($subject));
}

/**
* @test
*
* @param non-empty-string $firstSelector
* @param non-empty-string $secondSelector
*
* @dataProvider provideTwoSelectors
*/
public function setSelectorsSetsTwoCommaSeparatedSelectorsProvidedAsString(
string $firstSelector,
string $secondSelector
): void {
$joinedSelectors = $firstSelector . ', ' . $secondSelector;
$subject = new DeclarationBlock();

$subject->setSelectors($joinedSelectors);

$result = $subject->getSelectors();
self::assertSame([$firstSelector, $secondSelector], self::getSelectorsAsStrings($subject));
}

/**
* Provides selectors that would be parsed without error in the context of full CSS, but are nonetheless invalid.
*
* @return array<non-empty-string, array{0: non-empty-string}>
*/
public static function provideInvalidStandaloneSelector(): array
{
return [
'rogue `{`' => ['a { b'],
'rogue `}`' => ['a } b'],
];
}

/**
* @test
*
* @param non-empty-string $selector
*
* @dataProvider provideInvalidSelector
* @dataProvider provideInvalidStandaloneSelector
*/
public function setSelectorsThrowsExceptionWithInvalidSelector(string $selector): void
{
$this->expectException(UnexpectedTokenException::class);
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should also check for the exception message.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I looked at the messages and found they were unhelpful, so have fixed that.

I also found that the exceptions thrown by the code when { or end of string were not as expected were not being tested, so have added more test data for that.

$this->expectExceptionMessageMatches('/^Selector\\(s\\) string is not valid. /');

$subject = new DeclarationBlock();

$subject->setSelectors($selector);
}
}