[TASK] Add separate methods for RuleSet::removeRule(), with deprecation

CHANGELOG.md

@@ -10,6 +10,10 @@ Please also have a look at our
 
 ### 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:
@@ -46,6 +50,9 @@ Please also have a look at our
 
 ### 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 `Rule` to `RuleSet::getRules()` or `getRulesAssoc()` is deprecated,
   affecting the implementing classes `AtRuleSet` and `DeclarationBlock`
   (call e.g. `getRules($rule->getRule())` instead) (#1248)

src/RuleSet/RuleSet.php

@@ -212,18 +212,12 @@ public function getRulesAssoc($searchPattern = null): array
     }
 
     /**
-     * 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($rule->getRule())`.
+     * Removes a `Rule` from this `RuleSet` by identity.
      *
      * @param Rule|string|null $searchPattern
-     *        pattern to remove. If 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.
+     *        `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($searchPattern): void
     {
@@ -237,23 +231,44 @@ public function removeRule($searchPattern): void
                     unset($this->rules[$nameOfPropertyToRemove][$key]);
                 }
             }
+        } elseif ($searchPattern !== null) {
+            $this->removeMatchingRules($searchPattern);
         } else {
-            foreach ($this->rules as $propertyName => $rules) {
-                // 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 (
-                    $searchPattern === null || $propertyName === $searchPattern
-                    || (\strrpos($searchPattern, '-') === \strlen($searchPattern) - \strlen('-')
-                        && (\strpos($propertyName, $searchPattern) === 0
-                            || $propertyName === \substr($searchPattern, 0, -1)))
-                ) {
-                    unset($this->rules[$propertyName]);
-                }
+            $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(string $searchPattern): void
+    {
+        foreach ($this->rules 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->rules[$propertyName]);
             }
         }
     }
 
+    public function removeAllRules(): void
+    {
+        $this->rules = [];
+    }
+
     protected function renderRules(OutputFormat $outputFormat): string
     {
         $result = '';

tests/Unit/RuleSet/RuleSetTest.php

@@ -7,6 +7,7 @@
 use PHPUnit\Framework\TestCase;
 use Sabberworm\CSS\CSSElement;
 use Sabberworm\CSS\CSSList\CSSListItem;
+use Sabberworm\CSS\Rule\Rule;
 use Sabberworm\CSS\Tests\Unit\RuleSet\Fixtures\ConcreteRuleSet;
 
 /**
@@ -39,4 +40,173 @@ public function implementsCSSListItem(): void
     {
         self::assertInstanceOf(CSSListItem::class, $this->subject);
     }
+
+    /**
+     * @return array<string, array{0: list<Rule>, 1: string, 2: list<string>}>
+     */
+    public static function provideRulesAndPropertyNameToRemoveAndExpectedRemainingPropertyNames(): array
+    {
+        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<Rule> $rules
+     * @param list<string> $expectedRemainingPropertyNames
+     *
+     * @dataProvider provideRulesAndPropertyNameToRemoveAndExpectedRemainingPropertyNames
+     */
+    public function removeMatchingRulesRemovesRulesByPropertyNameAndKeepsOthers(
+        array $rules,
+        string $propertyName,
+        array $expectedRemainingPropertyNames
+    ): void {
+        $this->subject->setRules($rules);
+
+        $this->subject->removeMatchingRules($propertyName);
+
+        $remainingRules = $this->subject->getRulesAssoc();
+        self::assertArrayNotHasKey($propertyName, $remainingRules);
+        foreach ($expectedRemainingPropertyNames as $expectedPropertyName) {
+            self::assertArrayHasKey($expectedPropertyName, $remainingRules);
+        }
+    }
+
+    /**
+     * @return array<string, array{0: list<Rule>, 1: string, 2: list<string>}>
+     */
+    public static function provideRulesAndPropertyNamePrefixToRemoveAndExpectedRemainingPropertyNames(): array
+    {
+        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<Rule> $rules
+     * @param list<string> $expectedRemainingPropertyNames
+     *
+     * @dataProvider provideRulesAndPropertyNamePrefixToRemoveAndExpectedRemainingPropertyNames
+     */
+    public function removeMatchingRulesRemovesRulesByPropertyNamePrefixAndKeepsOthers(
+        array $rules,
+        string $propertyNamePrefix,
+        array $expectedRemainingPropertyNames
+    ): void {
+        $propertyNamePrefixWithHyphen = $propertyNamePrefix . '-';
+        $this->subject->setRules($rules);
+
+        $this->subject->removeMatchingRules($propertyNamePrefixWithHyphen);
+
+        $remainingRules = $this->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<string, array{0: list<Rule>}>
+     */
+    public static function provideRulesToRemove(): array
+    {
+        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<Rule> $rules
+     *
+     * @dataProvider provideRulesToRemove
+     */
+    public function removeAllRulesRemovesAllRules(array $rules): void
+    {
+        $this->subject->setRules($rules);
+
+        $this->subject->removeAllRules();
+
+        self::assertSame([], $this->subject->getRules());
+    }
 }