Skip to content

Commit 8626ec3

Browse files
JakeQZJakeQZ
committed
[BUGFIX] Allow comma-separated arguments in selectors
Fixes #138, #360 and #1289.
1 parent ec1f6dd commit 8626ec3

File tree

6 files changed

+118
-15
lines changed

6 files changed

+118
-15
lines changed

‎CHANGELOG.md

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

111111
### Fixed
112112

113+
- Selector functions (like `:not`) with comma-separated arguments are now
114+
parsed correclty (#1292)
113115
- Insert `Rule` before sibling even with different property name
114116
(in `RuleSet::addRule()`) (#1270)
115117
- Ensure `RuleSet::addRule()` sets non-negative column number when sibling

‎config/phpstan-baseline.neon

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ parameters:
4343
path: ../src/Rule/Rule.php
4444

4545
-
46-
message: '#^Loose comparison via "\!\=" is not allowed\.$#'
47-
identifier: notEqual.notAllowed
46+
message: '#^Class Sabberworm\\CSS\\Parsing\\UnexpectedTokenException constructor invoked with 1 parameter, 2\-4 required\.$#'
47+
identifier: arguments.count
4848
count: 1
4949
path: ../src/RuleSet/DeclarationBlock.php
5050

‎src/Parsing/ParserState.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,10 @@ public function consumeUntil(
345345
$start = $this->currentPosition;
346346

347347
while (!$this->isEnd()) {
348+
$comment = $this->consumeComment();
349+
if ($comment instanceof Comment) {
350+
$comments[] = $comment;
351+
}
348352
$character = $this->consume(1);
349353
if (\in_array($character, $stopCharacters, true)) {
350354
if ($includeEnd) {

‎src/Property/Selector.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@ class Selector implements Renderable
2424
public const SELECTOR_VALIDATION_RX = '/
2525
^(
2626
(?:
27-
[a-zA-Z0-9\\x{00A0}-\\x{FFFF}_^$|*="\'~\\[\\]()\\-\\s\\.:#+>]* # any sequence of valid unescaped characters
28-
(?:\\\\.)? # a single escaped character
29-
(?:([\'"]).*?(?<!\\\\)\\2)? # a quoted text like [id="example"]
27+
[a-zA-Z0-9\\x{00A0}-\\x{FFFF}_^$|*="\'~\\[\\]()\\-\\s\\.:#+>,]* # any sequence of valid unescaped characters
28+
(?:\\\\.)? # a single escaped character
29+
(?:([\'"]).*?(?<!\\\\)\\2)? # a quoted text like [id="example"]
3030
)*
3131
)$
3232
/ux';

‎src/RuleSet/DeclarationBlock.php

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -40,19 +40,48 @@ public static function parse(ParserState $parserState, ?CSSList $list = null): ?
4040
$comments = [];
4141
$result = new DeclarationBlock($parserState->currentLine());
4242
try {
43+
$selectors = [];
4344
$selectorParts = [];
45+
$functionNestingLevel = 0;
4446
do {
4547
$selectorParts[] = $parserState->consume(1)
46-
. $parserState->consumeUntil(['{', '}', '\'', '"'], false, false, $comments);
47-
if (\in_array($parserState->peek(), ['\'', '"'], true) && \substr(\end($selectorParts), -1) != '\\') {
48-
if (!isset($stringWrapperCharacter)) {
49-
$stringWrapperCharacter = $parserState->peek();
50-
} elseif ($stringWrapperCharacter === $parserState->peek()) {
51-
unset($stringWrapperCharacter);
52-
}
48+
. $parserState->consumeUntil(['{', '}', '\'', '"', '(', ')', ','], false, false, $comments);
49+
$nextCharacter = $parserState->peek();
50+
switch ($nextCharacter) {
51+
case '\'':
52+
case '"':
53+
if (!isset($stringWrapperCharacter)) {
54+
$stringWrapperCharacter = $nextCharacter;
55+
} elseif ($stringWrapperCharacter === $nextCharacter) {
56+
if (\substr(\end($selectorParts), -1) !== '\\') {
57+
unset($stringWrapperCharacter);
58+
}
59+
}
60+
break;
61+
case '(':
62+
if (!isset($stringWrapperCharacter)) {
63+
++$functionNestingLevel;
64+
}
65+
break;
66+
case ')':
67+
if (!isset($stringWrapperCharacter)) {
68+
if ($functionNestingLevel <= 0) {
69+
throw new UnexpectedTokenException('unexpected `)`');
70+
}
71+
--$functionNestingLevel;
72+
}
73+
break;
74+
case ',':
75+
if (!isset($stringWrapperCharacter) && $functionNestingLevel === 0) {
76+
$selectors[] = \implode('', $selectorParts);
77+
$selectorParts = [];
78+
$parserState->consume(1);
79+
}
80+
break;
5381
}
54-
} while (!\in_array($parserState->peek(), ['{', '}'], true) || isset($stringWrapperCharacter));
55-
$result->setSelectors(\implode('', $selectorParts), $list);
82+
} while (!\in_array($nextCharacter, ['{', '}'], true) || isset($stringWrapperCharacter));
83+
$selectors[] = \implode('', $selectorParts); // add final or only selector
84+
$result->setSelectors($selectors, $list);
5685
if ($parserState->comes('{')) {
5786
$parserState->consume(1);
5887
}
@@ -89,7 +118,7 @@ public function setSelectors($selectors, ?CSSList $list = null): void
89118
if (!Selector::isValid($selector)) {
90119
throw new UnexpectedTokenException(
91120
"Selector did not match '" . Selector::SELECTOR_VALIDATION_RX . "'.",
92-
$selectors,
121+
$selector,
93122
'custom'
94123
);
95124
}

‎tests/Unit/RuleSet/DeclarationBlockTest.php

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,11 @@
66

77
use PHPUnit\Framework\TestCase;
88
use Sabberworm\CSS\CSSList\CSSListItem;
9+
use Sabberworm\CSS\Parsing\ParserState;
10+
use Sabberworm\CSS\Property\Selector;
911
use Sabberworm\CSS\RuleSet\DeclarationBlock;
12+
use Sabberworm\CSS\Settings;
13+
use TRegx\PhpUnit\DataProviders\DataProvider;
1014

1115
/**
1216
* @covers \Sabberworm\CSS\RuleSet\DeclarationBlock
@@ -30,4 +34,68 @@ public function implementsCSSListItem(): void
3034
{
3135
self::assertInstanceOf(CSSListItem::class, $this->subject);
3236
}
37+
38+
/**
39+
* @return array<non-empty-string, array{0: non-empty-string}>
40+
*/
41+
public static function provideSelector(): array
42+
{
43+
return [
44+
'type' => ['body'],
45+
'class' => ['.teapot'],
46+
'id' => ['#my-mug'],
47+
'`not`' => [':not(#your-mug)'],
48+
'`not` with multiple arguments' => [':not(#your-mug, .their-mug)'],
49+
];
50+
}
51+
52+
/**
53+
* @test
54+
*
55+
* @param non-empty-string $selector
56+
*
57+
* @dataProvider provideSelector
58+
*/
59+
public function parsesAndReturnsSingleSelector(string $selector): void
60+
{
61+
$subject = DeclarationBlock::parse(new ParserState($selector . '{}', Settings::create()));
62+
63+
$resultSelectorStrings = \array_map(
64+
static function (Selector $selectorObject): string {
65+
return $selectorObject->getSelector();
66+
},
67+
$subject->getSelectors()
68+
);
69+
self::assertSame([$selector], $resultSelectorStrings);
70+
}
71+
72+
/**
73+
* @return DataProvider<non-empty-string, array{0: non-empty-string, 1: non-empty-string}>
74+
*/
75+
public static function provideTwoSelectors(): DataProvider
76+
{
77+
return DataProvider::cross(self::provideSelector(), self::provideSelector());
78+
}
79+
80+
/**
81+
* @test
82+
*
83+
* @param non-empty-string $firstSelector
84+
* @param non-empty-string $secondSelector
85+
*
86+
* @dataProvider provideTwoSelectors
87+
*/
88+
public function parsesAndReturnsTwoCommaSeparatedSelectors(string $firstSelector, string $secondSelector): void
89+
{
90+
$joinedSelectors = $firstSelector . ',' . $secondSelector;
91+
$subject = DeclarationBlock::parse(new ParserState($joinedSelectors . '{}', Settings::create()));
92+
93+
$resultSelectorStrings = \array_map(
94+
static function (Selector $selectorObject): string {
95+
return $selectorObject->getSelector();
96+
},
97+
$subject->getSelectors()
98+
);
99+
self::assertSame([$firstSelector, $secondSelector], $resultSelectorStrings);
100+
}
33101
}

0 commit comments

Comments
 (0)