Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions conf/config.neon
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,7 @@ services:
implicitThrows: %exceptions.implicitThrows%
treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain%
universalObjectCratesClasses: %universalObjectCratesClasses%
narrowMethodScopeFromConstructor: true

-
class: PHPStan\Analyser\ConstantResolver
Expand Down
86 changes: 78 additions & 8 deletions src/Analyser/MutatingScope.php
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,77 @@ public function enterDeclareStrictTypes(): self
);
}

/**
* @param array<string, ExpressionTypeHolder> $currentExpressionTypes
* @return array<string, ExpressionTypeHolder>
*/
private function rememberConstructorExpressions(array $currentExpressionTypes): array
{
$expressionTypes = [];
foreach ($currentExpressionTypes as $exprString => $expressionTypeHolder) {
$expr = $expressionTypeHolder->getExpr();
if ($expr instanceof FuncCall) {
if (
!$expr->name instanceof Name
|| !in_array($expr->name->name, ['class_exists', 'function_exists'], true)
) {
continue;
}
} elseif ($expr instanceof PropertyFetch) {
if (
!$expr->name instanceof Node\Identifier
|| !$expr->var instanceof Variable
|| $expr->var->name !== 'this'
|| !$this->phpVersion->supportsReadOnlyProperties()
) {
continue;
}

$propertyReflection = $this->propertyReflectionFinder->findPropertyReflectionFromNode($expr, $this);
if ($propertyReflection === null) {
continue;
}

$nativePropertyReflection = $propertyReflection->getNativeReflection();
if ($nativePropertyReflection === null || !$nativePropertyReflection->isReadOnly()) {
continue;
}
} elseif (!$expr instanceof ConstFetch && !$expr instanceof PropertyInitializationExpr) {
continue;
}

$expressionTypes[$exprString] = $expressionTypeHolder;
}

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(),
$this->rememberConstructorExpressions($this->expressionTypes),
$this->rememberConstructorExpressions($this->nativeExpressionTypes),
$this->conditionalExpressions,
$this->inClosureBindScopeClasses,
$this->anonymousFunctionReflection,
$this->inFirstLevelStatement,
[],
[],
$this->inFunctionCallsStack,
$this->afterExtractCall,
$this->parentScope,
$this->nativeTypesPromoted,
);
}

/** @api */
public function isInClass(): bool
{
Expand Down Expand Up @@ -3286,7 +3357,7 @@ public function enterFunction(

private function enterFunctionLike(
PhpFunctionFromParserNodeReflection $functionReflection,
bool $preserveThis,
bool $preserveConstructorScope,
): self
{
$parametersByName = [];
Expand All @@ -3298,6 +3369,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();

Expand Down Expand Up @@ -3348,13 +3425,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(),
Expand Down
55 changes: 54 additions & 1 deletion src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -271,6 +272,7 @@ public function __construct(
private readonly array $universalObjectCratesClasses,
private readonly bool $implicitThrows,
private readonly bool $treatPhpDocTypesAsCertain,
private readonly bool $narrowMethodScopeFromConstructor,
)
{
$earlyTerminatingMethodNames = [];
Expand Down Expand Up @@ -791,6 +793,38 @@ private function processStmtNode(
$classReflection,
$methodReflection,
), $methodScope);

if ($isConstructor && $this->narrowMethodScopeFromConstructor) {
$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_) {
$hasYield = false;
Expand Down Expand Up @@ -925,7 +959,26 @@ private function processStmtNode(
$classStatementsGatherer = new ClassStatementsGatherer($classReflection, $nodeCallback);
$this->processAttributeGroups($stmt, $stmt->attrGroups, $classScope, $classStatementsGatherer);

$this->processStmtNodes($stmt, $stmt->stmts, $classScope, $classStatementsGatherer, $context);
$classLikeStatements = $stmt->stmts;
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;
}

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);
Expand Down
24 changes: 23 additions & 1 deletion src/Rules/IssetCheck.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
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;
Expand Down Expand Up @@ -143,6 +144,27 @@ 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'
&& $propertyReflection->getNativeType()->isNull()->no()
&& $scope->hasExpressionType(new PropertyInitializationExpr($propertyReflection->getName()))->yes()
) {
return $this->generateError(
$propertyReflection->getNativeType(),
sprintf(
'%s %s',
$this->propertyDescriptor->describeProperty($propertyReflection, $scope, $expr),
$operatorDescription,
),
$typeMessageCallback,
$identifier,
'propertyNeverNullOrUninitialized',
);
}

if (!$scope->hasExpressionType($expr)->yes()) {
if ($expr instanceof Node\Expr\PropertyFetch) {
return $this->checkUndefined($expr->var, $scope, $operatorDescription, $identifier);
Expand Down Expand Up @@ -280,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
{
Expand Down
6 changes: 6 additions & 0 deletions src/Testing/RuleTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -261,6 +262,11 @@ protected function shouldFailOnPhpErrors(): bool
return true;
}

protected function shouldNarrowMethodScopeFromConstructor(): bool
{
return false;
}

public static function getAdditionalConfigFiles(): array
{
return [
Expand Down
1 change: 1 addition & 0 deletions src/Testing/TypeInferenceTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -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())));

Expand Down
1 change: 1 addition & 0 deletions tests/PHPStan/Analyser/AnalyserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,7 @@ private function createAnalyser(): Analyser
[stdClass::class],
true,
$this->shouldTreatPhpDocTypesAsCertain(),
true,
);
$lexer = new Lexer();
$fileAnalyser = new FileAnalyser(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php // lint >= 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;
}
}
}
Loading
Loading