Fix phpstan/phpstan#14314: strange behavior on regex capture groups#5239
Fix phpstan/phpstan#14314: strange behavior on regex capture groups#5239phpstan-bot wants to merge 2 commits intophpstan:2.1.xfrom
Conversation
…d constant arrays from union - Added fallback in TypeSpecifier::specifyTypesForCountFuncCall for falsey context - When recursive truthy call fails (remaining size includes 0), directly filter constant arrays by their exact sizes instead of using TypeCombinator::remove - This avoids the issue where remove() considers a shorter constant array as a supertype of a longer one, incorrectly eliminating both from the union - New regression test in tests/PHPStan/Analyser/nsrt/bug-14314.php
VincentLanglet
left a comment
There was a problem hiding this comment.
Can't the check $sizeType->isSuperTypeOf($arrayType->getArraySize())->yes() lose some array when the sizeType is an IntegerRange and the arrayType have optional keys ?
Please add test about this.
Also I wonder if we're loosing some other typespecifications from the early return ; please ensure it's not the case by adding more tests.
…eType Address review concerns from PR phpstan#5239: - Test that IntegerRange sizeType doesn't incorrectly lose arrays with optional keys (fully covering range correctly removes, partially covering range correctly keeps) - Test sequential count checks preserve narrowing across multiple branches - Test that other variable type specifications are not lost from the early return Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Done. Here's a summary of what was done to address the review: Review concern 1: Can No. The check
Review concern 2: Are other type specifications lost from the early return? No. The early return is in the falsey branch, and the additional
|
Summary
When using
count()to narrow a union of constant arrays with different sizes (e.g.,array{}|array{a, b}|array{a, b, c, d}), the falsey branch (count !== 2) incorrectly removed arrays whose count didn't match but whose shape was a subtype. This causedarray{a, b, c, d}to be removed along witharray{a, b}, producing false positives on subsequent count checks.Changes
src/Analyser/TypeSpecifier.phpin thespecifyTypesForCountFuncCallmethod's falsey handlerTypeCombinator::remove()which incorrectly treats shorter constant arrays as supertypes of longer onesRoot cause
ConstantArrayType::isSuperTypeOf()considersarray{string, string}a supertype ofarray{string, string, string, string}because it only checks that its own keys have matching values, ignoring extra keys. This is correct type-system behavior, but it causesTypeCombinator::remove(union, array{a,b})to also removearray{a,b,c,d}from the union. The falsey branch of count narrowing relied on this removal, which produced wrong results.The fix directly filters the constant arrays by their size (using
getArraySize()) rather than relying on type-shape removal.Test
Added
tests/PHPStan/Analyser/nsrt/bug-14314.phpwhich tests that afterpreg_matchproducing a union of 0, 2, and 4-element arrays, narrowing bycount === 2correctly preserves the 4-element variant in the falsey branch.Fixes phpstan/phpstan#14314