From 11264f9e3f14e210bfbd192f28bab5f0f75f56b2 Mon Sep 17 00:00:00 2001 From: Jake Hotson Date: Fri, 14 Mar 2025 23:22:42 +0000 Subject: [PATCH 1/3] [BUGFIX] Include comments for all rules in declaration block - `Rule::parse()` will no longer consume anything after the semicolon terminating the rule - it does not belong to that rule; - The whitespace and comments before a rule will be processed by `RuleSet::parseRuleSet()` and passed as a parameter to `Rule::parse()` - - This is only required while 'strict mode' parsing is an option, to avoid having an exception thrown during normal operation (i.e. when `Rule::parse()` encounters normal `}` as opposed to some other junk, which is not distinguished). Fixes #173. See also #672, #741. --- CHANGELOG.md | 1 + src/Rule/Rule.php | 8 ++++---- src/RuleSet/RuleSet.php | 10 +++++++--- tests/ParserTest.php | 2 -- 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f6bcb805..73fed056 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,6 +66,7 @@ Please also have a look at our ### Fixed +- Include comments for all rules in declaration block (#1169) - Render rules in line and column number order (#1059) - Don't render `rgb` colors with percentage values using hex notation (#803) - Parse `@font-face` `src` property as comma-delimited list (#790) diff --git a/src/Rule/Rule.php b/src/Rule/Rule.php index 42fef09d..dac05cd1 100644 --- a/src/Rule/Rule.php +++ b/src/Rule/Rule.php @@ -68,14 +68,16 @@ public function __construct($rule, int $lineNumber = 0, $columnNumber = 0) } /** + * @param list $commentsBeforeRule + * * @throws UnexpectedEOFException * @throws UnexpectedTokenException * * @internal since V8.8.0 */ - public static function parse(ParserState $parserState): Rule + public static function parse(ParserState $parserState, array $commentsBeforeRule = []): Rule { - $comments = $parserState->consumeWhiteSpace(); + $comments = \array_merge($commentsBeforeRule, $parserState->consumeWhiteSpace()); $rule = new Rule( $parserState->parseIdentifier(!$parserState->comes('--')), $parserState->currentLine(), @@ -98,8 +100,6 @@ public static function parse(ParserState $parserState): Rule $parserState->consume(';'); } - $parserState->consumeWhiteSpace(); - return $rule; } diff --git a/src/RuleSet/RuleSet.php b/src/RuleSet/RuleSet.php index 311f06be..6ff5b21c 100644 --- a/src/RuleSet/RuleSet.php +++ b/src/RuleSet/RuleSet.php @@ -65,11 +65,15 @@ public static function parseRuleSet(ParserState $parserState, RuleSet $ruleSet): while ($parserState->comes(';')) { $parserState->consume(';'); } - while (!$parserState->comes('}')) { + for (;;) { + $commentsBeforeRule = $parserState->consumeWhiteSpace(); + if ($parserState->comes('}')) { + break; + } $rule = null; if ($parserState->getSettings()->usesLenientParsing()) { try { - $rule = Rule::parse($parserState); + $rule = Rule::parse($parserState, $commentsBeforeRule); } catch (UnexpectedTokenException $e) { try { $consumedText = $parserState->consumeUntil(["\n", ';', '}'], true); @@ -87,7 +91,7 @@ public static function parseRuleSet(ParserState $parserState, RuleSet $ruleSet): } } } else { - $rule = Rule::parse($parserState); + $rule = Rule::parse($parserState, $commentsBeforeRule); } if ($rule instanceof Rule) { $ruleSet->addRule($rule); diff --git a/tests/ParserTest.php b/tests/ParserTest.php index 47ec1ffa..317d1c68 100644 --- a/tests/ParserTest.php +++ b/tests/ParserTest.php @@ -1103,8 +1103,6 @@ public function flatCommentExtractingOneComment(): void */ public function flatCommentExtractingTwoComments(): void { - self::markTestSkipped('This is currently broken.'); - $parser = new Parser('div {/*Find Me!*/left:10px; /*Find Me Too!*/text-align:left;}'); $document = $parser->parse(); $contents = $document->getContents(); From 310081adeef0889a8e8e1672f954d828cbb8c590 Mon Sep 17 00:00:00 2001 From: Jake Hotson Date: Sun, 16 Mar 2025 00:29:13 +0000 Subject: [PATCH 2/3] Add tests with two comments for one rule. Also add line-breaks in test methods to separate the three phases: - set up; - obtain results; - make assertions. --- tests/ParserTest.php | 40 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/tests/ParserTest.php b/tests/ParserTest.php index 317d1c68..930f12f7 100644 --- a/tests/ParserTest.php +++ b/tests/ParserTest.php @@ -1091,9 +1091,11 @@ public function flatCommentExtractingOneComment(): void { $parser = new Parser('div {/*Find Me!*/left:10px; text-align:left;}'); $document = $parser->parse(); + $contents = $document->getContents(); $divRules = $contents[0]->getRules(); $comments = $divRules[0]->getComments(); + self::assertCount(1, $comments); self::assertSame('Find Me!', $comments[0]->getComment()); } @@ -1101,14 +1103,50 @@ public function flatCommentExtractingOneComment(): void /** * @test */ - public function flatCommentExtractingTwoComments(): void + public function flatCommentExtractingTwoConjoinedCommentsForOneRule(): void + { + $parser = new Parser('div {/*Find Me!*//*Find Me Too!*/left:10px; text-align:left;}'); + $document = $parser->parse(); + + $contents = $document->getContents(); + $divRules = $contents[0]->getRules(); + $comments = $divRules[0]->getComments(); + + self::assertCount(2, $comments); + self::assertSame('Find Me!', $comments[0]->getComment()); + self::assertSame('Find Me Too!', $comments[1]->getComment()); + } + + /** + * @test + */ + public function flatCommentExtractingTwoSpaceSeparatedCommentsForOneRule(): void + { + $parser = new Parser('div { /*Find Me!*/ /*Find Me Too!*/ left:10px; text-align:left;}'); + $document = $parser->parse(); + + $contents = $document->getContents(); + $divRules = $contents[0]->getRules(); + $comments = $divRules[0]->getComments(); + + self::assertCount(2, $comments); + self::assertSame('Find Me!', $comments[0]->getComment()); + self::assertSame('Find Me Too!', $comments[1]->getComment()); + } + + /** + * @test + */ + public function flatCommentExtractingCommentsForTwoRules(): void { $parser = new Parser('div {/*Find Me!*/left:10px; /*Find Me Too!*/text-align:left;}'); $document = $parser->parse(); + $contents = $document->getContents(); $divRules = $contents[0]->getRules(); $rule1Comments = $divRules[0]->getComments(); $rule2Comments = $divRules[1]->getComments(); + self::assertCount(1, $rule1Comments); self::assertCount(1, $rule2Comments); self::assertSame('Find Me!', $rule1Comments[0]->getComment()); From d503f75aaa2014337868c4bf43787301449141c6 Mon Sep 17 00:00:00 2001 From: JakeQZ Date: Sun, 16 Mar 2025 16:16:09 +0000 Subject: [PATCH 3/3] Use `while (true)` instead of `for (;;)` Co-authored-by: Oliver Klee --- src/RuleSet/RuleSet.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/RuleSet/RuleSet.php b/src/RuleSet/RuleSet.php index 6ff5b21c..9d5ab411 100644 --- a/src/RuleSet/RuleSet.php +++ b/src/RuleSet/RuleSet.php @@ -65,7 +65,7 @@ public static function parseRuleSet(ParserState $parserState, RuleSet $ruleSet): while ($parserState->comes(';')) { $parserState->consume(';'); } - for (;;) { + while (true) { $commentsBeforeRule = $parserState->consumeWhiteSpace(); if ($parserState->comes('}')) { break;