Skip to content

Commit 3f57730

Browse files
authored
[CLEANUP] Tighten DeclarationBlock::selectors type (#1407)
Use a local variable in `setSelectors()` rather than temporarily assigning the property with a type it's not supposed to have. Ensure that the property is set to a non-sparse numerical array. Also tighten the return type of `getSelectors()`. (Precursor to #1330.)
1 parent a7e229b commit 3f57730

File tree

3 files changed

+30
-7
lines changed

3 files changed

+30
-7
lines changed

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ Please also have a look at our
1414

1515
### Changed
1616

17+
- The array keys passed to `DeclarationBlock::setSelectors()` are no longer
18+
preserved (#1407)
19+
1720
### Deprecated
1821

1922
### Removed

src/RuleSet/DeclarationBlock.php

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ class DeclarationBlock implements CSSElement, CSSListItem, Positionable, RuleCon
3636
use Position;
3737

3838
/**
39-
* @var array<Selector|string>
39+
* @var list<Selector>
4040
*/
4141
private $selectors = [];
4242

@@ -146,11 +146,13 @@ public static function parse(ParserState $parserState, ?CSSList $list = null): ?
146146
public function setSelectors($selectors, ?CSSList $list = null): void
147147
{
148148
if (\is_array($selectors)) {
149-
$this->selectors = $selectors;
149+
$selectorsToSet = $selectors;
150150
} else {
151-
$this->selectors = \explode(',', $selectors);
151+
$selectorsToSet = \explode(',', $selectors);
152152
}
153-
foreach ($this->selectors as $key => $selector) {
153+
154+
// Convert all items to a `Selector` if not already
155+
foreach ($selectorsToSet as $key => $selector) {
154156
if (!($selector instanceof Selector)) {
155157
if ($list === null || !($list instanceof KeyFrame)) {
156158
if (!Selector::isValid($selector)) {
@@ -160,7 +162,7 @@ public function setSelectors($selectors, ?CSSList $list = null): void
160162
'custom'
161163
);
162164
}
163-
$this->selectors[$key] = new Selector($selector);
165+
$selectorsToSet[$key] = new Selector($selector);
164166
} else {
165167
if (!KeyframeSelector::isValid($selector)) {
166168
throw new UnexpectedTokenException(
@@ -169,10 +171,13 @@ public function setSelectors($selectors, ?CSSList $list = null): void
169171
'custom'
170172
);
171173
}
172-
$this->selectors[$key] = new KeyframeSelector($selector);
174+
$selectorsToSet[$key] = new KeyframeSelector($selector);
173175
}
174176
}
175177
}
178+
179+
// Discard the keys and reindex the array
180+
$this->selectors = \array_values($selectorsToSet);
176181
}
177182

178183
/**
@@ -195,7 +200,7 @@ public function removeSelector($selectorToRemove): bool
195200
}
196201

197202
/**
198-
* @return array<Selector>
203+
* @return list<Selector>
199204
*/
200205
public function getSelectors(): array
201206
{

tests/Unit/RuleSet/DeclarationBlockTest.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,4 +278,19 @@ public function getRuleSetReturnsObjectWithLineNumberPassedToConstructor(?int $l
278278

279279
self::assertSame($lineNumber, $result->getLineNumber());
280280
}
281+
282+
/**
283+
* @test
284+
*
285+
* Any type of array may be passed to the method, but the resultant property should be a `list`.
286+
*/
287+
public function setSelectorsIgnoresKeys(): void
288+
{
289+
$subject = new DeclarationBlock();
290+
$subject->setSelectors(['Bob' => 'html', 'Mary' => 'body']);
291+
292+
$result = $subject->getSelectors();
293+
294+
self::assertSame([0, 1], \array_keys($result));
295+
}
281296
}

0 commit comments

Comments
 (0)