[TASK] Add native type declarations for CSSBlockList
#1183
Conversation
oliverklee
added this to the
9.0.0: Drop support for PHP < 7.2 and clean up things milestone
src/CSSList/CSSBlockList.php
Outdated
| } else { | ||
| // Non-List `Value` or `CSSString` (CSS identifier) | ||
| $result[] = $element; | ||
| if ($element instanceof Value) { |
There was a problem hiding this comment.
Could this just be an elseif?
src/CSSList/CSSBlockList.php
Outdated
| @@ -149,7 +146,7 @@ protected function allSelectors(array &$result, $specificitySearch = null): void | |||
| $comparatorMatched = $selectorSpecificity === $targetSpecificity; | |||
| break; | |||
| } | |||
| if ($comparatorMatched) { | |||
| if ($comparatorMatched && $selector instanceof Selector) { | |||
There was a problem hiding this comment.
If we did want to do the instanceof check, we should do it before calling getSpecificity(). However, I think it's unnecessary because DeclarationBlock::getSelectors() always returns an array of Selectors - and the types in the PHPDoc for DeclarationBlock::$selectors are incorrect. If we first fix the type hints for that property and its getter, I expect we'd find the instanceof test is not needed and PHPStan would be happy without it.
There was a problem hiding this comment.
I took a quick peek into that - that's quite a can of worms, involving covering setSelectors with tests, refactoring it to no longer temporarily storing strings in DeclarationBlock::selectors, and more. I'll have a look look at that later and create a ticket.
There was a problem hiding this comment.
Ah, yes, I see: during execution of setSelectors(), the elements in $selectors may temporarily become strings. And if an exception is thrown, they'll be left in that state. So it's not as straightforward as I thought. When you create the ticket, there should be a note to remove the instanceof check added here. For now, it can be added, but needs to be before the getSpecificity method call.
There was a problem hiding this comment.
I have added the first (smaller) part for this with #1184.
Also add some more type checks to ensure that the corresponding types are actually returned. Part of #811
oliverklee
force-pushed
the
task/types/cssblocklist
branch
from
1002e67 to
558831c
Compare
Also add some more type checks to ensure that the corresponding types are actually returned.
Part of #811