From 877eb3ab66ed13f81eb1bc73bdbaa93d816ff466 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 11 Apr 2025 10:32:51 +0200 Subject: [PATCH 01/23] Remember narrowed types from the constructor when analysing other methods --- src/Analyser/MutatingScope.php | 48 +++++++++++++++++++ src/Analyser/NodeScopeResolver.php | 17 ++++++- .../Rules/Constants/ConstantRuleTest.php | 16 +++++++ .../data/remembered-constructor-scope.php | 45 +++++++++++++++++ 4 files changed, 125 insertions(+), 1 deletion(-) create mode 100644 tests/PHPStan/Rules/Constants/data/remembered-constructor-scope.php diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 2d23a40665..02b98fcc94 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -294,6 +294,54 @@ public function enterDeclareStrictTypes(): self ); } + public function rememberConstructorScope(): self + { + $expressionTypes = []; + foreach ($this->expressionTypes as $exprString => $expressionTypeHolder) { + $expr = $expressionTypeHolder->getExpr(); + if (!$expr instanceof ConstFetch) { + continue; + } + $expressionTypes[$exprString] = $expressionTypeHolder; + } + + $nativeExpressionTypes = []; + foreach ($this->nativeExpressionTypes as $exprString => $expressionTypeHolder) { + $expr = $expressionTypeHolder->getExpr(); + if (!$expr instanceof ConstFetch) { + continue; + } + + $nativeExpressionTypes[$exprString] = $expressionTypeHolder; + } + + if (array_key_exists('$this', $this->expressionTypes)) { + $expressionTypes['$this'] = $this->expressionTypes['$this']; + } + if (array_key_exists('$this', $this->nativeExpressionTypes)) { + $nativeExpressionTypes['$this'] = $this->nativeExpressionTypes['$this']; + } + + return $this->scopeFactory->create( + $this->context, + $this->isDeclareStrictTypes(), + $this->getFunction(), + $this->getNamespace(), + $expressionTypes, + $nativeExpressionTypes, + $this->conditionalExpressions, + $this->inClosureBindScopeClasses, + $this->anonymousFunctionReflection, + $this->inFirstLevelStatement, + [], + [], + $this->inFunctionCallsStack, + $this->afterExtractCall, + $this->parentScope, + $this->nativeTypesPromoted, + ); + } + /** @api */ public function isInClass(): bool { diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 668c3aa9bd..fa088e8fd0 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -218,6 +218,7 @@ use function str_starts_with; use function strtolower; use function trim; +use function usort; use const PHP_VERSION_ID; use const SORT_NUMERIC; @@ -791,6 +792,10 @@ private function processStmtNode( $classReflection, $methodReflection, ), $methodScope); + + if ($isConstructor) { + $scope = $statementResult->getScope()->rememberConstructorScope(); + } } } elseif ($stmt instanceof Echo_) { $hasYield = false; @@ -925,7 +930,17 @@ private function processStmtNode( $classStatementsGatherer = new ClassStatementsGatherer($classReflection, $nodeCallback); $this->processAttributeGroups($stmt, $stmt->attrGroups, $classScope, $classStatementsGatherer); - $this->processStmtNodes($stmt, $stmt->stmts, $classScope, $classStatementsGatherer, $context); + // analyze static methods first, constructor next and instance methods last so we can carry over the scope + $classLikeStatements = $stmt->stmts; + usort($classLikeStatements, static function ($a, $b) { + if (!$a instanceof Node\Stmt\ClassMethod || !$b instanceof Node\Stmt\ClassMethod) { + return 0; + } + + return [!$a->isStatic(), $a->name->toLowerString() !== '__construct'] <=> [!$b->isStatic(), $b->name->toLowerString() !== '__construct']; + }); + + $this->processStmtNodes($stmt, $classLikeStatements, $classScope, $classStatementsGatherer, $context); $nodeCallback(new ClassPropertiesNode($stmt, $this->readWritePropertiesExtensionProvider, $classStatementsGatherer->getProperties(), $classStatementsGatherer->getPropertyUsages(), $classStatementsGatherer->getMethodCalls(), $classStatementsGatherer->getReturnStatementsNodes(), $classStatementsGatherer->getPropertyAssigns(), $classReflection), $classScope); $nodeCallback(new ClassMethodsNode($stmt, $classStatementsGatherer->getMethods(), $classStatementsGatherer->getMethodCalls(), $classReflection), $classScope); $nodeCallback(new ClassConstantsNode($stmt, $classStatementsGatherer->getConstants(), $classStatementsGatherer->getConstantFetches(), $classReflection), $classScope); diff --git a/tests/PHPStan/Rules/Constants/ConstantRuleTest.php b/tests/PHPStan/Rules/Constants/ConstantRuleTest.php index d30fbcec67..0b8a7ea695 100644 --- a/tests/PHPStan/Rules/Constants/ConstantRuleTest.php +++ b/tests/PHPStan/Rules/Constants/ConstantRuleTest.php @@ -80,4 +80,20 @@ public function testDefinedScopeMerge(): void ]); } + public function testRememberedConstructorScope(): void + { + $this->analyse([__DIR__ . '/data/remembered-constructor-scope.php'], [ + [ + 'Constant REMEMBERED_FOO not found.', + 23, + 'Learn more at https://phpstan.org/user-guide/discovering-symbols', + ], + [ + 'Constant REMEMBERED_FOO not found.', + 38, + 'Learn more at https://phpstan.org/user-guide/discovering-symbols', + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Constants/data/remembered-constructor-scope.php b/tests/PHPStan/Rules/Constants/data/remembered-constructor-scope.php new file mode 100644 index 0000000000..83c03759c7 --- /dev/null +++ b/tests/PHPStan/Rules/Constants/data/remembered-constructor-scope.php @@ -0,0 +1,45 @@ + Date: Fri, 11 Apr 2025 11:39:06 +0200 Subject: [PATCH 02/23] test 2nd class does not inherit previous __construct() scope --- tests/PHPStan/Rules/Constants/ConstantRuleTest.php | 5 +++++ .../Rules/Constants/data/remembered-constructor-scope.php | 8 ++++++++ 2 files changed, 13 insertions(+) diff --git a/tests/PHPStan/Rules/Constants/ConstantRuleTest.php b/tests/PHPStan/Rules/Constants/ConstantRuleTest.php index 0b8a7ea695..c2fec624b9 100644 --- a/tests/PHPStan/Rules/Constants/ConstantRuleTest.php +++ b/tests/PHPStan/Rules/Constants/ConstantRuleTest.php @@ -93,6 +93,11 @@ public function testRememberedConstructorScope(): void 38, 'Learn more at https://phpstan.org/user-guide/discovering-symbols', ], + [ + 'Constant REMEMBERED_FOO not found.', + 51, + 'Learn more at https://phpstan.org/user-guide/discovering-symbols', + ], ]); } diff --git a/tests/PHPStan/Rules/Constants/data/remembered-constructor-scope.php b/tests/PHPStan/Rules/Constants/data/remembered-constructor-scope.php index 83c03759c7..8a40df5d1f 100644 --- a/tests/PHPStan/Rules/Constants/data/remembered-constructor-scope.php +++ b/tests/PHPStan/Rules/Constants/data/remembered-constructor-scope.php @@ -43,3 +43,11 @@ public function returnFoo2(): string return REMEMBERED_FOO; } } + + +class AnotherClass { + public function myFoo(): string + { + return REMEMBERED_FOO; // should error, because this file cannot share the __constructor() scope with HelloWorld-class above. + } +} From 01975bdd409c7f95fbecd594a13b54d9ad508684 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 11 Apr 2025 11:40:44 +0200 Subject: [PATCH 03/23] More tests --- .../PHPStan/Rules/Constants/ConstantRuleTest.php | 5 +++++ .../data/remembered-constructor-scope.php | 16 ++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/tests/PHPStan/Rules/Constants/ConstantRuleTest.php b/tests/PHPStan/Rules/Constants/ConstantRuleTest.php index c2fec624b9..4526c98248 100644 --- a/tests/PHPStan/Rules/Constants/ConstantRuleTest.php +++ b/tests/PHPStan/Rules/Constants/ConstantRuleTest.php @@ -98,6 +98,11 @@ public function testRememberedConstructorScope(): void 51, 'Learn more at https://phpstan.org/user-guide/discovering-symbols', ], + [ + 'Constant REMEMBERED_FOO not found.', + 65, + 'Learn more at https://phpstan.org/user-guide/discovering-symbols', + ], ]); } diff --git a/tests/PHPStan/Rules/Constants/data/remembered-constructor-scope.php b/tests/PHPStan/Rules/Constants/data/remembered-constructor-scope.php index 8a40df5d1f..468cd7d58f 100644 --- a/tests/PHPStan/Rules/Constants/data/remembered-constructor-scope.php +++ b/tests/PHPStan/Rules/Constants/data/remembered-constructor-scope.php @@ -51,3 +51,19 @@ public function myFoo(): string return REMEMBERED_FOO; // should error, because this file cannot share the __constructor() scope with HelloWorld-class above. } } + +class AnotherClassWithOwnConstructor { + public function __construct() + { + if (!defined('ANOTHER_REMEMBERED_FOO')) { + throw new LogicException(); + } + } + + public function myFoo(): void + { + echo REMEMBERED_FOO; // should error, because this file cannot share the __constructor() scope with HelloWorld-class above. + + echo ANOTHER_REMEMBERED_FOO; + } +} From 4842d2b750455c60f8b52fb95332fa7cae690cbd Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 11 Apr 2025 12:32:13 +0200 Subject: [PATCH 04/23] Remember types of readonly properties --- src/Analyser/MutatingScope.php | 69 +++++++++++-------- ...remember-readonly-constructor-narrowed.php | 40 +++++++++++ 2 files changed, 82 insertions(+), 27 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/remember-readonly-constructor-narrowed.php diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 02b98fcc94..a166c8d6e7 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -294,41 +294,57 @@ public function enterDeclareStrictTypes(): self ); } - public function rememberConstructorScope(): self + /** + * @param array $currentExpressionTypes + * @return array + */ + private function rememberConstructorExpressions(array $currentExpressionTypes): array { $expressionTypes = []; - foreach ($this->expressionTypes as $exprString => $expressionTypeHolder) { + foreach ($currentExpressionTypes as $exprString => $expressionTypeHolder) { $expr = $expressionTypeHolder->getExpr(); - if (!$expr instanceof ConstFetch) { - continue; - } - $expressionTypes[$exprString] = $expressionTypeHolder; - } + if ($expr instanceof PropertyFetch) { + if ( + !$expr->name instanceof Node\Identifier + || !$expr->var instanceof Variable + || $expr->var->name !== 'this' + || !$this->phpVersion->supportsReadOnlyProperties() + ) { + continue; + } - $nativeExpressionTypes = []; - foreach ($this->nativeExpressionTypes as $exprString => $expressionTypeHolder) { - $expr = $expressionTypeHolder->getExpr(); - if (!$expr instanceof ConstFetch) { + $propertyReflection = $this->propertyReflectionFinder->findPropertyReflectionFromNode($expr, $this); + if ($propertyReflection === null) { + continue; + } + + $nativePropertyReflection = $propertyReflection->getNativeReflection(); + if ($nativePropertyReflection === null || !$nativePropertyReflection->isReadOnly()) { + continue; + } + } elseif (!$expr instanceof ConstFetch) { continue; } - $nativeExpressionTypes[$exprString] = $expressionTypeHolder; + $expressionTypes[$exprString] = $expressionTypeHolder; } - if (array_key_exists('$this', $this->expressionTypes)) { - $expressionTypes['$this'] = $this->expressionTypes['$this']; - } - if (array_key_exists('$this', $this->nativeExpressionTypes)) { - $nativeExpressionTypes['$this'] = $this->nativeExpressionTypes['$this']; + if (array_key_exists('$this', $currentExpressionTypes)) { + $expressionTypes['$this'] = $currentExpressionTypes['$this']; } + return $expressionTypes; + } + + public function rememberConstructorScope(): self + { return $this->scopeFactory->create( $this->context, $this->isDeclareStrictTypes(), $this->getFunction(), $this->getNamespace(), - $expressionTypes, - $nativeExpressionTypes, + $this->rememberConstructorExpressions($this->expressionTypes), + $this->rememberConstructorExpressions($this->nativeExpressionTypes), $this->conditionalExpressions, $this->inClosureBindScopeClasses, $this->anonymousFunctionReflection, @@ -3334,7 +3350,7 @@ public function enterFunction( private function enterFunctionLike( PhpFunctionFromParserNodeReflection $functionReflection, - bool $preserveThis, + bool $preserveConstructorScope, ): self { $parametersByName = []; @@ -3346,6 +3362,12 @@ private function enterFunctionLike( $expressionTypes = []; $nativeExpressionTypes = []; $conditionalTypes = []; + + if ($preserveConstructorScope) { + $expressionTypes = $this->rememberConstructorExpressions($this->expressionTypes); + $nativeExpressionTypes = $this->rememberConstructorExpressions($this->nativeExpressionTypes); + } + foreach ($functionReflection->getParameters() as $parameter) { $parameterType = $parameter->getType(); @@ -3396,13 +3418,6 @@ private function enterFunctionLike( $nativeExpressionTypes[$parameterOriginalValueExprString] = ExpressionTypeHolder::createYes($parameterOriginalValueExpr, $nativeParameterType); } - if ($preserveThis && array_key_exists('$this', $this->expressionTypes)) { - $expressionTypes['$this'] = $this->expressionTypes['$this']; - } - if ($preserveThis && array_key_exists('$this', $this->nativeExpressionTypes)) { - $nativeExpressionTypes['$this'] = $this->nativeExpressionTypes['$this']; - } - return $this->scopeFactory->create( $this->context, $this->isDeclareStrictTypes(), diff --git a/tests/PHPStan/Analyser/nsrt/remember-readonly-constructor-narrowed.php b/tests/PHPStan/Analyser/nsrt/remember-readonly-constructor-narrowed.php new file mode 100644 index 0000000000..ff2f34f04b --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/remember-readonly-constructor-narrowed.php @@ -0,0 +1,40 @@ += 8.1 + +namespace RememberReadOnlyConstructor; + +use function PHPStan\Testing\assertType; + +class HelloWorldReadonly { + private readonly int $i; + + public function __construct() + { + if (rand(0,1)) { + $this->i = 4; + } else { + $this->i = 10; + } + } + + public function doFoo() { + assertType('4|10', $this->i); + } +} + + +class HelloWorldRegular { + private int $i; + + public function __construct() + { + if (rand(0,1)) { + $this->i = 4; + } else { + $this->i = 10; + } + } + + public function doFoo() { + assertType('int', $this->i); + } +} From 6344cc135d0f2d657116f3d6fba436aa7a57a17c Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 11 Apr 2025 13:37:52 +0200 Subject: [PATCH 05/23] test property hooks --- src/Analyser/MutatingScope.php | 2 +- src/Analyser/NodeScopeResolver.php | 9 ++++- ...er-readonly-constructor-narrowed-hooks.php | 35 +++++++++++++++++++ 3 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/remember-readonly-constructor-narrowed-hooks.php diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index a166c8d6e7..f15c92b7f1 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -3350,7 +3350,7 @@ public function enterFunction( private function enterFunctionLike( PhpFunctionFromParserNodeReflection $functionReflection, - bool $preserveConstructorScope, + bool $preserveConstructorScope, ): self { $parametersByName = []; diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index fa088e8fd0..945e0b898b 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -930,9 +930,16 @@ private function processStmtNode( $classStatementsGatherer = new ClassStatementsGatherer($classReflection, $nodeCallback); $this->processAttributeGroups($stmt, $stmt->attrGroups, $classScope, $classStatementsGatherer); - // analyze static methods first, constructor next and instance methods last so we can carry over the scope + // analyze static methods first; constructor next; instance methods and property hooks last so we can carry over the scope $classLikeStatements = $stmt->stmts; usort($classLikeStatements, static function ($a, $b) { + if ($a instanceof Node\Stmt\Property) { + return 1; + } + if ($b instanceof Node\Stmt\Property) { + return -1; + } + if (!$a instanceof Node\Stmt\ClassMethod || !$b instanceof Node\Stmt\ClassMethod) { return 0; } diff --git a/tests/PHPStan/Analyser/nsrt/remember-readonly-constructor-narrowed-hooks.php b/tests/PHPStan/Analyser/nsrt/remember-readonly-constructor-narrowed-hooks.php new file mode 100644 index 0000000000..8f03858767 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/remember-readonly-constructor-narrowed-hooks.php @@ -0,0 +1,35 @@ += 8.4 + +namespace RememberReadOnlyConstructorInPropertyHookBodies; + +use function PHPStan\Testing\assertType; + +class User +{ + public string $name { + get { + assertType('1|2', $this->type); + return $this->name ; + } + set { + if (strlen($value) === 0) { + throw new ValueError("Name must be non-empty"); + } + assertType('1|2', $this->type); + $this->name = $value; + } + } + + private readonly int $type; + + public function __construct( + string $name + ) { + $this->name = $name; + if (rand(0,1)) { + $this->type = 1; + } else { + $this->type = 2; + } + } +} From e4bc05b1e6bed6866abe0b00c848e66d6c009a2e Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 11 Apr 2025 13:54:03 +0200 Subject: [PATCH 06/23] Remember class_exists() from constructor --- src/Analyser/MutatingScope.php | 9 ++++++- .../ExistingClassInInstanceOfRuleTest.php | 5 ++++ ...remember-class-exists-from-constructor.php | 24 +++++++++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 tests/PHPStan/Rules/Classes/data/remember-class-exists-from-constructor.php diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index f15c92b7f1..84576d5317 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -303,7 +303,14 @@ private function rememberConstructorExpressions(array $currentExpressionTypes): $expressionTypes = []; foreach ($currentExpressionTypes as $exprString => $expressionTypeHolder) { $expr = $expressionTypeHolder->getExpr(); - if ($expr instanceof PropertyFetch) { + if ($expr instanceof FuncCall) { + if ( + !$expr->name instanceof Name + || !in_array($expr->name->name, ['class_exists'], true) + ) { + continue; + } + } elseif ($expr instanceof PropertyFetch) { if ( !$expr->name instanceof Node\Identifier || !$expr->var instanceof Variable diff --git a/tests/PHPStan/Rules/Classes/ExistingClassInInstanceOfRuleTest.php b/tests/PHPStan/Rules/Classes/ExistingClassInInstanceOfRuleTest.php index 9841cd6ed4..c9a4a37452 100644 --- a/tests/PHPStan/Rules/Classes/ExistingClassInInstanceOfRuleTest.php +++ b/tests/PHPStan/Rules/Classes/ExistingClassInInstanceOfRuleTest.php @@ -81,4 +81,9 @@ public function testBug7720(): void ]); } + public function testRememberClassExistsFromConstructor(): void + { + $this->analyse([__DIR__ . '/data/remember-class-exists-from-constructor.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Classes/data/remember-class-exists-from-constructor.php b/tests/PHPStan/Rules/Classes/data/remember-class-exists-from-constructor.php new file mode 100644 index 0000000000..d212ae94e6 --- /dev/null +++ b/tests/PHPStan/Rules/Classes/data/remember-class-exists-from-constructor.php @@ -0,0 +1,24 @@ += 7.4 + +namespace RememberClassExistsFromConstructor; + +use SomeUnknownClass; + +class User +{ + public function __construct( + ) { + if (!class_exists('SomeUnknownClass')) { + throw new \LogicException(); + } + } + + public function doFoo($m): bool + { + if ($m instanceof SomeUnknownClass) { + return false; + } + return true; + } + +} From baa78f3621dfa06ef5695845f6b4e5ee3c4f956d Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 11 Apr 2025 13:57:37 +0200 Subject: [PATCH 07/23] Remember function_exists() from constructor --- src/Analyser/MutatingScope.php | 2 +- .../CallToNonExistentFunctionRuleTest.php | 5 +++++ ...ember-function-exists-from-constructor.php | 19 +++++++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 tests/PHPStan/Rules/Functions/data/remember-function-exists-from-constructor.php diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 84576d5317..bc557bff65 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -306,7 +306,7 @@ private function rememberConstructorExpressions(array $currentExpressionTypes): if ($expr instanceof FuncCall) { if ( !$expr->name instanceof Name - || !in_array($expr->name->name, ['class_exists'], true) + || !in_array($expr->name->name, ['class_exists', 'function_exists'], true) ) { continue; } diff --git a/tests/PHPStan/Rules/Functions/CallToNonExistentFunctionRuleTest.php b/tests/PHPStan/Rules/Functions/CallToNonExistentFunctionRuleTest.php index 4344c32b14..4cc6ce57d5 100644 --- a/tests/PHPStan/Rules/Functions/CallToNonExistentFunctionRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallToNonExistentFunctionRuleTest.php @@ -258,4 +258,9 @@ public function testBug10003(): void ]); } + public function testRememberFunctionExistsFromConstructor(): void + { + $this->analyse([__DIR__ . '/data/remember-function-exists-from-constructor.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Functions/data/remember-function-exists-from-constructor.php b/tests/PHPStan/Rules/Functions/data/remember-function-exists-from-constructor.php new file mode 100644 index 0000000000..f95f0945b2 --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/remember-function-exists-from-constructor.php @@ -0,0 +1,19 @@ += 7.4 + +namespace RememberFunctionExistsFromConstructor; + +class User +{ + public function __construct( + ) { + if (!function_exists('some_unknown_function')) { + throw new \LogicException(); + } + } + + public function doFoo(): void + { + some_unknown_function(); + } + +} From 8c9fa55b6a84c81bdb13a8a9925fb8b8750119e7 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 11 Apr 2025 13:59:15 +0200 Subject: [PATCH 08/23] Another test --- .../CallToNonExistentFunctionRuleTest.php | 8 +++++++- ...remember-function-exists-from-constructor.php | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/tests/PHPStan/Rules/Functions/CallToNonExistentFunctionRuleTest.php b/tests/PHPStan/Rules/Functions/CallToNonExistentFunctionRuleTest.php index 4cc6ce57d5..8a57461c65 100644 --- a/tests/PHPStan/Rules/Functions/CallToNonExistentFunctionRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallToNonExistentFunctionRuleTest.php @@ -260,7 +260,13 @@ public function testBug10003(): void public function testRememberFunctionExistsFromConstructor(): void { - $this->analyse([__DIR__ . '/data/remember-function-exists-from-constructor.php'], []); + $this->analyse([__DIR__ . '/data/remember-function-exists-from-constructor.php'], [ + [ + 'Function another_unknown_function not found.', + 32, + 'Learn more at https://phpstan.org/user-guide/discovering-symbols', + ] + ]); } } diff --git a/tests/PHPStan/Rules/Functions/data/remember-function-exists-from-constructor.php b/tests/PHPStan/Rules/Functions/data/remember-function-exists-from-constructor.php index f95f0945b2..1d8640a9ae 100644 --- a/tests/PHPStan/Rules/Functions/data/remember-function-exists-from-constructor.php +++ b/tests/PHPStan/Rules/Functions/data/remember-function-exists-from-constructor.php @@ -17,3 +17,19 @@ public function doFoo(): void } } + +class FooUser +{ + public function __construct( + ) { + if (!function_exists('another_unknown_function')) { + echo 'Function another_unknown_function does not exist'; + } + } + + public function doFoo(): void + { + another_unknown_function(); + } + +} From 21bfc391017eaeef1fdb1d5ec092f80fe73d530a Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 11 Apr 2025 16:43:50 +0200 Subject: [PATCH 09/23] Added bleeding-edge feature flag --- conf/bleedingEdge.neon | 1 + conf/config.neon | 2 ++ conf/parametersSchema.neon | 1 + src/Analyser/NodeScopeResolver.php | 31 ++++++++++--------- src/Testing/RuleTestCase.php | 6 ++++ src/Testing/TypeInferenceTestCase.php | 1 + tests/PHPStan/Analyser/AnalyserTest.php | 1 + .../ExistingClassInInstanceOfRuleTest.php | 5 +++ .../Rules/Constants/ConstantRuleTest.php | 5 +++ .../CallToNonExistentFunctionRuleTest.php | 7 ++++- 10 files changed, 45 insertions(+), 15 deletions(-) diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index 76dd0d8904..72bd9f7b3e 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -5,3 +5,4 @@ parameters: skipCheckGenericClasses!: [] stricterFunctionMap: true reportPreciseLineForUnusedFunctionParameter: true + narrowMethodScopeFromConstructor: true diff --git a/conf/config.neon b/conf/config.neon index 555e7ec53c..28dfc2f2e2 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -26,6 +26,7 @@ parameters: skipCheckGenericClasses: [] stricterFunctionMap: false reportPreciseLineForUnusedFunctionParameter: false + narrowMethodScopeFromConstructor: false fileExtensions: - php checkAdvancedIsset: false @@ -495,6 +496,7 @@ services: implicitThrows: %exceptions.implicitThrows% treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain% universalObjectCratesClasses: %universalObjectCratesClasses% + narrowMethodScopeFromConstructor: %featureToggles.narrowMethodScopeFromConstructor% - class: PHPStan\Analyser\ConstantResolver diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index 69fc9380b9..a214b270a6 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -32,6 +32,7 @@ parametersSchema: skipCheckGenericClasses: listOf(string()), stricterFunctionMap: bool() reportPreciseLineForUnusedFunctionParameter: bool() + narrowMethodScopeFromConstructor: bool() ]) fileExtensions: listOf(string()) checkAdvancedIsset: bool() diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 945e0b898b..3e4446620c 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -272,6 +272,7 @@ public function __construct( private readonly array $universalObjectCratesClasses, private readonly bool $implicitThrows, private readonly bool $treatPhpDocTypesAsCertain, + private readonly bool $narrowMethodScopeFromConstructor, ) { $earlyTerminatingMethodNames = []; @@ -793,7 +794,7 @@ private function processStmtNode( $methodReflection, ), $methodScope); - if ($isConstructor) { + if ($isConstructor && $this->narrowMethodScopeFromConstructor) { $scope = $statementResult->getScope()->rememberConstructorScope(); } } @@ -930,22 +931,24 @@ private function processStmtNode( $classStatementsGatherer = new ClassStatementsGatherer($classReflection, $nodeCallback); $this->processAttributeGroups($stmt, $stmt->attrGroups, $classScope, $classStatementsGatherer); - // analyze static methods first; constructor next; instance methods and property hooks last so we can carry over the scope $classLikeStatements = $stmt->stmts; - usort($classLikeStatements, static function ($a, $b) { - if ($a instanceof Node\Stmt\Property) { - return 1; - } - if ($b instanceof Node\Stmt\Property) { - return -1; - } + if ($this->narrowMethodScopeFromConstructor) { + // analyze static methods first; constructor next; instance methods and property hooks last so we can carry over the scope + usort($classLikeStatements, static function ($a, $b) { + if ($a instanceof Node\Stmt\Property) { + return 1; + } + if ($b instanceof Node\Stmt\Property) { + return -1; + } - if (!$a instanceof Node\Stmt\ClassMethod || !$b instanceof Node\Stmt\ClassMethod) { - return 0; - } + if (!$a instanceof Node\Stmt\ClassMethod || !$b instanceof Node\Stmt\ClassMethod) { + return 0; + } - return [!$a->isStatic(), $a->name->toLowerString() !== '__construct'] <=> [!$b->isStatic(), $b->name->toLowerString() !== '__construct']; - }); + return [!$a->isStatic(), $a->name->toLowerString() !== '__construct'] <=> [!$b->isStatic(), $b->name->toLowerString() !== '__construct']; + }); + } $this->processStmtNodes($stmt, $classLikeStatements, $classScope, $classStatementsGatherer, $context); $nodeCallback(new ClassPropertiesNode($stmt, $this->readWritePropertiesExtensionProvider, $classStatementsGatherer->getProperties(), $classStatementsGatherer->getPropertyUsages(), $classStatementsGatherer->getMethodCalls(), $classStatementsGatherer->getReturnStatementsNodes(), $classStatementsGatherer->getPropertyAssigns(), $classReflection), $classScope); diff --git a/src/Testing/RuleTestCase.php b/src/Testing/RuleTestCase.php index cd0f52534e..e1e3cdb200 100644 --- a/src/Testing/RuleTestCase.php +++ b/src/Testing/RuleTestCase.php @@ -110,6 +110,7 @@ private function getAnalyser(DirectRuleRegistry $ruleRegistry): Analyser self::getContainer()->getParameter('universalObjectCratesClasses'), self::getContainer()->getParameter('exceptions')['implicitThrows'], $this->shouldTreatPhpDocTypesAsCertain(), + $this->shouldNarrowMethodScopeFromConstructor(), ); $fileAnalyser = new FileAnalyser( $this->createScopeFactory($reflectionProvider, $typeSpecifier), @@ -261,6 +262,11 @@ protected function shouldFailOnPhpErrors(): bool return true; } + protected function shouldNarrowMethodScopeFromConstructor(): bool + { + return false; + } + public static function getAdditionalConfigFiles(): array { return [ diff --git a/src/Testing/TypeInferenceTestCase.php b/src/Testing/TypeInferenceTestCase.php index dc3e884c88..f693c1fe36 100644 --- a/src/Testing/TypeInferenceTestCase.php +++ b/src/Testing/TypeInferenceTestCase.php @@ -90,6 +90,7 @@ public static function processFile( self::getContainer()->getParameter('universalObjectCratesClasses'), self::getContainer()->getParameter('exceptions')['implicitThrows'], self::getContainer()->getParameter('treatPhpDocTypesAsCertain'), + true, ); $resolver->setAnalysedFiles(array_map(static fn (string $file): string => $fileHelper->normalizePath($file), array_merge([$file], static::getAdditionalAnalysedFiles()))); diff --git a/tests/PHPStan/Analyser/AnalyserTest.php b/tests/PHPStan/Analyser/AnalyserTest.php index bacd85a967..cb7a6e4a37 100644 --- a/tests/PHPStan/Analyser/AnalyserTest.php +++ b/tests/PHPStan/Analyser/AnalyserTest.php @@ -733,6 +733,7 @@ private function createAnalyser(): Analyser [stdClass::class], true, $this->shouldTreatPhpDocTypesAsCertain(), + false, ); $lexer = new Lexer(); $fileAnalyser = new FileAnalyser( diff --git a/tests/PHPStan/Rules/Classes/ExistingClassInInstanceOfRuleTest.php b/tests/PHPStan/Rules/Classes/ExistingClassInInstanceOfRuleTest.php index c9a4a37452..08f446b8de 100644 --- a/tests/PHPStan/Rules/Classes/ExistingClassInInstanceOfRuleTest.php +++ b/tests/PHPStan/Rules/Classes/ExistingClassInInstanceOfRuleTest.php @@ -29,6 +29,11 @@ protected function getRule(): Rule ); } + public function shouldNarrowMethodScopeFromConstructor(): bool + { + return true; + } + public function testClassDoesNotExist(): void { $this->analyse( diff --git a/tests/PHPStan/Rules/Constants/ConstantRuleTest.php b/tests/PHPStan/Rules/Constants/ConstantRuleTest.php index 4526c98248..2c07c911ce 100644 --- a/tests/PHPStan/Rules/Constants/ConstantRuleTest.php +++ b/tests/PHPStan/Rules/Constants/ConstantRuleTest.php @@ -17,6 +17,11 @@ protected function getRule(): Rule return new ConstantRule(true); } + public function shouldNarrowMethodScopeFromConstructor(): bool + { + return true; + } + public function testConstants(): void { define('FOO_CONSTANT', 'foo'); diff --git a/tests/PHPStan/Rules/Functions/CallToNonExistentFunctionRuleTest.php b/tests/PHPStan/Rules/Functions/CallToNonExistentFunctionRuleTest.php index 8a57461c65..6707068e04 100644 --- a/tests/PHPStan/Rules/Functions/CallToNonExistentFunctionRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallToNonExistentFunctionRuleTest.php @@ -17,6 +17,11 @@ protected function getRule(): Rule return new CallToNonExistentFunctionRule($this->createReflectionProvider(), true, true); } + public function shouldNarrowMethodScopeFromConstructor(): bool + { + return true; + } + public function testEmptyFile(): void { $this->analyse([__DIR__ . '/data/empty.php'], []); @@ -265,7 +270,7 @@ public function testRememberFunctionExistsFromConstructor(): void 'Function another_unknown_function not found.', 32, 'Learn more at https://phpstan.org/user-guide/discovering-symbols', - ] + ], ]); } From 9745da0a6be610606a801fc179973cf8814d6c62 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 11 Apr 2025 16:55:48 +0200 Subject: [PATCH 10/23] test readonly class --- ...remember-readonly-constructor-narrowed.php | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/tests/PHPStan/Analyser/nsrt/remember-readonly-constructor-narrowed.php b/tests/PHPStan/Analyser/nsrt/remember-readonly-constructor-narrowed.php index ff2f34f04b..1c4576c619 100644 --- a/tests/PHPStan/Analyser/nsrt/remember-readonly-constructor-narrowed.php +++ b/tests/PHPStan/Analyser/nsrt/remember-readonly-constructor-narrowed.php @@ -1,10 +1,10 @@ -= 8.1 += 8.2 namespace RememberReadOnlyConstructor; use function PHPStan\Testing\assertType; -class HelloWorldReadonly { +class HelloWorldReadonlyProperty { private readonly int $i; public function __construct() @@ -21,6 +21,23 @@ public function doFoo() { } } +readonly class HelloWorldReadonlyClass { + private int $i; + + public function __construct() + { + if (rand(0,1)) { + $this->i = 4; + } else { + $this->i = 10; + } + } + + public function doFoo() { + assertType('4|10', $this->i); + } +} + class HelloWorldRegular { private int $i; From 1ac6f76b49c66ee71c03862a18ccd6e0251defc9 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 12 Apr 2025 08:37:49 +0200 Subject: [PATCH 11/23] test interface_exists() --- ...remember-class-exists-from-constructor.php | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/tests/PHPStan/Rules/Classes/data/remember-class-exists-from-constructor.php b/tests/PHPStan/Rules/Classes/data/remember-class-exists-from-constructor.php index d212ae94e6..6c7b8cecbf 100644 --- a/tests/PHPStan/Rules/Classes/data/remember-class-exists-from-constructor.php +++ b/tests/PHPStan/Rules/Classes/data/remember-class-exists-from-constructor.php @@ -3,8 +3,9 @@ namespace RememberClassExistsFromConstructor; use SomeUnknownClass; +use SomeUnknownInterface; -class User +class UserWithClass { public function __construct( ) { @@ -22,3 +23,22 @@ public function doFoo($m): bool } } + +class UserWithInterface +{ + public function __construct( + ) { + if (!interface_exists('SomeUnknownInterface')) { + throw new \LogicException(); + } + } + + public function doFoo($m): bool + { + if ($m instanceof SomeUnknownInterface) { + return false; + } + return true; + } + +} From d775762562f6b06e7712fd2a0f5307c183a72a4a Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 12 Apr 2025 08:44:39 +0200 Subject: [PATCH 12/23] test narrowing oft *_exists() functions --- ...remember-readonly-constructor-narrowed.php | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/tests/PHPStan/Analyser/nsrt/remember-readonly-constructor-narrowed.php b/tests/PHPStan/Analyser/nsrt/remember-readonly-constructor-narrowed.php index 1c4576c619..2cdeaf3c7a 100644 --- a/tests/PHPStan/Analyser/nsrt/remember-readonly-constructor-narrowed.php +++ b/tests/PHPStan/Analyser/nsrt/remember-readonly-constructor-narrowed.php @@ -23,18 +23,46 @@ public function doFoo() { readonly class HelloWorldReadonlyClass { private int $i; + private string $class; + private string $interface; + private string $enum; + private string $trait; - public function __construct() + public function __construct(string $class, string $interface, string $enum, string $trait) { if (rand(0,1)) { $this->i = 4; } else { $this->i = 10; } + + if (!class_exists($class)) { + throw new \LogicException(); + } + $this->class = $class; + + if (!interface_exists($interface)) { + throw new \LogicException(); + } + $this->interface = $interface; + + if (!enum_exists($enum)) { + throw new \LogicException(); + } + $this->enum = $enum; + + if (!trait_exists($trait)) { + throw new \LogicException(); + } + $this->trait = $trait; } public function doFoo() { assertType('4|10', $this->i); + assertType('class-string', $this->class); + assertType('class-string', $this->interface); + assertType('class-string', $this->enum); + assertType('class-string', $this->trait); } } From 662804958680bf658ae3da92a8653f12595501ff Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 12 Apr 2025 08:55:00 +0200 Subject: [PATCH 13/23] more precise comment --- .../Rules/Constants/data/remembered-constructor-scope.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/PHPStan/Rules/Constants/data/remembered-constructor-scope.php b/tests/PHPStan/Rules/Constants/data/remembered-constructor-scope.php index 468cd7d58f..22ccae79a9 100644 --- a/tests/PHPStan/Rules/Constants/data/remembered-constructor-scope.php +++ b/tests/PHPStan/Rules/Constants/data/remembered-constructor-scope.php @@ -48,7 +48,7 @@ public function returnFoo2(): string class AnotherClass { public function myFoo(): string { - return REMEMBERED_FOO; // should error, because this file cannot share the __constructor() scope with HelloWorld-class above. + return REMEMBERED_FOO; // should error, because this class should not share the __constructor() scope with HelloWorld-class above. } } @@ -62,7 +62,7 @@ public function __construct() public function myFoo(): void { - echo REMEMBERED_FOO; // should error, because this file cannot share the __constructor() scope with HelloWorld-class above. + echo REMEMBERED_FOO; // should error, because this class should not share the __constructor() scope with HelloWorld-class above. echo ANOTHER_REMEMBERED_FOO; } From a04a5fd99abe11923b9f9ed3a7a5735a9665e7d8 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 12 Apr 2025 09:05:38 +0200 Subject: [PATCH 14/23] test remembers define() from __construct() --- .../Constants/data/remembered-constructor-scope.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/PHPStan/Rules/Constants/data/remembered-constructor-scope.php b/tests/PHPStan/Rules/Constants/data/remembered-constructor-scope.php index 22ccae79a9..5e0d0d256a 100644 --- a/tests/PHPStan/Rules/Constants/data/remembered-constructor-scope.php +++ b/tests/PHPStan/Rules/Constants/data/remembered-constructor-scope.php @@ -67,3 +67,13 @@ public function myFoo(): void echo ANOTHER_REMEMBERED_FOO; } } + +class MyClassWithDefine { + public function __construct() { + define('XYZ', '123'); + } + + public function doFoo(): void { + echo XYZ; + } +} From 3f793d95d026460bde88f637eb96f00857b888b6 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 12 Apr 2025 09:17:00 +0200 Subject: [PATCH 15/23] test more scenarios which should not remember scope --- .../Rules/Constants/ConstantRuleTest.php | 15 +++++++++++++ .../data/remembered-constructor-scope.php | 21 +++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/tests/PHPStan/Rules/Constants/ConstantRuleTest.php b/tests/PHPStan/Rules/Constants/ConstantRuleTest.php index 2c07c911ce..0da9819e08 100644 --- a/tests/PHPStan/Rules/Constants/ConstantRuleTest.php +++ b/tests/PHPStan/Rules/Constants/ConstantRuleTest.php @@ -108,6 +108,21 @@ public function testRememberedConstructorScope(): void 65, 'Learn more at https://phpstan.org/user-guide/discovering-symbols', ], + [ + 'Constant XYZ22 not found.', + 87, + 'Learn more at https://phpstan.org/user-guide/discovering-symbols', + ], + [ + 'Constant XYZ not found.', + 88, + 'Learn more at https://phpstan.org/user-guide/discovering-symbols', + ], + [ + 'Constant XYZ33 not found.', + 98, + 'Learn more at https://phpstan.org/user-guide/discovering-symbols', + ], ]); } diff --git a/tests/PHPStan/Rules/Constants/data/remembered-constructor-scope.php b/tests/PHPStan/Rules/Constants/data/remembered-constructor-scope.php index 5e0d0d256a..7be2b0bf7b 100644 --- a/tests/PHPStan/Rules/Constants/data/remembered-constructor-scope.php +++ b/tests/PHPStan/Rules/Constants/data/remembered-constructor-scope.php @@ -77,3 +77,24 @@ public function doFoo(): void { echo XYZ; } } + +class StaticMethodScopeNotRemembered { + static public function init() { + define('XYZ22', '123'); + } + + public function doFoo(): void { + echo XYZ22; // error because defined in static method + echo XYZ; // error, because should not mix scope of __constructor() scope of different class + } +} + +class InstanceMethodScopeNotRemembered { + public function init() { + define('XYZ33', '123'); + } + + public function doFoo(): void { + echo XYZ33; // error because defined in non-constructor method + } +} From b140ce416955c017dee5d717a0c79b7a2d67db4f Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 12 Apr 2025 09:27:12 +0200 Subject: [PATCH 16/23] enable narrowMethodScopeFromConstructor in AnalyserTest --- tests/PHPStan/Analyser/AnalyserTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/PHPStan/Analyser/AnalyserTest.php b/tests/PHPStan/Analyser/AnalyserTest.php index cb7a6e4a37..78bfdfa4d7 100644 --- a/tests/PHPStan/Analyser/AnalyserTest.php +++ b/tests/PHPStan/Analyser/AnalyserTest.php @@ -733,7 +733,7 @@ private function createAnalyser(): Analyser [stdClass::class], true, $this->shouldTreatPhpDocTypesAsCertain(), - false, + true, ); $lexer = new Lexer(); $fileAnalyser = new FileAnalyser( From 1756f857f0260324c6c1be08d1ae4eabe38da34c Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 12 Apr 2025 09:34:14 +0200 Subject: [PATCH 17/23] test narrowMethodScopeFromConstructor can be disabled --- .../ExistingClassInInstanceOfRuleTest.php | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/tests/PHPStan/Rules/Classes/ExistingClassInInstanceOfRuleTest.php b/tests/PHPStan/Rules/Classes/ExistingClassInInstanceOfRuleTest.php index 08f446b8de..4e026df3a6 100644 --- a/tests/PHPStan/Rules/Classes/ExistingClassInInstanceOfRuleTest.php +++ b/tests/PHPStan/Rules/Classes/ExistingClassInInstanceOfRuleTest.php @@ -15,6 +15,8 @@ class ExistingClassInInstanceOfRuleTest extends RuleTestCase { + private bool $shouldNarrowMethodScopeFromConstructor = true; + protected function getRule(): Rule { $reflectionProvider = $this->createReflectionProvider(); @@ -31,7 +33,7 @@ protected function getRule(): Rule public function shouldNarrowMethodScopeFromConstructor(): bool { - return true; + return $this->shouldNarrowMethodScopeFromConstructor; } public function testClassDoesNotExist(): void @@ -86,6 +88,24 @@ public function testBug7720(): void ]); } + public function testRememberClassExistsFromConstructorDisabled(): void + { + $this->shouldNarrowMethodScopeFromConstructor = false; + + $this->analyse([__DIR__ . '/data/remember-class-exists-from-constructor.php'], [ + [ + 'Class SomeUnknownClass not found.', + 19, + 'Learn more at https://phpstan.org/user-guide/discovering-symbols', + ], + [ + 'Class SomeUnknownInterface not found.', + 38, + 'Learn more at https://phpstan.org/user-guide/discovering-symbols', + ], + ]); + } + public function testRememberClassExistsFromConstructor(): void { $this->analyse([__DIR__ . '/data/remember-class-exists-from-constructor.php'], []); From 0fbc5685413f9760f2b04a9d988e6f6fb4188c64 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sun, 13 Apr 2025 13:26:55 +0200 Subject: [PATCH 18/23] enable for everyone --- conf/bleedingEdge.neon | 1 - conf/config.neon | 3 +-- conf/parametersSchema.neon | 1 - 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index 72bd9f7b3e..76dd0d8904 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -5,4 +5,3 @@ parameters: skipCheckGenericClasses!: [] stricterFunctionMap: true reportPreciseLineForUnusedFunctionParameter: true - narrowMethodScopeFromConstructor: true diff --git a/conf/config.neon b/conf/config.neon index 28dfc2f2e2..4d18b9552e 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -26,7 +26,6 @@ parameters: skipCheckGenericClasses: [] stricterFunctionMap: false reportPreciseLineForUnusedFunctionParameter: false - narrowMethodScopeFromConstructor: false fileExtensions: - php checkAdvancedIsset: false @@ -496,7 +495,7 @@ services: implicitThrows: %exceptions.implicitThrows% treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain% universalObjectCratesClasses: %universalObjectCratesClasses% - narrowMethodScopeFromConstructor: %featureToggles.narrowMethodScopeFromConstructor% + narrowMethodScopeFromConstructor: true - class: PHPStan\Analyser\ConstantResolver diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index a214b270a6..69fc9380b9 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -32,7 +32,6 @@ parametersSchema: skipCheckGenericClasses: listOf(string()), stricterFunctionMap: bool() reportPreciseLineForUnusedFunctionParameter: bool() - narrowMethodScopeFromConstructor: bool() ]) fileExtensions: listOf(string()) checkAdvancedIsset: bool() From 534b11bc1aba4160ed20368458c431691444353a Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sun, 13 Apr 2025 14:17:14 +0200 Subject: [PATCH 19/23] fix determination of final scope --- src/Analyser/NodeScopeResolver.php | 30 ++++++++++++++++++- ...remember-readonly-constructor-narrowed.php | 24 +++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 3e4446620c..e5cffa8560 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -795,7 +795,35 @@ private function processStmtNode( ), $methodScope); if ($isConstructor && $this->narrowMethodScopeFromConstructor) { - $scope = $statementResult->getScope()->rememberConstructorScope(); + $finalScope = null; + + foreach ($executionEnds as $executionEnd) { + if ($executionEnd->getStatementResult()->isAlwaysTerminating()) { + continue; + } + + $endScope = $executionEnd->getStatementResult()->getScope(); + if ($finalScope === null) { + $finalScope = $endScope; + continue; + } + + $finalScope = $finalScope->mergeWith($endScope); + } + + foreach ($gatheredReturnStatements as $statement) { + if ($finalScope === null) { + $finalScope = $statement->getScope(); + continue; + } + + $finalScope = $finalScope->mergeWith($statement->getScope()); + } + + if ($finalScope !== null) { + $scope = $finalScope->rememberConstructorScope(); + } + } } } elseif ($stmt instanceof Echo_) { diff --git a/tests/PHPStan/Analyser/nsrt/remember-readonly-constructor-narrowed.php b/tests/PHPStan/Analyser/nsrt/remember-readonly-constructor-narrowed.php index 2cdeaf3c7a..7ab2ea364f 100644 --- a/tests/PHPStan/Analyser/nsrt/remember-readonly-constructor-narrowed.php +++ b/tests/PHPStan/Analyser/nsrt/remember-readonly-constructor-narrowed.php @@ -83,3 +83,27 @@ public function doFoo() { assertType('int', $this->i); } } + +class HelloWorldReadonlyPropertySometimesThrowing { + private readonly int $i; + + public function __construct() + { + if (rand(0,1)) { + $this->i = 4; + + return; + } elseif (rand(10,100)) { + $this->i = 10; + return; + } else { + $this->i = 20; + } + + throw new \LogicException(); + } + + public function doFoo() { + assertType('4|10', $this->i); + } +} From 4e1889b7aac579b7eb6338edf4a9fc2fe86ed811 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sun, 13 Apr 2025 18:16:31 +0200 Subject: [PATCH 20/23] error on initialized non-nullable properties --- src/Analyser/MutatingScope.php | 2 +- src/Rules/IssetCheck.php | 19 +++++++ .../PHPStan/Rules/Variables/IssetRuleTest.php | 17 +++++++ .../Rules/Variables/NullCoalesceRuleTest.php | 17 +++++++ .../isset-after-remembered-constructor.php | 51 +++++++++++++++++++ 5 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 tests/PHPStan/Rules/Variables/data/isset-after-remembered-constructor.php diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index bc557bff65..42034f0ae3 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -329,7 +329,7 @@ private function rememberConstructorExpressions(array $currentExpressionTypes): if ($nativePropertyReflection === null || !$nativePropertyReflection->isReadOnly()) { continue; } - } elseif (!$expr instanceof ConstFetch) { + } elseif (!$expr instanceof ConstFetch && !$expr instanceof PropertyInitializationExpr) { continue; } diff --git a/src/Rules/IssetCheck.php b/src/Rules/IssetCheck.php index 528f037f8b..8788bd4166 100644 --- a/src/Rules/IssetCheck.php +++ b/src/Rules/IssetCheck.php @@ -5,10 +5,12 @@ use PhpParser\Node; use PhpParser\Node\Expr; use PHPStan\Analyser\Scope; +use PHPStan\Node\Expr\PropertyInitializationExpr; use PHPStan\Rules\Properties\PropertyDescriptor; use PHPStan\Rules\Properties\PropertyReflectionFinder; use PHPStan\Type\NeverType; use PHPStan\Type\Type; +use PHPStan\Type\TypeCombinator; use PHPStan\Type\VerbosityLevel; use function is_string; use function sprintf; @@ -143,6 +145,23 @@ public function check(Expr $expr, Scope $scope, string $operatorDescription, str } if ($propertyReflection->hasNativeType() && !$propertyReflection->isVirtual()->yes()) { + if ( + $expr instanceof Node\Expr\PropertyFetch + && $expr->name instanceof Node\Identifier + && $expr->var instanceof Expr\Variable + && $expr->var->name === 'this' + && !TypeCombinator::containsNull($propertyReflection->getNativeType()) + && $scope->hasExpressionType(new PropertyInitializationExpr($propertyReflection->getName()))->yes() + ) { + return RuleErrorBuilder::message( + sprintf( + '%s cannot be null or uninitialized %s.', + $this->propertyDescriptor->describeProperty($propertyReflection, $scope, $expr), + $operatorDescription, + ), + )->identifier(sprintf('%s.neverNullOrUninitialized', $identifier))->build(); + } + if (!$scope->hasExpressionType($expr)->yes()) { if ($expr instanceof Node\Expr\PropertyFetch) { return $this->checkUndefined($expr->var, $scope, $operatorDescription, $identifier); diff --git a/tests/PHPStan/Rules/Variables/IssetRuleTest.php b/tests/PHPStan/Rules/Variables/IssetRuleTest.php index f25e40b2d7..fa41949944 100644 --- a/tests/PHPStan/Rules/Variables/IssetRuleTest.php +++ b/tests/PHPStan/Rules/Variables/IssetRuleTest.php @@ -32,6 +32,11 @@ protected function shouldTreatPhpDocTypesAsCertain(): bool return $this->treatPhpDocTypesAsCertain; } + public function shouldNarrowMethodScopeFromConstructor(): bool + { + return true; + } + public function testRule(): void { $this->treatPhpDocTypesAsCertain = true; @@ -480,4 +485,16 @@ public function testBug12771(): void $this->analyse([__DIR__ . '/data/bug-12771.php'], []); } + public function testIssetAfterRememberedConstructor(): void + { + $this->treatPhpDocTypesAsCertain = true; + + $this->analyse([__DIR__ . '/data/isset-after-remembered-constructor.php'], [ + [ + 'Property IssetOrCoalesceOnNonNullableInitializedProperty\User::$string cannot be null or uninitialized in isset().', + 36, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php b/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php index 0745db66a6..a2affb0ea0 100644 --- a/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php +++ b/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php @@ -32,6 +32,11 @@ protected function shouldTreatPhpDocTypesAsCertain(): bool return $this->treatPhpDocTypesAsCertain; } + public function shouldNarrowMethodScopeFromConstructor(): bool + { + return true; + } + public function testCoalesceRule(): void { $this->treatPhpDocTypesAsCertain = true; @@ -356,4 +361,16 @@ public function testBug12553(): void $this->analyse([__DIR__ . '/data/bug-12553.php'], []); } + public function testIssetAfterRememberedConstructor(): void + { + $this->treatPhpDocTypesAsCertain = true; + + $this->analyse([__DIR__ . '/data/isset-after-remembered-constructor.php'], [ + [ + 'Property IssetOrCoalesceOnNonNullableInitializedProperty\User::$string cannot be null or uninitialized on left side of ??.', + 48, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Variables/data/isset-after-remembered-constructor.php b/tests/PHPStan/Rules/Variables/data/isset-after-remembered-constructor.php new file mode 100644 index 0000000000..fdafdf4fa8 --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/isset-after-remembered-constructor.php @@ -0,0 +1,51 @@ += 7.4 + +namespace IssetOrCoalesceOnNonNullableInitializedProperty; + +use function PHPStan\debugScope; + +class User +{ + private ?string $nullableString; + private string $maybeUninitializedString; + private string $string; + + private $untyped; + + public function __construct( + ) { + if (rand(0,1)) { + $this->nullableString = 'hello'; + $this->string = 'world'; + $this->maybeUninitializedString = 'something'; + } else { + $this->nullableString = null; + $this->string = 'world 2'; + $this->untyped = 123; + } + } + + public function doFoo(): void + { + if (isset($this->maybeUninitializedString)) { + echo $this->maybeUninitializedString; + } + if (isset($this->nullableString)) { + echo $this->nullableString; + } + if (isset($this->string)) { + echo $this->string; + } + if (isset($this->untyped)) { + echo $this->untyped; + } + } + + public function doBar(): void + { + echo $this->maybeUninitializedString ?? 'default'; + echo $this->nullableString ?? 'default'; + echo $this->string ?? 'default'; + echo $this->untyped ?? 'default'; + } +} From e33a5740a75b392a8ca5b7eaadfa87aedc676bd8 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sun, 13 Apr 2025 21:52:24 +0200 Subject: [PATCH 21/23] Cover EmptyRule --- src/Rules/IssetCheck.php | 15 +++-- .../PHPStan/Rules/Variables/EmptyRuleTest.php | 25 ++++++++ .../PHPStan/Rules/Variables/IssetRuleTest.php | 4 +- .../Rules/Variables/NullCoalesceRuleTest.php | 4 +- .../isset-after-remembered-constructor.php | 59 +++++++++++++++++-- 5 files changed, 91 insertions(+), 16 deletions(-) diff --git a/src/Rules/IssetCheck.php b/src/Rules/IssetCheck.php index 8788bd4166..303f0d2c0d 100644 --- a/src/Rules/IssetCheck.php +++ b/src/Rules/IssetCheck.php @@ -10,7 +10,6 @@ use PHPStan\Rules\Properties\PropertyReflectionFinder; use PHPStan\Type\NeverType; use PHPStan\Type\Type; -use PHPStan\Type\TypeCombinator; use PHPStan\Type\VerbosityLevel; use function is_string; use function sprintf; @@ -150,16 +149,20 @@ public function check(Expr $expr, Scope $scope, string $operatorDescription, str && $expr->name instanceof Node\Identifier && $expr->var instanceof Expr\Variable && $expr->var->name === 'this' - && !TypeCombinator::containsNull($propertyReflection->getNativeType()) + && $propertyReflection->getNativeType()->isNull()->no() && $scope->hasExpressionType(new PropertyInitializationExpr($propertyReflection->getName()))->yes() ) { - return RuleErrorBuilder::message( + return $this->generateError( + $propertyReflection->getNativeType(), sprintf( - '%s cannot be null or uninitialized %s.', + '%s %s', $this->propertyDescriptor->describeProperty($propertyReflection, $scope, $expr), $operatorDescription, ), - )->identifier(sprintf('%s.neverNullOrUninitialized', $identifier))->build(); + $typeMessageCallback, + $identifier, + 'propertyNeverNullOrUninitialized', + ); } if (!$scope->hasExpressionType($expr)->yes()) { @@ -299,7 +302,7 @@ private function checkUndefined(Expr $expr, Scope $scope, string $operatorDescri /** * @param callable(Type): ?string $typeMessageCallback * @param ErrorIdentifier $identifier - * @param 'variable'|'offset'|'property'|'expr' $identifierSecondPart + * @param 'variable'|'offset'|'property'|'expr'|'propertyNeverNullOrUninitialized' $identifierSecondPart */ private function generateError(Type $type, string $message, callable $typeMessageCallback, string $identifier, string $identifierSecondPart): ?IdentifierRuleError { diff --git a/tests/PHPStan/Rules/Variables/EmptyRuleTest.php b/tests/PHPStan/Rules/Variables/EmptyRuleTest.php index f45e41b774..6e7a5f8181 100644 --- a/tests/PHPStan/Rules/Variables/EmptyRuleTest.php +++ b/tests/PHPStan/Rules/Variables/EmptyRuleTest.php @@ -32,6 +32,11 @@ protected function shouldTreatPhpDocTypesAsCertain(): bool return $this->treatPhpDocTypesAsCertain; } + public function shouldNarrowMethodScopeFromConstructor(): bool + { + return true; + } + public function testRule(): void { $this->treatPhpDocTypesAsCertain = true; @@ -206,4 +211,24 @@ public function testBug12658(): void $this->analyse([__DIR__ . '/data/bug-12658.php'], []); } + public function testIssetAfterRememberedConstructor(): void + { + if (PHP_VERSION_ID < 80000) { + $this->markTestSkipped('Test requires PHP 8.0.'); + } + + $this->treatPhpDocTypesAsCertain = true; + + $this->analyse([__DIR__ . '/data/isset-after-remembered-constructor.php'], [ + [ + 'Property IssetOrCoalesceOnNonNullableInitializedProperty\MoreEmptyCases::$false in empty() is always falsy.', + 93, + ], + [ + 'Property IssetOrCoalesceOnNonNullableInitializedProperty\MoreEmptyCases::$true in empty() is not falsy.', + 95, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Variables/IssetRuleTest.php b/tests/PHPStan/Rules/Variables/IssetRuleTest.php index fa41949944..f92076b16a 100644 --- a/tests/PHPStan/Rules/Variables/IssetRuleTest.php +++ b/tests/PHPStan/Rules/Variables/IssetRuleTest.php @@ -491,8 +491,8 @@ public function testIssetAfterRememberedConstructor(): void $this->analyse([__DIR__ . '/data/isset-after-remembered-constructor.php'], [ [ - 'Property IssetOrCoalesceOnNonNullableInitializedProperty\User::$string cannot be null or uninitialized in isset().', - 36, + 'Property IssetOrCoalesceOnNonNullableInitializedProperty\User::$string in isset() is not nullable.', + 34, ], ]); } diff --git a/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php b/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php index a2affb0ea0..0e849ab84e 100644 --- a/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php +++ b/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php @@ -367,8 +367,8 @@ public function testIssetAfterRememberedConstructor(): void $this->analyse([__DIR__ . '/data/isset-after-remembered-constructor.php'], [ [ - 'Property IssetOrCoalesceOnNonNullableInitializedProperty\User::$string cannot be null or uninitialized on left side of ??.', - 48, + 'Property IssetOrCoalesceOnNonNullableInitializedProperty\User::$string on left side of ?? is not nullable.', + 46, ], ]); } diff --git a/tests/PHPStan/Rules/Variables/data/isset-after-remembered-constructor.php b/tests/PHPStan/Rules/Variables/data/isset-after-remembered-constructor.php index fdafdf4fa8..2c48e6249d 100644 --- a/tests/PHPStan/Rules/Variables/data/isset-after-remembered-constructor.php +++ b/tests/PHPStan/Rules/Variables/data/isset-after-remembered-constructor.php @@ -1,9 +1,7 @@ -= 7.4 += 8.2 namespace IssetOrCoalesceOnNonNullableInitializedProperty; -use function PHPStan\debugScope; - class User { private ?string $nullableString; @@ -12,9 +10,9 @@ class User private $untyped; - public function __construct( - ) { - if (rand(0,1)) { + public function __construct() + { + if (rand(0, 1)) { $this->nullableString = 'hello'; $this->string = 'world'; $this->maybeUninitializedString = 'something'; @@ -48,4 +46,53 @@ public function doBar(): void echo $this->string ?? 'default'; echo $this->untyped ?? 'default'; } + + public function doFooBar(): void + { + if (empty($this->maybeUninitializedString)) { + echo $this->maybeUninitializedString; + } + if (empty($this->nullableString)) { + echo $this->nullableString; + } + if (empty($this->string)) { + echo $this->string; + } + if (empty($this->untyped)) { + echo $this->untyped; + } + } +} + +class MoreEmptyCases +{ + private false|string $union; + private false $false; + private true $true; + private bool $bool; + + public function __construct() + { + if (rand(0, 1)) { + $this->union = 'nope'; + $this->bool = true; + } elseif (rand(10, 20)) { + $this->union = false; + $this->bool = false; + } + $this->false = false; + $this->true = true; + } + + public function doFoo(): void + { + if (empty($this->union)) { + } + if (empty($this->bool)) { + } + if (empty($this->false)) { + } + if (empty($this->true)) { + } + } } From 49e771e90edeb35fd921130eb64e528586b0d177 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 14 Apr 2025 08:50:48 +0200 Subject: [PATCH 22/23] Added regression test --- .../MissingReadOnlyPropertyAssignRuleTest.php | 17 ++++++++++++ .../Rules/Properties/data/bug-10048.php | 26 +++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 tests/PHPStan/Rules/Properties/data/bug-10048.php diff --git a/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php b/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php index 3b481b8f14..d0ac14646d 100644 --- a/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php +++ b/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php @@ -16,6 +16,8 @@ class MissingReadOnlyPropertyAssignRuleTest extends RuleTestCase { + private bool $shouldNarrowMethodScopeFromConstructor = false; + protected function getRule(): Rule { return new MissingReadOnlyPropertyAssignRule( @@ -31,6 +33,11 @@ protected function getRule(): Rule ); } + public function shouldNarrowMethodScopeFromConstructor(): bool + { + return $this->shouldNarrowMethodScopeFromConstructor; + } + protected function getReadWritePropertiesExtensions(): array { return [ @@ -375,6 +382,16 @@ public function testBug9863(): void ]); } + public function testBug10048(): void + { + if (PHP_VERSION_ID < 80100) { + $this->markTestSkipped('Test requires PHP 8.1.'); + } + + $this->shouldNarrowMethodScopeFromConstructor = true; + $this->analyse([__DIR__ . '/data/bug-10048.php'], []); + } + public function testBug9864(): void { if (PHP_VERSION_ID < 80100) { diff --git a/tests/PHPStan/Rules/Properties/data/bug-10048.php b/tests/PHPStan/Rules/Properties/data/bug-10048.php new file mode 100644 index 0000000000..a6b2b151d7 --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/bug-10048.php @@ -0,0 +1,26 @@ +bar = "hi"; + $this->useBar(); + echo $this->bar; + $this->callback = function() { + $this->useBar(); + }; + } + + private function useBar(): void { + echo $this->bar; + } + + public function useCallback(): void { + call_user_func($this->callback); + } +} + +(new Foo())->useCallback(); From e4d70b8ea71e72b80d1570e6569559ba898ebe5d Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 14 Apr 2025 08:58:17 +0200 Subject: [PATCH 23/23] Added regression test --- .../MissingReadOnlyPropertyAssignRuleTest.php | 10 ++++++ .../Rules/Properties/data/bug-10048.php | 2 +- .../Rules/Properties/data/bug-11828.php | 31 +++++++++++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 tests/PHPStan/Rules/Properties/data/bug-11828.php diff --git a/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php b/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php index d0ac14646d..f389c3f628 100644 --- a/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php +++ b/tests/PHPStan/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php @@ -392,6 +392,16 @@ public function testBug10048(): void $this->analyse([__DIR__ . '/data/bug-10048.php'], []); } + public function testBug11828(): void + { + if (PHP_VERSION_ID < 80100) { + $this->markTestSkipped('Test requires PHP 8.1.'); + } + + $this->shouldNarrowMethodScopeFromConstructor = true; + $this->analyse([__DIR__ . '/data/bug-11828.php'], []); + } + public function testBug9864(): void { if (PHP_VERSION_ID < 80100) { diff --git a/tests/PHPStan/Rules/Properties/data/bug-10048.php b/tests/PHPStan/Rules/Properties/data/bug-10048.php index a6b2b151d7..d537fb6527 100644 --- a/tests/PHPStan/Rules/Properties/data/bug-10048.php +++ b/tests/PHPStan/Rules/Properties/data/bug-10048.php @@ -1,4 +1,4 @@ -= 8.1 namespace Bug10048; diff --git a/tests/PHPStan/Rules/Properties/data/bug-11828.php b/tests/PHPStan/Rules/Properties/data/bug-11828.php new file mode 100644 index 0000000000..0a030d7bd0 --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/bug-11828.php @@ -0,0 +1,31 @@ += 8.1 + +namespace Bug11828; + +class Dummy +{ + /** + * @var callable + */ + private $callable; + private readonly int $foo; + + public function __construct(int $foo) + { + $this->foo = $foo; + + $this->callable = function () { + $foo = $this->getFoo(); + }; + } + + public function getFoo(): int + { + return $this->foo; + } + + public function getCallable(): callable + { + return $this->callable; + } +}