Skip to content

Widen empty ConstantArrayType to array when resolving dynamic constant types#5606

Merged
VincentLanglet merged 8 commits intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-5esgl9v
May 9, 2026
Merged

Widen empty ConstantArrayType to array when resolving dynamic constant types#5606
VincentLanglet merged 8 commits intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-5esgl9v

Conversation

@phpstan-bot
Copy link
Copy Markdown
Collaborator

Summary

When a constant is defined as an empty array (define('FOO', []) or const FOO = []) and listed in dynamicConstantNames, PHPStan should treat its type as array (non-constant). Instead, the type remained array{}, causing false positives like "Offset 'X' on array{} in isset() does not exist."

Changes

  • Added generalizeDynamicConstantType() helper method in src/Analyser/ConstantResolver.php that handles the case where generalize(lessSpecific) fails to widen the type (returns a type still satisfying isConstantValue()->yes())
  • Updated resolveConstantType() and resolveClassConstantType() to use the new helper
  • Added test cases for global dynamic empty array constants and class dynamic empty array constants in tests/PHPStan/Analyser/data/dynamic-constant.php and tests/PHPStan/Analyser/dynamic-constants.neon

Analogous cases probed

  • Class constants with empty arrays (no PHPDoc): Same root cause, fixed by the same resolveClassConstantType() change. Test added.
  • Class constants with PHPDoc types: Already correctly handled — resolveClassConstantType() uses the PHPDoc type before reaching generalize(). Existing test confirms (DYNAMIC_EMPTY_ARRAY_WITH_PHPDOC_CONSTANT).
  • Other scalar constant types (bool, int, string, float): Already correctly generalized — generalize(lessSpecific) widens true→bool, 42→int, etc. No fix needed.
  • NullType: generalize() returns null (itself), but null has no wider type, so this is correct. Users needing a wider type should use the explicit type syntax (FOO: 'string|null').
  • Non-empty ConstantArrayType: Already correctly generalized to ArrayType with generalized key/value types. No fix needed.

Root cause

ConstantArrayType::generalize(GeneralizePrecision::lessSpecific()) has an early return for empty arrays (count($this->keyTypes) === 0 → return $this). This means an empty constant array "generalizes" to itself — still a constant value. When ConstantResolver::resolveConstantType() calls generalize() to widen a dynamic constant's type, the empty array passes through unchanged.

Rather than changing generalize() globally (which would affect type descriptions in error messages, making unions like array{}|array{a: int} describe as array instead of array<string, int>), the fix is scoped to ConstantResolver: after calling generalize(), if the result is still a constant array value, fall back to ArrayType(MixedType, MixedType).

Test

  • Type inference test: Added assertType('array', GLOBAL_DYNAMIC_EMPTY_ARRAY) and assertType('array', DynamicConstantClass::DYNAMIC_EMPTY_ARRAY_NO_PHPDOC) in tests/PHPStan/Analyser/data/dynamic-constant.php. Both fail without the fix (actual type: array{}), pass with it.

Fixes phpstan/phpstan#8526

Copy link
Copy Markdown
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.

I don't think you really understood the issue.

Let's start with a (possibly failing test).

Use

dynamicConstantNames:
        - FOO
        - DYNAMICARRAY

with

define('FOO', true);
define('DYNAMICARRAY', []);

if (isset(DYNAMICARRAY['MyKey'])){
..
}
if (FOO) {
    echo 'foo';
}

and ensure that there is no

  • Offset 'MyKey' on array{} in isset() does not exist.
  • If condition is always true

errors

@phpstan-bot
Copy link
Copy Markdown
Collaborator Author

Done. Added an integration test (Bug8526IntegrationTest) that directly verifies the reviewer's requested scenario:

  • define('FOO', true) + define('DYNAMICARRAY', []) with dynamicConstantNames: [FOO, DYNAMICARRAY]
  • if (isset(DYNAMICARRAY['MyKey'])) must not produce "Offset 'MyKey' on array{} in isset() does not exist"
  • if (FOO) must not produce "If condition is always true"

I verified the test fails without the fix (showing the "Offset 'MyKey' on array{} in isset() does not exist" error) and passes with the fix. All existing tests and make phpstan pass cleanly.

Comment thread src/Analyser/ConstantResolver.php
Comment thread src/Analyser/ConstantResolver.php Outdated
Comment thread src/Analyser/ConstantResolver.php Outdated
@VincentLanglet VincentLanglet requested a review from staabm May 7, 2026 07:04
@VincentLanglet VincentLanglet self-assigned this May 7, 2026
VincentLanglet and others added 6 commits May 9, 2026 11:45
…stant types

- `ConstantArrayType::generalize(lessSpecific)` returns `$this` for empty
  arrays (0 keys), so `resolveConstantType()` and `resolveClassConstantType()`
  were returning `array{}` for dynamic constants defined as `[]`
- Add `generalizeDynamicConstantType()` helper in `ConstantResolver` that
  falls back to `ArrayType(MixedType, MixedType)` when `generalize()` returns
  a type that is still a constant value and is an array
- Fixes false positive "Offset X on array{} in isset() does not exist" for
  dynamic array constants
- Covers both global constants (`define('FOO', [])`) and class constants
  (`const FOO = []`) listed in `dynamicConstantNames`
…ynamic constants

Tests the actual user scenario: define('FOO', true) and define('DYNAMICARRAY', [])
with dynamicConstantNames should not report "Offset on array{} in isset() does not
exist" or "If condition is always true".

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The method operates on any constant value type, not specifically
dynamic constants. The previous name was misleading about its scope.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Narrow the fallback check from any constant array value to
specifically an empty ConstantArrayType using equals(). This is
safer against future changes to ConstantArrayType::generalize()
and documents the exact edge case being handled.

Also restores the generalizeConstantType name since the method
generalizes any constant type, not just dynamic ones.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The method is only called in contexts where the constant is in
dynamicConstantNames, so the "Dynamic" qualifier is appropriate.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@staabm staabm force-pushed the create-pull-request/patch-5esgl9v branch from 5cddc72 to d4a86dd Compare May 9, 2026 09:45
@VincentLanglet VincentLanglet merged commit c937b6d into phpstan:2.1.x May 9, 2026
658 of 659 checks passed
@VincentLanglet VincentLanglet deleted the create-pull-request/patch-5esgl9v branch May 9, 2026 14:13
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.

3 participants