From c3293736fcf50d8f896492d347bb76cbe3e46974 Mon Sep 17 00:00:00 2001 From: JakeQZ Date: Sun, 4 May 2025 09:25:23 +0100 Subject: [PATCH] [TASK] Add separate methods for `RuleSet::removeRule()`, with deprecation Passing a `string` or `null` to `removeRule()` is deprecated. The new methods handle that functionality. Relates to #1247. This is the backport of #1249. --- CHANGELOG.md | 7 ++ src/RuleSet/RuleSet.php | 60 ++++++---- tests/Unit/RuleSet/RuleSetTest.php | 175 +++++++++++++++++++++++++++++ 3 files changed, 219 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b08c38a2..cab8e1e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ This project adheres to [Semantic Versioning](https://semver.org/). ### Added +- `RuleSet::removeMatchingRules()` method + (for the implementing classes `AtRuleSet` and `DeclarationBlock`) (#1249) +- `RuleSet::removeAllRules()` method + (for the implementing classes `AtRuleSet` and `DeclarationBlock`) (#1249) - Add Interface `CSSElement` (#1231) - Methods `getLineNumber` and `getColumnNumber` which return a nullable `int` for the following classes: @@ -26,6 +30,9 @@ This project adheres to [Semantic Versioning](https://semver.org/). ### Deprecated +- Passing a `string` or `null` to `RuleSet::removeRule()` is deprecated + (implementing classes are `AtRuleSet` and `DeclarationBlock`); + use `removeMatchingRules()` or `removeAllRules()` instead (#1249) - Passing a string as the first argument to `getAllValues()` is deprecated; the search pattern should now be passed as the second argument (#1241) - Passing a Boolean as the second argument to `getAllValues()` is deprecated; diff --git a/src/RuleSet/RuleSet.php b/src/RuleSet/RuleSet.php index 12b8157a..8cd4904d 100644 --- a/src/RuleSet/RuleSet.php +++ b/src/RuleSet/RuleSet.php @@ -220,20 +220,12 @@ public function getRulesAssoc($mRule = null) } /** - * Removes a rule from this RuleSet. This accepts all the possible values that `getRules()` accepts. - * - * If given a Rule, it will only remove this particular rule (by identity). - * If given a name, it will remove all rules by that name. - * - * Note: this is different from pre-v.2.0 behaviour of PHP-CSS-Parser, where passing a Rule instance would - * remove all rules with the same name. To get the old behaviour, use `removeRule($oRule->getRule())`. + * Removes a `Rule` from this `RuleSet` by identity. * * @param Rule|string|null $mRule - * pattern to remove. If $mRule is null, all rules are removed. If the pattern ends in a dash, - * all rules starting with the pattern are removed as well as one matching the pattern with the dash - * excluded. Passing a Rule behaves matches by identity. - * - * @return void + * `Rule` to remove. + * Passing a `string` or `null` is deprecated in version 8.9.0, and will no longer work from v9.0. + * Use `removeMatchingRules()` or `removeAllRules()` instead. */ public function removeRule($mRule) { @@ -247,22 +239,44 @@ public function removeRule($mRule) unset($this->aRules[$sRule][$iKey]); } } + } elseif ($mRule !== null) { + $this->removeMatchingRules($mRule); } else { - foreach ($this->aRules as $sName => $aRules) { - // Either no search rule is given or the search rule matches the found rule exactly - // or the search rule ends in “-” and the found rule starts with the search rule or equals it - // (without the trailing dash). - if ( - !$mRule || $sName === $mRule - || (strrpos($mRule, '-') === strlen($mRule) - strlen('-') - && (strpos($sName, $mRule) === 0 || $sName === substr($mRule, 0, -1))) - ) { - unset($this->aRules[$sName]); - } + $this->removeAllRules(); + } + } + + /** + * Removes rules by property name or search pattern. + * + * @param string $searchPattern + * pattern to remove. + * If the pattern ends in a dash, + * all rules starting with the pattern are removed as well as one matching the pattern with the dash + * excluded. + */ + public function removeMatchingRules($searchPattern) + { + foreach ($this->aRules as $propertyName => $rules) { + // Either the search rule matches the found rule exactly + // or the search rule ends in “-” and the found rule starts with the search rule or equals it + // (without the trailing dash). + if ( + $propertyName === $searchPattern + || (\strrpos($searchPattern, '-') === \strlen($searchPattern) - \strlen('-') + && (\strpos($propertyName, $searchPattern) === 0 + || $propertyName === \substr($searchPattern, 0, -1))) + ) { + unset($this->aRules[$propertyName]); } } } + public function removeAllRules() + { + $this->aRules = []; + } + /** * @return string * diff --git a/tests/Unit/RuleSet/RuleSetTest.php b/tests/Unit/RuleSet/RuleSetTest.php index d5231e1f..fada2a28 100644 --- a/tests/Unit/RuleSet/RuleSetTest.php +++ b/tests/Unit/RuleSet/RuleSetTest.php @@ -6,6 +6,7 @@ use PHPUnit\Framework\TestCase; use Sabberworm\CSS\CSSElement; +use Sabberworm\CSS\Rule\Rule; use Sabberworm\CSS\Tests\Unit\RuleSet\Fixtures\ConcreteRuleSet; /** @@ -24,4 +25,178 @@ public function implementsCSSElement() self::assertInstanceOf(CSSElement::class, $subject); } + + /** + * @return array, 1: string, 2: list}> + */ + public static function provideRulesAndPropertyNameToRemoveAndExpectedRemainingPropertyNames() + { + return [ + 'removing single rule' => [ + [new Rule('color')], + 'color', + [], + ], + 'removing first rule' => [ + [new Rule('color'), new Rule('display')], + 'color', + ['display'], + ], + 'removing last rule' => [ + [new Rule('color'), new Rule('display')], + 'display', + ['color'], + ], + 'removing middle rule' => [ + [new Rule('color'), new Rule('display'), new Rule('width')], + 'display', + ['color', 'width'], + ], + 'removing multiple rules' => [ + [new Rule('color'), new Rule('color')], + 'color', + [], + ], + 'removing multiple rules with another kept' => [ + [new Rule('color'), new Rule('color'), new Rule('display')], + 'color', + ['display'], + ], + 'removing nonexistent rule from empty list' => [ + [], + 'color', + [], + ], + 'removing nonexistent rule from nonempty list' => [ + [new Rule('color'), new Rule('display')], + 'width', + ['color', 'display'], + ], + ]; + } + + /** + * @test + * + * @param list $rules + * @param string $propertyName + * @param list $expectedRemainingPropertyNames + * + * @dataProvider provideRulesAndPropertyNameToRemoveAndExpectedRemainingPropertyNames + */ + public function removeMatchingRulesRemovesRulesByPropertyNameAndKeepsOthers( + array $rules, + $propertyName, + array $expectedRemainingPropertyNames + ) { + $subject = new ConcreteRuleSet(); + $subject->setRules($rules); + + $subject->removeMatchingRules($propertyName); + + $remainingRules = $subject->getRulesAssoc(); + self::assertArrayNotHasKey($propertyName, $remainingRules); + foreach ($expectedRemainingPropertyNames as $expectedPropertyName) { + self::assertArrayHasKey($expectedPropertyName, $remainingRules); + } + } + + /** + * @return array, 1: string, 2: list}> + */ + public static function provideRulesAndPropertyNamePrefixToRemoveAndExpectedRemainingPropertyNames() + { + return [ + 'removing shorthand rule' => [ + [new Rule('font')], + 'font', + [], + ], + 'removing longhand rule' => [ + [new Rule('font-size')], + 'font', + [], + ], + 'removing shorthand and longhand rule' => [ + [new Rule('font'), new Rule('font-size')], + 'font', + [], + ], + 'removing shorthand rule with another kept' => [ + [new Rule('font'), new Rule('color')], + 'font', + ['color'], + ], + 'removing longhand rule with another kept' => [ + [new Rule('font-size'), new Rule('color')], + 'font', + ['color'], + ], + 'keeping other rules whose property names begin with the same characters' => [ + [new Rule('contain'), new Rule('container'), new Rule('container-type')], + 'contain', + ['container', 'container-type'], + ], + ]; + } + + /** + * @test + * + * @param list $rules + * @param string $propertyNamePrefix + * @param list $expectedRemainingPropertyNames + * + * @dataProvider provideRulesAndPropertyNamePrefixToRemoveAndExpectedRemainingPropertyNames + */ + public function removeMatchingRulesRemovesRulesByPropertyNamePrefixAndKeepsOthers( + array $rules, + $propertyNamePrefix, + array $expectedRemainingPropertyNames + ) { + $propertyNamePrefixWithHyphen = $propertyNamePrefix . '-'; + $subject = new ConcreteRuleSet(); + $subject->setRules($rules); + + $subject->removeMatchingRules($propertyNamePrefixWithHyphen); + + $remainingRules = $subject->getRulesAssoc(); + self::assertArrayNotHasKey($propertyNamePrefix, $remainingRules); + foreach (\array_keys($remainingRules) as $remainingPropertyName) { + self::assertStringStartsNotWith($propertyNamePrefixWithHyphen, $remainingPropertyName); + } + foreach ($expectedRemainingPropertyNames as $expectedPropertyName) { + self::assertArrayHasKey($expectedPropertyName, $remainingRules); + } + } + + /** + * @return array}> + */ + public static function provideRulesToRemove() + { + return [ + 'no rules' => [[]], + 'one rule' => [[new Rule('color')]], + 'two rules for different properties' => [[new Rule('color'), new Rule('display')]], + 'two rules for the same property' => [[new Rule('color'), new Rule('color')]], + ]; + } + + /** + * @test + * + * @param list $rules + * + * @dataProvider provideRulesToRemove + */ + public function removeAllRulesRemovesAllRules(array $rules) + { + $subject = new ConcreteRuleSet(); + $subject->setRules($rules); + + $subject->removeAllRules(); + + self::assertSame([], $subject->getRules()); + } }