Skip to content

Commit da74773

Browse files
staabmstaabm
authored
Remember narrowed types from the constructor when analysing other methods
1 parent cd2ca64 commit da74773

22 files changed

+820
-10
lines changed

Diff for: ‎conf/config.neon

+1
Original file line numberDiff line numberDiff line change
@@ -495,6 +495,7 @@ services:
495495
implicitThrows: %exceptions.implicitThrows%
496496
treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain%
497497
universalObjectCratesClasses: %universalObjectCratesClasses%
498+
narrowMethodScopeFromConstructor: true
498499

499500
-
500501
class: PHPStan\Analyser\ConstantResolver

Diff for: ‎src/Analyser/MutatingScope.php

+78-8
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,77 @@ public function enterDeclareStrictTypes(): self
294294
);
295295
}
296296

297+
/**
298+
* @param array<string, ExpressionTypeHolder> $currentExpressionTypes
299+
* @return array<string, ExpressionTypeHolder>
300+
*/
301+
private function rememberConstructorExpressions(array $currentExpressionTypes): array
302+
{
303+
$expressionTypes = [];
304+
foreach ($currentExpressionTypes as $exprString => $expressionTypeHolder) {
305+
$expr = $expressionTypeHolder->getExpr();
306+
if ($expr instanceof FuncCall) {
307+
if (
308+
!$expr->name instanceof Name
309+
|| !in_array($expr->name->name, ['class_exists', 'function_exists'], true)
310+
) {
311+
continue;
312+
}
313+
} elseif ($expr instanceof PropertyFetch) {
314+
if (
315+
!$expr->name instanceof Node\Identifier
316+
|| !$expr->var instanceof Variable
317+
|| $expr->var->name !== 'this'
318+
|| !$this->phpVersion->supportsReadOnlyProperties()
319+
) {
320+
continue;
321+
}
322+
323+
$propertyReflection = $this->propertyReflectionFinder->findPropertyReflectionFromNode($expr, $this);
324+
if ($propertyReflection === null) {
325+
continue;
326+
}
327+
328+
$nativePropertyReflection = $propertyReflection->getNativeReflection();
329+
if ($nativePropertyReflection === null || !$nativePropertyReflection->isReadOnly()) {
330+
continue;
331+
}
332+
} elseif (!$expr instanceof ConstFetch && !$expr instanceof PropertyInitializationExpr) {
333+
continue;
334+
}
335+
336+
$expressionTypes[$exprString] = $expressionTypeHolder;
337+
}
338+
339+
if (array_key_exists('$this', $currentExpressionTypes)) {
340+
$expressionTypes['$this'] = $currentExpressionTypes['$this'];
341+
}
342+
343+
return $expressionTypes;
344+
}
345+
346+
public function rememberConstructorScope(): self
347+
{
348+
return $this->scopeFactory->create(
349+
$this->context,
350+
$this->isDeclareStrictTypes(),
351+
$this->getFunction(),
352+
$this->getNamespace(),
353+
$this->rememberConstructorExpressions($this->expressionTypes),
354+
$this->rememberConstructorExpressions($this->nativeExpressionTypes),
355+
$this->conditionalExpressions,
356+
$this->inClosureBindScopeClasses,
357+
$this->anonymousFunctionReflection,
358+
$this->inFirstLevelStatement,
359+
[],
360+
[],
361+
$this->inFunctionCallsStack,
362+
$this->afterExtractCall,
363+
$this->parentScope,
364+
$this->nativeTypesPromoted,
365+
);
366+
}
367+
297368
/** @api */
298369
public function isInClass(): bool
299370
{
@@ -3286,7 +3357,7 @@ public function enterFunction(
32863357

32873358
private function enterFunctionLike(
32883359
PhpFunctionFromParserNodeReflection $functionReflection,
3289-
bool $preserveThis,
3360+
bool $preserveConstructorScope,
32903361
): self
32913362
{
32923363
$parametersByName = [];
@@ -3298,6 +3369,12 @@ private function enterFunctionLike(
32983369
$expressionTypes = [];
32993370
$nativeExpressionTypes = [];
33003371
$conditionalTypes = [];
3372+
3373+
if ($preserveConstructorScope) {
3374+
$expressionTypes = $this->rememberConstructorExpressions($this->expressionTypes);
3375+
$nativeExpressionTypes = $this->rememberConstructorExpressions($this->nativeExpressionTypes);
3376+
}
3377+
33013378
foreach ($functionReflection->getParameters() as $parameter) {
33023379
$parameterType = $parameter->getType();
33033380

@@ -3348,13 +3425,6 @@ private function enterFunctionLike(
33483425
$nativeExpressionTypes[$parameterOriginalValueExprString] = ExpressionTypeHolder::createYes($parameterOriginalValueExpr, $nativeParameterType);
33493426
}
33503427

3351-
if ($preserveThis && array_key_exists('$this', $this->expressionTypes)) {
3352-
$expressionTypes['$this'] = $this->expressionTypes['$this'];
3353-
}
3354-
if ($preserveThis && array_key_exists('$this', $this->nativeExpressionTypes)) {
3355-
$nativeExpressionTypes['$this'] = $this->nativeExpressionTypes['$this'];
3356-
}
3357-
33583428
return $this->scopeFactory->create(
33593429
$this->context,
33603430
$this->isDeclareStrictTypes(),

Diff for: ‎src/Analyser/NodeScopeResolver.php

+54-1
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,7 @@
218218
use function str_starts_with;
219219
use function strtolower;
220220
use function trim;
221+
use function usort;
221222
use const PHP_VERSION_ID;
222223
use const SORT_NUMERIC;
223224

@@ -271,6 +272,7 @@ public function __construct(
271272
private readonly array $universalObjectCratesClasses,
272273
private readonly bool $implicitThrows,
273274
private readonly bool $treatPhpDocTypesAsCertain,
275+
private readonly bool $narrowMethodScopeFromConstructor,
274276
)
275277
{
276278
$earlyTerminatingMethodNames = [];
@@ -791,6 +793,38 @@ private function processStmtNode(
791793
$classReflection,
792794
$methodReflection,
793795
), $methodScope);
796+
797+
if ($isConstructor && $this->narrowMethodScopeFromConstructor) {
798+
$finalScope = null;
799+
800+
foreach ($executionEnds as $executionEnd) {
801+
if ($executionEnd->getStatementResult()->isAlwaysTerminating()) {
802+
continue;
803+
}
804+
805+
$endScope = $executionEnd->getStatementResult()->getScope();
806+
if ($finalScope === null) {
807+
$finalScope = $endScope;
808+
continue;
809+
}
810+
811+
$finalScope = $finalScope->mergeWith($endScope);
812+
}
813+
814+
foreach ($gatheredReturnStatements as $statement) {
815+
if ($finalScope === null) {
816+
$finalScope = $statement->getScope();
817+
continue;
818+
}
819+
820+
$finalScope = $finalScope->mergeWith($statement->getScope());
821+
}
822+
823+
if ($finalScope !== null) {
824+
$scope = $finalScope->rememberConstructorScope();
825+
}
826+
827+
}
794828
}
795829
} elseif ($stmt instanceof Echo_) {
796830
$hasYield = false;
@@ -925,7 +959,26 @@ private function processStmtNode(
925959
$classStatementsGatherer = new ClassStatementsGatherer($classReflection, $nodeCallback);
926960
$this->processAttributeGroups($stmt, $stmt->attrGroups, $classScope, $classStatementsGatherer);
927961

928-
$this->processStmtNodes($stmt, $stmt->stmts, $classScope, $classStatementsGatherer, $context);
962+
$classLikeStatements = $stmt->stmts;
963+
if ($this->narrowMethodScopeFromConstructor) {
964+
// analyze static methods first; constructor next; instance methods and property hooks last so we can carry over the scope
965+
usort($classLikeStatements, static function ($a, $b) {
966+
if ($a instanceof Node\Stmt\Property) {
967+
return 1;
968+
}
969+
if ($b instanceof Node\Stmt\Property) {
970+
return -1;
971+
}
972+
973+
if (!$a instanceof Node\Stmt\ClassMethod || !$b instanceof Node\Stmt\ClassMethod) {
974+
return 0;
975+
}
976+
977+
return [!$a->isStatic(), $a->name->toLowerString() !== '__construct'] <=> [!$b->isStatic(), $b->name->toLowerString() !== '__construct'];
978+
});
979+
}
980+
981+
$this->processStmtNodes($stmt, $classLikeStatements, $classScope, $classStatementsGatherer, $context);
929982
$nodeCallback(new ClassPropertiesNode($stmt, $this->readWritePropertiesExtensionProvider, $classStatementsGatherer->getProperties(), $classStatementsGatherer->getPropertyUsages(), $classStatementsGatherer->getMethodCalls(), $classStatementsGatherer->getReturnStatementsNodes(), $classStatementsGatherer->getPropertyAssigns(), $classReflection), $classScope);
930983
$nodeCallback(new ClassMethodsNode($stmt, $classStatementsGatherer->getMethods(), $classStatementsGatherer->getMethodCalls(), $classReflection), $classScope);
931984
$nodeCallback(new ClassConstantsNode($stmt, $classStatementsGatherer->getConstants(), $classStatementsGatherer->getConstantFetches(), $classReflection), $classScope);

Diff for: ‎src/Rules/IssetCheck.php

+23-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use PhpParser\Node;
66
use PhpParser\Node\Expr;
77
use PHPStan\Analyser\Scope;
8+
use PHPStan\Node\Expr\PropertyInitializationExpr;
89
use PHPStan\Rules\Properties\PropertyDescriptor;
910
use PHPStan\Rules\Properties\PropertyReflectionFinder;
1011
use PHPStan\Type\NeverType;
@@ -143,6 +144,27 @@ public function check(Expr $expr, Scope $scope, string $operatorDescription, str
143144
}
144145

145146
if ($propertyReflection->hasNativeType() && !$propertyReflection->isVirtual()->yes()) {
147+
if (
148+
$expr instanceof Node\Expr\PropertyFetch
149+
&& $expr->name instanceof Node\Identifier
150+
&& $expr->var instanceof Expr\Variable
151+
&& $expr->var->name === 'this'
152+
&& $propertyReflection->getNativeType()->isNull()->no()
153+
&& $scope->hasExpressionType(new PropertyInitializationExpr($propertyReflection->getName()))->yes()
154+
) {
155+
return $this->generateError(
156+
$propertyReflection->getNativeType(),
157+
sprintf(
158+
'%s %s',
159+
$this->propertyDescriptor->describeProperty($propertyReflection, $scope, $expr),
160+
$operatorDescription,
161+
),
162+
$typeMessageCallback,
163+
$identifier,
164+
'propertyNeverNullOrUninitialized',
165+
);
166+
}
167+
146168
if (!$scope->hasExpressionType($expr)->yes()) {
147169
if ($expr instanceof Node\Expr\PropertyFetch) {
148170
return $this->checkUndefined($expr->var, $scope, $operatorDescription, $identifier);
@@ -280,7 +302,7 @@ private function checkUndefined(Expr $expr, Scope $scope, string $operatorDescri
280302
/**
281303
* @param callable(Type): ?string $typeMessageCallback
282304
* @param ErrorIdentifier $identifier
283-
* @param 'variable'|'offset'|'property'|'expr' $identifierSecondPart
305+
* @param 'variable'|'offset'|'property'|'expr'|'propertyNeverNullOrUninitialized' $identifierSecondPart
284306
*/
285307
private function generateError(Type $type, string $message, callable $typeMessageCallback, string $identifier, string $identifierSecondPart): ?IdentifierRuleError
286308
{

Diff for: ‎src/Testing/RuleTestCase.php

+6
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ private function getAnalyser(DirectRuleRegistry $ruleRegistry): Analyser
110110
self::getContainer()->getParameter('universalObjectCratesClasses'),
111111
self::getContainer()->getParameter('exceptions')['implicitThrows'],
112112
$this->shouldTreatPhpDocTypesAsCertain(),
113+
$this->shouldNarrowMethodScopeFromConstructor(),
113114
);
114115
$fileAnalyser = new FileAnalyser(
115116
$this->createScopeFactory($reflectionProvider, $typeSpecifier),
@@ -261,6 +262,11 @@ protected function shouldFailOnPhpErrors(): bool
261262
return true;
262263
}
263264

265+
protected function shouldNarrowMethodScopeFromConstructor(): bool
266+
{
267+
return false;
268+
}
269+
264270
public static function getAdditionalConfigFiles(): array
265271
{
266272
return [

Diff for: ‎src/Testing/TypeInferenceTestCase.php

+1
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ public static function processFile(
9090
self::getContainer()->getParameter('universalObjectCratesClasses'),
9191
self::getContainer()->getParameter('exceptions')['implicitThrows'],
9292
self::getContainer()->getParameter('treatPhpDocTypesAsCertain'),
93+
true,
9394
);
9495
$resolver->setAnalysedFiles(array_map(static fn (string $file): string => $fileHelper->normalizePath($file), array_merge([$file], static::getAdditionalAnalysedFiles())));
9596

Diff for: ‎tests/PHPStan/Analyser/AnalyserTest.php

+1
Original file line numberDiff line numberDiff line change
@@ -733,6 +733,7 @@ private function createAnalyser(): Analyser
733733
[stdClass::class],
734734
true,
735735
$this->shouldTreatPhpDocTypesAsCertain(),
736+
true,
736737
);
737738
$lexer = new Lexer();
738739
$fileAnalyser = new FileAnalyser(

Diff for: ‎tests/PHPStan/Analyser/nsrt/remember-readonly-constructor-narrowed-hooks.php

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<?php // lint >= 8.4
2+
3+
namespace RememberReadOnlyConstructorInPropertyHookBodies;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
class User
8+
{
9+
public string $name {
10+
get {
11+
assertType('1|2', $this->type);
12+
return $this->name ;
13+
}
14+
set {
15+
if (strlen($value) === 0) {
16+
throw new ValueError("Name must be non-empty");
17+
}
18+
assertType('1|2', $this->type);
19+
$this->name = $value;
20+
}
21+
}
22+
23+
private readonly int $type;
24+
25+
public function __construct(
26+
string $name
27+
) {
28+
$this->name = $name;
29+
if (rand(0,1)) {
30+
$this->type = 1;
31+
} else {
32+
$this->type = 2;
33+
}
34+
}
35+
}

0 commit comments

Comments
 (0)