Mark class_exists, interface_exists, trait_exists, and enum_exists as having no side effects in function metadata#5607
Conversation
…ists` as having no side effects in function metadata - Add `class_exists`, `interface_exists`, `trait_exists`, and `enum_exists` to `resources/functionMetadata.php` with `hasSideEffects => false`, matching the existing `function_exists` entry - When `rememberPossiblyImpureFunctionValues` was false, the type narrowing from these functions was blocked in `TypeSpecifier::createForExpr()` because they lacked metadata and defaulted to `hasSideEffects = maybe` - Update `AutoloadSourceLocatorTest` to use the return value of `class_exists()` to avoid the now-triggered `function.resultUnused` rule
VincentLanglet
left a comment
There was a problem hiding this comment.
This is semantic wrong because those function depends on other parameters.
One *_exists function could return false and then true later.
But there is something specific to them, as soon as they return true, it stay true. Once a class is loaded in PHP, it cannot be unloaded within the same process.
…pe narrowing Instead of incorrectly marking class_exists, interface_exists, trait_exists, and enum_exists as having no side effects (they trigger autoloading), wrap the FuncCall in AlwaysRememberedExpr in ClassExistsFunctionTypeSpecifyingExtension. This bypasses the rememberPossiblyImpureFunctionValues check in TypeSpecifier::createForExpr() while keeping the correct hasSideEffects semantics. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Done. Here's a summary of what changed: Problem: The reviewer correctly pointed out that Root cause: When Fix: Instead of lying about side effects, wrap the Changes:
|
VincentLanglet
left a comment
There was a problem hiding this comment.
Add test to ensure that a false result for class_exists is still not always remembered when we don't remember possibly impure function.
When rememberPossiblyImpureFunctionValues is false, a false result from class_exists() must not be permanently cached — a subsequent class_exists() call for the same class should still be able to return true (since the class could be autoloaded between calls). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Pushed. Here's what I added to address the review:
Both scenarios run with |
|
|
||
| use PHPStan\Testing\PHPStanTestCase; | ||
|
|
||
| class Bug8579Test extends PHPStanTestCase |
There was a problem hiding this comment.
why do we need such new test? can't we test it with a regular rule-test?
There was a problem hiding this comment.
Done. Here's what I changed to address staabm's review:
-
Converted
Bug8579Testto a standard rule test: CreatedInstantiationDoNotRememberPossiblyImpureValuesRuleTestextendingRuleTestCase<InstantiationRule>withrememberPossiblyImpureFunctionValues: falseconfig, following the same pattern asMatchExpressionDoNotRememberPossiblyImpureValuesRuleTest. -
Converted the false-not-remembered test to assertType: Added two
assertType('bool', class_exists(...))assertions to the existingdo-not-remember-possibly-impure-function-values.phptest data file, verifying that aclass_existsfalse result is not permanently remembered after branch merges. -
Deleted the old custom test: Removed
Bug8579Test.phpand its data files (bug-8579.php,bug-8579-false-not-remembered.php) from the Analyser test directory.
All 11997 tests pass and make phpstan reports no errors.
| // The false result must not be permanently remembered, | ||
| // so a subsequent class_exists check can still return true |
There was a problem hiding this comment.
can this be tested via assertType() in a nsrt test?
Move the Bug8579 regression test from a custom Analyser test to a standard RuleTestCase for InstantiationRule with rememberPossiblyImpureFunctionValues: false config. Convert the class_exists false-not-remembered test to assertType assertions in the existing DoNotRememberPossiblyImpureFunctionValues test data file. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| // class_exists returned false here, but we don't exit | ||
| } | ||
|
|
||
| assertType('bool', class_exists('Bug8579FalseNotRememberedA')); |
There was a problem hiding this comment.
Also add assert inside the if
There was a problem hiding this comment.
Done. Added assertType('bool', class_exists(...)) assertions inside the if branches as requested by VincentLanglet. The expected type is bool (not true/false) because with rememberPossiblyImpureFunctionValues: false, function call results are not narrowed within branches — only the class existence narrowing (used by instantiation checks) is preserved via AlwaysRememberedExpr. All 12000 tests pass and make phpstan reports no errors.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
When
rememberPossiblyImpureFunctionValuesis set tofalse, PHPStan incorrectly reports "Instantiated class X not found" even when the instantiation is guarded byclass_exists(). The same issue affectsinterface_exists,trait_exists, andenum_exists.Changes
class_exists,interface_exists,trait_exists, andenum_existstoresources/functionMetadata.phpwith'hasSideEffects' => falsetests/PHPStan/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocatorTest.phpto use the return value ofclass_exists()(avoids the now-correctly-triggeredfunction.resultUnusedwarning)tests/PHPStan/Analyser/Bug8579Test.phpwith test dataRoot cause
TypeSpecifier::createForExpr()blocks type narrowing for function calls whenrememberPossiblyImpureFunctionValuesisfalseandhasSideEffectsis not definitivelyno. Sinceclass_existsand its siblings were missing fromresources/functionMetadata.php, they defaulted tohasSideEffects = maybe(set inNativeFunctionReflectionProvider::findFunctionReflection()). This caused the condition at line 2481 ofTypeSpecifier.php(!$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no()) to evaluate totrue, returning emptySpecifiedTypesand preventing the truthy/falsey narrowing from being stored in the scope.The
function_existsfunction was already correctly marked ashasSideEffects => falsein the metadata and was unaffected. The fix brings the other*_existsfunctions in line with it.Test
tests/PHPStan/Analyser/Bug8579Test.php— runs full analysis withrememberPossiblyImpureFunctionValues: falseconfig and verifies thatclass_exists()guards prevent the "class not found" errorinterface_exists,trait_exists,enum_exists) were verified via direct analysis and confirmed workingFixes phpstan/phpstan#8579