Skip to content

Commit f87eb61

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 f87eb61

File tree

3 files changed

+224
-25
lines changed

3 files changed

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

25-
self::assertInstanceOf(CSSElement::class, $subject);
203+
self::assertSame([], $this->subject->getRules());
26204
}
27205
}

0 commit comments

Comments
 (0)