Skip to content

Commit 8d123a2

Browse files
authored
[BUGFIX] Allow commas in attributes in setSelectors (#1419)
This fixes an issue noted in #1324. Also add additional test data to fully exercise the code.
1 parent a5d1172 commit 8d123a2

File tree

3 files changed

+93
-2
lines changed

3 files changed

+93
-2
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ Please also have a look at our
2424

2525
### Fixed
2626

27+
- Support attribute selectors with values containing commas in
28+
`DeclarationBlock::setSelectors()` (#1419)
2729
- Allow `removeDeclarationBlockBySelector()` to be order-insensitve (#1406)
2830
- Fix parsing of `calc` expressions when a newline immediately precedes or
2931
follows a `+` or `-` operator (#1399)

src/RuleSet/DeclarationBlock.php

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use Sabberworm\CSS\Property\KeyframeSelector;
2121
use Sabberworm\CSS\Property\Selector;
2222
use Sabberworm\CSS\Rule\Rule;
23+
use Sabberworm\CSS\Settings;
2324

2425
/**
2526
* This class represents a `RuleSet` constrained by a `Selector`.
@@ -98,7 +99,25 @@ public function setSelectors($selectors, ?CSSList $list = null): void
9899
if (\is_array($selectors)) {
99100
$selectorsToSet = $selectors;
100101
} else {
101-
$selectorsToSet = \explode(',', $selectors);
102+
// A string of comma-separated selectors requires parsing.
103+
// Parse as if it's the opening part of a rule.
104+
try {
105+
$parserState = new ParserState($selectors . '{', Settings::create());
106+
$selectorsToSet = self::parseSelectors($parserState);
107+
$parserState->consume('{'); // throw exception if this is not next
108+
if (!$parserState->isEnd()) {
109+
throw new UnexpectedTokenException('EOF', 'more');
110+
}
111+
} catch (UnexpectedTokenException $exception) {
112+
// The exception message from parsing may refer to the faux `{` block start token,
113+
// which would be confusing.
114+
// Rethrow with a more useful message, that also includes the selector(s) string that was passed.
115+
throw new UnexpectedTokenException(
116+
'Selector(s) string is not valid.',
117+
$selectors,
118+
'custom'
119+
);
120+
}
102121
}
103122

104123
// Convert all items to a `Selector` if not already
@@ -261,7 +280,7 @@ public function render(OutputFormat $outputFormat): string
261280
*
262281
* @throws UnexpectedTokenException
263282
*/
264-
private static function parseSelectors(ParserState $parserState, array &$comments): array
283+
private static function parseSelectors(ParserState $parserState, array &$comments = []): array
265284
{
266285
$selectors = [];
267286
$selectorParts = [];

tests/Unit/RuleSet/DeclarationBlockTest.php

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use Sabberworm\CSS\CSSElement;
99
use Sabberworm\CSS\CSSList\CSSListItem;
1010
use Sabberworm\CSS\Parsing\ParserState;
11+
use Sabberworm\CSS\Parsing\UnexpectedTokenException;
1112
use Sabberworm\CSS\Position\Positionable;
1213
use Sabberworm\CSS\Property\Selector;
1314
use Sabberworm\CSS\Rule\Rule;
@@ -293,4 +294,73 @@ public function setSelectorsIgnoresKeys(): void
293294

294295
self::assertSame([0, 1], \array_keys($result));
295296
}
297+
298+
/**
299+
* @test
300+
*
301+
* @param non-empty-string $selector
302+
*
303+
* @dataProvider provideSelector
304+
*/
305+
public function setSelectorsSetsSingleSelectorProvidedAsString(string $selector): void
306+
{
307+
$subject = new DeclarationBlock();
308+
309+
$subject->setSelectors($selector);
310+
311+
$result = $subject->getSelectors();
312+
self::assertSame([$selector], self::getSelectorsAsStrings($subject));
313+
}
314+
315+
/**
316+
* @test
317+
*
318+
* @param non-empty-string $firstSelector
319+
* @param non-empty-string $secondSelector
320+
*
321+
* @dataProvider provideTwoSelectors
322+
*/
323+
public function setSelectorsSetsTwoCommaSeparatedSelectorsProvidedAsString(
324+
string $firstSelector,
325+
string $secondSelector
326+
): void {
327+
$joinedSelectors = $firstSelector . ', ' . $secondSelector;
328+
$subject = new DeclarationBlock();
329+
330+
$subject->setSelectors($joinedSelectors);
331+
332+
$result = $subject->getSelectors();
333+
self::assertSame([$firstSelector, $secondSelector], self::getSelectorsAsStrings($subject));
334+
}
335+
336+
/**
337+
* Provides selectors that would be parsed without error in the context of full CSS, but are nonetheless invalid.
338+
*
339+
* @return array<non-empty-string, array{0: non-empty-string}>
340+
*/
341+
public static function provideInvalidStandaloneSelector(): array
342+
{
343+
return [
344+
'rogue `{`' => ['a { b'],
345+
'rogue `}`' => ['a } b'],
346+
];
347+
}
348+
349+
/**
350+
* @test
351+
*
352+
* @param non-empty-string $selector
353+
*
354+
* @dataProvider provideInvalidSelector
355+
* @dataProvider provideInvalidStandaloneSelector
356+
*/
357+
public function setSelectorsThrowsExceptionWithInvalidSelector(string $selector): void
358+
{
359+
$this->expectException(UnexpectedTokenException::class);
360+
$this->expectExceptionMessageMatches('/^Selector\\(s\\) string is not valid. /');
361+
362+
$subject = new DeclarationBlock();
363+
364+
$subject->setSelectors($selector);
365+
}
296366
}

0 commit comments

Comments
 (0)