Skip to content

Commit 807a11b

Browse files
JakeQZJakeQZ
authored
[TASK] Add separate methods for RuleSet::removeRule(), with deprecation (#1249)
Passing `string` or `null` to `removeRule()` is deprecated. The new method handles that functionality. Relates to #1247.
1 parent f6b5cd3 commit 807a11b

File tree

3 files changed

+214
-22
lines changed

3 files changed

+214
-22
lines changed

‎CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@ Please also have a look at our
1010

1111
### Added
1212

13+
- `RuleSet::removeMatchingRules()` method
14+
(for the implementing classes `AtRuleSet` and `DeclarationBlock`) (#1249)
15+
- `RuleSet::removeAllRules()` method
16+
(for the implementing classes `AtRuleSet` and `DeclarationBlock`) (#1249)
1317
- Add Interface `CSSElement` (#1231)
1418
- Methods `getLineNumber` and `getColumnNumber` which return a nullable `int`
1519
for the following classes:
@@ -46,6 +50,9 @@ Please also have a look at our
4650

4751
### Deprecated
4852

53+
- Passing a `string` or `null` to `RuleSet::removeRule()` is deprecated
54+
(implementing classes are `AtRuleSet` and `DeclarationBlock`);
55+
use `removeMatchingRules()` or `removeAllRules()` instead (#1249)
4956
- Passing a `Rule` to `RuleSet::getRules()` or `getRulesAssoc()` is deprecated,
5057
affecting the implementing classes `AtRuleSet` and `DeclarationBlock`
5158
(call e.g. `getRules($rule->getRule())` instead) (#1248)

‎src/RuleSet/RuleSet.php

Lines changed: 37 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -212,18 +212,12 @@ public function getRulesAssoc($searchPattern = null): array
212212
}
213213

214214
/**
215-
* Removes a rule from this RuleSet. This accepts all the possible values that `getRules()` accepts.
216-
*
217-
* If given a Rule, it will only remove this particular rule (by identity).
218-
* If given a name, it will remove all rules by that name.
219-
*
220-
* Note: this is different from pre-v.2.0 behaviour of PHP-CSS-Parser, where passing a Rule instance would
221-
* remove all rules with the same name. To get the old behaviour, use `removeRule($rule->getRule())`.
215+
* Removes a `Rule` from this `RuleSet` by identity.
222216
*
223217
* @param Rule|string|null $searchPattern
224-
* pattern to remove. If null, all rules are removed. If the pattern ends in a dash,
225-
* all rules starting with the pattern are removed as well as one matching the pattern with the dash
226-
* excluded. Passing a Rule behaves matches by identity.
218+
* `Rule` to remove.
219+
* Passing a `string` or `null` is deprecated in version 8.9.0, and will no longer work from v9.0.
220+
* Use `removeMatchingRules()` or `removeAllRules()` instead.
227221
*/
228222
public function removeRule($searchPattern): void
229223
{
@@ -237,23 +231,44 @@ public function removeRule($searchPattern): void
237231
unset($this->rules[$nameOfPropertyToRemove][$key]);
238232
}
239233
}
234+
} elseif ($searchPattern !== null) {
235+
$this->removeMatchingRules($searchPattern);
240236
} else {
241-
foreach ($this->rules as $propertyName => $rules) {
242-
// Either no search rule is given or the search rule matches the found rule exactly
243-
// or the search rule ends in “-” and the found rule starts with the search rule or equals it
244-
// (without the trailing dash).
245-
if (
246-
$searchPattern === null || $propertyName === $searchPattern
247-
|| (\strrpos($searchPattern, '-') === \strlen($searchPattern) - \strlen('-')
248-
&& (\strpos($propertyName, $searchPattern) === 0
249-
|| $propertyName === \substr($searchPattern, 0, -1)))
250-
) {
251-
unset($this->rules[$propertyName]);
252-
}
237+
$this->removeAllRules();
238+
}
239+
}
240+
241+
/**
242+
* Removes rules by property name or search pattern.
243+
*
244+
* @param string $searchPattern
245+
* pattern to remove.
246+
* If the pattern ends in a dash,
247+
* all rules starting with the pattern are removed as well as one matching the pattern with the dash
248+
* excluded.
249+
*/
250+
public function removeMatchingRules(string $searchPattern): void
251+
{
252+
foreach ($this->rules as $propertyName => $rules) {
253+
// Either the search rule matches the found rule exactly
254+
// or the search rule ends in “-” and the found rule starts with the search rule or equals it
255+
// (without the trailing dash).
256+
if (
257+
$propertyName === $searchPattern
258+
|| (\strrpos($searchPattern, '-') === \strlen($searchPattern) - \strlen('-')
259+
&& (\strpos($propertyName, $searchPattern) === 0
260+
|| $propertyName === \substr($searchPattern, 0, -1)))
261+
) {
262+
unset($this->rules[$propertyName]);
253263
}
254264
}
255265
}
256266

267+
public function removeAllRules(): void
268+
{
269+
$this->rules = [];
270+
}
271+
257272
protected function renderRules(OutputFormat $outputFormat): string
258273
{
259274
$result = '';

‎tests/Unit/RuleSet/RuleSetTest.php

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use PHPUnit\Framework\TestCase;
88
use Sabberworm\CSS\CSSElement;
99
use Sabberworm\CSS\CSSList\CSSListItem;
10+
use Sabberworm\CSS\Rule\Rule;
1011
use Sabberworm\CSS\Tests\Unit\RuleSet\Fixtures\ConcreteRuleSet;
1112

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

0 commit comments

Comments
 (0)