From a83c3dcefef85bf648881d2f620c8d5bb0ca245c Mon Sep 17 00:00:00 2001 From: Martin Herndl Date: Mon, 30 Dec 2024 13:54:37 +0100 Subject: [PATCH 01/36] Remove duplicated PHPDoc from InternalScopeFactory classes --- src/Analyser/DirectInternalScopeFactory.php | 11 ----------- src/Analyser/InternalScopeFactory.php | 2 +- src/Analyser/LazyInternalScopeFactory.php | 11 ----------- 3 files changed, 1 insertion(+), 23 deletions(-) diff --git a/src/Analyser/DirectInternalScopeFactory.php b/src/Analyser/DirectInternalScopeFactory.php index 74b43d80c9..22f709cbc9 100644 --- a/src/Analyser/DirectInternalScopeFactory.php +++ b/src/Analyser/DirectInternalScopeFactory.php @@ -7,10 +7,7 @@ use PHPStan\Node\Printer\ExprPrinter; use PHPStan\Parser\Parser; use PHPStan\Php\PhpVersion; -use PHPStan\Reflection\FunctionReflection; use PHPStan\Reflection\InitializerExprTypeResolver; -use PHPStan\Reflection\MethodReflection; -use PHPStan\Reflection\ParameterReflection; use PHPStan\Reflection\ParametersAcceptor; use PHPStan\Reflection\Php\PhpFunctionFromParserNodeReflection; use PHPStan\Reflection\ReflectionProvider; @@ -40,14 +37,6 @@ public function __construct( { } - /** - * @param array $expressionTypes - * @param array $nativeExpressionTypes - * @param array $conditionalExpressions - * @param list $inFunctionCallsStack - * @param array $currentlyAssignedExpressions - * @param array $currentlyAllowedUndefinedExpressions - */ public function create( ScopeContext $context, bool $declareStrictTypes = false, diff --git a/src/Analyser/InternalScopeFactory.php b/src/Analyser/InternalScopeFactory.php index 6d8608ec18..8d8daa714f 100644 --- a/src/Analyser/InternalScopeFactory.php +++ b/src/Analyser/InternalScopeFactory.php @@ -18,7 +18,7 @@ interface InternalScopeFactory * @param list $inClosureBindScopeClasses * @param array $currentlyAssignedExpressions * @param array $currentlyAllowedUndefinedExpressions - * @param list $inFunctionCallsStack + * @param list $inFunctionCallsStack */ public function create( ScopeContext $context, diff --git a/src/Analyser/LazyInternalScopeFactory.php b/src/Analyser/LazyInternalScopeFactory.php index ac5b757991..657cb8c865 100644 --- a/src/Analyser/LazyInternalScopeFactory.php +++ b/src/Analyser/LazyInternalScopeFactory.php @@ -7,10 +7,7 @@ use PHPStan\DependencyInjection\Type\ExpressionTypeResolverExtensionRegistryProvider; use PHPStan\Node\Printer\ExprPrinter; use PHPStan\Php\PhpVersion; -use PHPStan\Reflection\FunctionReflection; use PHPStan\Reflection\InitializerExprTypeResolver; -use PHPStan\Reflection\MethodReflection; -use PHPStan\Reflection\ParameterReflection; use PHPStan\Reflection\ParametersAcceptor; use PHPStan\Reflection\Php\PhpFunctionFromParserNodeReflection; use PHPStan\Reflection\ReflectionProvider; @@ -25,14 +22,6 @@ public function __construct( { } - /** - * @param array $expressionTypes - * @param array $nativeExpressionTypes - * @param array $conditionalExpressions - * @param array $currentlyAssignedExpressions - * @param array $currentlyAllowedUndefinedExpressions - * @param list $inFunctionCallsStack - */ public function create( ScopeContext $context, bool $declareStrictTypes = false, From 068be33ffe572a64647d369e7b63115b58ef1d40 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 29 Dec 2024 16:06:05 +0100 Subject: [PATCH 02/36] Test AccessPropertiesInAssignRule private(set) with array append --- .../Properties/AccessPropertiesInAssignRuleTest.php | 4 ++++ .../Properties/data/write-asymmetric-visibility.php | 11 +++++++++++ 2 files changed, 15 insertions(+) diff --git a/tests/PHPStan/Rules/Properties/AccessPropertiesInAssignRuleTest.php b/tests/PHPStan/Rules/Properties/AccessPropertiesInAssignRuleTest.php index cd318cfedc..53f852ae8e 100644 --- a/tests/PHPStan/Rules/Properties/AccessPropertiesInAssignRuleTest.php +++ b/tests/PHPStan/Rules/Properties/AccessPropertiesInAssignRuleTest.php @@ -157,6 +157,10 @@ public function testAsymmetricVisibility(): void 'Assign to private(set) property WriteAsymmetricVisibility\ReadonlyProps::$c.', 72, ], + [ + 'Assign to private(set) property WriteAsymmetricVisibility\ArrayProp::$a.', + 83, + ], ]); } diff --git a/tests/PHPStan/Rules/Properties/data/write-asymmetric-visibility.php b/tests/PHPStan/Rules/Properties/data/write-asymmetric-visibility.php index 6ea2ab154b..a6273caf09 100644 --- a/tests/PHPStan/Rules/Properties/data/write-asymmetric-visibility.php +++ b/tests/PHPStan/Rules/Properties/data/write-asymmetric-visibility.php @@ -71,3 +71,14 @@ function (ReadonlyProps $foo): void { $foo->b = 1; $foo->c = 1; }; + +class ArrayProp +{ + + public private(set) array $a = []; + +} + +function (ArrayProp $foo): void { + $foo->a[] = 1; +}; From e36bb83477da13b9475efff78f663016c1622d53 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 2 Jan 2025 18:24:09 +0700 Subject: [PATCH 03/36] Introduce `getNextStatements` in UnreachableStatementNode Co-authored-by: Ondrej Mirtes --- src/Analyser/NodeScopeResolver.php | 61 +++++++++--- src/Node/UnreachableStatementNode.php | 13 ++- ...achableStatementNextStatementsRuleTest.php | 94 +++++++++++++++++++ .../DeadCode/UnreachableStatementRuleTest.php | 11 +++ .../DeadCode/data/multiple_unreachable.php | 23 +++++ .../data/multiple_unreachable_top_level.php | 17 ++++ 6 files changed, 203 insertions(+), 16 deletions(-) create mode 100644 tests/PHPStan/Rules/DeadCode/UnreachableStatementNextStatementsRuleTest.php create mode 100644 tests/PHPStan/Rules/DeadCode/data/multiple_unreachable.php create mode 100644 tests/PHPStan/Rules/DeadCode/data/multiple_unreachable_top_level.php diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 791a8920b7..fb72cfeb6b 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -316,13 +316,38 @@ public function processNodes( } $alreadyTerminated = true; - $nextStmt = $this->getFirstUnreachableNode(array_slice($nodes, $i + 1), true); - if (!$nextStmt instanceof Node\Stmt) { + $nextStmts = $this->getNextUnreachableStatements(array_slice($nodes, $i + 1), true); + $this->processUnreachableStatement($nextStmts, $scope, $nodeCallback); + } + } + + /** + * @param Node\Stmt[] $nextStmts + * @param callable(Node $node, Scope $scope): void $nodeCallback + */ + private function processUnreachableStatement(array $nextStmts, MutatingScope $scope, callable $nodeCallback): void + { + if ($nextStmts === []) { + return; + } + + $unreachableStatement = null; + $nextStatements = []; + + foreach ($nextStmts as $key => $nextStmt) { + if ($key === 0) { + $unreachableStatement = $nextStmt; continue; } - $nodeCallback(new UnreachableStatementNode($nextStmt), $scope); + $nextStatements[] = $nextStmt; + } + + if (!$unreachableStatement instanceof Node\Stmt) { + return; } + + $nodeCallback(new UnreachableStatementNode($unreachableStatement, $nextStatements), $scope); } /** @@ -409,11 +434,8 @@ public function processStmtNodes( } $alreadyTerminated = true; - $nextStmt = $this->getFirstUnreachableNode(array_slice($stmts, $i + 1), $parentNode instanceof Node\Stmt\Namespace_); - if ($nextStmt === null) { - continue; - } - $nodeCallback(new UnreachableStatementNode($nextStmt), $scope); + $nextStmts = $this->getNextUnreachableStatements(array_slice($stmts, $i + 1), $parentNode instanceof Node\Stmt\Namespace_); + $this->processUnreachableStatement($nextStmts, $scope, $nodeCallback); } $statementResult = new StatementResult($scope, $hasYield, $alreadyTerminated, $exitPoints, $throwPoints, $impurePoints); @@ -6514,22 +6536,31 @@ private function getPhpDocReturnType(ResolvedPhpDocBlock $resolvedPhpDoc, Type $ } /** - * @template T of Node - * @param array $nodes - * @return T|null + * @param array $nodes + * @return list */ - private function getFirstUnreachableNode(array $nodes, bool $earlyBinding): ?Node + private function getNextUnreachableStatements(array $nodes, bool $earlyBinding): array { + $stmts = []; + $isPassedUnreachableStatement = false; foreach ($nodes as $node) { + if ($earlyBinding && ($node instanceof Node\Stmt\Function_ || $node instanceof Node\Stmt\ClassLike || $node instanceof Node\Stmt\HaltCompiler)) { + continue; + } + if ($isPassedUnreachableStatement && $node instanceof Node\Stmt) { + $stmts[] = $node; + continue; + } if ($node instanceof Node\Stmt\Nop) { continue; } - if ($earlyBinding && ($node instanceof Node\Stmt\Function_ || $node instanceof Node\Stmt\ClassLike || $node instanceof Node\Stmt\HaltCompiler)) { + if (!$node instanceof Node\Stmt) { continue; } - return $node; + $stmts[] = $node; + $isPassedUnreachableStatement = true; } - return null; + return $stmts; } } diff --git a/src/Node/UnreachableStatementNode.php b/src/Node/UnreachableStatementNode.php index e0c8cb0af9..603b7d6f2f 100644 --- a/src/Node/UnreachableStatementNode.php +++ b/src/Node/UnreachableStatementNode.php @@ -10,9 +10,12 @@ final class UnreachableStatementNode extends Stmt implements VirtualNode { - public function __construct(private Stmt $originalStatement) + /** @param Stmt[] $nextStatements */ + public function __construct(private Stmt $originalStatement, private array $nextStatements = []) { parent::__construct($originalStatement->getAttributes()); + + $this->nextStatements = $nextStatements; } public function getOriginalStatement(): Stmt @@ -33,4 +36,12 @@ public function getSubNodeNames(): array return []; } + /** + * @return Stmt[] + */ + public function getNextStatements(): array + { + return $this->nextStatements; + } + } diff --git a/tests/PHPStan/Rules/DeadCode/UnreachableStatementNextStatementsRuleTest.php b/tests/PHPStan/Rules/DeadCode/UnreachableStatementNextStatementsRuleTest.php new file mode 100644 index 0000000000..36aa85b6bb --- /dev/null +++ b/tests/PHPStan/Rules/DeadCode/UnreachableStatementNextStatementsRuleTest.php @@ -0,0 +1,94 @@ + + */ +class UnreachableStatementNextStatementsRuleTest extends RuleTestCase +{ + + /** + * @return Rule + */ + protected function getRule(): Rule + { + return new class implements Rule { + + public function getNodeType(): string + { + return UnreachableStatementNode::class; + } + + /** + * @param UnreachableStatementNode $node + */ + public function processNode(Node $node, Scope $scope): array + { + $errors = [ + RuleErrorBuilder::message('First unreachable') + ->identifier('tests.nextUnreachableStatements') + ->build(), + ]; + + foreach ($node->getNextStatements() as $nextStatement) { + $errors[] = RuleErrorBuilder::message('Another unreachable') + ->line($nextStatement->getStartLine()) + ->identifier('tests.nextUnreachableStatements') + ->build(); + } + + return $errors; + } + + }; + } + + public function testRule(): void + { + $this->analyse([__DIR__ . '/data/multiple_unreachable.php'], [ + [ + 'First unreachable', + 14, + ], + [ + 'Another unreachable', + 15, + ], + [ + 'Another unreachable', + 17, + ], + [ + 'Another unreachable', + 22, + ], + ]); + } + + public function testRuleTopLevel(): void + { + $this->analyse([__DIR__ . '/data/multiple_unreachable_top_level.php'], [ + [ + 'First unreachable', + 9, + ], + [ + 'Another unreachable', + 10, + ], + [ + 'Another unreachable', + 17, + ], + ]); + } + +} diff --git a/tests/PHPStan/Rules/DeadCode/UnreachableStatementRuleTest.php b/tests/PHPStan/Rules/DeadCode/UnreachableStatementRuleTest.php index da076db1c7..ec97b0481a 100644 --- a/tests/PHPStan/Rules/DeadCode/UnreachableStatementRuleTest.php +++ b/tests/PHPStan/Rules/DeadCode/UnreachableStatementRuleTest.php @@ -230,4 +230,15 @@ public function testBug11992(): void $this->analyse([__DIR__ . '/data/bug-11992.php'], []); } + public function testMultipleUnreachable(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/multiple_unreachable.php'], [ + [ + 'Unreachable statement - code above always terminates.', + 14, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/DeadCode/data/multiple_unreachable.php b/tests/PHPStan/Rules/DeadCode/data/multiple_unreachable.php new file mode 100644 index 0000000000..0e9ab15119 --- /dev/null +++ b/tests/PHPStan/Rules/DeadCode/data/multiple_unreachable.php @@ -0,0 +1,23 @@ + Date: Wed, 25 Dec 2024 13:00:35 +0100 Subject: [PATCH 04/36] Improve loose comparison on string types --- .../Accessory/AccessoryNonEmptyStringType.php | 5 + .../Accessory/AccessoryNonFalsyStringType.php | 7 ++ .../Accessory/AccessoryNumericStringType.php | 9 ++ src/Type/StringType.php | 5 + .../Analyser/nsrt/loose-comparisons-php7.php | 11 ++ .../Analyser/nsrt/loose-comparisons-php8.php | 11 ++ .../Analyser/nsrt/loose-comparisons.php | 110 +++++++++++++++++- 7 files changed, 156 insertions(+), 2 deletions(-) diff --git a/src/Type/Accessory/AccessoryNonEmptyStringType.php b/src/Type/Accessory/AccessoryNonEmptyStringType.php index f31e3108b4..d630cad147 100644 --- a/src/Type/Accessory/AccessoryNonEmptyStringType.php +++ b/src/Type/Accessory/AccessoryNonEmptyStringType.php @@ -323,9 +323,14 @@ public function isScalar(): TrinaryLogic public function looseCompare(Type $type, PhpVersion $phpVersion): BooleanType { + if ($type->isNull()->yes()) { + return new ConstantBooleanType(false); + } + if ($type->isString()->yes() && $type->isNonEmptyString()->no()) { return new ConstantBooleanType(false); } + return new BooleanType(); } diff --git a/src/Type/Accessory/AccessoryNonFalsyStringType.php b/src/Type/Accessory/AccessoryNonFalsyStringType.php index 04fd4fb60e..faac90150e 100644 --- a/src/Type/Accessory/AccessoryNonFalsyStringType.php +++ b/src/Type/Accessory/AccessoryNonFalsyStringType.php @@ -11,6 +11,7 @@ use PHPStan\Type\BooleanType; use PHPStan\Type\CompoundType; use PHPStan\Type\Constant\ConstantArrayType; +use PHPStan\Type\Constant\ConstantBooleanType; use PHPStan\Type\Constant\ConstantIntegerType; use PHPStan\Type\ErrorType; use PHPStan\Type\FloatType; @@ -19,6 +20,7 @@ use PHPStan\Type\IntersectionType; use PHPStan\Type\IsSuperTypeOfResult; use PHPStan\Type\ObjectWithoutClassType; +use PHPStan\Type\StaticTypeFactory; use PHPStan\Type\StringType; use PHPStan\Type\Traits\MaybeCallableTypeTrait; use PHPStan\Type\Traits\NonArrayTypeTrait; @@ -322,6 +324,11 @@ public function isScalar(): TrinaryLogic public function looseCompare(Type $type, PhpVersion $phpVersion): BooleanType { + $falseyTypes = StaticTypeFactory::falsey(); + if ($falseyTypes->isSuperTypeOf($type)->yes()) { + return new ConstantBooleanType(false); + } + return new BooleanType(); } diff --git a/src/Type/Accessory/AccessoryNumericStringType.php b/src/Type/Accessory/AccessoryNumericStringType.php index e0f4964c93..9429c7ea73 100644 --- a/src/Type/Accessory/AccessoryNumericStringType.php +++ b/src/Type/Accessory/AccessoryNumericStringType.php @@ -11,6 +11,7 @@ use PHPStan\Type\BooleanType; use PHPStan\Type\CompoundType; use PHPStan\Type\Constant\ConstantArrayType; +use PHPStan\Type\Constant\ConstantBooleanType; use PHPStan\Type\Constant\ConstantIntegerType; use PHPStan\Type\Constant\ConstantStringType; use PHPStan\Type\ErrorType; @@ -324,6 +325,14 @@ public function isScalar(): TrinaryLogic public function looseCompare(Type $type, PhpVersion $phpVersion): BooleanType { + if ($type->isNull()->yes()) { + return new ConstantBooleanType(false); + } + + if ($type->isString()->yes() && $type->isNumericString()->no()) { + return new ConstantBooleanType(false); + } + return new BooleanType(); } diff --git a/src/Type/StringType.php b/src/Type/StringType.php index 4605c92efe..eeedd4bc68 100644 --- a/src/Type/StringType.php +++ b/src/Type/StringType.php @@ -10,6 +10,7 @@ use PHPStan\TrinaryLogic; use PHPStan\Type\Accessory\AccessoryNonEmptyStringType; use PHPStan\Type\Constant\ConstantArrayType; +use PHPStan\Type\Constant\ConstantBooleanType; use PHPStan\Type\Constant\ConstantIntegerType; use PHPStan\Type\Constant\ConstantStringType; use PHPStan\Type\Traits\MaybeCallableTypeTrait; @@ -267,6 +268,10 @@ public function isScalar(): TrinaryLogic public function looseCompare(Type $type, PhpVersion $phpVersion): BooleanType { + if ($type->isArray()->yes()) { + return new ConstantBooleanType(false); + } + return new BooleanType(); } diff --git a/tests/PHPStan/Analyser/nsrt/loose-comparisons-php7.php b/tests/PHPStan/Analyser/nsrt/loose-comparisons-php7.php index 9e00dd0f65..cc5b0f4eb4 100644 --- a/tests/PHPStan/Analyser/nsrt/loose-comparisons-php7.php +++ b/tests/PHPStan/Analyser/nsrt/loose-comparisons-php7.php @@ -61,4 +61,15 @@ public function sayInt( assertType('bool', $int == $phpStr); assertType('bool', $int == 'a'); } + + /** + * @param "abc"|"def" $constNonFalsy + */ + public function sayConstUnion( + $constNonFalsy, + ): void + { + assertType('true', $constNonFalsy == 0); + assertType('true', "" == 0); + } } diff --git a/tests/PHPStan/Analyser/nsrt/loose-comparisons-php8.php b/tests/PHPStan/Analyser/nsrt/loose-comparisons-php8.php index a3ca84cf64..3c12092eb7 100644 --- a/tests/PHPStan/Analyser/nsrt/loose-comparisons-php8.php +++ b/tests/PHPStan/Analyser/nsrt/loose-comparisons-php8.php @@ -68,4 +68,15 @@ public function sayInt( assertType('false', $intRange == 'a'); } + /** + * @param "abc"|"def" $constNonFalsy + */ + public function sayConstUnion( + $constNonFalsy, + ): void + { + assertType('false', $constNonFalsy == 0); + assertType('false', "" == 0); + } + } diff --git a/tests/PHPStan/Analyser/nsrt/loose-comparisons.php b/tests/PHPStan/Analyser/nsrt/loose-comparisons.php index 16c414170b..243b7672fd 100644 --- a/tests/PHPStan/Analyser/nsrt/loose-comparisons.php +++ b/tests/PHPStan/Analyser/nsrt/loose-comparisons.php @@ -526,6 +526,7 @@ public function sayEmptyArray( * @param array{} $emptyArr * @param 'php' $phpStr * @param '' $emptyStr + * @param non-falsy-string $nonFalsyString */ public function sayNonFalsyStr( $true, @@ -540,7 +541,8 @@ public function sayNonFalsyStr( $null, $emptyArr, $phpStr, - $emptyStr + $emptyStr, + $nonFalsyString ): void { assertType('true', $phpStr == $true); @@ -555,6 +557,100 @@ public function sayNonFalsyStr( assertType('false', $phpStr == $emptyArr); assertType('true', $phpStr == $phpStr); assertType('false', $phpStr == $emptyStr); + + assertType('bool', $nonFalsyString == $true); + assertType('false', $nonFalsyString == $false); + assertType('bool', $nonFalsyString == $one); + assertType('false', $nonFalsyString == $zero); + assertType('bool', $nonFalsyString == $minusOne); + assertType('bool', $nonFalsyString == $oneStr); + assertType('false', $nonFalsyString == $zeroStr); + assertType('bool', $nonFalsyString == $minusOneStr); + assertType('bool', $nonFalsyString == $plusOneStr); + assertType('false', $nonFalsyString == $null); + assertType('false', $nonFalsyString == $emptyArr); + assertType('bool', $nonFalsyString == $phpStr); + assertType('false', $nonFalsyString == $emptyStr); + } + + /** + * @param true $true + * @param false $false + * @param 1 $one + * @param 0 $zero + * @param -1 $minusOne + * @param '1' $oneStr + * @param '0' $zeroStr + * @param '-1' $minusOneStr + * @param '+1' $plusOneStr + * @param null $null + * @param array{} $emptyArr + * @param 'php' $phpStr + * @param '' $emptyStr + * @param numeric-string $numericStr + */ + public function sayStr( + $true, + $false, + $one, + $zero, + $minusOne, + $oneStr, + $zeroStr, + $minusOneStr, + $plusOneStr, + $null, + $emptyArr, + string $string, + $phpStr, + $emptyStr, + $numericStr, + ?string $stringOrNull, + ): void + { + assertType('bool', $string == $true); + assertType('bool', $string == $false); + assertType('bool', $string == $one); + assertType('bool', $string == $zero); + assertType('bool', $string == $minusOne); + assertType('bool', $string == $oneStr); + assertType('bool', $string == $zeroStr); + assertType('bool', $string == $minusOneStr); + assertType('bool', $string == $plusOneStr); + assertType('bool', $string == $null); + assertType('bool', $string == $stringOrNull); + assertType('false', $string == $emptyArr); + assertType('bool', $string == $phpStr); + assertType('bool', $string == $emptyStr); + assertType('bool', $string == $numericStr); + + assertType('bool', $numericStr == $true); + assertType('bool', $numericStr == $false); + assertType('bool', $numericStr == $one); + assertType('bool', $numericStr == $zero); + assertType('bool', $numericStr == $minusOne); + assertType('bool', $numericStr == $oneStr); + assertType('bool', $numericStr == $zeroStr); + assertType('bool', $numericStr == $minusOneStr); + assertType('bool', $numericStr == $plusOneStr); + assertType('false', $numericStr == $null); + assertType('bool', $numericStr == $stringOrNull); + assertType('false', $numericStr == $emptyArr); + assertType('bool', $numericStr == $string); + assertType('false', $numericStr == $phpStr); + assertType('false', $numericStr == $emptyStr); + if (is_numeric($string)) { + assertType('bool', $numericStr == $string); + } + + assertType('false', "" == 1); + assertType('true', "" == null); + assertType('false', "" == true); + assertType('true', "" == false); + assertType('false', "" == "1"); + assertType('false', "" == "0"); + assertType('false', "" == "-1"); + assertType('false', "" == []); } /** @@ -657,11 +753,13 @@ public function sayInt( * @param true|1|"1" $looseOne * @param false|0|"0" $looseZero * @param false|1 $constMix + * @param "abc"|"def" $constNonFalsy */ public function sayConstUnion( $looseOne, $looseZero, - $constMix + $constMix, + $constNonFalsy, ): void { assertType('true', $looseOne == 1); @@ -696,6 +794,14 @@ public function sayConstUnion( assertType('bool', $constMix == $looseOne); assertType('bool', $looseZero == $constMix); assertType('bool', $constMix == $looseZero); + + assertType('false', $constNonFalsy == 1); + assertType('false', $constNonFalsy == null); + assertType('true', $constNonFalsy == true); + assertType('false', $constNonFalsy == false); + assertType('false', $constNonFalsy == "1"); + assertType('false', $constNonFalsy == "0"); + assertType('false', $constNonFalsy == []); } /** From a742e51a9f5f984141e30b9dd96cd68ba33ba59f Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Thu, 2 Jan 2025 12:33:30 +0100 Subject: [PATCH 05/36] Assignment not needed --- src/Node/UnreachableStatementNode.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Node/UnreachableStatementNode.php b/src/Node/UnreachableStatementNode.php index 603b7d6f2f..3e2c72e29b 100644 --- a/src/Node/UnreachableStatementNode.php +++ b/src/Node/UnreachableStatementNode.php @@ -14,8 +14,6 @@ final class UnreachableStatementNode extends Stmt implements VirtualNode public function __construct(private Stmt $originalStatement, private array $nextStatements = []) { parent::__construct($originalStatement->getAttributes()); - - $this->nextStatements = $nextStatements; } public function getOriginalStatement(): Stmt From b614f70e0154010f74e36dc9264962facac8122e Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Thu, 2 Jan 2025 17:19:09 +0100 Subject: [PATCH 06/36] GetNonVirtualPropertyHookReadRule - do not report if get hook is not present at all --- .../Properties/GetNonVirtualPropertyHookReadRule.php | 12 +++++++++++- .../data/get-non-virtual-property-hook-read.php | 9 +++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/Rules/Properties/GetNonVirtualPropertyHookReadRule.php b/src/Rules/Properties/GetNonVirtualPropertyHookReadRule.php index 61b1c32a6c..9a2fc917e7 100644 --- a/src/Rules/Properties/GetNonVirtualPropertyHookReadRule.php +++ b/src/Rules/Properties/GetNonVirtualPropertyHookReadRule.php @@ -70,7 +70,17 @@ public function processNode(Node $node, Scope $scope): array $errors = []; foreach ($node->getProperties() as $propertyNode) { - if (!$propertyNode->hasHooks()) { + $hasGetHook = false; + foreach ($propertyNode->getHooks() as $hook) { + if ($hook->name->toLowerString() !== 'get') { + continue; + } + + $hasGetHook = true; + break; + } + + if (!$hasGetHook) { continue; } diff --git a/tests/PHPStan/Rules/Properties/data/get-non-virtual-property-hook-read.php b/tests/PHPStan/Rules/Properties/data/get-non-virtual-property-hook-read.php index 077792c406..76ceabe408 100644 --- a/tests/PHPStan/Rules/Properties/data/get-non-virtual-property-hook-read.php +++ b/tests/PHPStan/Rules/Properties/data/get-non-virtual-property-hook-read.php @@ -46,3 +46,12 @@ class Foo } } + +class GetHookIsNotPresentAtAll +{ + public int $i { + set { + $this->i = $value + 10; + } + } +} From 3119fe90be7f055de2fe6848a14310e0040f8935 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sat, 4 Jan 2025 13:20:07 +0100 Subject: [PATCH 07/36] Update PHP-Parser and BetterReflection --- composer.json | 6 +++--- composer.lock | 40 ++++++++++++++++++++-------------------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/composer.json b/composer.json index 6e2c1cb8c4..6ac10b2742 100644 --- a/composer.json +++ b/composer.json @@ -15,16 +15,16 @@ "hoa/compiler": "3.17.08.08", "hoa/exception": "^1.0", "hoa/file": "1.17.07.11", - "jetbrains/phpstorm-stubs": "dev-master#db675e059f57071e8209c99075128b92d8a727e7", + "jetbrains/phpstorm-stubs": "dev-master#dfcad4524db603bd20bdec3aab1a31c5f5128ea3", "nette/bootstrap": "^3.0", "nette/di": "^3.1.4", "nette/neon": "3.3.4", "nette/php-generator": "3.6.9", "nette/schema": "^1.2.2", "nette/utils": "^3.2.5", - "nikic/php-parser": "^5.3.0", + "nikic/php-parser": "^5.4.0", "ondram/ci-detector": "^3.4.0", - "ondrejmirtes/better-reflection": "6.49.0.0", + "ondrejmirtes/better-reflection": "6.51.0.1", "phpstan/php-8-stubs": "0.4.9", "phpstan/phpdoc-parser": "2.0.0", "psr/http-message": "^1.1", diff --git a/composer.lock b/composer.lock index 880e113451..039bd64a30 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "f3a19a9abe4cf8cfbe9a6a76cf161369", + "content-hash": "f5964f498aa14ffd6b984c06676417aa", "packages": [ { "name": "clue/ndjson-react", @@ -1442,12 +1442,12 @@ "source": { "type": "git", "url": "https://github.com/JetBrains/phpstorm-stubs.git", - "reference": "db675e059f57071e8209c99075128b92d8a727e7" + "reference": "dfcad4524db603bd20bdec3aab1a31c5f5128ea3" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/JetBrains/phpstorm-stubs/zipball/db675e059f57071e8209c99075128b92d8a727e7", - "reference": "db675e059f57071e8209c99075128b92d8a727e7", + "url": "https://api.github.com/repos/JetBrains/phpstorm-stubs/zipball/dfcad4524db603bd20bdec3aab1a31c5f5128ea3", + "reference": "dfcad4524db603bd20bdec3aab1a31c5f5128ea3", "shasum": "" }, "require-dev": { @@ -1482,7 +1482,7 @@ "support": { "source": "https://github.com/JetBrains/phpstorm-stubs/tree/master" }, - "time": "2024-12-23T11:36:45+00:00" + "time": "2025-01-02T13:51:39+00:00" }, { "name": "nette/bootstrap", @@ -2057,16 +2057,16 @@ }, { "name": "nikic/php-parser", - "version": "v5.3.1", + "version": "v5.4.0", "source": { "type": "git", "url": "https://github.com/nikic/PHP-Parser.git", - "reference": "8eea230464783aa9671db8eea6f8c6ac5285794b" + "reference": "447a020a1f875a434d62f2a401f53b82a396e494" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/nikic/PHP-Parser/zipball/8eea230464783aa9671db8eea6f8c6ac5285794b", - "reference": "8eea230464783aa9671db8eea6f8c6ac5285794b", + "url": "https://api.github.com/repos/nikic/PHP-Parser/zipball/447a020a1f875a434d62f2a401f53b82a396e494", + "reference": "447a020a1f875a434d62f2a401f53b82a396e494", "shasum": "" }, "require": { @@ -2109,9 +2109,9 @@ ], "support": { "issues": "https://github.com/nikic/PHP-Parser/issues", - "source": "https://github.com/nikic/PHP-Parser/tree/v5.3.1" + "source": "https://github.com/nikic/PHP-Parser/tree/v5.4.0" }, - "time": "2024-10-08T18:51:32+00:00" + "time": "2024-12-30T11:07:19+00:00" }, { "name": "ondram/ci-detector", @@ -2187,22 +2187,22 @@ }, { "name": "ondrejmirtes/better-reflection", - "version": "6.49.0.0", + "version": "6.51.0.1", "source": { "type": "git", "url": "https://github.com/ondrejmirtes/BetterReflection.git", - "reference": "11abb6b4c9c8b29ee2730a3307ebae77b17fa94d" + "reference": "739c4cc0a01ef79055688606be07cff93551815d" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/ondrejmirtes/BetterReflection/zipball/11abb6b4c9c8b29ee2730a3307ebae77b17fa94d", - "reference": "11abb6b4c9c8b29ee2730a3307ebae77b17fa94d", + "url": "https://api.github.com/repos/ondrejmirtes/BetterReflection/zipball/739c4cc0a01ef79055688606be07cff93551815d", + "reference": "739c4cc0a01ef79055688606be07cff93551815d", "shasum": "" }, "require": { "ext-json": "*", - "jetbrains/phpstorm-stubs": "dev-master#b61d4a5f40c3940be440d85355fef4e2416b8527", - "nikic/php-parser": "^5.3.1", + "jetbrains/phpstorm-stubs": "dev-master#dfcad4524db603bd20bdec3aab1a31c5f5128ea3", + "nikic/php-parser": "^5.4.0", "php": "^7.4 || ^8.0" }, "conflict": { @@ -2212,7 +2212,7 @@ "doctrine/coding-standard": "^12.0.0", "phpstan/phpstan": "^1.10.60", "phpstan/phpstan-phpunit": "^1.3.16", - "phpunit/phpunit": "^11.5.1", + "phpunit/phpunit": "^11.5.2", "rector/rector": "1.2.10" }, "suggest": { @@ -2252,9 +2252,9 @@ ], "description": "Better Reflection - an improved code reflection API", "support": { - "source": "https://github.com/ondrejmirtes/BetterReflection/tree/6.49.0.0" + "source": "https://github.com/ondrejmirtes/BetterReflection/tree/6.51.0.1" }, - "time": "2024-12-20T19:27:15+00:00" + "time": "2025-01-04T12:23:15+00:00" }, { "name": "phpstan/php-8-stubs", From d5a6a6310118249041b7126a19001756c98f125e Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sat, 4 Jan 2025 13:39:35 +0100 Subject: [PATCH 08/36] Simplify code thanks to PHP-Parser update --- .../Php/PhpMethodFromParserNodeReflection.php | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/Reflection/Php/PhpMethodFromParserNodeReflection.php b/src/Reflection/Php/PhpMethodFromParserNodeReflection.php index af7d9a80f4..9e36a632ae 100644 --- a/src/Reflection/Php/PhpMethodFromParserNodeReflection.php +++ b/src/Reflection/Php/PhpMethodFromParserNodeReflection.php @@ -2,7 +2,6 @@ namespace PHPStan\Reflection\Php; -use PhpParser\Modifiers; use PhpParser\Node; use PhpParser\Node\Stmt\ClassMethod; use PHPStan\Reflection\Assertions; @@ -230,7 +229,7 @@ public function isFinal(): TrinaryLogic { $method = $this->getClassMethod(); if ($method instanceof Node\PropertyHook) { - return TrinaryLogic::createFromBoolean((bool) ($method->flags & Modifiers::FINAL)); + return TrinaryLogic::createFromBoolean($method->isFinal()); } return TrinaryLogic::createFromBoolean($method->isFinal() || $this->isFinal); @@ -238,12 +237,7 @@ public function isFinal(): TrinaryLogic public function isFinalByKeyword(): TrinaryLogic { - $method = $this->getClassMethod(); - if ($method instanceof Node\PropertyHook) { - return TrinaryLogic::createFromBoolean((bool) ($method->flags & Modifiers::FINAL)); - } - - return TrinaryLogic::createFromBoolean($method->isFinal()); + return TrinaryLogic::createFromBoolean($this->getClassMethod()->isFinal()); } public function isBuiltin(): bool From 01ade6ed1ffae4d96cdd23a783855e2f2a1e7dfd Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sat, 4 Jan 2025 13:40:50 +0100 Subject: [PATCH 09/36] Simplify code thanks to BetterReflection update --- src/Reflection/Php/PhpPropertyReflection.php | 33 ++----------------- .../AccessPropertiesInAssignRuleTest.php | 4 +-- .../Properties/PropertyAssignRefRuleTest.php | 2 +- 3 files changed, 6 insertions(+), 33 deletions(-) diff --git a/src/Reflection/Php/PhpPropertyReflection.php b/src/Reflection/Php/PhpPropertyReflection.php index 3ff948eb3a..1284d4a699 100644 --- a/src/Reflection/Php/PhpPropertyReflection.php +++ b/src/Reflection/Php/PhpPropertyReflection.php @@ -234,14 +234,7 @@ public function isAbstract(): TrinaryLogic public function isFinal(): TrinaryLogic { - if ($this->reflection->isFinal()) { - return TrinaryLogic::createYes(); - } - if ($this->reflection->isPrivate()) { - return TrinaryLogic::createNo(); - } - - return TrinaryLogic::createFromBoolean($this->isPrivateSet()); + return TrinaryLogic::createFromBoolean($this->reflection->isFinal()); } public function isVirtual(): TrinaryLogic @@ -277,32 +270,12 @@ public function getHook(string $hookType): ExtendedMethodReflection public function isProtectedSet(): bool { - if ($this->reflection->isProtectedSet()) { - return true; - } - - if ($this->isReadOnly()) { - return !$this->isPrivate() && !$this->reflection->isPrivateSet(); - } - - return false; + return $this->reflection->isProtectedSet(); } public function isPrivateSet(): bool { - if ($this->reflection->isPrivateSet()) { - return true; - } - - if ($this->reflection->isProtectedSet()) { - return false; - } - - if ($this->isReadOnly()) { - return $this->isPrivate(); - } - - return false; + return $this->reflection->isPrivateSet(); } } diff --git a/tests/PHPStan/Rules/Properties/AccessPropertiesInAssignRuleTest.php b/tests/PHPStan/Rules/Properties/AccessPropertiesInAssignRuleTest.php index 53f852ae8e..d1e43fd0e1 100644 --- a/tests/PHPStan/Rules/Properties/AccessPropertiesInAssignRuleTest.php +++ b/tests/PHPStan/Rules/Properties/AccessPropertiesInAssignRuleTest.php @@ -150,11 +150,11 @@ public function testAsymmetricVisibility(): void 70, ], [ - 'Assign to protected(set) property WriteAsymmetricVisibility\ReadonlyProps::$b.', + 'Access to protected property WriteAsymmetricVisibility\ReadonlyProps::$b.', 71, ], [ - 'Assign to private(set) property WriteAsymmetricVisibility\ReadonlyProps::$c.', + 'Access to private property WriteAsymmetricVisibility\ReadonlyProps::$c.', 72, ], [ diff --git a/tests/PHPStan/Rules/Properties/PropertyAssignRefRuleTest.php b/tests/PHPStan/Rules/Properties/PropertyAssignRefRuleTest.php index 7f24e6f628..3d78abbee9 100644 --- a/tests/PHPStan/Rules/Properties/PropertyAssignRefRuleTest.php +++ b/tests/PHPStan/Rules/Properties/PropertyAssignRefRuleTest.php @@ -26,7 +26,7 @@ public function testRule(): void $this->analyse([__DIR__ . '/data/property-assign-ref.php'], [ [ - 'Property PropertyAssignRef\Foo::$foo with private(set) visibility is assigned by reference.', + 'Property PropertyAssignRef\Foo::$foo with private visibility is assigned by reference.', 25, ], [ From cc83891d25da951ada41ad1dba4995bf70827554 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sat, 4 Jan 2025 13:53:37 +0100 Subject: [PATCH 10/36] Always call processStmtNodes on property hook body thanks to `getStmts()` --- src/Analyser/NodeScopeResolver.php | 96 +++++++++++++++------------- src/Parser/LineAttributesVisitor.php | 28 ++++++++ 2 files changed, 80 insertions(+), 44 deletions(-) create mode 100644 src/Parser/LineAttributesVisitor.php diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index fb72cfeb6b..eb05b36f3b 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -124,6 +124,7 @@ use PHPStan\Parser\ArrowFunctionArgVisitor; use PHPStan\Parser\ClosureArgVisitor; use PHPStan\Parser\ImmediatelyInvokedClosureVisitor; +use PHPStan\Parser\LineAttributesVisitor; use PHPStan\Parser\Parser; use PHPStan\Parser\PropertyHookNameVisitor; use PHPStan\Php\PhpVersion; @@ -4830,54 +4831,61 @@ private function processPropertyHooks( $hook, ), $hookScope); + $stmts = $hook->getStmts(); + if ($stmts === null) { + return; + } + if ($hook->body instanceof Expr) { - $this->processExprNode($stmt, $hook->body, $hookScope, $nodeCallback, ExpressionContext::createTopLevel()); - $nodeCallback(new PropertyAssignNode(new PropertyFetch(new Variable('this'), $propertyName, $hook->body->getAttributes()), $hook->body, false), $hookScope); - } elseif (is_array($hook->body)) { - $gatheredReturnStatements = []; - $executionEnds = []; - $methodImpurePoints = []; - $statementResult = $this->processStmtNodes(new PropertyHookStatementNode($hook), $hook->body, $hookScope, static function (Node $node, Scope $scope) use ($nodeCallback, $hookScope, &$gatheredReturnStatements, &$executionEnds, &$hookImpurePoints): void { - $nodeCallback($node, $scope); - if ($scope->getFunction() !== $hookScope->getFunction()) { - return; - } - if ($scope->isInAnonymousFunction()) { - return; - } - if ($node instanceof PropertyAssignNode) { - $hookImpurePoints[] = new ImpurePoint( - $scope, - $node, - 'propertyAssign', - 'property assignment', - true, - ); - return; - } - if ($node instanceof ExecutionEndNode) { - $executionEnds[] = $node; - return; - } - if (!$node instanceof Return_) { - return; - } + // enrich attributes of nodes in short hook body statements + $traverser = new NodeTraverser( + new LineAttributesVisitor($hook->body->getStartLine(), $hook->body->getEndLine()), + ); + $traverser->traverse($stmts); + } - $gatheredReturnStatements[] = new ReturnStatement($scope, $node); - }, StatementContext::createTopLevel()); + $gatheredReturnStatements = []; + $executionEnds = []; + $methodImpurePoints = []; + $statementResult = $this->processStmtNodes(new PropertyHookStatementNode($hook), $stmts, $hookScope, static function (Node $node, Scope $scope) use ($nodeCallback, $hookScope, &$gatheredReturnStatements, &$executionEnds, &$hookImpurePoints): void { + $nodeCallback($node, $scope); + if ($scope->getFunction() !== $hookScope->getFunction()) { + return; + } + if ($scope->isInAnonymousFunction()) { + return; + } + if ($node instanceof PropertyAssignNode) { + $hookImpurePoints[] = new ImpurePoint( + $scope, + $node, + 'propertyAssign', + 'property assignment', + true, + ); + return; + } + if ($node instanceof ExecutionEndNode) { + $executionEnds[] = $node; + return; + } + if (!$node instanceof Return_) { + return; + } - $nodeCallback(new PropertyHookReturnStatementsNode( - $hook, - $gatheredReturnStatements, - $statementResult, - $executionEnds, - array_merge($statementResult->getImpurePoints(), $methodImpurePoints), - $classReflection, - $hookReflection, - $propertyReflection, - ), $hookScope); - } + $gatheredReturnStatements[] = new ReturnStatement($scope, $node); + }, StatementContext::createTopLevel()); + $nodeCallback(new PropertyHookReturnStatementsNode( + $hook, + $gatheredReturnStatements, + $statementResult, + $executionEnds, + array_merge($statementResult->getImpurePoints(), $methodImpurePoints), + $classReflection, + $hookReflection, + $propertyReflection, + ), $hookScope); } } diff --git a/src/Parser/LineAttributesVisitor.php b/src/Parser/LineAttributesVisitor.php new file mode 100644 index 0000000000..53f3f3f50f --- /dev/null +++ b/src/Parser/LineAttributesVisitor.php @@ -0,0 +1,28 @@ +getStartLine() === -1) { + $node->setAttribute('startLine', $this->startLine); + } + + if ($node->getEndLine() === -1) { + $node->setAttribute('endLine', $this->endLine); + } + + return $node; + } + +} From b2c257625fe9a3544fdcaafde72cc7a6b0b5c870 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sat, 4 Jan 2025 14:06:57 +0100 Subject: [PATCH 11/36] PropertyHookReturnStatementsNode is invoked for short body hooks --- ...ckedExceptionInPropertyHookThrowsRuleTest.php | 4 ++++ ...ropertyHookWithExplicitThrowPointRuleTest.php | 16 ++++++++++++++++ .../TooWidePropertyHookThrowTypeRuleTest.php | 4 ++++ .../missing-exception-property-hook-throws.php | 4 ++++ .../data/throws-void-property-hook.php | 7 +++++++ .../data/too-wide-throws-property-hook.php | 5 +++++ .../set-non-virtual-property-hook-assign.php | 7 +++++++ 7 files changed, 47 insertions(+) diff --git a/tests/PHPStan/Rules/Exceptions/MissingCheckedExceptionInPropertyHookThrowsRuleTest.php b/tests/PHPStan/Rules/Exceptions/MissingCheckedExceptionInPropertyHookThrowsRuleTest.php index e7f6130d67..cf01fb3852 100644 --- a/tests/PHPStan/Rules/Exceptions/MissingCheckedExceptionInPropertyHookThrowsRuleTest.php +++ b/tests/PHPStan/Rules/Exceptions/MissingCheckedExceptionInPropertyHookThrowsRuleTest.php @@ -45,6 +45,10 @@ public function testRule(): void 'Get hook for property MissingExceptionPropertyHookThrows\Foo::$m throws checked exception InvalidArgumentException but it\'s missing from the PHPDoc @throws tag.', 38, ], + [ + 'Get hook for property MissingExceptionPropertyHookThrows\Foo::$n throws checked exception InvalidArgumentException but it\'s missing from the PHPDoc @throws tag.', + 43, + ], ]); } diff --git a/tests/PHPStan/Rules/Exceptions/ThrowsVoidPropertyHookWithExplicitThrowPointRuleTest.php b/tests/PHPStan/Rules/Exceptions/ThrowsVoidPropertyHookWithExplicitThrowPointRuleTest.php index fecb9cfdc5..6072db088b 100644 --- a/tests/PHPStan/Rules/Exceptions/ThrowsVoidPropertyHookWithExplicitThrowPointRuleTest.php +++ b/tests/PHPStan/Rules/Exceptions/ThrowsVoidPropertyHookWithExplicitThrowPointRuleTest.php @@ -44,6 +44,10 @@ public function dataRule(): array 'Get hook for property ThrowsVoidPropertyHook\Foo::$i throws exception ThrowsVoidPropertyHook\MyException but the PHPDoc contains @throws void.', 18, ], + [ + 'Get hook for property ThrowsVoidPropertyHook\Foo::$j throws exception ThrowsVoidPropertyHook\MyException but the PHPDoc contains @throws void.', + 26, + ], ], ], [ @@ -59,6 +63,10 @@ public function dataRule(): array 'Get hook for property ThrowsVoidPropertyHook\Foo::$i throws exception ThrowsVoidPropertyHook\MyException but the PHPDoc contains @throws void.', 18, ], + [ + 'Get hook for property ThrowsVoidPropertyHook\Foo::$j throws exception ThrowsVoidPropertyHook\MyException but the PHPDoc contains @throws void.', + 26, + ], ], ], [ @@ -69,6 +77,10 @@ public function dataRule(): array 'Get hook for property ThrowsVoidPropertyHook\Foo::$i throws exception ThrowsVoidPropertyHook\MyException but the PHPDoc contains @throws void.', 18, ], + [ + 'Get hook for property ThrowsVoidPropertyHook\Foo::$j throws exception ThrowsVoidPropertyHook\MyException but the PHPDoc contains @throws void.', + 26, + ], ], ], [ @@ -79,6 +91,10 @@ public function dataRule(): array 'Get hook for property ThrowsVoidPropertyHook\Foo::$i throws exception ThrowsVoidPropertyHook\MyException but the PHPDoc contains @throws void.', 18, ], + [ + 'Get hook for property ThrowsVoidPropertyHook\Foo::$j throws exception ThrowsVoidPropertyHook\MyException but the PHPDoc contains @throws void.', + 26, + ], ], ], ]; diff --git a/tests/PHPStan/Rules/Exceptions/TooWidePropertyHookThrowTypeRuleTest.php b/tests/PHPStan/Rules/Exceptions/TooWidePropertyHookThrowTypeRuleTest.php index 0c3d0f75a1..6fed25e6c2 100644 --- a/tests/PHPStan/Rules/Exceptions/TooWidePropertyHookThrowTypeRuleTest.php +++ b/tests/PHPStan/Rules/Exceptions/TooWidePropertyHookThrowTypeRuleTest.php @@ -43,6 +43,10 @@ public function testRule(): void 'Get hook for property TooWideThrowsPropertyHook\Foo::$j has DomainException in PHPDoc @throws tag but it\'s not thrown.', 76, ], + [ + 'Get hook for property TooWideThrowsPropertyHook\Foo::$k has DomainException in PHPDoc @throws tag but it\'s not thrown.', + 83, + ], ]); } diff --git a/tests/PHPStan/Rules/Exceptions/data/missing-exception-property-hook-throws.php b/tests/PHPStan/Rules/Exceptions/data/missing-exception-property-hook-throws.php index d9fba8d0f1..773f849d74 100644 --- a/tests/PHPStan/Rules/Exceptions/data/missing-exception-property-hook-throws.php +++ b/tests/PHPStan/Rules/Exceptions/data/missing-exception-property-hook-throws.php @@ -39,4 +39,8 @@ class Foo } } + public int $n { + get => throw new \InvalidArgumentException(); // error + } + } diff --git a/tests/PHPStan/Rules/Exceptions/data/throws-void-property-hook.php b/tests/PHPStan/Rules/Exceptions/data/throws-void-property-hook.php index 82c4c2381f..08e3f10940 100644 --- a/tests/PHPStan/Rules/Exceptions/data/throws-void-property-hook.php +++ b/tests/PHPStan/Rules/Exceptions/data/throws-void-property-hook.php @@ -19,4 +19,11 @@ class Foo } } + public int $j { + /** + * @throws void + */ + get => throw new MyException(); + } + } diff --git a/tests/PHPStan/Rules/Exceptions/data/too-wide-throws-property-hook.php b/tests/PHPStan/Rules/Exceptions/data/too-wide-throws-property-hook.php index 92998bafa4..6cf4b55072 100644 --- a/tests/PHPStan/Rules/Exceptions/data/too-wide-throws-property-hook.php +++ b/tests/PHPStan/Rules/Exceptions/data/too-wide-throws-property-hook.php @@ -78,4 +78,9 @@ class Foo } } + public int $k { + /** @throws \DomainException */ + get => 11; // error - DomainException unused + } + } diff --git a/tests/PHPStan/Rules/Properties/data/set-non-virtual-property-hook-assign.php b/tests/PHPStan/Rules/Properties/data/set-non-virtual-property-hook-assign.php index ffc0e559f6..56133fb556 100644 --- a/tests/PHPStan/Rules/Properties/data/set-non-virtual-property-hook-assign.php +++ b/tests/PHPStan/Rules/Properties/data/set-non-virtual-property-hook-assign.php @@ -65,4 +65,11 @@ class Foo } } + public int $k5 { + get { + return $this->k4 + 1; + } + set => $value; // short body always assigns + } + } From 947e74e22356db4edb1ea7950367dd968523cd82 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sat, 4 Jan 2025 14:13:22 +0100 Subject: [PATCH 12/36] ShortGetPropertyHookReturnTypeRule is no longer needed --- conf/config.level3.neon | 1 - .../ShortGetPropertyHookReturnTypeRule.php | 75 ------------------- .../Rules/Methods/ReturnTypeRuleTest.php | 33 ++++++++ .../data/short-get-property-hook-return.php | 0 ...ShortGetPropertyHookReturnTypeRuleTest.php | 57 -------------- 5 files changed, 33 insertions(+), 133 deletions(-) delete mode 100644 src/Rules/Properties/ShortGetPropertyHookReturnTypeRule.php rename tests/PHPStan/Rules/{Properties => Methods}/data/short-get-property-hook-return.php (100%) delete mode 100644 tests/PHPStan/Rules/Properties/ShortGetPropertyHookReturnTypeRuleTest.php diff --git a/conf/config.level3.neon b/conf/config.level3.neon index c946a5ee3f..4e5f80c5ef 100644 --- a/conf/config.level3.neon +++ b/conf/config.level3.neon @@ -22,7 +22,6 @@ rules: - PHPStan\Rules\Properties\ReadOnlyPropertyAssignRefRule - PHPStan\Rules\Properties\ReadOnlyByPhpDocPropertyAssignRefRule - PHPStan\Rules\Properties\SetNonVirtualPropertyHookAssignRule - - PHPStan\Rules\Properties\ShortGetPropertyHookReturnTypeRule - PHPStan\Rules\Properties\TypesAssignedToPropertiesRule - PHPStan\Rules\Variables\ParameterOutAssignedTypeRule - PHPStan\Rules\Variables\ParameterOutExecutionEndTypeRule diff --git a/src/Rules/Properties/ShortGetPropertyHookReturnTypeRule.php b/src/Rules/Properties/ShortGetPropertyHookReturnTypeRule.php deleted file mode 100644 index cf30777b19..0000000000 --- a/src/Rules/Properties/ShortGetPropertyHookReturnTypeRule.php +++ /dev/null @@ -1,75 +0,0 @@ - - */ -final class ShortGetPropertyHookReturnTypeRule implements Rule -{ - - public function __construct(private FunctionReturnTypeCheck $returnTypeCheck) - { - } - - public function getNodeType(): string - { - return InPropertyHookNode::class; - } - - public function processNode(Node $node, Scope $scope): array - { - // return statements in long property hook bodies are checked by Methods\ReturnTypeRule - // short set property hook type is checked by TypesAssignedToPropertiesRule - $hookReflection = $node->getHookReflection(); - if ($hookReflection->getPropertyHookName() !== 'get') { - return []; - } - - $originalHookNode = $node->getOriginalNode(); - $hookBody = $originalHookNode->body; - if (!$hookBody instanceof Node\Expr) { - return []; - } - - $methodDescription = sprintf( - 'Get hook for property %s::$%s', - $hookReflection->getDeclaringClass()->getDisplayName(), - $hookReflection->getHookedPropertyName(), - ); - - $returnType = $hookReflection->getReturnType(); - - return $this->returnTypeCheck->checkReturnType( - $scope, - $returnType, - $hookBody, - $node, - sprintf( - '%s should return %%s but empty return statement found.', - $methodDescription, - ), - sprintf( - '%s with return type void returns %%s but should not return anything.', - $methodDescription, - ), - sprintf( - '%s should return %%s but returns %%s.', - $methodDescription, - ), - sprintf( - '%s should never return but return statement found.', - $methodDescription, - ), - $hookReflection->isGenerator(), - ); - } - -} diff --git a/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php b/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php index c524382174..98bb11b0b5 100644 --- a/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php +++ b/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php @@ -1138,4 +1138,37 @@ public function testPropertyHooks(): void ]); } + public function testShortGetPropertyHook(): void + { + if (PHP_VERSION_ID < 80400) { + $this->markTestSkipped('Test requires PHP 8.4.'); + } + + $this->analyse([__DIR__ . '/data/short-get-property-hook-return.php'], [ + [ + 'Get hook for property ShortGetPropertyHookReturn\Foo::$i should return int but returns string.', + 9, + ], + [ + 'Get hook for property ShortGetPropertyHookReturn\Foo::$s should return non-empty-string but returns \'\'.', + 18, + ], + [ + 'Get hook for property ShortGetPropertyHookReturn\GenericFoo::$a should return T of ShortGetPropertyHookReturn\Foo but returns ShortGetPropertyHookReturn\Foo.', + 36, + 'Type ShortGetPropertyHookReturn\Foo is not always the same as T. It breaks the contract for some argument types, typically subtypes.', + ], + [ + 'Get hook for property ShortGetPropertyHookReturn\GenericFoo::$b should return T of ShortGetPropertyHookReturn\Foo but returns ShortGetPropertyHookReturn\Foo.', + 50, + 'Type ShortGetPropertyHookReturn\Foo is not always the same as T. It breaks the contract for some argument types, typically subtypes.', + ], + [ + 'Get hook for property ShortGetPropertyHookReturn\GenericFoo::$c should return T of ShortGetPropertyHookReturn\Foo but returns ShortGetPropertyHookReturn\Foo.', + 59, + 'Type ShortGetPropertyHookReturn\Foo is not always the same as T. It breaks the contract for some argument types, typically subtypes.', + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Properties/data/short-get-property-hook-return.php b/tests/PHPStan/Rules/Methods/data/short-get-property-hook-return.php similarity index 100% rename from tests/PHPStan/Rules/Properties/data/short-get-property-hook-return.php rename to tests/PHPStan/Rules/Methods/data/short-get-property-hook-return.php diff --git a/tests/PHPStan/Rules/Properties/ShortGetPropertyHookReturnTypeRuleTest.php b/tests/PHPStan/Rules/Properties/ShortGetPropertyHookReturnTypeRuleTest.php deleted file mode 100644 index 8a318db7ed..0000000000 --- a/tests/PHPStan/Rules/Properties/ShortGetPropertyHookReturnTypeRuleTest.php +++ /dev/null @@ -1,57 +0,0 @@ - - */ -final class ShortGetPropertyHookReturnTypeRuleTest extends RuleTestCase -{ - - protected function getRule(): Rule - { - return new ShortGetPropertyHookReturnTypeRule( - new FunctionReturnTypeCheck(new RuleLevelHelper($this->createReflectionProvider(), true, false, true, true, false, false)), - ); - } - - public function testRule(): void - { - if (PHP_VERSION_ID < 80400) { - $this->markTestSkipped('Test requires PHP 8.4.'); - } - - $this->analyse([__DIR__ . '/data/short-get-property-hook-return.php'], [ - [ - 'Get hook for property ShortGetPropertyHookReturn\Foo::$i should return int but returns string.', - 9, - ], - [ - 'Get hook for property ShortGetPropertyHookReturn\Foo::$s should return non-empty-string but returns \'\'.', - 18, - ], - [ - 'Get hook for property ShortGetPropertyHookReturn\GenericFoo::$a should return T of ShortGetPropertyHookReturn\Foo but returns ShortGetPropertyHookReturn\Foo.', - 36, - 'Type ShortGetPropertyHookReturn\Foo is not always the same as T. It breaks the contract for some argument types, typically subtypes.', - ], - [ - 'Get hook for property ShortGetPropertyHookReturn\GenericFoo::$b should return T of ShortGetPropertyHookReturn\Foo but returns ShortGetPropertyHookReturn\Foo.', - 50, - 'Type ShortGetPropertyHookReturn\Foo is not always the same as T. It breaks the contract for some argument types, typically subtypes.', - ], - [ - 'Get hook for property ShortGetPropertyHookReturn\GenericFoo::$c should return T of ShortGetPropertyHookReturn\Foo but returns ShortGetPropertyHookReturn\Foo.', - 59, - 'Type ShortGetPropertyHookReturn\Foo is not always the same as T. It breaks the contract for some argument types, typically subtypes.', - ], - ]); - } - -} From 317a9974e295838cb5083527f6061965ab70dca5 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sat, 4 Jan 2025 14:13:53 +0100 Subject: [PATCH 13/36] PropertyHookNameVisitor is no longer needed, PHP-parser comes with `propertyName` attribute --- conf/config.neon | 5 -- src/Analyser/NodeScopeResolver.php | 3 +- src/Parser/CleaningVisitor.php | 2 +- src/Parser/PropertyHookNameVisitor.php | 60 --------------------- src/Parser/SimpleParser.php | 2 - src/Reflection/InitializerExprContext.php | 5 +- src/Type/FileTypeMapper.php | 11 ++-- tests/PHPStan/Parser/CleaningParserTest.php | 1 - 8 files changed, 9 insertions(+), 80 deletions(-) delete mode 100644 src/Parser/PropertyHookNameVisitor.php diff --git a/conf/config.neon b/conf/config.neon index b2d222ebb3..c3c2c7bded 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -320,11 +320,6 @@ services: tags: - phpstan.parser.richParserNodeVisitor - - - class: PHPStan\Parser\PropertyHookNameVisitor - tags: - - phpstan.parser.richParserNodeVisitor - - class: PHPStan\Node\Printer\ExprPrinter diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index eb05b36f3b..ab61ff1486 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -126,7 +126,6 @@ use PHPStan\Parser\ImmediatelyInvokedClosureVisitor; use PHPStan\Parser\LineAttributesVisitor; use PHPStan\Parser\Parser; -use PHPStan\Parser\PropertyHookNameVisitor; use PHPStan\Php\PhpVersion; use PHPStan\PhpDoc\PhpDocInheritanceResolver; use PHPStan\PhpDoc\ResolvedPhpDocBlock; @@ -6436,7 +6435,7 @@ public function getPhpDocs(Scope $scope, Node\FunctionLike|Node\Stmt\Property $n } elseif ($node instanceof Node\Stmt\Function_) { $functionName = trim($scope->getNamespace() . '\\' . $node->name->name, '\\'); } elseif ($node instanceof Node\PropertyHook) { - $propertyName = $node->getAttribute(PropertyHookNameVisitor::ATTRIBUTE_NAME); + $propertyName = $node->getAttribute('propertyName'); if ($propertyName !== null) { $functionName = sprintf('$%s::%s', $propertyName, $node->name->toString()); } diff --git a/src/Parser/CleaningVisitor.php b/src/Parser/CleaningVisitor.php index eb9492f3cd..80f5b2f594 100644 --- a/src/Parser/CleaningVisitor.php +++ b/src/Parser/CleaningVisitor.php @@ -37,7 +37,7 @@ public function enterNode(Node $node): ?Node } if ($node instanceof Node\PropertyHook && is_array($node->body)) { - $propertyName = $node->getAttribute(PropertyHookNameVisitor::ATTRIBUTE_NAME); + $propertyName = $node->getAttribute('propertyName'); if ($propertyName !== null) { $node->body = $this->keepVariadicsAndYields($node->body, $propertyName); return $node; diff --git a/src/Parser/PropertyHookNameVisitor.php b/src/Parser/PropertyHookNameVisitor.php deleted file mode 100644 index 5a49a70915..0000000000 --- a/src/Parser/PropertyHookNameVisitor.php +++ /dev/null @@ -1,60 +0,0 @@ -hooks) === 0) { - return null; - } - - $propertyName = null; - foreach ($node->props as $prop) { - $propertyName = $prop->name->toString(); - break; - } - - if (!isset($propertyName)) { - return null; - } - - foreach ($node->hooks as $hook) { - $hook->setAttribute(self::ATTRIBUTE_NAME, $propertyName); - } - - return $node; - } - - if ($node instanceof Node\Param) { - if (count($node->hooks) === 0) { - return null; - } - if (!$node->var instanceof Node\Expr\Variable) { - return null; - } - if (!is_string($node->var->name)) { - return null; - } - - foreach ($node->hooks as $hook) { - $hook->setAttribute(self::ATTRIBUTE_NAME, $node->var->name); - } - - return $node; - } - - return null; - } - -} diff --git a/src/Parser/SimpleParser.php b/src/Parser/SimpleParser.php index 71bab19964..8fbd112742 100644 --- a/src/Parser/SimpleParser.php +++ b/src/Parser/SimpleParser.php @@ -17,7 +17,6 @@ public function __construct( private NameResolver $nameResolver, private VariadicMethodsVisitor $variadicMethodsVisitor, private VariadicFunctionsVisitor $variadicFunctionsVisitor, - private PropertyHookNameVisitor $propertyHookNameVisitor, ) { } @@ -53,7 +52,6 @@ public function parseString(string $sourceCode): array $nodeTraverser->addVisitor($this->nameResolver); $nodeTraverser->addVisitor($this->variadicMethodsVisitor); $nodeTraverser->addVisitor($this->variadicFunctionsVisitor); - $nodeTraverser->addVisitor($this->propertyHookNameVisitor); /** @var array */ return $nodeTraverser->traverse($nodes); diff --git a/src/Reflection/InitializerExprContext.php b/src/Reflection/InitializerExprContext.php index eb64cacdbb..650408f349 100644 --- a/src/Reflection/InitializerExprContext.php +++ b/src/Reflection/InitializerExprContext.php @@ -9,7 +9,6 @@ use PHPStan\BetterReflection\Reflection\Adapter\ReflectionFunction; use PHPStan\BetterReflection\Reflection\Adapter\ReflectionParameter; use PHPStan\BetterReflection\Reflection\ReflectionConstant; -use PHPStan\Parser\PropertyHookNameVisitor; use PHPStan\Reflection\Php\PhpMethodFromParserNodeReflection; use PHPStan\ShouldNotHappenException; use function array_slice; @@ -144,7 +143,7 @@ public static function fromStubParameter( } elseif ($function instanceof ClassMethod) { $functionName = $function->name->toString(); } elseif ($function instanceof PropertyHook) { - $propertyName = $function->getAttribute(PropertyHookNameVisitor::ATTRIBUTE_NAME); + $propertyName = $function->getAttribute('propertyName'); $functionName = sprintf('$%s::%s', $propertyName, $function->name->toString()); } @@ -152,7 +151,7 @@ public static function fromStubParameter( if ($function instanceof ClassMethod && $className !== null) { $methodName = sprintf('%s::%s', $className, $function->name->toString()); } elseif ($function instanceof PropertyHook) { - $propertyName = $function->getAttribute(PropertyHookNameVisitor::ATTRIBUTE_NAME); + $propertyName = $function->getAttribute('propertyName'); $methodName = sprintf('%s::$%s::%s', $className, $propertyName, $function->name->toString()); } elseif ($function instanceof Function_ && $function->namespacedName !== null) { $methodName = $function->namespacedName->toString(); diff --git a/src/Type/FileTypeMapper.php b/src/Type/FileTypeMapper.php index 3cc5e6c62e..614ef44910 100644 --- a/src/Type/FileTypeMapper.php +++ b/src/Type/FileTypeMapper.php @@ -9,7 +9,6 @@ use PHPStan\Broker\AnonymousClassNameHelper; use PHPStan\File\FileHelper; use PHPStan\Parser\Parser; -use PHPStan\Parser\PropertyHookNameVisitor; use PHPStan\PhpDoc\PhpDocNodeResolver; use PHPStan\PhpDoc\PhpDocStringResolver; use PHPStan\PhpDoc\ResolvedPhpDocBlock; @@ -281,7 +280,7 @@ function (Node $node) use ($fileName, $lookForTrait, &$traitFound, $traitMethodA } elseif ($node instanceof Node\Stmt\Function_) { $functionStack[] = ltrim(sprintf('%s\\%s', $namespace, $node->name->name), '\\'); } elseif ($node instanceof Node\PropertyHook) { - $propertyName = $node->getAttribute(PropertyHookNameVisitor::ATTRIBUTE_NAME); + $propertyName = $node->getAttribute('propertyName'); if ($propertyName !== null) { $functionStack[] = sprintf('$%s::%s', $propertyName, $node->name->toString()); } @@ -299,7 +298,7 @@ function (Node $node) use ($fileName, $lookForTrait, &$traitFound, $traitMethodA return null; } elseif ($node instanceof Node\PropertyHook) { - $propertyName = $node->getAttribute(PropertyHookNameVisitor::ATTRIBUTE_NAME); + $propertyName = $node->getAttribute('propertyName'); if ($propertyName !== null) { $docComment = GetLastDocComment::forNode($node); if ($docComment !== null) { @@ -394,7 +393,7 @@ static function (Node $node) use (&$namespace, &$functionStack, &$classStack): v array_pop($functionStack); } elseif ($node instanceof Node\PropertyHook) { - $propertyName = $node->getAttribute(PropertyHookNameVisitor::ATTRIBUTE_NAME); + $propertyName = $node->getAttribute('propertyName'); if ($propertyName !== null) { if (count($functionStack) === 0) { throw new ShouldNotHappenException(); @@ -503,7 +502,7 @@ function (Node $node) use ($fileName, $lookForTrait, $phpDocNodeMap, &$traitFoun } elseif ($node instanceof Node\Stmt\Function_) { $functionStack[] = ltrim(sprintf('%s\\%s', $namespace, $node->name->name), '\\'); } elseif ($node instanceof Node\PropertyHook) { - $propertyName = $node->getAttribute(PropertyHookNameVisitor::ATTRIBUTE_NAME); + $propertyName = $node->getAttribute('propertyName'); if ($propertyName !== null) { $functionStack[] = sprintf('$%s::%s', $propertyName, $node->name->toString()); } @@ -742,7 +741,7 @@ static function (Node $node, $callbackResult) use (&$namespace, &$functionStack, array_pop($functionStack); } elseif ($node instanceof Node\PropertyHook) { - $propertyName = $node->getAttribute(PropertyHookNameVisitor::ATTRIBUTE_NAME); + $propertyName = $node->getAttribute('propertyName'); if ($propertyName !== null) { if (count($functionStack) === 0) { throw new ShouldNotHappenException(); diff --git a/tests/PHPStan/Parser/CleaningParserTest.php b/tests/PHPStan/Parser/CleaningParserTest.php index 8dbf569171..69c0e8af10 100644 --- a/tests/PHPStan/Parser/CleaningParserTest.php +++ b/tests/PHPStan/Parser/CleaningParserTest.php @@ -75,7 +75,6 @@ public function testParse( new NameResolver(), new VariadicMethodsVisitor(), new VariadicFunctionsVisitor(), - new PropertyHookNameVisitor(), ), new PhpVersion($phpVersionId), ); From 17d6b2938621a42bc1f49d5d05eeadc06fde98aa Mon Sep 17 00:00:00 2001 From: Martin Herndl Date: Thu, 28 Nov 2024 10:56:02 +0100 Subject: [PATCH 14/36] Enforce safe constructor overrides with `@phpstan-consistent-constructor` --- conf/config.neon | 3 ++ src/PhpDoc/StubValidator.php | 11 +++- .../Methods/ConsistentConstructorRule.php | 7 ++- .../MethodVisibilityComparisonHelper.php | 51 +++++++++++++++++++ src/Rules/Methods/OverridingMethodRule.php | 28 +--------- .../Methods/ConsistentConstructorRuleTest.php | 15 +++++- .../Rules/Methods/MethodSignatureRuleTest.php | 1 + .../Methods/OverridingMethodRuleTest.php | 1 + .../PHPStan/Rules/Methods/data/bug-12137.php | 23 +++++++++ 9 files changed, 111 insertions(+), 29 deletions(-) create mode 100755 src/Rules/Methods/MethodVisibilityComparisonHelper.php create mode 100755 tests/PHPStan/Rules/Methods/data/bug-12137.php diff --git a/conf/config.neon b/conf/config.neon index c3c2c7bded..b9c52c9a57 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -986,6 +986,9 @@ services: - class: PHPStan\Rules\Methods\MethodParameterComparisonHelper + - + class: PHPStan\Rules\Methods\MethodVisibilityComparisonHelper + - class: PHPStan\Rules\MissingTypehintCheck arguments: diff --git a/src/PhpDoc/StubValidator.php b/src/PhpDoc/StubValidator.php index 33fde1e46e..0374aa268f 100644 --- a/src/PhpDoc/StubValidator.php +++ b/src/PhpDoc/StubValidator.php @@ -68,6 +68,7 @@ use PHPStan\Rules\Methods\ExistingClassesInTypehintsRule; use PHPStan\Rules\Methods\MethodParameterComparisonHelper; use PHPStan\Rules\Methods\MethodSignatureRule; +use PHPStan\Rules\Methods\MethodVisibilityComparisonHelper; use PHPStan\Rules\Methods\MissingMethodParameterTypehintRule; use PHPStan\Rules\Methods\MissingMethodReturnTypehintRule; use PHPStan\Rules\Methods\MissingMethodSelfOutTypeRule; @@ -209,7 +210,15 @@ private function getRuleRegistry(Container $container): RuleRegistry new ExistingClassesInTypehintsRule($functionDefinitionCheck), new \PHPStan\Rules\Functions\ExistingClassesInTypehintsRule($functionDefinitionCheck), new ExistingClassesInPropertiesRule($reflectionProvider, $classNameCheck, $unresolvableTypeHelper, $phpVersion, true, false), - new OverridingMethodRule($phpVersion, new MethodSignatureRule($phpClassReflectionExtension, true, true), true, new MethodParameterComparisonHelper($phpVersion), $phpClassReflectionExtension, $container->getParameter('checkMissingOverrideMethodAttribute')), + new OverridingMethodRule( + $phpVersion, + new MethodSignatureRule($phpClassReflectionExtension, true, true), + true, + new MethodParameterComparisonHelper($phpVersion), + new MethodVisibilityComparisonHelper(), + $phpClassReflectionExtension, + $container->getParameter('checkMissingOverrideMethodAttribute'), + ), new DuplicateDeclarationRule(), new LocalTypeAliasesRule($localTypeAliasesCheck), new LocalTypeTraitAliasesRule($localTypeAliasesCheck, $reflectionProvider), diff --git a/src/Rules/Methods/ConsistentConstructorRule.php b/src/Rules/Methods/ConsistentConstructorRule.php index ab8553b5cc..16226a1074 100644 --- a/src/Rules/Methods/ConsistentConstructorRule.php +++ b/src/Rules/Methods/ConsistentConstructorRule.php @@ -7,6 +7,7 @@ use PHPStan\Node\InClassMethodNode; use PHPStan\Reflection\Dummy\DummyConstructorReflection; use PHPStan\Rules\Rule; +use function array_merge; use function strtolower; /** @implements Rule */ @@ -15,6 +16,7 @@ final class ConsistentConstructorRule implements Rule public function __construct( private MethodParameterComparisonHelper $methodParameterComparisonHelper, + private MethodVisibilityComparisonHelper $methodVisibilityComparisonHelper, ) { } @@ -47,7 +49,10 @@ public function processNode(Node $node, Scope $scope): array return []; } - return $this->methodParameterComparisonHelper->compare($parentConstructor, $parentConstructor->getDeclaringClass(), $method, true); + return array_merge( + $this->methodParameterComparisonHelper->compare($parentConstructor, $parentConstructor->getDeclaringClass(), $method, true), + $this->methodVisibilityComparisonHelper->compare($parentConstructor, $parentConstructor->getDeclaringClass(), $method), + ); } } diff --git a/src/Rules/Methods/MethodVisibilityComparisonHelper.php b/src/Rules/Methods/MethodVisibilityComparisonHelper.php new file mode 100755 index 0000000000..4807453f2a --- /dev/null +++ b/src/Rules/Methods/MethodVisibilityComparisonHelper.php @@ -0,0 +1,51 @@ + */ + public function compare(ExtendedMethodReflection $prototype, ClassReflection $prototypeDeclaringClass, PhpMethodFromParserNodeReflection $method): array + { + /** @var list $messages */ + $messages = []; + + if ($prototype->isPublic()) { + if (!$method->isPublic()) { + $messages[] = RuleErrorBuilder::message(sprintf( + '%s method %s::%s() overriding public method %s::%s() should also be public.', + $method->isPrivate() ? 'Private' : 'Protected', + $method->getDeclaringClass()->getDisplayName(), + $method->getName(), + $prototypeDeclaringClass->getDisplayName(true), + $prototype->getName(), + )) + ->nonIgnorable() + ->identifier('method.visibility') + ->build(); + } + } elseif ($method->isPrivate()) { + $messages[] = RuleErrorBuilder::message(sprintf( + 'Private method %s::%s() overriding protected method %s::%s() should be protected or public.', + $method->getDeclaringClass()->getDisplayName(), + $method->getName(), + $prototypeDeclaringClass->getDisplayName(true), + $prototype->getName(), + )) + ->nonIgnorable() + ->identifier('method.visibility') + ->build(); + } + + return $messages; + } + +} diff --git a/src/Rules/Methods/OverridingMethodRule.php b/src/Rules/Methods/OverridingMethodRule.php index 38c588f9db..81a9b1b0e1 100644 --- a/src/Rules/Methods/OverridingMethodRule.php +++ b/src/Rules/Methods/OverridingMethodRule.php @@ -35,6 +35,7 @@ public function __construct( private MethodSignatureRule $methodSignatureRule, private bool $checkPhpDocMethodSignatures, private MethodParameterComparisonHelper $methodParameterComparisonHelper, + private MethodVisibilityComparisonHelper $methodVisibilityComparisonHelper, private PhpClassReflectionExtension $phpClassReflectionExtension, private bool $checkMissingOverrideMethodAttribute, ) @@ -165,32 +166,7 @@ public function processNode(Node $node, Scope $scope): array } if ($checkVisibility) { - if ($prototype->isPublic()) { - if (!$method->isPublic()) { - $messages[] = RuleErrorBuilder::message(sprintf( - '%s method %s::%s() overriding public method %s::%s() should also be public.', - $method->isPrivate() ? 'Private' : 'Protected', - $method->getDeclaringClass()->getDisplayName(), - $method->getName(), - $prototypeDeclaringClass->getDisplayName(true), - $prototype->getName(), - )) - ->nonIgnorable() - ->identifier('method.visibility') - ->build(); - } - } elseif ($method->isPrivate()) { - $messages[] = RuleErrorBuilder::message(sprintf( - 'Private method %s::%s() overriding protected method %s::%s() should be protected or public.', - $method->getDeclaringClass()->getDisplayName(), - $method->getName(), - $prototypeDeclaringClass->getDisplayName(true), - $prototype->getName(), - )) - ->nonIgnorable() - ->identifier('method.visibility') - ->build(); - } + $messages = array_merge($messages, $this->methodVisibilityComparisonHelper->compare($prototype, $prototypeDeclaringClass, $method)); } $prototypeVariants = $prototype->getVariants(); diff --git a/tests/PHPStan/Rules/Methods/ConsistentConstructorRuleTest.php b/tests/PHPStan/Rules/Methods/ConsistentConstructorRuleTest.php index 28b0091af9..adcb69cdbe 100644 --- a/tests/PHPStan/Rules/Methods/ConsistentConstructorRuleTest.php +++ b/tests/PHPStan/Rules/Methods/ConsistentConstructorRuleTest.php @@ -12,7 +12,10 @@ class ConsistentConstructorRuleTest extends RuleTestCase protected function getRule(): Rule { - return new ConsistentConstructorRule(self::getContainer()->getByType(MethodParameterComparisonHelper::class)); + return new ConsistentConstructorRule( + self::getContainer()->getByType(MethodParameterComparisonHelper::class), + self::getContainer()->getByType(MethodVisibilityComparisonHelper::class), + ); } public function testRule(): void @@ -42,4 +45,14 @@ public function testRuleNoErrors(): void $this->analyse([__DIR__ . '/data/consistent-constructor-no-errors.php'], []); } + public function testBug12137(): void + { + $this->analyse([__DIR__ . '/data/bug-12137.php'], [ + [ + 'Private method Bug12137\ChildClass::__construct() overriding protected method Bug12137\ParentClass::__construct() should be protected or public.', + 20, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Methods/MethodSignatureRuleTest.php b/tests/PHPStan/Rules/Methods/MethodSignatureRuleTest.php index 5f75cd0554..580d21787c 100644 --- a/tests/PHPStan/Rules/Methods/MethodSignatureRuleTest.php +++ b/tests/PHPStan/Rules/Methods/MethodSignatureRuleTest.php @@ -29,6 +29,7 @@ protected function getRule(): Rule new MethodSignatureRule($phpClassReflectionExtension, $this->reportMaybes, $this->reportStatic), true, new MethodParameterComparisonHelper($phpVersion), + new MethodVisibilityComparisonHelper(), $phpClassReflectionExtension, false, ); diff --git a/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php b/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php index abbe2927ab..9c65a99d35 100644 --- a/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php +++ b/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php @@ -31,6 +31,7 @@ protected function getRule(): Rule new MethodSignatureRule($phpClassReflectionExtension, true, true), false, new MethodParameterComparisonHelper($phpVersion), + new MethodVisibilityComparisonHelper(), $phpClassReflectionExtension, $this->checkMissingOverrideMethodAttribute, ); diff --git a/tests/PHPStan/Rules/Methods/data/bug-12137.php b/tests/PHPStan/Rules/Methods/data/bug-12137.php new file mode 100755 index 0000000000..eacb78bbf3 --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/bug-12137.php @@ -0,0 +1,23 @@ + Date: Wed, 25 Dec 2024 11:39:10 +0100 Subject: [PATCH 15/36] Improve loose comparison on constant types --- src/Type/Constant/ConstantArrayType.php | 22 +++++++-- .../Analyser/nsrt/loose-comparisons.php | 45 ++++++++++++++++++- 2 files changed, 63 insertions(+), 4 deletions(-) diff --git a/src/Type/Constant/ConstantArrayType.php b/src/Type/Constant/ConstantArrayType.php index 1514781d0f..114843f993 100644 --- a/src/Type/Constant/ConstantArrayType.php +++ b/src/Type/Constant/ConstantArrayType.php @@ -418,9 +418,25 @@ public function isSuperTypeOf(Type $type): IsSuperTypeOfResult public function looseCompare(Type $type, PhpVersion $phpVersion): BooleanType { - if ($this->isIterableAtLeastOnce()->no() && count($type->getConstantScalarValues()) === 1) { - // @phpstan-ignore equal.invalid, equal.notAllowed - return new ConstantBooleanType($type->getConstantScalarValues()[0] == []); // phpcs:ignore + if ($type->isInteger()->yes()) { + return new ConstantBooleanType(false); + } + + if ($this->isIterableAtLeastOnce()->no()) { + if ($type->isIterableAtLeastOnce()->yes()) { + return new ConstantBooleanType(false); + } + + $constantScalarValues = $type->getConstantScalarValues(); + if (count($constantScalarValues) > 0) { + $results = []; + foreach ($constantScalarValues as $constantScalarValue) { + // @phpstan-ignore equal.invalid, equal.notAllowed + $results[] = TrinaryLogic::createFromBoolean($constantScalarValue == []); // phpcs:ignore + } + + return TrinaryLogic::extremeIdentity(...$results)->toBooleanType(); + } } return new BooleanType(); diff --git a/tests/PHPStan/Analyser/nsrt/loose-comparisons.php b/tests/PHPStan/Analyser/nsrt/loose-comparisons.php index 243b7672fd..415cc07a73 100644 --- a/tests/PHPStan/Analyser/nsrt/loose-comparisons.php +++ b/tests/PHPStan/Analyser/nsrt/loose-comparisons.php @@ -729,6 +729,8 @@ public function sayInt( array $array, int $int, int $intRange, + string $emptyStr, + string $phpStr, ): void { assertType('bool', $int == $true); @@ -747,6 +749,20 @@ public function sayInt( assertType('false', $intRange == $emptyArr); assertType('false', $intRange == $array); + assertType('false', 5 == $emptyArr); + assertType('false', $emptyArr == 5); + assertType('false', 5 == $array); + assertType('false', $array == 5); + assertType('false', [] == 5); + assertType('false', 5 == []); + + assertType('false', 5 == $emptyStr); + assertType('false', 5 == $phpStr); + assertType('false', 5 == 'a'); + + assertType('false', $emptyStr == 5); + assertType('false', $phpStr == 5); + assertType('false', 'a' == 5); } /** @@ -754,12 +770,16 @@ public function sayInt( * @param false|0|"0" $looseZero * @param false|1 $constMix * @param "abc"|"def" $constNonFalsy + * @param array{abc: string, num?: int, nullable: ?string} $arrShape + * @param array{} $emptyArr */ public function sayConstUnion( $looseOne, $looseZero, $constMix, $constNonFalsy, + array $arrShape, + array $emptyArr ): void { assertType('true', $looseOne == 1); @@ -802,6 +822,14 @@ public function sayConstUnion( assertType('false', $constNonFalsy == "1"); assertType('false', $constNonFalsy == "0"); assertType('false', $constNonFalsy == []); + + assertType('false', $emptyArr == $looseOne); + assertType('bool', $emptyArr == $constMix); + assertType('bool', $emptyArr == $looseZero); + + assertType('bool', $arrShape == $looseOne); + assertType('bool', $arrShape == $constMix); + assertType('bool', $arrShape == $looseZero); } /** @@ -809,6 +837,7 @@ public function sayConstUnion( * @param lowercase-string $lower * @param array{} $emptyArr * @param non-empty-array $nonEmptyArr + * @param array{abc: string, num?: int, nullable: ?string} $arrShape * @param int<10, 20> $intRange */ public function sayIntersection( @@ -818,6 +847,7 @@ public function sayIntersection( array $emptyArr, array $nonEmptyArr, array $arr, + array $arrShape, int $i, int $intRange, ): void @@ -849,11 +879,24 @@ public function sayIntersection( assertType('false', $nonEmptyArr == $i); assertType('false', $arr == $intRange); assertType('false', $nonEmptyArr == $intRange); - assertType('bool', $emptyArr == $nonEmptyArr); // should be false + assertType('false', $emptyArr == $nonEmptyArr); assertType('false', $nonEmptyArr == $emptyArr); assertType('bool', $arr == $nonEmptyArr); assertType('bool', $nonEmptyArr == $arr); + assertType('false', 5 == $arr); + assertType('false', $arr == 5); + assertType('false', 5 == $emptyArr); + assertType('false', $emptyArr == 5); + assertType('false', 5 == $nonEmptyArr); + assertType('false', $nonEmptyArr == 5); + assertType('false', 5 == $arrShape); + assertType('false', $arrShape == 5); + if (count($arr) > 0) { + assertType('false', 5 == $arr); + assertType('false', $arr == 5); + } + assertType('bool', '' == $lower); if ($lower != '') { assertType('false', '' == $lower); From 13f6406cfe1fe1ff96e613004fa8c266c0c16b81 Mon Sep 17 00:00:00 2001 From: Jan Nedbal Date: Sat, 4 Jan 2025 14:57:51 +0100 Subject: [PATCH 16/36] ConstantArrayType: fix returned ConstantArrayTypeAndMethod --- src/Type/Constant/ConstantArrayType.php | 3 +- .../Type/Constant/ConstantArrayTypeTest.php | 30 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/Type/Constant/ConstantArrayType.php b/src/Type/Constant/ConstantArrayType.php index 114843f993..7659db1e60 100644 --- a/src/Type/Constant/ConstantArrayType.php +++ b/src/Type/Constant/ConstantArrayType.php @@ -499,6 +499,7 @@ public function getCallableParametersAcceptors(ClassMemberAccessAnswerer $scope) } $method = $typeAndMethodName->getType() + ->getObjectTypeOrClassStringObjectType() ->getMethod($typeAndMethodName->getMethod(), $scope); if (!$scope->canCallMethod($method)) { @@ -583,7 +584,7 @@ public function findTypeAndMethodNames(): array $has = $has->and(TrinaryLogic::createMaybe()); } - $typeAndMethods[] = ConstantArrayTypeAndMethod::createConcrete($type, $method->getValue(), $has); + $typeAndMethods[] = ConstantArrayTypeAndMethod::createConcrete($classOrObject, $method->getValue(), $has); } return $typeAndMethods; diff --git a/tests/PHPStan/Type/Constant/ConstantArrayTypeTest.php b/tests/PHPStan/Type/Constant/ConstantArrayTypeTest.php index b047b86a69..5697dee6b2 100644 --- a/tests/PHPStan/Type/Constant/ConstantArrayTypeTest.php +++ b/tests/PHPStan/Type/Constant/ConstantArrayTypeTest.php @@ -1051,4 +1051,34 @@ public function testValuesArray(ConstantArrayType $type, ConstantArrayType $expe $this->assertSame($expectedType->getNextAutoIndexes(), $actualType->getNextAutoIndexes()); } + public function testFindTypeAndMethodNames(): void + { + $classStringArray = new ConstantArrayType([ + new ConstantIntegerType(0), + new ConstantIntegerType(1), + ], [ + new ConstantStringType(Closure::class, true), + new ConstantStringType('bind'), + ]); + $objectArray = new ConstantArrayType([ + new ConstantIntegerType(0), + new ConstantIntegerType(1), + ], [ + new ObjectType(Closure::class, null, $this->createReflectionProvider()->getClass(Closure::class)), + new ConstantStringType('bind'), + ]); + + $classStringResult = $classStringArray->findTypeAndMethodNames(); + $objectResult = $objectArray->findTypeAndMethodNames(); + + $this->assertCount(1, $classStringResult); + $this->assertCount(1, $objectResult); + $this->assertInstanceOf(ConstantStringType::class, $classStringResult[0]->getType()); + $this->assertInstanceOf(ObjectType::class, $objectResult[0]->getType()); + $this->assertSame('bind', $classStringResult[0]->getMethod()); + $this->assertSame('bind', $objectResult[0]->getMethod()); + $this->assertSame(TrinaryLogic::createYes(), $classStringResult[0]->getCertainty()); + $this->assertSame(TrinaryLogic::createYes(), $objectResult[0]->getCertainty()); + } + } From b102d983419ac78cb173c3f120ded847de0933af Mon Sep 17 00:00:00 2001 From: Martin Herndl Date: Sat, 4 Jan 2025 15:08:24 +0100 Subject: [PATCH 17/36] Support named arguments after unpacking --- src/Php/PhpVersions.php | 5 +++ src/Rules/FunctionCallParametersCheck.php | 23 +++++++------ .../CallToFunctionParametersRuleTest.php | 32 +++++++++++++++++++ .../Rules/Functions/data/bug-11418.php | 9 ++++++ .../PHPStan/Rules/Functions/data/bug-8046.php | 11 +++++++ .../data/named-arguments-after-unpacking.php | 14 ++++++++ 6 files changed, 84 insertions(+), 10 deletions(-) create mode 100755 tests/PHPStan/Rules/Functions/data/bug-11418.php create mode 100644 tests/PHPStan/Rules/Functions/data/bug-8046.php create mode 100755 tests/PHPStan/Rules/Functions/data/named-arguments-after-unpacking.php diff --git a/src/Php/PhpVersions.php b/src/Php/PhpVersions.php index 229dccb72d..96bf233209 100644 --- a/src/Php/PhpVersions.php +++ b/src/Php/PhpVersions.php @@ -38,4 +38,9 @@ public function supportsNamedArguments(): TrinaryLogic return IntegerRangeType::fromInterval(80000, null)->isSuperTypeOf($this->phpVersions)->result; } + public function supportsNamedArgumentAfterUnpackedArgument(): TrinaryLogic + { + return IntegerRangeType::fromInterval(80100, null)->isSuperTypeOf($this->phpVersions)->result; + } + } diff --git a/src/Rules/FunctionCallParametersCheck.php b/src/Rules/FunctionCallParametersCheck.php index 50371ab23c..bd637913bc 100644 --- a/src/Rules/FunctionCallParametersCheck.php +++ b/src/Rules/FunctionCallParametersCheck.php @@ -101,6 +101,12 @@ public function check( $hasUnpackedArgument = false; $errors = []; foreach ($args as $arg) { + $argumentName = null; + if ($arg->name !== null) { + $hasNamedArguments = true; + $argumentName = $arg->name->toString(); + } + if ($hasNamedArguments && $arg->unpack) { $errors[] = RuleErrorBuilder::message('Named argument cannot be followed by an unpacked (...) argument.') ->identifier('argument.unpackAfterNamed') @@ -109,20 +115,17 @@ public function check( ->build(); } if ($hasUnpackedArgument && !$arg->unpack) { - $errors[] = RuleErrorBuilder::message('Unpacked argument (...) cannot be followed by a non-unpacked argument.') - ->identifier('argument.nonUnpackAfterUnpacked') - ->line($arg->getStartLine()) - ->nonIgnorable() - ->build(); + if ($argumentName === null || !$scope->getPhpVersion()->supportsNamedArgumentAfterUnpackedArgument()->yes()) { + $errors[] = RuleErrorBuilder::message('Unpacked argument (...) cannot be followed by a non-unpacked argument.') + ->identifier('argument.nonUnpackAfterUnpacked') + ->line($arg->getStartLine()) + ->nonIgnorable() + ->build(); + } } if ($arg->unpack) { $hasUnpackedArgument = true; } - $argumentName = null; - if ($arg->name !== null) { - $hasNamedArguments = true; - $argumentName = $arg->name->toString(); - } if ($arg->unpack) { $type = $scope->getType($arg->value); $arrays = $type->getConstantArrays(); diff --git a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php index a6513f03cb..2e8afa35a4 100644 --- a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php @@ -499,6 +499,20 @@ public function testNamedArguments(): void $this->analyse([__DIR__ . '/data/named-arguments.php'], $errors); } + public function testNamedArgumentsAfterUnpacking(): void + { + if (PHP_VERSION_ID < 80100) { + $this->markTestSkipped('Test requires PHP 8.1.'); + } + + $this->analyse([__DIR__ . '/data/named-arguments-after-unpacking.php'], [ + [ + 'Named parameter cannot overwrite already unpacked argument $b.', + 14, + ], + ]); + } + public function testBug4514(): void { $this->analyse([__DIR__ . '/data/bug-4514.php'], []); @@ -1936,4 +1950,22 @@ public function testBug12051(): void $this->analyse([__DIR__ . '/data/bug-12051.php'], []); } + public function testBug8046(): void + { + if (PHP_VERSION_ID < 80100) { + $this->markTestSkipped('Test requires PHP 8.1.'); + } + + $this->analyse([__DIR__ . '/data/bug-8046.php'], []); + } + + public function testBug11418(): void + { + if (PHP_VERSION_ID < 80100) { + $this->markTestSkipped('Test requires PHP 8.1.'); + } + + $this->analyse([__DIR__ . '/data/bug-11418.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Functions/data/bug-11418.php b/tests/PHPStan/Rules/Functions/data/bug-11418.php new file mode 100755 index 0000000000..8172892d95 --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/bug-11418.php @@ -0,0 +1,9 @@ + 7]; + +var_dump(add(...$args, b: 8)); diff --git a/tests/PHPStan/Rules/Functions/data/named-arguments-after-unpacking.php b/tests/PHPStan/Rules/Functions/data/named-arguments-after-unpacking.php new file mode 100755 index 0000000000..29d9ac8b4e --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/named-arguments-after-unpacking.php @@ -0,0 +1,14 @@ + 2, 'a' => 1], d: 40)); // 46 + +var_dump(foo(...[1, 2], b: 20)); // Fatal error. Named parameter $b overwrites previous argument From a245a648d371d08b318d3c5310a6f18c33bdd0dd Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sat, 4 Jan 2025 15:09:10 +0100 Subject: [PATCH 18/36] Revert "ConstantArrayType: fix returned ConstantArrayTypeAndMethod" This reverts commit 13f6406cfe1fe1ff96e613004fa8c266c0c16b81. --- src/Type/Constant/ConstantArrayType.php | 3 +- .../Type/Constant/ConstantArrayTypeTest.php | 30 ------------------- 2 files changed, 1 insertion(+), 32 deletions(-) diff --git a/src/Type/Constant/ConstantArrayType.php b/src/Type/Constant/ConstantArrayType.php index 7659db1e60..114843f993 100644 --- a/src/Type/Constant/ConstantArrayType.php +++ b/src/Type/Constant/ConstantArrayType.php @@ -499,7 +499,6 @@ public function getCallableParametersAcceptors(ClassMemberAccessAnswerer $scope) } $method = $typeAndMethodName->getType() - ->getObjectTypeOrClassStringObjectType() ->getMethod($typeAndMethodName->getMethod(), $scope); if (!$scope->canCallMethod($method)) { @@ -584,7 +583,7 @@ public function findTypeAndMethodNames(): array $has = $has->and(TrinaryLogic::createMaybe()); } - $typeAndMethods[] = ConstantArrayTypeAndMethod::createConcrete($classOrObject, $method->getValue(), $has); + $typeAndMethods[] = ConstantArrayTypeAndMethod::createConcrete($type, $method->getValue(), $has); } return $typeAndMethods; diff --git a/tests/PHPStan/Type/Constant/ConstantArrayTypeTest.php b/tests/PHPStan/Type/Constant/ConstantArrayTypeTest.php index 5697dee6b2..b047b86a69 100644 --- a/tests/PHPStan/Type/Constant/ConstantArrayTypeTest.php +++ b/tests/PHPStan/Type/Constant/ConstantArrayTypeTest.php @@ -1051,34 +1051,4 @@ public function testValuesArray(ConstantArrayType $type, ConstantArrayType $expe $this->assertSame($expectedType->getNextAutoIndexes(), $actualType->getNextAutoIndexes()); } - public function testFindTypeAndMethodNames(): void - { - $classStringArray = new ConstantArrayType([ - new ConstantIntegerType(0), - new ConstantIntegerType(1), - ], [ - new ConstantStringType(Closure::class, true), - new ConstantStringType('bind'), - ]); - $objectArray = new ConstantArrayType([ - new ConstantIntegerType(0), - new ConstantIntegerType(1), - ], [ - new ObjectType(Closure::class, null, $this->createReflectionProvider()->getClass(Closure::class)), - new ConstantStringType('bind'), - ]); - - $classStringResult = $classStringArray->findTypeAndMethodNames(); - $objectResult = $objectArray->findTypeAndMethodNames(); - - $this->assertCount(1, $classStringResult); - $this->assertCount(1, $objectResult); - $this->assertInstanceOf(ConstantStringType::class, $classStringResult[0]->getType()); - $this->assertInstanceOf(ObjectType::class, $objectResult[0]->getType()); - $this->assertSame('bind', $classStringResult[0]->getMethod()); - $this->assertSame('bind', $objectResult[0]->getMethod()); - $this->assertSame(TrinaryLogic::createYes(), $classStringResult[0]->getCertainty()); - $this->assertSame(TrinaryLogic::createYes(), $objectResult[0]->getCertainty()); - } - } From 44e28390db88e8e23c39af601eeb8ba8e476161b Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 2 Jan 2025 16:46:03 +0100 Subject: [PATCH 19/36] Improve loose comparison on IntegerRange containing zero --- src/Type/IntegerRangeType.php | 55 ++++++++++++++++- .../Analyser/nsrt/loose-comparisons.php | 60 ++++++++++++++++++- .../ConstantLooseComparisonRuleTest.php | 10 +++- 3 files changed, 119 insertions(+), 6 deletions(-) diff --git a/src/Type/IntegerRangeType.php b/src/Type/IntegerRangeType.php index b90f4d31a7..1c956015dd 100644 --- a/src/Type/IntegerRangeType.php +++ b/src/Type/IntegerRangeType.php @@ -315,6 +315,16 @@ public function isSmallerThan(Type $otherType, PhpVersion $phpVersion): TrinaryL $maxIsSmaller = (new ConstantIntegerType($this->max))->isSmallerThan($otherType, $phpVersion); } + // 0 can have different results in contrast to the interval edges, see https://3v4l.org/iGoti + $zeroInt = new ConstantIntegerType(0); + if (!$zeroInt->isSuperTypeOf($this)->no()) { + return TrinaryLogic::extremeIdentity( + $zeroInt->isSmallerThan($otherType, $phpVersion), + $minIsSmaller, + $maxIsSmaller, + ); + } + return TrinaryLogic::extremeIdentity($minIsSmaller, $maxIsSmaller); } @@ -332,6 +342,16 @@ public function isSmallerThanOrEqual(Type $otherType, PhpVersion $phpVersion): T $maxIsSmaller = (new ConstantIntegerType($this->max))->isSmallerThanOrEqual($otherType, $phpVersion); } + // 0 can have different results in contrast to the interval edges, see https://3v4l.org/iGoti + $zeroInt = new ConstantIntegerType(0); + if (!$zeroInt->isSuperTypeOf($this)->no()) { + return TrinaryLogic::extremeIdentity( + $zeroInt->isSmallerThanOrEqual($otherType, $phpVersion), + $minIsSmaller, + $maxIsSmaller, + ); + } + return TrinaryLogic::extremeIdentity($minIsSmaller, $maxIsSmaller); } @@ -349,6 +369,16 @@ public function isGreaterThan(Type $otherType, PhpVersion $phpVersion): TrinaryL $maxIsSmaller = $otherType->isSmallerThan((new ConstantIntegerType($this->max)), $phpVersion); } + // 0 can have different results in contrast to the interval edges, see https://3v4l.org/iGoti + $zeroInt = new ConstantIntegerType(0); + if (!$zeroInt->isSuperTypeOf($this)->no()) { + return TrinaryLogic::extremeIdentity( + $otherType->isSmallerThan($zeroInt, $phpVersion), + $minIsSmaller, + $maxIsSmaller, + ); + } + return TrinaryLogic::extremeIdentity($minIsSmaller, $maxIsSmaller); } @@ -366,6 +396,16 @@ public function isGreaterThanOrEqual(Type $otherType, PhpVersion $phpVersion): T $maxIsSmaller = $otherType->isSmallerThanOrEqual((new ConstantIntegerType($this->max)), $phpVersion); } + // 0 can have different results in contrast to the interval edges, see https://3v4l.org/iGoti + $zeroInt = new ConstantIntegerType(0); + if (!$zeroInt->isSuperTypeOf($this)->no()) { + return TrinaryLogic::extremeIdentity( + $otherType->isSmallerThanOrEqual($zeroInt, $phpVersion), + $minIsSmaller, + $maxIsSmaller, + ); + } + return TrinaryLogic::extremeIdentity($minIsSmaller, $maxIsSmaller); } @@ -694,7 +734,20 @@ public function toPhpDocNode(): TypeNode public function looseCompare(Type $type, PhpVersion $phpVersion): BooleanType { - if ($this->isSmallerThan($type, $phpVersion)->yes() || $this->isGreaterThan($type, $phpVersion)->yes()) { + $zeroInt = new ConstantIntegerType(0); + if ($zeroInt->isSuperTypeOf($this)->no()) { + if ($type->isTrue()->yes()) { + return new ConstantBooleanType(true); + } + if ($type->isFalse()->yes()) { + return new ConstantBooleanType(false); + } + } + + if ( + $this->isSmallerThan($type, $phpVersion)->yes() + || $this->isGreaterThan($type, $phpVersion)->yes() + ) { return new ConstantBooleanType(false); } diff --git a/tests/PHPStan/Analyser/nsrt/loose-comparisons.php b/tests/PHPStan/Analyser/nsrt/loose-comparisons.php index 415cc07a73..c385548cf5 100644 --- a/tests/PHPStan/Analyser/nsrt/loose-comparisons.php +++ b/tests/PHPStan/Analyser/nsrt/loose-comparisons.php @@ -712,7 +712,9 @@ public function sayEmptyStr( * @param array{} $emptyArr * @param 'php' $phpStr * @param '' $emptyStr - * @param int<10, 20> $intRange + * @param int<10, 20> $positiveIntRange + * @param int<-20, -10> $negativeIntRange + * @param int<-10, 10> $minusTenToTen */ public function sayInt( $true, @@ -731,6 +733,9 @@ public function sayInt( int $intRange, string $emptyStr, string $phpStr, + int $positiveIntRange, + int $negativeIntRange, + int $minusTenToTen, ): void { assertType('bool', $int == $true); @@ -746,8 +751,57 @@ public function sayInt( assertType('false', $int == $emptyArr); assertType('false', $int == $array); - assertType('false', $intRange == $emptyArr); - assertType('false', $intRange == $array); + assertType('true', $positiveIntRange == $true); + assertType('false', $positiveIntRange == $false); + assertType('false', $positiveIntRange == $one); + assertType('false', $positiveIntRange == $zero); + assertType('false', $positiveIntRange == $minusOne); + assertType('false', $positiveIntRange == $oneStr); + assertType('false', $positiveIntRange == $zeroStr); + assertType('false', $positiveIntRange == $minusOneStr); + assertType('false', $positiveIntRange == $plusOneStr); + assertType('false', $positiveIntRange == $null); + assertType('false', $positiveIntRange == $emptyArr); + assertType('false', $positiveIntRange == $array); + + assertType('true', $negativeIntRange == $true); + assertType('false', $negativeIntRange == $false); + assertType('false', $negativeIntRange == $one); + assertType('false', $negativeIntRange == $zero); + assertType('false', $negativeIntRange == $minusOne); + assertType('false', $negativeIntRange == $oneStr); + assertType('false', $negativeIntRange == $zeroStr); + assertType('false', $negativeIntRange == $minusOneStr); + assertType('false', $negativeIntRange == $plusOneStr); + assertType('false', $negativeIntRange == $null); + assertType('false', $negativeIntRange == $emptyArr); + assertType('false', $negativeIntRange == $array); + + // see https://3v4l.org/VudDK + assertType('bool', $minusTenToTen == $true); + assertType('bool', $minusTenToTen == $false); + assertType('bool', $minusTenToTen == $one); + assertType('bool', $minusTenToTen == $zero); + assertType('bool', $minusTenToTen == $minusOne); + assertType('bool', $minusTenToTen == $oneStr); + assertType('bool', $minusTenToTen == $zeroStr); + assertType('bool', $minusTenToTen == $minusOneStr); + assertType('bool', $minusTenToTen == $plusOneStr); + assertType('bool', $minusTenToTen == $null); + assertType('false', $minusTenToTen == $emptyArr); + assertType('false', $minusTenToTen == $array); + + // see https://3v4l.org/oJl3K + assertType('false', $minusTenToTen < $null); + assertType('bool', $minusTenToTen > $null); + assertType('bool', $minusTenToTen <= $null); + assertType('true', $minusTenToTen >= $null); + + // see https://3v4l.org/oRSgU + assertType('bool', $null < $minusTenToTen); + assertType('false', $null > $minusTenToTen); + assertType('true', $null <= $minusTenToTen); + assertType('bool', $null >= $minusTenToTen); assertType('false', 5 == $emptyArr); assertType('false', $emptyArr == 5); diff --git a/tests/PHPStan/Rules/Comparison/ConstantLooseComparisonRuleTest.php b/tests/PHPStan/Rules/Comparison/ConstantLooseComparisonRuleTest.php index 6e56f9dfcd..e49a6100f7 100644 --- a/tests/PHPStan/Rules/Comparison/ConstantLooseComparisonRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ConstantLooseComparisonRuleTest.php @@ -210,15 +210,21 @@ public function testBug11694(): void 39, 'Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.', ], + [ + 'Loose comparison using == between true and int<10, 20> will always evaluate to true.', + 41, + ], + [ + 'Loose comparison using == between int<10, 20> and true will always evaluate to true.', + 42, + ], [ 'Loose comparison using == between false and int<10, 20> will always evaluate to false.', 44, - 'Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.', ], [ 'Loose comparison using == between int<10, 20> and false will always evaluate to false.', 45, - 'Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.', ], ]); From cdf51107e19c8e90a3b64616eaf9bd60d7d8ae5b Mon Sep 17 00:00:00 2001 From: Martin Herndl Date: Sat, 4 Jan 2025 15:29:35 +0100 Subject: [PATCH 20/36] Fix test --- .../Rules/Functions/CallToFunctionParametersRuleTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php index 2e8afa35a4..7fde83eb12 100644 --- a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php @@ -507,7 +507,7 @@ public function testNamedArgumentsAfterUnpacking(): void $this->analyse([__DIR__ . '/data/named-arguments-after-unpacking.php'], [ [ - 'Named parameter cannot overwrite already unpacked argument $b.', + 'Argument for parameter $b has already been passed.', 14, ], ]); From 7c281ba7ba38186cd62df366ba62fe28037b98ef Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 3 Jan 2025 16:57:46 +0100 Subject: [PATCH 21/36] NodeScopeResolver: 10x Faster constant array processing --- src/Analyser/NodeScopeResolver.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index ab61ff1486..f8a09a5c4e 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -5771,7 +5771,10 @@ private function produceArrayDimFetchAssignValueToWrite(array $offsetTypes, Type foreach (array_reverse($offsetTypes) as $i => $offsetType) { /** @var Type $offsetValueType */ $offsetValueType = array_pop($offsetValueTypeStack); - if (!$offsetValueType instanceof MixedType) { + if ( + !$offsetValueType instanceof MixedType + && !$offsetValueType->isConstantArray()->yes() + ) { $types = [ new ArrayType(new MixedType(), new MixedType()), new ObjectType(ArrayAccess::class), From a063119ee422460615adaa7a37bc4c5d2e755971 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 5 Jan 2025 13:34:35 +0100 Subject: [PATCH 22/36] Fix inferring type of `new` with generic type with constructor in parent class --- src/Analyser/MutatingScope.php | 66 ++++++++++- tests/PHPStan/Analyser/nsrt/bug-2735.php | 134 +++++++++++++++++++++++ tests/PHPStan/Analyser/nsrt/generics.php | 2 +- 3 files changed, 200 insertions(+), 2 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-2735.php diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 52ae676397..9f726ede71 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -5525,13 +5525,77 @@ private function exactInstantiation(New_ $node, string $className): ?Type } } - if ($constructorMethod instanceof DummyConstructorReflection || $constructorMethod->getDeclaringClass()->getName() !== $classReflection->getName()) { + if ($constructorMethod instanceof DummyConstructorReflection) { return new GenericObjectType( $resolvedClassName, $classReflection->typeMapToList($classReflection->getTemplateTypeMap()->resolveToBounds()), ); } + if ($constructorMethod->getDeclaringClass()->getName() !== $classReflection->getName()) { + if (!$constructorMethod->getDeclaringClass()->isGeneric()) { + return new GenericObjectType( + $resolvedClassName, + $classReflection->typeMapToList($classReflection->getTemplateTypeMap()->resolveToBounds()), + ); + } + $newType = new GenericObjectType($resolvedClassName, $classReflection->typeMapToList($classReflection->getTemplateTypeMap())); + $ancestorType = $newType->getAncestorWithClassName($constructorMethod->getDeclaringClass()->getName()); + if ($ancestorType === null) { + return new GenericObjectType( + $resolvedClassName, + $classReflection->typeMapToList($classReflection->getTemplateTypeMap()->resolveToBounds()), + ); + } + $ancestorClassReflections = $ancestorType->getObjectClassReflections(); + if (count($ancestorClassReflections) !== 1) { + return new GenericObjectType( + $resolvedClassName, + $classReflection->typeMapToList($classReflection->getTemplateTypeMap()->resolveToBounds()), + ); + } + + $newParentNode = new New_(new Name($constructorMethod->getDeclaringClass()->getName()), $node->args); + $newParentType = $this->getType($newParentNode); + $newParentTypeClassReflections = $newParentType->getObjectClassReflections(); + if (count($newParentTypeClassReflections) !== 1) { + return new GenericObjectType( + $resolvedClassName, + $classReflection->typeMapToList($classReflection->getTemplateTypeMap()->resolveToBounds()), + ); + } + $newParentTypeClassReflection = $newParentTypeClassReflections[0]; + + $ancestorClassReflection = $ancestorClassReflections[0]; + $ancestorMapping = []; + foreach ($ancestorClassReflection->getActiveTemplateTypeMap()->getTypes() as $typeName => $templateType) { + if (!$templateType instanceof TemplateType) { + continue; + } + + $ancestorMapping[$typeName] = $templateType->getName(); + } + + $resolvedTypeMap = []; + foreach ($newParentTypeClassReflection->getActiveTemplateTypeMap()->getTypes() as $typeName => $type) { + if (!array_key_exists($typeName, $ancestorMapping)) { + continue; + } + + if (!array_key_exists($ancestorMapping[$typeName], $resolvedTypeMap)) { + $resolvedTypeMap[$ancestorMapping[$typeName]] = $type; + continue; + } + + $resolvedTypeMap[$ancestorMapping[$typeName]] = TypeCombinator::union($resolvedTypeMap[$ancestorMapping[$typeName]], $type); + } + + return new GenericObjectType( + $resolvedClassName, + $classReflection->typeMapToList(new TemplateTypeMap($resolvedTypeMap)), + ); + } + $parametersAcceptor = ParametersAcceptorSelector::selectFromArgs( $this, $methodCall->getArgs(), diff --git a/tests/PHPStan/Analyser/nsrt/bug-2735.php b/tests/PHPStan/Analyser/nsrt/bug-2735.php new file mode 100644 index 0000000000..a486eda5c0 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-2735.php @@ -0,0 +1,134 @@ + */ + protected $arr = []; + + /** + * @param array $arr + */ + public function __construct(array $arr) { + $this->arr = $arr; + } + + /** + * @return T + */ + public function last() + { + if (!$this->arr) { + throw new \Exception('bad'); + } + return end($this->arr); + } +} + +/** + * @template T + * @extends Collection + */ +class CollectionChild extends Collection { +} + +$dogs = new CollectionChild([new Dog(), new Dog()]); +assertType('Bug2735\\CollectionChild', $dogs); + +/** + * @template X + * @template Y + */ +class ParentWithConstructor +{ + + /** + * @param X $x + * @param Y $y + */ + public function __construct($x, $y) + { + } + +} + +/** + * @template T + * @extends ParentWithConstructor + */ +class ChildOne extends ParentWithConstructor +{ + +} + +function (): void { + $a = new ChildOne(1, new Dog()); + assertType('Bug2735\\ChildOne', $a); +}; + +/** + * @template T + * @extends ParentWithConstructor + */ +class ChildTwo extends ParentWithConstructor +{ + +} + +function (): void { + $a = new ChildTwo(new Cat(), 2); + assertType('Bug2735\\ChildTwo', $a); +}; + +/** + * @template T + * @extends ParentWithConstructor + */ +class ChildThree extends ParentWithConstructor +{ + +} + +function (): void { + $a = new ChildThree(new Cat(), new Dog()); + assertType('Bug2735\\ChildThree', $a); +}; + +/** + * @template T + * @template U + * @extends ParentWithConstructor + */ +class ChildFour extends ParentWithConstructor +{ + +} + +function (): void { + $a = new ChildFour(new Cat(), new Dog()); + assertType('Bug2735\\ChildFour', $a); +}; + +/** + * @template T + * @template U + * @extends ParentWithConstructor + */ +class ChildFive extends ParentWithConstructor +{ + +} + +function (): void { + $a = new ChildFive(new Cat(), new Dog()); + assertType('Bug2735\\ChildFive', $a); +}; diff --git a/tests/PHPStan/Analyser/nsrt/generics.php b/tests/PHPStan/Analyser/nsrt/generics.php index 80f10030e8..6f69f3b78f 100644 --- a/tests/PHPStan/Analyser/nsrt/generics.php +++ b/tests/PHPStan/Analyser/nsrt/generics.php @@ -741,7 +741,7 @@ function testClasses() assertType('DateTime', $ab->getB(new \DateTime())); $noConstructor = new NoConstructor(1); - assertType('PHPStan\Generics\FunctionsAssertType\NoConstructor', $noConstructor); + assertType('PHPStan\Generics\FunctionsAssertType\NoConstructor', $noConstructor); assertType('stdClass', acceptsClassString(\stdClass::class)); assertType('class-string', returnsClassString(new \stdClass())); From fd7bad3ef259b3f9990b6cf88c2c01bae24b725e Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sun, 5 Jan 2025 13:08:46 +0100 Subject: [PATCH 23/36] Cleanup `instanceof ConstantBooleanType` checks --- phpstan-baseline.neon | 10 ---------- src/Type/Php/ArrayFilterFunctionReturnTypeHelper.php | 3 +-- .../ArraySearchFunctionDynamicReturnTypeExtension.php | 2 +- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 1662d1fa9a..1011e39a00 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1307,11 +1307,6 @@ parameters: count: 1 path: src/Type/Php/ArrayCombineFunctionReturnTypeExtension.php - - - message: "#^Doing instanceof PHPStan\\\\Type\\\\Constant\\\\ConstantBooleanType is error\\-prone and deprecated\\. Use Type\\:\\:isTrue\\(\\) or Type\\:\\:isFalse\\(\\) instead\\.$#" - count: 1 - path: src/Type/Php/ArrayFilterFunctionReturnTypeHelper.php - - message: "#^Doing instanceof PHPStan\\\\Type\\\\Constant\\\\ConstantStringType is error\\-prone and deprecated\\. Use Type\\:\\:getConstantStrings\\(\\) instead\\.$#" count: 1 @@ -1322,11 +1317,6 @@ parameters: count: 4 path: src/Type/Php/ArrayMergeFunctionDynamicReturnTypeExtension.php - - - message: "#^Doing instanceof PHPStan\\\\Type\\\\Constant\\\\ConstantBooleanType is error\\-prone and deprecated\\. Use Type\\:\\:isTrue\\(\\) or Type\\:\\:isFalse\\(\\) instead\\.$#" - count: 1 - path: src/Type/Php/ArraySearchFunctionDynamicReturnTypeExtension.php - - message: "#^Doing instanceof PHPStan\\\\Type\\\\ConstantScalarType is error\\-prone and deprecated\\. Use Type\\:\\:isConstantScalarValue\\(\\) or Type\\:\\:getConstantScalarTypes\\(\\) or Type\\:\\:getConstantScalarValues\\(\\) instead\\.$#" count: 16 diff --git a/src/Type/Php/ArrayFilterFunctionReturnTypeHelper.php b/src/Type/Php/ArrayFilterFunctionReturnTypeHelper.php index 5291652129..8c158da87b 100644 --- a/src/Type/Php/ArrayFilterFunctionReturnTypeHelper.php +++ b/src/Type/Php/ArrayFilterFunctionReturnTypeHelper.php @@ -21,7 +21,6 @@ use PHPStan\Type\BenevolentUnionType; use PHPStan\Type\Constant\ConstantArrayType; use PHPStan\Type\Constant\ConstantArrayTypeBuilder; -use PHPStan\Type\Constant\ConstantBooleanType; use PHPStan\Type\Constant\ConstantIntegerType; use PHPStan\Type\ErrorType; use PHPStan\Type\MixedType; @@ -255,7 +254,7 @@ private function processKeyAndItemType(MutatingScope $scope, Type $keyType, Type return [ $keyVarName !== null ? $scope->getVariableType($keyVarName) : $keyType, $itemVarName !== null ? $scope->getVariableType($itemVarName) : $itemType, - !$booleanResult instanceof ConstantBooleanType, + !$booleanResult->isTrue()->yes(), ]; } diff --git a/src/Type/Php/ArraySearchFunctionDynamicReturnTypeExtension.php b/src/Type/Php/ArraySearchFunctionDynamicReturnTypeExtension.php index 8560faf5a9..762e577211 100644 --- a/src/Type/Php/ArraySearchFunctionDynamicReturnTypeExtension.php +++ b/src/Type/Php/ArraySearchFunctionDynamicReturnTypeExtension.php @@ -43,7 +43,7 @@ public function getTypeFromFunctionCall(FunctionReflection $functionReflection, } $strictArgType = $scope->getType($functionCall->getArgs()[2]->value); - if (!$strictArgType instanceof ConstantBooleanType || $strictArgType->getValue() === false) { + if (!$strictArgType->isTrue()->yes()) { return TypeCombinator::union($haystackArgType->getIterableKeyType(), new ConstantBooleanType(false)); } From 3ff4184f7319e20104a172a3669cdd6b2b99e112 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 5 Jan 2025 14:07:46 +0100 Subject: [PATCH 24/36] Regression tests Closes https://github.com/phpstan/phpstan/issues/10580 Closes https://github.com/phpstan/phpstan/issues/11939 Closes https://github.com/phpstan/phpstan/issues/10338 Closes https://github.com/phpstan/phpstan/issues/12048 Closes https://github.com/phpstan/phpstan/issues/3107 --- tests/PHPStan/Analyser/nsrt/bug-10338.php | 12 +++++ .../CallToFunctionParametersRuleTest.php | 5 ++ .../PHPStan/Rules/Functions/data/bug-3107.php | 23 ++++++++++ .../Rules/Methods/ReturnTypeRuleTest.php | 46 +++++++++++++++++++ .../PHPStan/Rules/Methods/data/bug-10580.php | 36 +++++++++++++++ .../MethodConditionalReturnTypeRuleTest.php | 5 ++ tests/PHPStan/Rules/PhpDoc/data/bug-11939.php | 36 +++++++++++++++ .../PHPStan/Rules/Pure/PureMethodRuleTest.php | 6 +++ tests/PHPStan/Rules/Pure/data/bug-12048.php | 19 ++++++++ 9 files changed, 188 insertions(+) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-10338.php create mode 100644 tests/PHPStan/Rules/Functions/data/bug-3107.php create mode 100644 tests/PHPStan/Rules/Methods/data/bug-10580.php create mode 100644 tests/PHPStan/Rules/PhpDoc/data/bug-11939.php create mode 100644 tests/PHPStan/Rules/Pure/data/bug-12048.php diff --git a/tests/PHPStan/Analyser/nsrt/bug-10338.php b/tests/PHPStan/Analyser/nsrt/bug-10338.php new file mode 100644 index 0000000000..89934bb502 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-10338.php @@ -0,0 +1,12 @@ +analyse([__DIR__ . '/data/bug-11418.php'], []); } + public function testBug3107(): void + { + $this->analyse([__DIR__ . '/data/bug-3107.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Functions/data/bug-3107.php b/tests/PHPStan/Rules/Functions/data/bug-3107.php new file mode 100644 index 0000000000..12ed0edfd0 --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/bug-3107.php @@ -0,0 +1,23 @@ +val = $mixed; + + $a = []; + $a[$holder->val] = 1; + take($a); +} + +/** @param array $a */ +function take($a): void {} diff --git a/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php b/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php index 98bb11b0b5..73350821e1 100644 --- a/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php +++ b/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php @@ -1171,4 +1171,50 @@ public function testShortGetPropertyHook(): void ]); } + public function testBug1O580(): void + { + if (PHP_VERSION_ID < 80000) { + $this->markTestSkipped('Test requires PHP 8.0.'); + } + + $this->analyse([__DIR__ . '/data/bug-10580.php'], [ + [ + 'Method Bug10580\FooA::fooThisInterface() should return $this(Bug10580\FooA) but returns Bug10580\FooA.', + 18, + ], + [ + 'Method Bug10580\FooA::fooThisClass() should return $this(Bug10580\FooA) but returns Bug10580\FooA.', + 19, + ], + [ + 'Method Bug10580\FooA::fooThisSelf() should return $this(Bug10580\FooA) but returns Bug10580\FooA.', + 20, + ], + [ + 'Method Bug10580\FooA::fooThisStatic() should return $this(Bug10580\FooA) but returns Bug10580\FooA.', + 21, + ], + [ + 'Method Bug10580\FooB::fooThisInterface() should return $this(Bug10580\FooB) but returns Bug10580\FooB.', + 27, + ], + [ + 'Method Bug10580\FooB::fooThisClass() should return $this(Bug10580\FooB) but returns Bug10580\FooB.', + 29, + ], + [ + 'Method Bug10580\FooB::fooThisSelf() should return $this(Bug10580\FooB) but returns Bug10580\FooB.', + 31, + ], + [ + 'Method Bug10580\FooB::fooThisStatic() should return $this(Bug10580\FooB) but returns Bug10580\FooB.', + 33, + ], + [ + 'Method Bug10580\FooB::fooThis() should return $this(Bug10580\FooB) but returns Bug10580\FooB.', + 35, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Methods/data/bug-10580.php b/tests/PHPStan/Rules/Methods/data/bug-10580.php new file mode 100644 index 0000000000..b9c479000a --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/bug-10580.php @@ -0,0 +1,36 @@ += 8.0 + +namespace Bug10580; + +interface FooI { + /** @return $this */ + public function fooThisInterface(): FooI; + /** @return $this */ + public function fooThisClass(): FooI; + /** @return $this */ + public function fooThisSelf(): self; + /** @return $this */ + public function fooThisStatic(): static; +} + +final class FooA implements FooI +{ + public function fooThisInterface(): FooI { return new FooA(); } + public function fooThisClass(): FooA { return new FooA(); } + public function fooThisSelf(): self { return new FooA(); } + public function fooThisStatic(): static { return new FooA(); } +} + +final class FooB implements FooI +{ + /** @return $this */ + public function fooThisInterface(): FooI { return new FooB(); } + /** @return $this */ + public function fooThisClass(): FooB { return new FooB(); } + /** @return $this */ + public function fooThisSelf(): self { return new FooB(); } + /** @return $this */ + public function fooThisStatic(): static { return new FooB(); } + /** @return $this */ + public function fooThis(): static { return new FooB(); } +} diff --git a/tests/PHPStan/Rules/PhpDoc/MethodConditionalReturnTypeRuleTest.php b/tests/PHPStan/Rules/PhpDoc/MethodConditionalReturnTypeRuleTest.php index 48258202dc..1fa91d3fe6 100644 --- a/tests/PHPStan/Rules/PhpDoc/MethodConditionalReturnTypeRuleTest.php +++ b/tests/PHPStan/Rules/PhpDoc/MethodConditionalReturnTypeRuleTest.php @@ -95,4 +95,9 @@ public function testBug7310(): void $this->analyse([__DIR__ . '/data/bug-7310.php'], []); } + public function testBug11939(): void + { + $this->analyse([__DIR__ . '/data/bug-11939.php'], []); + } + } diff --git a/tests/PHPStan/Rules/PhpDoc/data/bug-11939.php b/tests/PHPStan/Rules/PhpDoc/data/bug-11939.php new file mode 100644 index 0000000000..759c3b5bb5 --- /dev/null +++ b/tests/PHPStan/Rules/PhpDoc/data/bug-11939.php @@ -0,0 +1,36 @@ += 8.1 + +declare(strict_types=1); + +namespace Bug11939; + +enum What +{ + case This; + case That; + + /** + * @return ($this is self::This ? 'here' : 'there') + */ + public function where(): string + { + return match ($this) { + self::This => 'here', + self::That => 'there' + }; + } +} + +class Where +{ + /** + * @return ($what is What::This ? 'here' : 'there') + */ + public function __invoke(What $what): string + { + return match ($what) { + What::This => 'here', + What::That => 'there' + }; + } +} diff --git a/tests/PHPStan/Rules/Pure/PureMethodRuleTest.php b/tests/PHPStan/Rules/Pure/PureMethodRuleTest.php index 4ec5028172..64ee811d81 100644 --- a/tests/PHPStan/Rules/Pure/PureMethodRuleTest.php +++ b/tests/PHPStan/Rules/Pure/PureMethodRuleTest.php @@ -194,4 +194,10 @@ public function dataBug11207(): array ]; } + public function testBug12048(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-12048.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Pure/data/bug-12048.php b/tests/PHPStan/Rules/Pure/data/bug-12048.php new file mode 100644 index 0000000000..ab5d44154a --- /dev/null +++ b/tests/PHPStan/Rules/Pure/data/bug-12048.php @@ -0,0 +1,19 @@ + Date: Sat, 4 Jan 2025 21:48:11 +0100 Subject: [PATCH 25/36] Support tagged unions in `array_merge` --- phpstan-baseline.neon | 5 -- ...ergeFunctionDynamicReturnTypeExtension.php | 54 +++++++++---------- tests/PHPStan/Analyser/nsrt/array-merge2.php | 4 ++ .../CallToFunctionParametersRuleTest.php | 9 ++++ .../PHPStan/Rules/Functions/data/bug-9559.php | 12 +++++ .../Rules/Methods/ReturnTypeRuleTest.php | 10 ++++ tests/PHPStan/Rules/Methods/data/bug-7857.php | 17 ++++++ tests/PHPStan/Rules/Methods/data/bug-8632.php | 26 +++++++++ 8 files changed, 103 insertions(+), 34 deletions(-) create mode 100644 tests/PHPStan/Rules/Functions/data/bug-9559.php create mode 100644 tests/PHPStan/Rules/Methods/data/bug-7857.php create mode 100644 tests/PHPStan/Rules/Methods/data/bug-8632.php diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 1011e39a00..9977a1992d 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1312,11 +1312,6 @@ parameters: count: 1 path: src/Type/Php/ArrayKeyExistsFunctionTypeSpecifyingExtension.php - - - message: "#^Doing instanceof PHPStan\\\\Type\\\\Constant\\\\ConstantArrayType is error\\-prone and deprecated\\. Use Type\\:\\:getConstantArrays\\(\\) instead\\.$#" - count: 4 - path: src/Type/Php/ArrayMergeFunctionDynamicReturnTypeExtension.php - - message: "#^Doing instanceof PHPStan\\\\Type\\\\ConstantScalarType is error\\-prone and deprecated\\. Use Type\\:\\:isConstantScalarValue\\(\\) or Type\\:\\:getConstantScalarTypes\\(\\) or Type\\:\\:getConstantScalarValues\\(\\) instead\\.$#" count: 16 diff --git a/src/Type/Php/ArrayMergeFunctionDynamicReturnTypeExtension.php b/src/Type/Php/ArrayMergeFunctionDynamicReturnTypeExtension.php index 5b177a9f68..01d2143b25 100644 --- a/src/Type/Php/ArrayMergeFunctionDynamicReturnTypeExtension.php +++ b/src/Type/Php/ArrayMergeFunctionDynamicReturnTypeExtension.php @@ -5,13 +5,14 @@ use PhpParser\Node\Expr\FuncCall; use PHPStan\Analyser\Scope; use PHPStan\Reflection\FunctionReflection; -use PHPStan\ShouldNotHappenException; +use PHPStan\TrinaryLogic; use PHPStan\Type\Accessory\AccessoryArrayListType; use PHPStan\Type\Accessory\NonEmptyArrayType; use PHPStan\Type\ArrayType; use PHPStan\Type\Constant\ConstantArrayType; use PHPStan\Type\Constant\ConstantArrayTypeBuilder; use PHPStan\Type\Constant\ConstantIntegerType; +use PHPStan\Type\Constant\ConstantStringType; use PHPStan\Type\DynamicFunctionReturnTypeExtension; use PHPStan\Type\IntegerType; use PHPStan\Type\NeverType; @@ -39,58 +40,53 @@ public function getTypeFromFunctionCall(FunctionReflection $functionReflection, $argTypes = []; $optionalArgTypes = []; - $allConstant = true; foreach ($args as $arg) { $argType = $scope->getType($arg->value); if ($arg->unpack) { - if ($argType instanceof ConstantArrayType) { - $argTypesFound = $argType->getValueTypes(); - } else { - $argTypesFound = [$argType->getIterableValueType()]; - } - - foreach ($argTypesFound as $argTypeFound) { - $argTypes[] = $argTypeFound; - if ($argTypeFound instanceof ConstantArrayType) { - continue; + if ($argType->isConstantArray()->yes()) { + foreach ($argType->getConstantArrays() as $constantArray) { + foreach ($constantArray->getValueTypes() as $valueType) { + $argTypes[] = $valueType; + } } - $allConstant = false; + } else { + $argTypes[] = $argType->getIterableValueType(); } if (!$argType->isIterableAtLeastOnce()->yes()) { // unpacked params can be empty, making them optional $optionalArgTypesOffset = count($argTypes) - 1; - foreach (array_keys($argTypesFound) as $key) { + foreach (array_keys($argTypes) as $key) { $optionalArgTypes[] = $optionalArgTypesOffset + $key; } } } else { $argTypes[] = $argType; - if (!$argType instanceof ConstantArrayType) { - $allConstant = false; - } } } - if ($allConstant) { + $allConstant = TrinaryLogic::createYes()->lazyAnd( + $argTypes, + static fn (Type $argType) => $argType->isConstantArray(), + ); + + if ($allConstant->yes()) { $newArrayBuilder = ConstantArrayTypeBuilder::createEmpty(); foreach ($argTypes as $argType) { - if (!$argType instanceof ConstantArrayType) { - throw new ShouldNotHappenException(); + /** @var array $keyTypes */ + $keyTypes = []; + foreach ($argType->getConstantArrays() as $constantArray) { + foreach ($constantArray->getKeyTypes() as $keyType) { + $keyTypes[$keyType->getValue()] = $keyType; + } } - $keyTypes = $argType->getKeyTypes(); - $valueTypes = $argType->getValueTypes(); - $optionalKeys = $argType->getOptionalKeys(); - - foreach ($keyTypes as $k => $keyType) { - $isOptional = in_array($k, $optionalKeys, true); - + foreach ($keyTypes as $keyType) { $newArrayBuilder->setOffsetValueType( $keyType instanceof ConstantIntegerType ? null : $keyType, - $valueTypes[$k], - $isOptional, + $argType->getOffsetValueType($keyType), + !$argType->hasOffsetValueType($keyType)->yes(), ); } } diff --git a/tests/PHPStan/Analyser/nsrt/array-merge2.php b/tests/PHPStan/Analyser/nsrt/array-merge2.php index a52f640ef2..f0d86e61b6 100644 --- a/tests/PHPStan/Analyser/nsrt/array-merge2.php +++ b/tests/PHPStan/Analyser/nsrt/array-merge2.php @@ -21,6 +21,10 @@ public function arrayMergeArrayShapes($array1, $array2): void assertType("array{foo: '1', bar: '2', lall2: '3', 0: '4', 1: '6', lall: '3', 2: '2', 3: '3'}", array_merge($array2, $array1)); assertType("array{foo: 3, bar: '2', lall2: '3', 0: '4', 1: '6', lall: '3', 2: '2', 3: '3'}", array_merge($array2, $array1, ['foo' => 3])); assertType("array{foo: 3, bar: '2', lall2: '3', 0: '4', 1: '6', lall: '3', 2: '2', 3: '3'}", array_merge($array2, $array1, ...[['foo' => 3]])); + assertType("array{foo: '1', bar: '2'|'4', lall?: '3', 0: '2'|'4', 1: '3'|'6', lall2?: '3'}", array_merge(rand(0, 1) ? $array1 : $array2, [])); + assertType("array{foo?: 3, bar?: 3}", array_merge([], ...[rand(0, 1) ? ['foo' => 3] : ['bar' => 3]])); + assertType("array{foo: '1', bar: '2'|'4', lall?: '3', 0: '2'|'4', 1: '3'|'6', lall2?: '3'}", array_merge([], ...[rand(0, 1) ? $array1 : $array2])); + assertType("array{foo: 1, bar: 2, 0: 2, 1: 3}", array_merge(['foo' => 4, 'bar' => 5], ...[['foo' => 1, 'bar' => 2], [2, 3]])); } /** diff --git a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php index a89705028b..a3ffa071b7 100644 --- a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php @@ -1598,6 +1598,15 @@ public function testBug9399(): void $this->analyse([__DIR__ . '/data/bug-9399.php'], []); } + public function testBug9559(): void + { + if (PHP_VERSION_ID < 80000) { + $this->markTestSkipped('Test requires PHP 8.0'); + } + + $this->analyse([__DIR__ . '/data/bug-9559.php'], []); + } + public function testBug9923(): void { if (PHP_VERSION_ID < 80000) { diff --git a/tests/PHPStan/Rules/Functions/data/bug-9559.php b/tests/PHPStan/Rules/Functions/data/bug-9559.php new file mode 100644 index 0000000000..8f452e90f0 --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/bug-9559.php @@ -0,0 +1,12 @@ + "3" ])); +} diff --git a/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php b/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php index 13082d79b0..98e54bce0e 100644 --- a/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php +++ b/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php @@ -846,6 +846,16 @@ public function testBug8573(): void $this->analyse([__DIR__ . '/data/bug-8573.php'], []); } + public function testBug8632(): void + { + $this->analyse([__DIR__ . '/data/bug-8632.php'], []); + } + + public function testBug7857(): void + { + $this->analyse([__DIR__ . '/data/bug-7857.php'], []); + } + public function testBug8879(): void { $this->analyse([__DIR__ . '/data/bug-8879.php'], []); diff --git a/tests/PHPStan/Rules/Methods/data/bug-7857.php b/tests/PHPStan/Rules/Methods/data/bug-7857.php new file mode 100644 index 0000000000..269bbd5a87 --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/bug-7857.php @@ -0,0 +1,17 @@ + $page], + $perPage !== null ? ['perPage' => $perPage] : [] + ); + } +} diff --git a/tests/PHPStan/Rules/Methods/data/bug-8632.php b/tests/PHPStan/Rules/Methods/data/bug-8632.php new file mode 100644 index 0000000000..17c2aa50f6 --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/bug-8632.php @@ -0,0 +1,26 @@ + 1, + 'categories' => ['news'], + ]; + } else { + $arr = []; + } + + return array_merge($arr, []); + } +} From 33a45f4f64303ba31bb7407404f4b9c8cb8b058a Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 5 Jan 2025 14:28:29 +0100 Subject: [PATCH 26/36] Fix build --- tests/PHPStan/Analyser/nsrt/bug-10338.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/PHPStan/Analyser/nsrt/bug-10338.php b/tests/PHPStan/Analyser/nsrt/bug-10338.php index 89934bb502..cb9103eae0 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-10338.php +++ b/tests/PHPStan/Analyser/nsrt/bug-10338.php @@ -8,5 +8,5 @@ function (): void { die; } - assertType('string', $content); + assertType('non-empty-string', $content); }; From 7dc98b6bfd02a76bbcf6c766972e1eba07e9c070 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 5 Jan 2025 14:37:08 +0100 Subject: [PATCH 27/36] Playground rule - StaticVarWithoutTypeRule --- issue-bot/playground.neon | 8 ++ .../Playground/StaticVarWithoutTypeRule.php | 81 +++++++++++++++++++ .../StaticVarWithoutTypeRuleTest.php | 34 ++++++++ .../data/static-var-without-type.php | 31 +++++++ 4 files changed, 154 insertions(+) create mode 100644 src/Rules/Playground/StaticVarWithoutTypeRule.php create mode 100644 tests/PHPStan/Rules/Playground/StaticVarWithoutTypeRuleTest.php create mode 100644 tests/PHPStan/Rules/Playground/data/static-var-without-type.php diff --git a/issue-bot/playground.neon b/issue-bot/playground.neon index 2f9743d575..d4072a4170 100644 --- a/issue-bot/playground.neon +++ b/issue-bot/playground.neon @@ -3,3 +3,11 @@ rules: - PHPStan\Rules\Playground\MethodNeverRule - PHPStan\Rules\Playground\NotAnalysedTraitRule - PHPStan\Rules\Playground\NoPhpCodeRule + +conditionalTags: + PHPStan\Rules\Playground\StaticVarWithoutTypeRule: + phpstan.rules.rule: %checkImplicitMixed% + +services: + - + class: PHPStan\Rules\Playground\StaticVarWithoutTypeRule diff --git a/src/Rules/Playground/StaticVarWithoutTypeRule.php b/src/Rules/Playground/StaticVarWithoutTypeRule.php new file mode 100644 index 0000000000..28f5369952 --- /dev/null +++ b/src/Rules/Playground/StaticVarWithoutTypeRule.php @@ -0,0 +1,81 @@ + + */ +final class StaticVarWithoutTypeRule implements Rule +{ + + public function __construct( + private FileTypeMapper $fileTypeMapper, + ) + { + } + + public function getNodeType(): string + { + return Node\Stmt\Static_::class; + } + + public function processNode(Node $node, Scope $scope): array + { + $docComment = $node->getDocComment(); + $ruleError = RuleErrorBuilder::message('Static variable needs to be typed with PHPDoc @var tag.') + ->identifier('phpstanPlayground.staticWithoutType') + ->build(); + if ($docComment === null) { + return [$ruleError]; + } + $variableNames = []; + foreach ($node->vars as $var) { + if (!is_string($var->var->name)) { + throw new ShouldNotHappenException(); + } + + $variableNames[] = $var->var->name; + } + + $function = $scope->getFunction(); + $resolvedPhpDoc = $this->fileTypeMapper->getResolvedPhpDoc( + $scope->getFile(), + $scope->isInClass() ? $scope->getClassReflection()->getName() : null, + $scope->isInTrait() ? $scope->getTraitReflection()->getName() : null, + $function !== null ? $function->getName() : null, + $docComment->getText(), + ); + $varTags = []; + foreach ($resolvedPhpDoc->getVarTags() as $key => $varTag) { + $varTags[$key] = $varTag; + } + + if (count($varTags) === 0) { + return [$ruleError]; + } + + if (count($variableNames) === 1 && count($varTags) === 1 && isset($varTags[0])) { + return []; + } + + foreach ($variableNames as $variableName) { + if (isset($varTags[$variableName])) { + continue; + } + + return [$ruleError]; + } + + return []; + } + +} diff --git a/tests/PHPStan/Rules/Playground/StaticVarWithoutTypeRuleTest.php b/tests/PHPStan/Rules/Playground/StaticVarWithoutTypeRuleTest.php new file mode 100644 index 0000000000..4ef6334828 --- /dev/null +++ b/tests/PHPStan/Rules/Playground/StaticVarWithoutTypeRuleTest.php @@ -0,0 +1,34 @@ + + */ +class StaticVarWithoutTypeRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return new StaticVarWithoutTypeRule(self::getContainer()->getByType(FileTypeMapper::class)); + } + + public function testRule(): void + { + $this->analyse([__DIR__ . '/data/static-var-without-type.php'], [ + [ + 'Static variable needs to be typed with PHPDoc @var tag.', + 23, + ], + [ + 'Static variable needs to be typed with PHPDoc @var tag.', + 28, + ], + ]); + } + +} diff --git a/tests/PHPStan/Rules/Playground/data/static-var-without-type.php b/tests/PHPStan/Rules/Playground/data/static-var-without-type.php new file mode 100644 index 0000000000..10455d131d --- /dev/null +++ b/tests/PHPStan/Rules/Playground/data/static-var-without-type.php @@ -0,0 +1,31 @@ + Date: Sun, 5 Jan 2025 14:49:31 +0100 Subject: [PATCH 28/36] Fix build --- .../Rules/PhpDoc/MethodConditionalReturnTypeRuleTest.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/PHPStan/Rules/PhpDoc/MethodConditionalReturnTypeRuleTest.php b/tests/PHPStan/Rules/PhpDoc/MethodConditionalReturnTypeRuleTest.php index 1fa91d3fe6..ff465ea7e5 100644 --- a/tests/PHPStan/Rules/PhpDoc/MethodConditionalReturnTypeRuleTest.php +++ b/tests/PHPStan/Rules/PhpDoc/MethodConditionalReturnTypeRuleTest.php @@ -4,6 +4,7 @@ use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; +use const PHP_VERSION_ID; /** * @extends RuleTestCase @@ -97,6 +98,10 @@ public function testBug7310(): void public function testBug11939(): void { + if (PHP_VERSION_ID < 80100) { + $this->markTestSkipped('Test requires PHP 8.1.'); + } + $this->analyse([__DIR__ . '/data/bug-11939.php'], []); } From 59ccf550b002043e044f620d23432dbcbd3d6bfc Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 5 Jan 2025 14:51:28 +0100 Subject: [PATCH 29/36] Playground rule - PromoteParameterRule --- src/Rules/Playground/PromoteParameterRule.php | 57 +++++++++++++++++++ .../Playground/PromoteParameterRuleTest.php | 41 +++++++++++++ .../Playground/data/promote-parameter.php | 10 ++++ 3 files changed, 108 insertions(+) create mode 100644 src/Rules/Playground/PromoteParameterRule.php create mode 100644 tests/PHPStan/Rules/Playground/PromoteParameterRuleTest.php create mode 100644 tests/PHPStan/Rules/Playground/data/promote-parameter.php diff --git a/src/Rules/Playground/PromoteParameterRule.php b/src/Rules/Playground/PromoteParameterRule.php new file mode 100644 index 0000000000..10a7af56f1 --- /dev/null +++ b/src/Rules/Playground/PromoteParameterRule.php @@ -0,0 +1,57 @@ + + */ +final class PromoteParameterRule implements Rule +{ + + /** + * @param Rule $rule + * @param class-string $nodeType + */ + public function __construct( + private Rule $rule, + private string $nodeType, + private bool $parameterValue, + private string $parameterName, + ) + { + } + + public function getNodeType(): string + { + return $this->nodeType; + } + + public function processNode(Node $node, Scope $scope): array + { + if ($this->parameterValue) { + return []; + } + + if ($this->nodeType !== $this->rule->getNodeType()) { + return []; + } + + $errors = []; + foreach ($this->rule->processNode($node, $scope) as $error) { + $errors[] = RuleErrorBuilder::message($error->getMessage()) + ->identifier('phpstanPlayground.configParameter') + ->tip(sprintf('This error would be reported if the %s: true parameter was enabled in your %%configurationFile%%.', $this->parameterName)) + ->build(); + } + + return $errors; + } + +} diff --git a/tests/PHPStan/Rules/Playground/PromoteParameterRuleTest.php b/tests/PHPStan/Rules/Playground/PromoteParameterRuleTest.php new file mode 100644 index 0000000000..f95e1b5573 --- /dev/null +++ b/tests/PHPStan/Rules/Playground/PromoteParameterRuleTest.php @@ -0,0 +1,41 @@ +> + */ +class PromoteParameterRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return new PromoteParameterRule( + new UninitializedPropertyRule(new ConstructorsHelper( + self::getContainer(), + [], + )), + ClassPropertiesNode::class, + false, + 'checkUninitializedProperties', + ); + } + + public function testRule(): void + { + $this->analyse([__DIR__ . '/data/promote-parameter.php'], [ + [ + 'Class PromoteParameter\Foo has an uninitialized property $test. Give it default value or assign it in the constructor.', + 5, + 'This error would be reported if the checkUninitializedProperties: true parameter was enabled in your %configurationFile%.', + ], + ]); + } + +} diff --git a/tests/PHPStan/Rules/Playground/data/promote-parameter.php b/tests/PHPStan/Rules/Playground/data/promote-parameter.php new file mode 100644 index 0000000000..da1ea8ad08 --- /dev/null +++ b/tests/PHPStan/Rules/Playground/data/promote-parameter.php @@ -0,0 +1,10 @@ + Date: Sun, 5 Jan 2025 17:08:44 +0100 Subject: [PATCH 30/36] UninitializedPropertyRule should be always reported when `checkUninitializedProperties` is enabled --- conf/config.level0.neon | 12 ------------ conf/config.neon | 10 ++++++++++ 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/conf/config.level0.neon b/conf/config.level0.neon index dbb2b4836c..9160281651 100644 --- a/conf/config.level0.neon +++ b/conf/config.level0.neon @@ -1,10 +1,6 @@ parameters: customRulesetUsed: false -conditionalTags: - PHPStan\Rules\Properties\UninitializedPropertyRule: - phpstan.rules.rule: %checkUninitializedProperties% - rules: - PHPStan\Rules\Api\ApiInstanceofRule - PHPStan\Rules\Api\ApiInstanceofTypeRule @@ -218,9 +214,6 @@ services: tags: - phpstan.rules.rule - - - class: PHPStan\Rules\Properties\UninitializedPropertyRule - - class: PHPStan\Rules\Properties\WritingToReadOnlyPropertiesRule arguments: @@ -250,11 +243,6 @@ services: tags: - phpstan.rules.rule - - - class: PHPStan\Reflection\ConstructorsHelper - arguments: - additionalConstructors: %additionalConstructors% - - class: PHPStan\Rules\Keywords\RequireFileExistsRule arguments: diff --git a/conf/config.neon b/conf/config.neon index b9c52c9a57..9d62f32c60 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -211,6 +211,8 @@ conditionalTags: phpstan.rules.rule: %exceptions.check.missingCheckedExceptionInThrows% PHPStan\Rules\Exceptions\MissingCheckedExceptionInPropertyHookThrowsRule: phpstan.rules.rule: %exceptions.check.missingCheckedExceptionInThrows% + PHPStan\Rules\Properties\UninitializedPropertyRule: + phpstan.rules.rule: %checkUninitializedProperties% services: - @@ -734,6 +736,11 @@ services: tags: - phpstan.broker.dynamicMethodReturnTypeExtension + - + class: PHPStan\Reflection\ConstructorsHelper + arguments: + additionalConstructors: %additionalConstructors% + - class: PHPStan\Reflection\RequireExtension\RequireExtendsMethodsClassReflectionExtension @@ -1037,6 +1044,9 @@ services: reportMagicProperties: %reportMagicProperties% checkDynamicProperties: %checkDynamicProperties% + - + class: PHPStan\Rules\Properties\UninitializedPropertyRule + - class: PHPStan\Rules\Properties\LazyReadWritePropertiesExtensionProvider From 2f712479fe1aa86f618a49fe4f36a3531230484b Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 5 Jan 2025 17:16:28 +0100 Subject: [PATCH 31/36] PromoteParameterRule - more precise line for LineRuleError --- src/Rules/Playground/PromoteParameterRule.php | 10 +++++++--- .../Rules/Playground/PromoteParameterRuleTest.php | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/Rules/Playground/PromoteParameterRule.php b/src/Rules/Playground/PromoteParameterRule.php index 10a7af56f1..cee351e3ff 100644 --- a/src/Rules/Playground/PromoteParameterRule.php +++ b/src/Rules/Playground/PromoteParameterRule.php @@ -4,6 +4,7 @@ use PhpParser\Node; use PHPStan\Analyser\Scope; +use PHPStan\Rules\LineRuleError; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use function sprintf; @@ -45,10 +46,13 @@ public function processNode(Node $node, Scope $scope): array $errors = []; foreach ($this->rule->processNode($node, $scope) as $error) { - $errors[] = RuleErrorBuilder::message($error->getMessage()) + $builder = RuleErrorBuilder::message($error->getMessage()) ->identifier('phpstanPlayground.configParameter') - ->tip(sprintf('This error would be reported if the %s: true parameter was enabled in your %%configurationFile%%.', $this->parameterName)) - ->build(); + ->tip(sprintf('This error would be reported if the %s: true parameter was enabled in your %%configurationFile%%.', $this->parameterName)); + if ($error instanceof LineRuleError) { + $builder->line($error->getLine()); + } + $errors[] = $builder->build(); } return $errors; diff --git a/tests/PHPStan/Rules/Playground/PromoteParameterRuleTest.php b/tests/PHPStan/Rules/Playground/PromoteParameterRuleTest.php index f95e1b5573..e97673ff5f 100644 --- a/tests/PHPStan/Rules/Playground/PromoteParameterRuleTest.php +++ b/tests/PHPStan/Rules/Playground/PromoteParameterRuleTest.php @@ -32,7 +32,7 @@ public function testRule(): void $this->analyse([__DIR__ . '/data/promote-parameter.php'], [ [ 'Class PromoteParameter\Foo has an uninitialized property $test. Give it default value or assign it in the constructor.', - 5, + 8, 'This error would be reported if the checkUninitializedProperties: true parameter was enabled in your %configurationFile%.', ], ]); From 1f64c159c599408fde0d7285fb89920c00b15446 Mon Sep 17 00:00:00 2001 From: ondrejmirtes <104888+ondrejmirtes@users.noreply.github.com> Date: Tue, 7 Jan 2025 00:03:54 +0000 Subject: [PATCH 32/36] Update PhpStorm stubs --- composer.json | 2 +- composer.lock | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/composer.json b/composer.json index 6ac10b2742..6edf082303 100644 --- a/composer.json +++ b/composer.json @@ -15,7 +15,7 @@ "hoa/compiler": "3.17.08.08", "hoa/exception": "^1.0", "hoa/file": "1.17.07.11", - "jetbrains/phpstorm-stubs": "dev-master#dfcad4524db603bd20bdec3aab1a31c5f5128ea3", + "jetbrains/phpstorm-stubs": "dev-master#62a683f61d9ea11ef8caf8b2ad54e59e92b2c670", "nette/bootstrap": "^3.0", "nette/di": "^3.1.4", "nette/neon": "3.3.4", diff --git a/composer.lock b/composer.lock index 039bd64a30..f9f801df4c 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "f5964f498aa14ffd6b984c06676417aa", + "content-hash": "bf40a89cec9c4598324b1e8394b7367c", "packages": [ { "name": "clue/ndjson-react", @@ -1442,12 +1442,12 @@ "source": { "type": "git", "url": "https://github.com/JetBrains/phpstorm-stubs.git", - "reference": "dfcad4524db603bd20bdec3aab1a31c5f5128ea3" + "reference": "62a683f61d9ea11ef8caf8b2ad54e59e92b2c670" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/JetBrains/phpstorm-stubs/zipball/dfcad4524db603bd20bdec3aab1a31c5f5128ea3", - "reference": "dfcad4524db603bd20bdec3aab1a31c5f5128ea3", + "url": "https://api.github.com/repos/JetBrains/phpstorm-stubs/zipball/62a683f61d9ea11ef8caf8b2ad54e59e92b2c670", + "reference": "62a683f61d9ea11ef8caf8b2ad54e59e92b2c670", "shasum": "" }, "require-dev": { @@ -1482,7 +1482,7 @@ "support": { "source": "https://github.com/JetBrains/phpstorm-stubs/tree/master" }, - "time": "2025-01-02T13:51:39+00:00" + "time": "2025-01-04T20:30:22+00:00" }, { "name": "nette/bootstrap", From 8b2794326fcfea43111df419a948d197219f589a Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Mon, 6 Jan 2025 11:53:02 +0100 Subject: [PATCH 33/36] Calling to a constructor with promoted properties has side effects --- src/Analyser/MutatingScope.php | 2 ++ src/Analyser/NodeScopeResolver.php | 7 ++++-- .../Php/PhpMethodFromParserNodeReflection.php | 11 +++++++++- ...onstructorWithoutImpurePointsCollector.php | 3 +-- .../MethodWithoutImpurePointsCollector.php | 6 +---- src/Rules/Pure/FunctionPurityCheck.php | 9 +------- src/Rules/Pure/PureFunctionRule.php | 1 + src/Rules/Pure/PureMethodRule.php | 1 + ...odStatementWithoutImpurePointsRuleTest.php | 8 +++++++ .../PHPStan/Rules/DeadCode/data/bug-12379.php | 22 +++++++++++++++++++ 10 files changed, 52 insertions(+), 18 deletions(-) create mode 100644 tests/PHPStan/Rules/DeadCode/data/bug-12379.php diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 9f726ede71..274d8392e4 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -2969,6 +2969,7 @@ public function enterClassMethod( array $parameterOutTypes = [], array $immediatelyInvokedCallableParameters = [], array $phpDocClosureThisTypeParameters = [], + bool $isConstructor = false, ): self { if (!$this->isInClass()) { @@ -2999,6 +3000,7 @@ public function enterClassMethod( array_map(fn (Type $type): Type => $this->transformStaticType(TemplateTypeHelper::toArgument($type)), $parameterOutTypes), $immediatelyInvokedCallableParameters, array_map(fn (Type $type): Type => $this->transformStaticType(TemplateTypeHelper::toArgument($type)), $phpDocClosureThisTypeParameters), + $isConstructor, ), !$classMethod->isStatic(), ); diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 5f518077a2..c0617856f7 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -614,6 +614,9 @@ private function processStmtNode( $nodeCallback($stmt->returnType, $scope); } + $isFromTrait = $stmt->getAttribute('originalTraitMethodName') === '__construct'; + $isConstructor = $isFromTrait || $stmt->name->toLowerString() === '__construct'; + $methodScope = $scope->enterClassMethod( $stmt, $templateTypeMap, @@ -632,14 +635,14 @@ private function processStmtNode( $phpDocParameterOutTypes, $phpDocImmediatelyInvokedCallableParameters, $phpDocClosureThisTypeParameters, + $isConstructor, ); if (!$scope->isInClass()) { throw new ShouldNotHappenException(); } - $isFromTrait = $stmt->getAttribute('originalTraitMethodName') === '__construct'; - if ($isFromTrait || $stmt->name->toLowerString() === '__construct') { + if ($isConstructor) { foreach ($stmt->params as $param) { if ($param->flags === 0) { continue; diff --git a/src/Reflection/Php/PhpMethodFromParserNodeReflection.php b/src/Reflection/Php/PhpMethodFromParserNodeReflection.php index a05f550105..f105115df7 100644 --- a/src/Reflection/Php/PhpMethodFromParserNodeReflection.php +++ b/src/Reflection/Php/PhpMethodFromParserNodeReflection.php @@ -60,10 +60,14 @@ public function __construct( array $parameterOutTypes, array $immediatelyInvokedCallableParameters, array $phpDocClosureThisTypeParameters, + private bool $isConstructor, ) { $name = strtolower($classMethod->name->name); - if (in_array($name, ['__construct', '__destruct', '__unset', '__wakeup', '__clone'], true)) { + if ($this->isConstructor) { + $realReturnType = new VoidType(); + } + if (in_array($name, ['__destruct', '__unset', '__wakeup', '__clone'], true)) { $realReturnType = new VoidType(); } if ($name === '__tostring') { @@ -175,6 +179,11 @@ public function isAbstract(): TrinaryLogic return TrinaryLogic::createFromBoolean($this->getClassMethod()->isAbstract()); } + public function isConstructor(): bool + { + return $this->isConstructor; + } + public function hasSideEffects(): TrinaryLogic { if ( diff --git a/src/Rules/DeadCode/ConstructorWithoutImpurePointsCollector.php b/src/Rules/DeadCode/ConstructorWithoutImpurePointsCollector.php index 2dca4eb85e..47b2feb4df 100644 --- a/src/Rules/DeadCode/ConstructorWithoutImpurePointsCollector.php +++ b/src/Rules/DeadCode/ConstructorWithoutImpurePointsCollector.php @@ -7,7 +7,6 @@ use PHPStan\Collectors\Collector; use PHPStan\Node\MethodReturnStatementsNode; use function count; -use function strtolower; /** * @implements Collector @@ -23,7 +22,7 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope) { $method = $node->getMethodReflection(); - if (strtolower($method->getName()) !== '__construct') { + if (!$method->isConstructor()) { return null; } diff --git a/src/Rules/DeadCode/MethodWithoutImpurePointsCollector.php b/src/Rules/DeadCode/MethodWithoutImpurePointsCollector.php index 1ec55619b6..675d150d52 100644 --- a/src/Rules/DeadCode/MethodWithoutImpurePointsCollector.php +++ b/src/Rules/DeadCode/MethodWithoutImpurePointsCollector.php @@ -49,11 +49,7 @@ public function processNode(Node $node, Scope $scope) return null; } - $declaringClass = $method->getDeclaringClass(); - if ( - $declaringClass->hasConstructor() - && $declaringClass->getConstructor()->getName() === $method->getName() - ) { + if ($method->isConstructor()) { return null; } diff --git a/src/Rules/Pure/FunctionPurityCheck.php b/src/Rules/Pure/FunctionPurityCheck.php index e70d2eb292..56c78e874b 100644 --- a/src/Rules/Pure/FunctionPurityCheck.php +++ b/src/Rules/Pure/FunctionPurityCheck.php @@ -40,18 +40,11 @@ public function check( array $impurePoints, array $throwPoints, array $statements, + bool $isConstructor, ): array { $errors = []; $isPure = $functionReflection->isPure(); - $isConstructor = false; - if ( - $functionReflection instanceof ExtendedMethodReflection - && $functionReflection->getDeclaringClass()->hasConstructor() - && $functionReflection->getDeclaringClass()->getConstructor()->getName() === $functionReflection->getName() - ) { - $isConstructor = true; - } if ($isPure->yes()) { foreach ($parameters as $parameter) { diff --git a/src/Rules/Pure/PureFunctionRule.php b/src/Rules/Pure/PureFunctionRule.php index 622a093e0b..e05a0be902 100644 --- a/src/Rules/Pure/PureFunctionRule.php +++ b/src/Rules/Pure/PureFunctionRule.php @@ -36,6 +36,7 @@ public function processNode(Node $node, Scope $scope): array $node->getImpurePoints(), $node->getStatementResult()->getThrowPoints(), $node->getStatements(), + false, ); } diff --git a/src/Rules/Pure/PureMethodRule.php b/src/Rules/Pure/PureMethodRule.php index 5dd972f709..8ef6f87c66 100644 --- a/src/Rules/Pure/PureMethodRule.php +++ b/src/Rules/Pure/PureMethodRule.php @@ -36,6 +36,7 @@ public function processNode(Node $node, Scope $scope): array $node->getImpurePoints(), $node->getStatementResult()->getThrowPoints(), $node->getStatements(), + $method->isConstructor(), ); } diff --git a/tests/PHPStan/Rules/DeadCode/CallToMethodStatementWithoutImpurePointsRuleTest.php b/tests/PHPStan/Rules/DeadCode/CallToMethodStatementWithoutImpurePointsRuleTest.php index 77feb89d7d..67f6f42b4e 100644 --- a/tests/PHPStan/Rules/DeadCode/CallToMethodStatementWithoutImpurePointsRuleTest.php +++ b/tests/PHPStan/Rules/DeadCode/CallToMethodStatementWithoutImpurePointsRuleTest.php @@ -72,6 +72,14 @@ public function testBug11011(): void ]); } + public function testBug12379(): void + { + if (PHP_VERSION_ID < 80000) { + $this->markTestSkipped('Test requires PHP 8.1.'); + } + $this->analyse([__DIR__ . '/data/bug-12379.php'], []); + } + protected function getCollectors(): array { return [ diff --git a/tests/PHPStan/Rules/DeadCode/data/bug-12379.php b/tests/PHPStan/Rules/DeadCode/data/bug-12379.php new file mode 100644 index 0000000000..f8dc4ede85 --- /dev/null +++ b/tests/PHPStan/Rules/DeadCode/data/bug-12379.php @@ -0,0 +1,22 @@ += 8.1 + +namespace Bug12379; + +class HelloWorld +{ + use myTrait{ + myTrait::__construct as private __myTraitConstruct; + } + + public function __construct( + int $entityManager + ){ + $this->__myTraitConstruct($entityManager); + } +} + +trait myTrait{ + public function __construct( + private readonly int $entityManager + ){} +} From 65be2b21be5614c7a1d8841a0381fcfb7a1330b8 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 7 Jan 2025 11:40:15 +0100 Subject: [PATCH 34/36] Support arrays with union value-types in `implode()` --- phpstan-baseline.neon | 5 ---- .../ImplodeFunctionReturnTypeExtension.php | 23 +++++++++++--- tests/PHPStan/Analyser/nsrt/bug-11854.php | 18 +++++++++++ tests/PHPStan/Analyser/nsrt/implode.php | 30 +++++++++++++++++++ 4 files changed, 67 insertions(+), 9 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-11854.php diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 9977a1992d..93649e9087 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1362,11 +1362,6 @@ parameters: count: 1 path: src/Type/Php/FunctionExistsFunctionTypeSpecifyingExtension.php - - - message: "#^Doing instanceof PHPStan\\\\Type\\\\ConstantScalarType is error\\-prone and deprecated\\. Use Type\\:\\:isConstantScalarValue\\(\\) or Type\\:\\:getConstantScalarTypes\\(\\) or Type\\:\\:getConstantScalarValues\\(\\) instead\\.$#" - count: 1 - path: src/Type/Php/ImplodeFunctionReturnTypeExtension.php - - message: """ #^Call to deprecated method getConstantScalars\\(\\) of class PHPStan\\\\Type\\\\TypeUtils\\: diff --git a/src/Type/Php/ImplodeFunctionReturnTypeExtension.php b/src/Type/Php/ImplodeFunctionReturnTypeExtension.php index 5c680b5b62..a052a43416 100644 --- a/src/Type/Php/ImplodeFunctionReturnTypeExtension.php +++ b/src/Type/Php/ImplodeFunctionReturnTypeExtension.php @@ -4,7 +4,9 @@ use PhpParser\Node\Expr\FuncCall; use PHPStan\Analyser\Scope; +use PHPStan\Internal\CombinationsHelper; use PHPStan\Reflection\FunctionReflection; +use PHPStan\Reflection\InitializerExprTypeResolver; use PHPStan\Type\Accessory\AccessoryLiteralStringType; use PHPStan\Type\Accessory\AccessoryLowercaseStringType; use PHPStan\Type\Accessory\AccessoryNonEmptyStringType; @@ -12,7 +14,6 @@ use PHPStan\Type\Accessory\AccessoryUppercaseStringType; use PHPStan\Type\Constant\ConstantArrayType; use PHPStan\Type\Constant\ConstantStringType; -use PHPStan\Type\ConstantScalarType; use PHPStan\Type\DynamicFunctionReturnTypeExtension; use PHPStan\Type\IntersectionType; use PHPStan\Type\StringType; @@ -114,14 +115,28 @@ private function inferConstantType(ConstantArrayType $arrayType, ConstantStringT $valueTypes = $array->getValueTypes(); $arrayValues = []; + $combinationsCount = 1; foreach ($valueTypes as $valueType) { - if (!$valueType instanceof ConstantScalarType) { + $constScalars = $valueType->getConstantScalarValues(); + if (count($constScalars) === 0) { return null; } - $arrayValues[] = $valueType->getValue(); + $arrayValues[] = $constScalars; + $combinationsCount *= count($constScalars); } - $strings[] = new ConstantStringType(implode($separatorType->getValue(), $arrayValues)); + if ($combinationsCount > InitializerExprTypeResolver::CALCULATE_SCALARS_LIMIT) { + return null; + } + + $combinations = CombinationsHelper::combinations($arrayValues); + foreach ($combinations as $combination) { + $strings[] = new ConstantStringType(implode($separatorType->getValue(), $combination)); + } + } + + if (count($strings) > InitializerExprTypeResolver::CALCULATE_SCALARS_LIMIT) { + return null; } return TypeCombinator::union(...$strings); diff --git a/tests/PHPStan/Analyser/nsrt/bug-11854.php b/tests/PHPStan/Analyser/nsrt/bug-11854.php new file mode 100644 index 0000000000..48a49258cc --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-11854.php @@ -0,0 +1,18 @@ + Date: Tue, 7 Jan 2025 14:35:06 +0100 Subject: [PATCH 35/36] Add support for result cache meta extensions --- .github/workflows/e2e-tests.yml | 9 +++++ composer.json | 3 ++ e2e/result-cache-meta-extension/hash.txt | 1 + e2e/result-cache-meta-extension/phpstan.neon | 10 +++++ .../src/DummyResultCacheMetaExtension.php | 21 ++++++++++ .../ResultCache/ResultCacheManager.php | 28 +++++++++++++ .../ResultCache/ResultCacheMetaExtension.php | 39 +++++++++++++++++++ .../ConditionalTagsExtension.php | 2 + 8 files changed, 113 insertions(+) create mode 100644 e2e/result-cache-meta-extension/hash.txt create mode 100644 e2e/result-cache-meta-extension/phpstan.neon create mode 100644 e2e/result-cache-meta-extension/src/DummyResultCacheMetaExtension.php create mode 100644 src/Analyser/ResultCache/ResultCacheMetaExtension.php diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index 872d536314..5b77182b86 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -233,6 +233,15 @@ jobs: cd e2e/bug-11857 composer install ../../bin/phpstan + - script: | + cd e2e/result-cache-meta-extension + ../../bin/phpstan -vvv + ../../bin/phpstan -vvv --fail-without-result-cache + echo 'modified-hash' > hash.txt + OUTPUT=$(../bashunit -a exit_code "2" "../../bin/phpstan -vvv --fail-without-result-cache") + echo "$OUTPUT" + ../bashunit -a matches "Note: Using configuration file .+phpstan.neon." "$OUTPUT" + ../bashunit -a contains 'Result cache not used because the metadata do not match: metaExtensions' "$OUTPUT" steps: - name: "Checkout" diff --git a/composer.json b/composer.json index 6edf082303..4a524d5ff7 100644 --- a/composer.json +++ b/composer.json @@ -140,6 +140,9 @@ "classmap": [ "tests/e2e", "tests/PHPStan" + ], + "files": [ + "e2e/result-cache-meta-extension/src/DummyResultCacheMetaExtension.php" ] }, "repositories": [ diff --git a/e2e/result-cache-meta-extension/hash.txt b/e2e/result-cache-meta-extension/hash.txt new file mode 100644 index 0000000000..1f34c8dfd2 --- /dev/null +++ b/e2e/result-cache-meta-extension/hash.txt @@ -0,0 +1 @@ +initial-hash diff --git a/e2e/result-cache-meta-extension/phpstan.neon b/e2e/result-cache-meta-extension/phpstan.neon new file mode 100644 index 0000000000..f2f9c41148 --- /dev/null +++ b/e2e/result-cache-meta-extension/phpstan.neon @@ -0,0 +1,10 @@ +parameters: + level: 8 + paths: + - src + +services: + - + class: ResultCacheE2E\MetaExtension\DummyResultCacheMetaExtension + tags: + - phpstan.resultCacheMetaExtension diff --git a/e2e/result-cache-meta-extension/src/DummyResultCacheMetaExtension.php b/e2e/result-cache-meta-extension/src/DummyResultCacheMetaExtension.php new file mode 100644 index 0000000000..81b6332f96 --- /dev/null +++ b/e2e/result-cache-meta-extension/src/DummyResultCacheMetaExtension.php @@ -0,0 +1,21 @@ + self::CACHE_VERSION, 'phpstanVersion' => ComposerHelper::getPhpStanVersion(), + 'metaExtensions' => $this->getMetaFromPhpStanExtensions(), 'phpVersion' => PHP_VERSION_ID, 'projectConfig' => $projectConfigArray, 'analysedPaths' => $this->analysedPaths, @@ -1036,4 +1039,29 @@ private function getStubFiles(): array return $stubFiles; } + /** + * @return array + * @throws ShouldNotHappenException + */ + private function getMetaFromPhpStanExtensions(): array + { + $meta = []; + + /** @var ResultCacheMetaExtension $extension */ + foreach ($this->container->getServicesByTag(ResultCacheMetaExtension::EXTENSION_TAG) as $extension) { + if (array_key_exists($extension->getKey(), $meta)) { + throw new ShouldNotHappenException(sprintf( + 'Duplicate ResultCacheMetaExtension with key "%s" found.', + $extension->getKey(), + )); + } + + $meta[$extension->getKey()] = $extension->getHash(); + } + + ksort($meta); + + return $meta; + } + } diff --git a/src/Analyser/ResultCache/ResultCacheMetaExtension.php b/src/Analyser/ResultCache/ResultCacheMetaExtension.php new file mode 100644 index 0000000000..11aac2512f --- /dev/null +++ b/src/Analyser/ResultCache/ResultCacheMetaExtension.php @@ -0,0 +1,39 @@ + $bool, LazyParameterOutTypeExtensionProvider::STATIC_METHOD_TAG => $bool, DiagnoseExtension::EXTENSION_TAG => $bool, + ResultCacheMetaExtension::EXTENSION_TAG => $bool, ])->min(1)); } From 33dc7579dec5e6de1f72406ec0b4b0bbfd75533c Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Tue, 7 Jan 2025 14:41:43 +0100 Subject: [PATCH 36/36] Moved the autoload-dev file to e2e/result-cache-meta-extension --- .github/workflows/e2e-tests.yml | 1 + composer.json | 3 --- e2e/result-cache-meta-extension/.gitignore | 1 + e2e/result-cache-meta-extension/composer.json | 5 +++++ e2e/result-cache-meta-extension/composer.lock | 18 ++++++++++++++++++ 5 files changed, 25 insertions(+), 3 deletions(-) create mode 100644 e2e/result-cache-meta-extension/.gitignore create mode 100644 e2e/result-cache-meta-extension/composer.json create mode 100644 e2e/result-cache-meta-extension/composer.lock diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index 5b77182b86..208df4952f 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -235,6 +235,7 @@ jobs: ../../bin/phpstan - script: | cd e2e/result-cache-meta-extension + composer install ../../bin/phpstan -vvv ../../bin/phpstan -vvv --fail-without-result-cache echo 'modified-hash' > hash.txt diff --git a/composer.json b/composer.json index 4a524d5ff7..6edf082303 100644 --- a/composer.json +++ b/composer.json @@ -140,9 +140,6 @@ "classmap": [ "tests/e2e", "tests/PHPStan" - ], - "files": [ - "e2e/result-cache-meta-extension/src/DummyResultCacheMetaExtension.php" ] }, "repositories": [ diff --git a/e2e/result-cache-meta-extension/.gitignore b/e2e/result-cache-meta-extension/.gitignore new file mode 100644 index 0000000000..61ead86667 --- /dev/null +++ b/e2e/result-cache-meta-extension/.gitignore @@ -0,0 +1 @@ +/vendor diff --git a/e2e/result-cache-meta-extension/composer.json b/e2e/result-cache-meta-extension/composer.json new file mode 100644 index 0000000000..a072011fe8 --- /dev/null +++ b/e2e/result-cache-meta-extension/composer.json @@ -0,0 +1,5 @@ +{ + "autoload-dev": { + "classmap": ["src/"] + } +} diff --git a/e2e/result-cache-meta-extension/composer.lock b/e2e/result-cache-meta-extension/composer.lock new file mode 100644 index 0000000000..b383d88ac5 --- /dev/null +++ b/e2e/result-cache-meta-extension/composer.lock @@ -0,0 +1,18 @@ +{ + "_readme": [ + "This file locks the dependencies of your project to a known state", + "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", + "This file is @generated automatically" + ], + "content-hash": "d751713988987e9331980363e24189ce", + "packages": [], + "packages-dev": [], + "aliases": [], + "minimum-stability": "stable", + "stability-flags": {}, + "prefer-stable": false, + "prefer-lowest": false, + "platform": {}, + "platform-dev": {}, + "plugin-api-version": "2.6.0" +}