[BUGFIX] Allow comma-separated arguments in selectors

Fixes #138, #360 and #1289.

Author: JakeQZ

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)
 - Allow comma in selectors (e.g. `:not(html, body)`) (#1293)
 - Insert `Rule` before sibling even with different property name
   (in `RuleSet::addRule()`) (#1270)

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) {

src/RuleSet/DeclarationBlock.php

@@ -40,11 +40,13 @@ public static function parse(ParserState $parserState, ?CSSList $list = null): ?
         $comments = [];
         $result = new DeclarationBlock($parserState->currentLine());
         try {
+            $selectors = [];
             $selectorParts = [];
             $stringWrapperCharacter = null;
+            $functionNestingLevel = 0;
             do {
                 $selectorParts[] = $parserState->consume(1)
-                    . $parserState->consumeUntil(['{', '}', '\'', '"'], false, false, $comments);
+                    . $parserState->consumeUntil(['{', '}', '\'', '"', '(', ')', ','], false, false, $comments);
                 $nextCharacter = $parserState->peek();
                 switch ($nextCharacter) {
                     case '\'':
@@ -58,9 +60,30 @@ public static function parse(ParserState $parserState, ?CSSList $list = null): ?
                             }
                         }
                         break;
+                    case '(':
+                        if (!isset($stringWrapperCharacter)) {
+                            ++$functionNestingLevel;
+                        }
+                        break;
+                    case ')':
+                        if (!isset($stringWrapperCharacter)) {
+                            if ($functionNestingLevel <= 0) {
+                                throw new UnexpectedTokenException('anything but', ')');
+                            }
+                            --$functionNestingLevel;
+                        }
+                        break;
+                    case ',':
+                        if (!isset($stringWrapperCharacter) && $functionNestingLevel === 0) {
+                            $selectors[] = \implode('', $selectorParts);
+                            $selectorParts = [];
+                            $parserState->consume(1);
+                        }
+                        break;
                 }
             } while (!\in_array($nextCharacter, ['{', '}'], true) || \is_string($stringWrapperCharacter));
-            $result->setSelectors(\implode('', $selectorParts), $list);
+            $selectors[] = \implode('', $selectorParts); // add final or only selector
+            $result->setSelectors($selectors, $list);
             if ($parserState->comes('{')) {
                 $parserState->consume(1);
             }
@@ -97,7 +120,7 @@ public function setSelectors($selectors, ?CSSList $list = null): void
                     if (!Selector::isValid($selector)) {
                         throw new UnexpectedTokenException(
                             "Selector did not match '" . Selector::SELECTOR_VALIDATION_RX . "'.",
-                            $selectors,
+                            $selector,
                             'custom'
                         );
                     }

tests/Unit/RuleSet/DeclarationBlockTest.php

@@ -6,7 +6,11 @@
 
 use PHPUnit\Framework\TestCase;
 use Sabberworm\CSS\CSSList\CSSListItem;
+use Sabberworm\CSS\Parsing\ParserState;
+use Sabberworm\CSS\Property\Selector;
 use Sabberworm\CSS\RuleSet\DeclarationBlock;
+use Sabberworm\CSS\Settings;
+use TRegx\PhpUnit\DataProviders\DataProvider;
 
 /**
  * @covers \Sabberworm\CSS\RuleSet\DeclarationBlock
@@ -30,4 +34,68 @@ public function implementsCSSListItem(): void
     {
         self::assertInstanceOf(CSSListItem::class, $this->subject);
     }
+
+    /**
+     * @return array<non-empty-string, array{0: non-empty-string}>
+     */
+    public static function provideSelector(): array
+    {
+        return [
+            'type' => ['body'],
+            'class' => ['.teapot'],
+            'id' => ['#my-mug'],
+            '`not`' => [':not(#your-mug)'],
+            '`not` with multiple arguments' => [':not(#your-mug, .their-mug)'],
+        ];
+    }
+
+    /**
+     * @test
+     *
+     * @param non-empty-string $selector
+     *
+     * @dataProvider provideSelector
+     */
+    public function parsesAndReturnsSingleSelector(string $selector): void
+    {
+        $subject = DeclarationBlock::parse(new ParserState($selector . '{}', Settings::create()));
+
+        $resultSelectorStrings = \array_map(
+            static function (Selector $selectorObject): string {
+                return $selectorObject->getSelector();
+            },
+            $subject->getSelectors()
+        );
+        self::assertSame([$selector], $resultSelectorStrings);
+    }
+
+    /**
+     * @return DataProvider<non-empty-string, array{0: non-empty-string, 1: non-empty-string}>
+     */
+    public static function provideTwoSelectors(): DataProvider
+    {
+        return DataProvider::cross(self::provideSelector(), self::provideSelector());
+    }
+
+    /**
+     * @test
+     *
+     * @param non-empty-string $firstSelector
+     * @param non-empty-string $secondSelector
+     *
+     * @dataProvider provideTwoSelectors
+     */
+    public function parsesAndReturnsTwoCommaSeparatedSelectors(string $firstSelector, string $secondSelector): void
+    {
+        $joinedSelectors = $firstSelector . ',' . $secondSelector;
+        $subject = DeclarationBlock::parse(new ParserState($joinedSelectors . '{}', Settings::create()));
+
+        $resultSelectorStrings = \array_map(
+            static function (Selector $selectorObject): string {
+                return $selectorObject->getSelector();
+            },
+            $subject->getSelectors()
+        );
+        self::assertSame([$firstSelector, $secondSelector], $resultSelectorStrings);
+    }
 }