From 1e8ca9073b0afab2912f50f598d6bb36687b8dbc Mon Sep 17 00:00:00 2001 From: Jake Hotson Date: Sun, 23 Mar 2025 01:50:47 +0000 Subject: [PATCH 1/2] [CLEANUP] Return `null` from `DeclarationBlock::parse()` on failure Also add clarification of meaning of return value from `CSSList::parseListItem()`, where `null` and `false` have different meanings. Part of #1176. --- config/phpstan-baseline.neon | 6 ------ src/CSSList/CSSList.php | 7 +++++-- src/RuleSet/DeclarationBlock.php | 6 ++---- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/config/phpstan-baseline.neon b/config/phpstan-baseline.neon index 416fa9d1..47b197a1 100644 --- a/config/phpstan-baseline.neon +++ b/config/phpstan-baseline.neon @@ -48,12 +48,6 @@ parameters: count: 1 path: ../src/RuleSet/DeclarationBlock.php - - - message: '#^Returning false in non return bool class method\. Use null with type\|null instead or add bool return type$#' - identifier: typePerfect.nullOverFalse - count: 1 - path: ../src/RuleSet/DeclarationBlock.php - - message: '#^Only booleans are allowed in a negated boolean, string\|null given\.$#' identifier: booleanNot.exprNotBoolean diff --git a/src/CSSList/CSSList.php b/src/CSSList/CSSList.php index 36c248cc..3cd76206 100644 --- a/src/CSSList/CSSList.php +++ b/src/CSSList/CSSList.php @@ -106,6 +106,9 @@ public static function parseList(ParserState $parserState, CSSList $list): void /** * @return AtRuleBlockList|KeyFrame|Charset|CSSNamespace|Import|AtRuleSet|DeclarationBlock|false|null + * If `null` is returned, it means the end of the list has been reached. + * If `false` is returned, it means an invalid item has been encountered, + * but parsing of the next item should proceed. * * @throws SourceException * @throws UnexpectedEOFException @@ -139,7 +142,7 @@ private static function parseListItem(ParserState $parserState, CSSList $list) } elseif ($parserState->comes('}')) { if ($isRoot) { if ($parserState->getSettings()->usesLenientParsing()) { - return DeclarationBlock::parse($parserState); + return DeclarationBlock::parse($parserState) ?? false; } else { throw new SourceException('Unopened {', $parserState->currentLine()); } @@ -148,7 +151,7 @@ private static function parseListItem(ParserState $parserState, CSSList $list) return null; } } else { - return DeclarationBlock::parse($parserState, $list); + return DeclarationBlock::parse($parserState, $list) ?? false; } } diff --git a/src/RuleSet/DeclarationBlock.php b/src/RuleSet/DeclarationBlock.php index 9a1a0ca0..06829fa2 100644 --- a/src/RuleSet/DeclarationBlock.php +++ b/src/RuleSet/DeclarationBlock.php @@ -30,14 +30,12 @@ class DeclarationBlock extends RuleSet private $selectors = []; /** - * @return DeclarationBlock|false - * * @throws UnexpectedTokenException * @throws UnexpectedEOFException * * @internal since V8.8.0 */ - public static function parse(ParserState $parserState, ?CSSList $list = null) + public static function parse(ParserState $parserState, ?CSSList $list = null): ?DeclarationBlock { $comments = []; $result = new DeclarationBlock($parserState->currentLine()); @@ -63,7 +61,7 @@ public static function parse(ParserState $parserState, ?CSSList $list = null) if (!$parserState->comes('}')) { $parserState->consumeUntil('}', false, true); } - return false; + return null; } else { throw $e; } From 39148120b34efac133a148edfce77f7dab369973 Mon Sep 17 00:00:00 2001 From: Jake Hotson Date: Mon, 24 Mar 2025 18:13:03 +0000 Subject: [PATCH 2/2] Add test for DeclarationBlock returning `null` on failure --- .../RuleSet/DeclarationBlockTest.php | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/Functional/RuleSet/DeclarationBlockTest.php b/tests/Functional/RuleSet/DeclarationBlockTest.php index ca3d3f87..fe6b9e51 100644 --- a/tests/Functional/RuleSet/DeclarationBlockTest.php +++ b/tests/Functional/RuleSet/DeclarationBlockTest.php @@ -6,15 +6,43 @@ use PHPUnit\Framework\TestCase; use Sabberworm\CSS\OutputFormat; +use Sabberworm\CSS\Parsing\ParserState; use Sabberworm\CSS\Property\Selector; use Sabberworm\CSS\Rule\Rule; use Sabberworm\CSS\RuleSet\DeclarationBlock; +use Sabberworm\CSS\Settings; /** * @covers \Sabberworm\CSS\RuleSet\DeclarationBlock */ final class DeclarationBlockTest extends TestCase { + /** + * @return array + */ + public static function provideInvalidDeclarationBlock(): array + { + return [ + 'no selector' => ['{ color: red; }'], + 'invalid selector' => ['/ { color: red; }'], + 'no opening brace' => ['body color: red; }'], + ]; + } + + /** + * @test + * + * @dataProvider provideInvalidDeclarationBlock + */ + public function parseReturnsNullForInvalidDeclarationBlock(string $invalidDeclarationBlock): void + { + $parserState = new ParserState($invalidDeclarationBlock, Settings::create()); + + $result = DeclarationBlock::parse($parserState); + + self::assertNull($result); + } + /** * @test */