Skip to content

Commit fbdf55c

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 fbdf55c

File tree

3 files changed

+226
-25
lines changed

3 files changed

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

25-
self::assertInstanceOf(CSSElement::class, $subject);
205+
self::assertSame([], $this->subject->getRules());
26206
}
27207
}

0 commit comments

Comments
 (0)