Generalize dynamicConstantNames constants whose type cannot be widened by generalize() to mixed#5457
Open
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Open
Conversation
…ned by `generalize()` to `mixed` - NullType::generalize() returns $this because null has no broader type family, so dynamicConstantNames had no effect on null-valued constants - After calling generalize(), check if the result is still a constant value; if so, return MixedType instead - Fix applies to both resolveConstantType (global constants) and resolveClassConstantType (class constants) - Also fixes the analogous case for empty ConstantArrayType ([]), whose generalize() also returns $this
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When a constant with value
null(or empty array[]) was listed indynamicConstantNames, PHPStan still treated it as having its literal type becauseNullType::generalize()returns$this— there is no broader "null family" type to generalize to. This madedynamicConstantNamesineffective for null-valued constants.Changes
ConstantResolver::resolveConstantType()insrc/Analyser/ConstantResolver.phpto check whethergeneralize()actually widened the type; if the result still reportsisConstantValue()->yes(), returnMixedTypeinsteadConstantResolver::resolveClassConstantType()for class constantstests/PHPStan/Analyser/data/dynamic-constant.phpdynamicConstantNamesentries intests/PHPStan/Analyser/dynamic-constants.neonAnalogous cases probed
true/falseconstants: Already generalize correctly tobool— confirmed with tests (already passing)ConstantArrayType([]):generalize()returns$thisfor empty arrays — was broken, now fixed by the same checkEnumCaseObjectType: ReportsisConstantValue()->no(), so not affectedVoidType: ReportsisConstantValue()->no(), so not affectedarray_key_exists): UsesTypeStringResolverto resolve user-specified type strings — not affected$nativeType): Returns the declared native type (PHP 8.3+) — not affectedRoot cause
NullType::generalize()returns$thisbecause null is a singleton type with no broader family. Similarly,ConstantArrayType::generalize()returns$thisfor empty arrays. WhendynamicConstantNamescalledgeneralize()on these types, the result was unchanged, so the constant was still treated as having its literal value. The fix detects this case by checking whether the generalized type is still a constant value and falls back tomixed.Test
assertType('mixed', DynamicConstantClass::DYNAMIC_NULL_CONSTANT)— null-valued class constantassertType('mixed', GLOBAL_DYNAMIC_NULL_CONSTANT)— null-valued global constantassertType('mixed', DynamicConstantClass::DYNAMIC_EMPTY_ARRAY_CONSTANT)— empty-array class constantassertType('bool', DynamicConstantClass::DYNAMIC_TRUE_CONSTANT)andassertType('bool', DynamicConstantClass::DYNAMIC_FALSE_CONSTANT)— confirm boolean constants still generalize correctlyFixes phpstan/phpstan#9218