Skip to content

Commit 725de58

Browse files
JakeQZJakeQZ
committed
[TASK] Add separate methods for RuleSet::removeRule(), with deprecation
Passing `string` or `null` to `removeRule()` is deprecated. The new methods handle that functionality. Relates to #1247. This is the backport of #1249.
1 parent f5fc39a commit 725de58

File tree

3 files changed

+214
-23
lines changed

3 files changed

+214
-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(string $searchPattern)
259+
{
260+
foreach ($this->rules 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->rules[$propertyName]);
262271
}
263272
}
264273
}
265274

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

‎tests/Unit/RuleSet/RuleSetTest.php‎

Lines changed: 170 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,173 @@ 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 list<string> $expectedRemainingPropertyNames
83+
*
84+
* @dataProvider provideRulesAndPropertyNameToRemoveAndExpectedRemainingPropertyNames
85+
*/
86+
public function removeMatchingRulesRemovesRulesByPropertyNameAndKeepsOthers(
87+
array $rules,
88+
string $propertyName,
89+
array $expectedRemainingPropertyNames
90+
) {
91+
$this->subject->setRules($rules);
92+
93+
$this->subject->removeMatchingRules($propertyName);
94+
95+
$remainingRules = $this->subject->getRulesAssoc();
96+
self::assertArrayNotHasKey($propertyName, $remainingRules);
97+
foreach ($expectedRemainingPropertyNames as $expectedPropertyName) {
98+
self::assertArrayHasKey($expectedPropertyName, $remainingRules);
99+
}
100+
}
101+
102+
/**
103+
* @return array<string, array{0: list<Rule>, 1: string, 2: list<string>}>
104+
*/
105+
public static function provideRulesAndPropertyNamePrefixToRemoveAndExpectedRemainingPropertyNames()
106+
{
107+
return [
108+
'removing shorthand rule' => [
109+
[new Rule('font')],
110+
'font',
111+
[],
112+
],
113+
'removing longhand rule' => [
114+
[new Rule('font-size')],
115+
'font',
116+
[],
117+
],
118+
'removing shorthand and longhand rule' => [
119+
[new Rule('font'), new Rule('font-size')],
120+
'font',
121+
[],
122+
],
123+
'removing shorthand rule with another kept' => [
124+
[new Rule('font'), new Rule('color')],
125+
'font',
126+
['color'],
127+
],
128+
'removing longhand rule with another kept' => [
129+
[new Rule('font-size'), new Rule('color')],
130+
'font',
131+
['color'],
132+
],
133+
'keeping other rules whose property names begin with the same characters' => [
134+
[new Rule('contain'), new Rule('container'), new Rule('container-type')],
135+
'contain',
136+
['container', 'container-type'],
137+
],
138+
];
139+
}
140+
141+
/**
142+
* @test
143+
*
144+
* @param list<Rule> $rules
145+
* @param list<string> $expectedRemainingPropertyNames
146+
*
147+
* @dataProvider provideRulesAndPropertyNamePrefixToRemoveAndExpectedRemainingPropertyNames
148+
*/
149+
public function removeMatchingRulesRemovesRulesByPropertyNamePrefixAndKeepsOthers(
150+
array $rules,
151+
string $propertyNamePrefix,
152+
array $expectedRemainingPropertyNames
153+
) {
154+
$propertyNamePrefixWithHyphen = $propertyNamePrefix . '-';
155+
$this->subject->setRules($rules);
156+
157+
$this->subject->removeMatchingRules($propertyNamePrefixWithHyphen);
158+
159+
$remainingRules = $this->subject->getRulesAssoc();
160+
self::assertArrayNotHasKey($propertyNamePrefix, $remainingRules);
161+
foreach (\array_keys($remainingRules) as $remainingPropertyName) {
162+
self::assertStringStartsNotWith($propertyNamePrefixWithHyphen, $remainingPropertyName);
163+
}
164+
foreach ($expectedRemainingPropertyNames as $expectedPropertyName) {
165+
self::assertArrayHasKey($expectedPropertyName, $remainingRules);
166+
}
167+
}
168+
169+
/**
170+
* @return array<string, array{0: list<Rule>}>
171+
*/
172+
public static function provideRulesToRemove()
173+
{
174+
return [
175+
'no rules' => [[]],
176+
'one rule' => [[new Rule('color')]],
177+
'two rules for different properties' => [[new Rule('color'), new Rule('display')]],
178+
'two rules for the same property' => [[new Rule('color'), new Rule('color')]],
179+
];
180+
}
181+
182+
/**
183+
* @test
184+
*
185+
* @param list<Rule> $rules
186+
*
187+
* @dataProvider provideRulesToRemove
188+
*/
189+
public function removeAllRulesRemovesAllRules(array $rules)
190+
{
191+
$this->subject->setRules($rules);
192+
193+
$this->subject->removeAllRules();
194+
195+
self::assertSame([], $this->subject->getRules());
196+
}
27197
}

0 commit comments

Comments
 (0)