Skip to content

Fix phpstan/phpstan#14314: strange behavior on regex capture groups#5239

Open
phpstan-bot wants to merge 2 commits intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-n3ujwwh
Open

Fix phpstan/phpstan#14314: strange behavior on regex capture groups#5239
phpstan-bot wants to merge 2 commits intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-n3ujwwh

Conversation

@phpstan-bot
Copy link
Collaborator

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 caused array{a, b, c, d} to be removed along with array{a, b}, producing false positives on subsequent count checks.

Changes

  • Added a fallback path in src/Analyser/TypeSpecifier.php in the specifyTypesForCountFuncCall method's falsey handler
  • When the recursive truthy call fails (because the remaining size includes 0), directly filter constant arrays by their exact sizes and use truthy context to set the result
  • This bypasses TypeCombinator::remove() which incorrectly treats shorter constant arrays as supertypes of longer ones

Root cause

ConstantArrayType::isSuperTypeOf() considers array{string, string} a supertype of array{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 causes TypeCombinator::remove(union, array{a,b}) to also remove array{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.php which tests that after preg_match producing a union of 0, 2, and 4-element arrays, narrowing by count === 2 correctly preserves the 4-element variant in the falsey branch.

Fixes phpstan/phpstan#14314

…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
Copy link
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
@phpstan-bot
Copy link
Collaborator Author

Done. Here's a summary of what was done to address the review:

Review concern 1: Can isSuperTypeOf lose arrays when sizeType is IntegerRange and arrays have optional keys?

No. The check $sizeType->isSuperTypeOf($arrayType->getArraySize())->yes() is conservative - it only removes an array if ALL its possible sizes fall within the sizeType range. Added three tests to demonstrate this:

  • testIntRangeFullyCoveringOptionalKeys: int<2,3> fully covers array{0: string, 1: string, 2?: string} (size range 2..3) - array is correctly removed in falsey branch
  • testIntRangePartiallyCoveringOptionalKeys: int<2,3> does NOT fully cover array{0: string, 1: string, 2?: string, 3?: string} (size range 2..4) - array is correctly kept
  • testOptionalKeysWithEmpty: count === 3 doesn't fully cover size range 2..3 - array is correctly kept

Review concern 2: Are other type specifications lost from the early return?

No. The early return is in the falsey branch, and the additional HasOffsetValueType specifications (lines 1427-1464) only run for truthy context. Added tests:

  • sequential count checks: Three consecutive count narrows on a 4-variant union, verifying each step preserves the correct remaining variants
  • other variable types preserved: Verifying that $x's int<1, max> narrowing from $x > 0 is preserved through count narrowing of a different variable

@VincentLanglet VincentLanglet requested a review from staabm March 17, 2026 22:27
@VincentLanglet VincentLanglet self-assigned this Mar 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants