Skip to content

Commit 726967e

Browse files
JakeQZJakeQZ
committed
[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 (and some minor dependent changes).
1 parent f5fc39a commit 726967e

File tree

3 files changed

+219
-23
lines changed

3 files changed

+219
-23
lines changed

‎CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ This project adheres to [Semantic Versioning](https://semver.org/).
77

88
### Added
99

10+
- `RuleSet::removeMatchingRules()` method
11+
(for the implementing classes `AtRuleSet` and `DeclarationBlock`) (#1249)
12+
- `RuleSet::removeAllRules()` method
13+
(for the implementing classes `AtRuleSet` and `DeclarationBlock`) (#1249)
1014
- Add Interface `CSSElement` (#1231)
1115
- Methods `getLineNumber` and `getColumnNumber` which return a nullable `int`
1216
for the following classes:
@@ -26,6 +30,9 @@ This project adheres to [Semantic Versioning](https://semver.org/).
2630

2731
### Deprecated
2832

33+
- Passing a `string` or `null` to `RuleSet::removeRule()` is deprecated
34+
(implementing classes are `AtRuleSet` and `DeclarationBlock`);
35+
use `removeMatchingRules()` or `removeAllRules()` instead (#1249)
2936
- Passing a string as the first argument to `getAllValues()` is deprecated;
3037
the search pattern should now be passed as the second argument (#1241)
3138
- Passing a Boolean as the second argument to `getAllValues()` is deprecated;

‎src/RuleSet/RuleSet.php

Lines changed: 37 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -220,20 +220,12 @@ public function getRulesAssoc($mRule = null)
220220
}
221221

222222
/**
223-
* Removes a rule from this RuleSet. This accepts all the possible values that `getRules()` accepts.
224-
*
225-
* If given a Rule, it will only remove this particular rule (by identity).
226-
* If given a name, it will remove all rules by that name.
227-
*
228-
* Note: this is different from pre-v.2.0 behaviour of PHP-CSS-Parser, where passing a Rule instance would
229-
* remove all rules with the same name. To get the old behaviour, use `removeRule($oRule->getRule())`.
223+
* Removes a `Rule` from this `RuleSet` by identity.
230224
*
231225
* @param Rule|string|null $mRule
232-
* pattern to remove. If $mRule is null, all rules are removed. If the pattern ends in a dash,
233-
* all rules starting with the pattern are removed as well as one matching the pattern with the dash
234-
* excluded. Passing a Rule behaves matches by identity.
235-
*
236-
* @return void
226+
* `Rule` to remove.
227+
* Passing a `string` or `null` is deprecated in version 8.9.0, and will no longer work from v9.0.
228+
* Use `removeMatchingRules()` or `removeAllRules()` instead.
237229
*/
238230
public function removeRule($mRule)
239231
{
@@ -247,22 +239,44 @@ public function removeRule($mRule)
247239
unset($this->aRules[$sRule][$iKey]);
248240
}
249241
}
242+
} elseif ($mRule !== null) {
243+
$this->removeMatchingRules($mRule);
250244
} else {
251-
foreach ($this->aRules as $sName => $aRules) {
252-
// Either no search rule is given or the search rule matches the found rule exactly
253-
// or the search rule ends in “-” and the found rule starts with the search rule or equals it
254-
// (without the trailing dash).
255-
if (
256-
!$mRule || $sName === $mRule
257-
|| (strrpos($mRule, '-') === strlen($mRule) - strlen('-')
258-
&& (strpos($sName, $mRule) === 0 || $sName === substr($mRule, 0, -1)))
259-
) {
260-
unset($this->aRules[$sName]);
261-
}
245+
$this->removeAllRules();
246+
}
247+
}
248+
249+
/**
250+
* Removes rules by property name or search pattern.
251+
*
252+
* @param string $searchPattern
253+
* pattern to remove.
254+
* If the pattern ends in a dash,
255+
* all rules starting with the pattern are removed as well as one matching the pattern with the dash
256+
* excluded.
257+
*/
258+
public function removeMatchingRules($searchPattern)
259+
{
260+
foreach ($this->aRules as $propertyName => $rules) {
261+
// Either the search rule matches the found rule exactly
262+
// or the search rule ends in “-” and the found rule starts with the search rule or equals it
263+
// (without the trailing dash).
264+
if (
265+
$propertyName === $searchPattern
266+
|| (\strrpos($searchPattern, '-') === \strlen($searchPattern) - \strlen('-')
267+
&& (\strpos($propertyName, $searchPattern) === 0
268+
|| $propertyName === \substr($searchPattern, 0, -1)))
269+
) {
270+
unset($this->aRules[$propertyName]);
262271
}
263272
}
264273
}
265274

275+
public function removeAllRules()
276+
{
277+
$this->aRules = [];
278+
}
279+
266280
/**
267281
* @return string
268282
*

‎tests/Unit/RuleSet/RuleSetTest.php

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
use PHPUnit\Framework\TestCase;
88
use Sabberworm\CSS\CSSElement;
9+
use Sabberworm\CSS\Rule\Rule;
910
use Sabberworm\CSS\Tests\Unit\RuleSet\Fixtures\ConcreteRuleSet;
1011

1112
/**
@@ -24,4 +25,178 @@ public function implementsCSSElement()
2425

2526
self::assertInstanceOf(CSSElement::class, $subject);
2627
}
28+
29+
/**
30+
* @return array<string, array{0: list<Rule>, 1: string, 2: list<string>}>
31+
*/
32+
public static function provideRulesAndPropertyNameToRemoveAndExpectedRemainingPropertyNames()
33+
{
34+
return [
35+
'removing single rule' => [
36+
[new Rule('color')],
37+
'color',
38+
[],
39+
],
40+
'removing first rule' => [
41+
[new Rule('color'), new Rule('display')],
42+
'color',
43+
['display'],
44+
],
45+
'removing last rule' => [
46+
[new Rule('color'), new Rule('display')],
47+
'display',
48+
['color'],
49+
],
50+
'removing middle rule' => [
51+
[new Rule('color'), new Rule('display'), new Rule('width')],
52+
'display',
53+
['color', 'width'],
54+
],
55+
'removing multiple rules' => [
56+
[new Rule('color'), new Rule('color')],
57+
'color',
58+
[],
59+
],
60+
'removing multiple rules with another kept' => [
61+
[new Rule('color'), new Rule('color'), new Rule('display')],
62+
'color',
63+
['display'],
64+
],
65+
'removing nonexistent rule from empty list' => [
66+
[],
67+
'color',
68+
[],
69+
],
70+
'removing nonexistent rule from nonempty list' => [
71+
[new Rule('color'), new Rule('display')],
72+
'width',
73+
['color', 'display'],
74+
],
75+
];
76+
}
77+
78+
/**
79+
* @test
80+
*
81+
* @param list<Rule> $rules
82+
* @param string $propertyName
83+
* @param list<string> $expectedRemainingPropertyNames
84+
*
85+
* @dataProvider provideRulesAndPropertyNameToRemoveAndExpectedRemainingPropertyNames
86+
*/
87+
public function removeMatchingRulesRemovesRulesByPropertyNameAndKeepsOthers(
88+
array $rules,
89+
$propertyName,
90+
array $expectedRemainingPropertyNames
91+
) {
92+
$subject = new ConcreteRuleSet();
93+
$subject->setRules($rules);
94+
95+
$subject->removeMatchingRules($propertyName);
96+
97+
$remainingRules = $subject->getRulesAssoc();
98+
self::assertArrayNotHasKey($propertyName, $remainingRules);
99+
foreach ($expectedRemainingPropertyNames as $expectedPropertyName) {
100+
self::assertArrayHasKey($expectedPropertyName, $remainingRules);
101+
}
102+
}
103+
104+
/**
105+
* @return array<string, array{0: list<Rule>, 1: string, 2: list<string>}>
106+
*/
107+
public static function provideRulesAndPropertyNamePrefixToRemoveAndExpectedRemainingPropertyNames()
108+
{
109+
return [
110+
'removing shorthand rule' => [
111+
[new Rule('font')],
112+
'font',
113+
[],
114+
],
115+
'removing longhand rule' => [
116+
[new Rule('font-size')],
117+
'font',
118+
[],
119+
],
120+
'removing shorthand and longhand rule' => [
121+
[new Rule('font'), new Rule('font-size')],
122+
'font',
123+
[],
124+
],
125+
'removing shorthand rule with another kept' => [
126+
[new Rule('font'), new Rule('color')],
127+
'font',
128+
['color'],
129+
],
130+
'removing longhand rule with another kept' => [
131+
[new Rule('font-size'), new Rule('color')],
132+
'font',
133+
['color'],
134+
],
135+
'keeping other rules whose property names begin with the same characters' => [
136+
[new Rule('contain'), new Rule('container'), new Rule('container-type')],
137+
'contain',
138+
['container', 'container-type'],
139+
],
140+
];
141+
}
142+
143+
/**
144+
* @test
145+
*
146+
* @param list<Rule> $rules
147+
* @param string $propertyNamePrefix
148+
* @param list<string> $expectedRemainingPropertyNames
149+
*
150+
* @dataProvider provideRulesAndPropertyNamePrefixToRemoveAndExpectedRemainingPropertyNames
151+
*/
152+
public function removeMatchingRulesRemovesRulesByPropertyNamePrefixAndKeepsOthers(
153+
array $rules,
154+
$propertyNamePrefix,
155+
array $expectedRemainingPropertyNames
156+
) {
157+
$propertyNamePrefixWithHyphen = $propertyNamePrefix . '-';
158+
$subject = new ConcreteRuleSet();
159+
$subject->setRules($rules);
160+
161+
$subject->removeMatchingRules($propertyNamePrefixWithHyphen);
162+
163+
$remainingRules = $subject->getRulesAssoc();
164+
self::assertArrayNotHasKey($propertyNamePrefix, $remainingRules);
165+
foreach (\array_keys($remainingRules) as $remainingPropertyName) {
166+
self::assertStringStartsNotWith($propertyNamePrefixWithHyphen, $remainingPropertyName);
167+
}
168+
foreach ($expectedRemainingPropertyNames as $expectedPropertyName) {
169+
self::assertArrayHasKey($expectedPropertyName, $remainingRules);
170+
}
171+
}
172+
173+
/**
174+
* @return array<string, array{0: list<Rule>}>
175+
*/
176+
public static function provideRulesToRemove()
177+
{
178+
return [
179+
'no rules' => [[]],
180+
'one rule' => [[new Rule('color')]],
181+
'two rules for different properties' => [[new Rule('color'), new Rule('display')]],
182+
'two rules for the same property' => [[new Rule('color'), new Rule('color')]],
183+
];
184+
}
185+
186+
/**
187+
* @test
188+
*
189+
* @param list<Rule> $rules
190+
*
191+
* @dataProvider provideRulesToRemove
192+
*/
193+
public function removeAllRulesRemovesAllRules(array $rules)
194+
{
195+
$subject = new ConcreteRuleSet();
196+
$subject->setRules($rules);
197+
198+
$subject->removeAllRules();
199+
200+
self::assertSame([], $subject->getRules());
201+
}
27202
}

0 commit comments

Comments
 (0)