From 0425f1ef018f4b9e63a625f0f5137eae8d2559a5 Mon Sep 17 00:00:00 2001 From: Jake Hotson Date: Sat, 28 Jun 2025 03:44:35 +0100 Subject: [PATCH 1/5] [BUGFIX] Allow comma-separated arguments in selectors Fixes #138. Fixes #360. Fixes #1289. --- CHANGELOG.md | 2 ++ src/Parsing/ParserState.php | 4 ++++ src/RuleSet/DeclarationBlock.php | 18 ++++++++++++++++-- tests/Unit/RuleSet/DeclarationBlockTest.php | 1 + 4 files changed, 23 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 08dfef29..194a8d8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -110,6 +110,8 @@ Please also have a look at our ### Fixed +- Selector functions (like `:not`) with comma-separated arguments are now + parsed correclty (#1292) - Parse quoted attribute selector value containing comma (#1323) - Allow comma in selectors (e.g. `:not(html, body)`) (#1293) - Insert `Rule` before sibling even with different property name diff --git a/src/Parsing/ParserState.php b/src/Parsing/ParserState.php index e90650d5..04d2f5df 100644 --- a/src/Parsing/ParserState.php +++ b/src/Parsing/ParserState.php @@ -345,6 +345,10 @@ public function consumeUntil( $start = $this->currentPosition; while (!$this->isEnd()) { + $comment = $this->consumeComment(); + if ($comment instanceof Comment) { + $comments[] = $comment; + } $character = $this->consume(1); if (\in_array($character, $stopCharacters, true)) { if ($includeEnd) { diff --git a/src/RuleSet/DeclarationBlock.php b/src/RuleSet/DeclarationBlock.php index d0b9654c..0c43c785 100644 --- a/src/RuleSet/DeclarationBlock.php +++ b/src/RuleSet/DeclarationBlock.php @@ -43,8 +43,9 @@ public static function parse(ParserState $parserState, ?CSSList $list = null): ? $selectors = []; $selectorParts = []; $stringWrapperCharacter = null; + $functionNestingLevel = 0; $consumedNextCharacter = false; - static $stopCharacters = ['{', '}', '\'', '"', ',']; + static $stopCharacters = ['{', '}', '\'', '"', '(', ')', ',']; do { if (!$consumedNextCharacter) { $selectorParts[] = $parserState->consume(1); @@ -64,8 +65,21 @@ public static function parse(ParserState $parserState, ?CSSList $list = null): ? } } break; - case ',': + case '(': + if (!\is_string($stringWrapperCharacter)) { + ++$functionNestingLevel; + } + break; + case ')': if (!\is_string($stringWrapperCharacter)) { + if ($functionNestingLevel <= 0) { + throw new UnexpectedTokenException('anything but', ')'); + } + --$functionNestingLevel; + } + break; + case ',': + if (!\is_string($stringWrapperCharacter) && $functionNestingLevel === 0) { $selectors[] = \implode('', $selectorParts); $selectorParts = []; $parserState->consume(1); diff --git a/tests/Unit/RuleSet/DeclarationBlockTest.php b/tests/Unit/RuleSet/DeclarationBlockTest.php index 468a8515..b9842cec 100644 --- a/tests/Unit/RuleSet/DeclarationBlockTest.php +++ b/tests/Unit/RuleSet/DeclarationBlockTest.php @@ -67,6 +67,7 @@ public static function provideSelector(): array 'pseudo-class' => [':hover'], 'type & pseudo-class' => ['a:hover'], '`not`' => [':not(#your-mug)'], + '`not` with multiple arguments' => [':not(#your-mug, .their-mug)'], 'pseudo-element' => ['::before'], 'attribute with `"`' => ['[alt="{}()[]\\"\',"]'], 'attribute with `\'`' => ['[alt=\'{}()[]"\\\',\']'], From 7a06c91a6f295e569136fc31e263a53516e0ee65 Mon Sep 17 00:00:00 2001 From: Jake Hotson Date: Tue, 8 Jul 2025 02:34:23 +0100 Subject: [PATCH 2/5] Don't consume character after `,` without checking Also remove fix for `consumeUntil` bug, which #1320 will address. --- src/Parsing/ParserState.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Parsing/ParserState.php b/src/Parsing/ParserState.php index 04d2f5df..e90650d5 100644 --- a/src/Parsing/ParserState.php +++ b/src/Parsing/ParserState.php @@ -345,10 +345,6 @@ public function consumeUntil( $start = $this->currentPosition; while (!$this->isEnd()) { - $comment = $this->consumeComment(); - if ($comment instanceof Comment) { - $comments[] = $comment; - } $character = $this->consume(1); if (\in_array($character, $stopCharacters, true)) { if ($includeEnd) { From 18756bf6065ad7a658257b174def65dfe86c89a0 Mon Sep 17 00:00:00 2001 From: JakeQZ Date: Tue, 8 Jul 2025 16:03:58 +0100 Subject: [PATCH 3/5] Fix typo in CHANGELOG.md Co-authored-by: Oliver Klee --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 194a8d8c..df82c76e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -111,7 +111,7 @@ Please also have a look at our ### Fixed - Selector functions (like `:not`) with comma-separated arguments are now - parsed correclty (#1292) + parsed correctly (#1292) - Parse quoted attribute selector value containing comma (#1323) - Allow comma in selectors (e.g. `:not(html, body)`) (#1293) - Insert `Rule` before sibling even with different property name From 4fa98e3b6a6b15f892fd9e7dee756b78e15303db Mon Sep 17 00:00:00 2001 From: Jake Hotson Date: Thu, 10 Jul 2025 09:56:50 +0100 Subject: [PATCH 4/5] Succinct changelog entry (Can succinct be used as a verb? The typo in 'correctly' vanishes.) --- CHANGELOG.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index df82c76e..a40e5ff2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -110,8 +110,7 @@ Please also have a look at our ### Fixed -- Selector functions (like `:not`) with comma-separated arguments are now - parsed correctly (#1292) +- Parse selector functions (like `:not`) with comma-separated arguments (#1292) - Parse quoted attribute selector value containing comma (#1323) - Allow comma in selectors (e.g. `:not(html, body)`) (#1293) - Insert `Rule` before sibling even with different property name From 0612c6535504fb1afad96471c24e4ee475649389 Mon Sep 17 00:00:00 2001 From: Jake Hotson Date: Thu, 10 Jul 2025 12:07:30 +0100 Subject: [PATCH 5/5] Test rejection of selectors with unmatched parentheses Also reject selectors with unclosed parentheses. --- src/RuleSet/DeclarationBlock.php | 3 ++ tests/Unit/RuleSet/DeclarationBlockTest.php | 34 +++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/src/RuleSet/DeclarationBlock.php b/src/RuleSet/DeclarationBlock.php index 0c43c785..68926deb 100644 --- a/src/RuleSet/DeclarationBlock.php +++ b/src/RuleSet/DeclarationBlock.php @@ -88,6 +88,9 @@ public static function parse(ParserState $parserState, ?CSSList $list = null): ? break; } } while (!\in_array($nextCharacter, ['{', '}'], true) || \is_string($stringWrapperCharacter)); + if ($functionNestingLevel !== 0) { + throw new UnexpectedTokenException(')', $nextCharacter); + } $selectors[] = \implode('', $selectorParts); // add final or only selector $result->setSelectors($selectors, $list); if ($parserState->comes('{')) { diff --git a/tests/Unit/RuleSet/DeclarationBlockTest.php b/tests/Unit/RuleSet/DeclarationBlockTest.php index b9842cec..4419ba0f 100644 --- a/tests/Unit/RuleSet/DeclarationBlockTest.php +++ b/tests/Unit/RuleSet/DeclarationBlockTest.php @@ -115,6 +115,40 @@ public function parsesTwoCommaSeparatedSelectors(string $firstSelector, string $ self::assertSame([$firstSelector, $secondSelector], self::getSelectorsAsStrings($subject)); } + /** + * @return array + */ + public static function provideInvalidSelector(): array + { + // TODO: the `parse` method consumes the first character without inspection, + // so the 'lone' test strings are prefixed with a space. + return [ + 'lone `(`' => [' ('], + 'lone `)`' => [' )'], + 'unclosed `(`' => [':not(#your-mug'], + 'extra `)`' => [':not(#your-mug))'], + ]; + } + + /** + * @test + * + * @param non-empty-string $selector + * + * @dataProvider provideInvalidSelector + */ + public function parseSkipsBlockWithInvalidSelector(string $selector): void + { + static $nextCss = ' .next {}'; + $css = $selector . ' {}' . $nextCss; + $parserState = new ParserState($css, Settings::create()); + + $subject = DeclarationBlock::parse($parserState); + + self::assertNull($subject); + self::assertTrue($parserState->comes($nextCss)); + } + /** * @return array */