From accc8314f258b0a2348b7676b1edd12eaa1477fc Mon Sep 17 00:00:00 2001 From: "l.gersen@visymo.com" Date: Mon, 2 Dec 2019 16:42:15 +0100 Subject: [PATCH 1/4] Add rule checking coalesces --- src/Rules/Variables/CoalesceRule.php | 74 +++++++++++++++++++ .../Rules/Variables/CoalesceRuleTest.php | 39 ++++++++++ .../PHPStan/Rules/Variables/data/coalesce.php | 19 +++++ 3 files changed, 132 insertions(+) create mode 100644 src/Rules/Variables/CoalesceRule.php create mode 100644 tests/PHPStan/Rules/Variables/CoalesceRuleTest.php create mode 100644 tests/PHPStan/Rules/Variables/data/coalesce.php diff --git a/src/Rules/Variables/CoalesceRule.php b/src/Rules/Variables/CoalesceRule.php new file mode 100644 index 00000000000..13f75b883d4 --- /dev/null +++ b/src/Rules/Variables/CoalesceRule.php @@ -0,0 +1,74 @@ + + */ +class CoalesceRule implements \PHPStan\Rules\Rule +{ + + public function getNodeType(): string + { + return Node\Expr\BinaryOp\Coalesce::class; + } + + public function processNode(Node $node, Scope $scope): array + { + $error = $this->canBeCoalesced($node->left, $scope); + + if ($error === null) { + return []; + } + + return [$error]; + } + + private function canBeCoalesced(Node $node, Scope $scope): ?RuleError + { + if ($node instanceof Node\Expr\Variable && is_string($node->name)) { + + $hasVariable = $scope->hasVariableType($node->name); + + if ($hasVariable->no()) { + return RuleErrorBuilder::message( + sprintf('Coalesce of undefined variable $%s.', $node->name) + )->line($node->getLine())->build(); + } + + $variableType = $scope->getVariableType($node->name); + + if ($variableType->isSuperTypeOf(new NullType())->no()) { + return RuleErrorBuilder::message( + sprintf('Coalesce of variable $%s, which cannot be null.', $node->name) + )->line($node->getLine())->build(); + } + + } elseif ($node instanceof Node\Expr\ArrayDimFetch && $node->dim !== null) { + $type = $scope->getType($node->var); + $dimType = $scope->getType($node->dim); + + if ($type->isOffsetAccessible()->no() || $type->hasOffsetValueType($dimType)->no()) { + return RuleErrorBuilder::message( + sprintf( + 'Coalesce of invalid offset %s on %s.', + $dimType->describe(VerbosityLevel::value()), + $type->describe(VerbosityLevel::value()) + ) + )->line($node->getLine())->build(); + } + + return $this->canBeCoalesced($node->var, $scope); + } + + return null; + } + +} diff --git a/tests/PHPStan/Rules/Variables/CoalesceRuleTest.php b/tests/PHPStan/Rules/Variables/CoalesceRuleTest.php new file mode 100644 index 00000000000..042098eec57 --- /dev/null +++ b/tests/PHPStan/Rules/Variables/CoalesceRuleTest.php @@ -0,0 +1,39 @@ + + */ +class CoalesceRuleTest extends \PHPStan\Testing\RuleTestCase +{ + + protected function getRule(): \PHPStan\Rules\Rule + { + return new CoalesceRule(); + } + + public function testUnsetRule(): void + { + require_once __DIR__ . '/data/coalesce.php'; + $this->analyse([__DIR__ . '/data/coalesce.php'], [ + [ + 'Coalesce of variable $scalar, which cannot be null.', + 7, + ], + [ + 'Coalesce of invalid offset \'string\' on array(1, 2, 3).', + 11, + ], + [ + 'Coalesce of invalid offset \'string\' on array(array(1), array(2), array(3)).', + 15, + ], + [ + 'Coalesce of undefined variable $doesNotExist.', + 17, + ], + ]); + } + +} diff --git a/tests/PHPStan/Rules/Variables/data/coalesce.php b/tests/PHPStan/Rules/Variables/data/coalesce.php new file mode 100644 index 00000000000..e20e22456ac --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/coalesce.php @@ -0,0 +1,19 @@ + Date: Tue, 3 Dec 2019 10:10:15 +0100 Subject: [PATCH 2/4] Add coalesce rule to level one, handle known nullable offsets --- conf/config.level1.neon | 1 + src/Rules/Variables/CoalesceRule.php | 20 +++++++++++++++++-- .../Rules/Variables/CoalesceRuleTest.php | 4 ++++ .../PHPStan/Rules/Variables/data/coalesce.php | 17 ++++++++++++++++ 4 files changed, 40 insertions(+), 2 deletions(-) diff --git a/conf/config.level1.neon b/conf/config.level1.neon index 769c4922cf3..54d25c8a43a 100644 --- a/conf/config.level1.neon +++ b/conf/config.level1.neon @@ -12,3 +12,4 @@ rules: - PHPStan\Rules\Constants\ConstantRule - PHPStan\Rules\Functions\UnusedClosureUsesRule - PHPStan\Rules\Variables\VariableCertaintyInIssetRule + - PHPStan\Rules\Variables\CoalesceRule diff --git a/src/Rules/Variables/CoalesceRule.php b/src/Rules/Variables/CoalesceRule.php index 13f75b883d4..7fc3368c9dc 100644 --- a/src/Rules/Variables/CoalesceRule.php +++ b/src/Rules/Variables/CoalesceRule.php @@ -45,7 +45,8 @@ private function canBeCoalesced(Node $node, Scope $scope): ?RuleError $variableType = $scope->getVariableType($node->name); - if ($variableType->isSuperTypeOf(new NullType())->no()) { + // For hasVariable->maybe a coalesce is valid + if ($hasVariable->yes() && $variableType->isSuperTypeOf(new NullType())->no()) { return RuleErrorBuilder::message( sprintf('Coalesce of variable $%s, which cannot be null.', $node->name) )->line($node->getLine())->build(); @@ -54,8 +55,9 @@ private function canBeCoalesced(Node $node, Scope $scope): ?RuleError } elseif ($node instanceof Node\Expr\ArrayDimFetch && $node->dim !== null) { $type = $scope->getType($node->var); $dimType = $scope->getType($node->dim); + $hasOffsetValue = $type->hasOffsetValueType($dimType); - if ($type->isOffsetAccessible()->no() || $type->hasOffsetValueType($dimType)->no()) { + if ($type->isOffsetAccessible()->no() || $hasOffsetValue->no()) { return RuleErrorBuilder::message( sprintf( 'Coalesce of invalid offset %s on %s.', @@ -65,6 +67,20 @@ private function canBeCoalesced(Node $node, Scope $scope): ?RuleError )->line($node->getLine())->build(); } + if ($hasOffsetValue->maybe()) { + return null; + } + + if ($hasOffsetValue->yes() && $type->isSuperTypeOf(new NullType())->no()) { + return RuleErrorBuilder::message( + sprintf( + 'Coalesce of offset %s on %s, which cannot be null.', + $dimType->describe(VerbosityLevel::value()), + $type->describe(VerbosityLevel::value()) + ) + )->line($node->getLine())->build(); + } + return $this->canBeCoalesced($node->var, $scope); } diff --git a/tests/PHPStan/Rules/Variables/CoalesceRuleTest.php b/tests/PHPStan/Rules/Variables/CoalesceRuleTest.php index 042098eec57..055ae483d7b 100644 --- a/tests/PHPStan/Rules/Variables/CoalesceRuleTest.php +++ b/tests/PHPStan/Rules/Variables/CoalesceRuleTest.php @@ -33,6 +33,10 @@ public function testUnsetRule(): void 'Coalesce of undefined variable $doesNotExist.', 17, ], + [ + 'Coalesce of offset \'dim\' on array(\'dim\' => 1), which cannot be null.', + 27, + ], ]); } diff --git a/tests/PHPStan/Rules/Variables/data/coalesce.php b/tests/PHPStan/Rules/Variables/data/coalesce.php index e20e22456ac..59b9cf6e2e6 100644 --- a/tests/PHPStan/Rules/Variables/data/coalesce.php +++ b/tests/PHPStan/Rules/Variables/data/coalesce.php @@ -16,4 +16,21 @@ function coalesce () { echo $doesNotExist ?? 0; + if(rand() > 0.5) { + $maybeVariable = 3; + } + + echo $maybeVariable ?? 0; + + $fixedDimArray = ['dim' => 1]; + + echo $fixedDimArray['dim'] ?? 0; +} + +/** + * @param array $array + */ +function coalesceStringOffset(array $array) +{ + echo $array['string'] ?? 0; } From d458464dbf688396e67b1cc64439da38ba4be5f7 Mon Sep 17 00:00:00 2001 From: "l.gersen@visymo.com" Date: Tue, 21 Jan 2020 16:48:44 +0100 Subject: [PATCH 3/4] Support AssignOp\Coalesce in CoalesceRule, add test cases for ArrayDimFetch --- src/Rules/Variables/CoalesceRule.php | 44 ++++++++++----- .../Rules/Variables/CoalesceRuleTest.php | 53 +++++++++++++++--- .../Rules/Variables/data/coalesce-assign.php | 55 +++++++++++++++++++ .../PHPStan/Rules/Variables/data/coalesce.php | 23 +++++++- 4 files changed, 150 insertions(+), 25 deletions(-) create mode 100644 tests/PHPStan/Rules/Variables/data/coalesce-assign.php diff --git a/src/Rules/Variables/CoalesceRule.php b/src/Rules/Variables/CoalesceRule.php index 7fc3368c9dc..83827385062 100644 --- a/src/Rules/Variables/CoalesceRule.php +++ b/src/Rules/Variables/CoalesceRule.php @@ -10,19 +10,25 @@ use PHPStan\Type\VerbosityLevel; /** - * @implements \PHPStan\Rules\Rule<\PhpParser\Node\Expr\BinaryOp\Coalesce> + * @implements \PHPStan\Rules\Rule<\PhpParser\Node\Expr> */ class CoalesceRule implements \PHPStan\Rules\Rule { public function getNodeType(): string { - return Node\Expr\BinaryOp\Coalesce::class; + return \PhpParser\Node\Expr::class; } public function processNode(Node $node, Scope $scope): array { - $error = $this->canBeCoalesced($node->left, $scope); + if ($node instanceof Node\Expr\BinaryOp\Coalesce) { + $error = $this->canBeCoalesced($node->left, $scope, 'Coalesce'); + } elseif ($node instanceof Node\Expr\AssignOp\Coalesce) { + $error = $this->canBeCoalesced($node->var, $scope, 'Null-coalescing assignment'); + } else { + return []; + } if ($error === null) { return []; @@ -31,15 +37,15 @@ public function processNode(Node $node, Scope $scope): array return [$error]; } - private function canBeCoalesced(Node $node, Scope $scope): ?RuleError + private function canBeCoalesced(Node $node, Scope $scope, string $action, ?RuleError $error = null): ?RuleError { if ($node instanceof Node\Expr\Variable && is_string($node->name)) { $hasVariable = $scope->hasVariableType($node->name); if ($hasVariable->no()) { - return RuleErrorBuilder::message( - sprintf('Coalesce of undefined variable $%s.', $node->name) + return $error ?? RuleErrorBuilder::message( + sprintf('%s of undefined variable $%s.', $action, $node->name) )->line($node->getLine())->build(); } @@ -47,20 +53,22 @@ private function canBeCoalesced(Node $node, Scope $scope): ?RuleError // For hasVariable->maybe a coalesce is valid if ($hasVariable->yes() && $variableType->isSuperTypeOf(new NullType())->no()) { - return RuleErrorBuilder::message( - sprintf('Coalesce of variable $%s, which cannot be null.', $node->name) + return $error ?? RuleErrorBuilder::message( + sprintf('%s of variable $%s, which cannot be null.', $action, $node->name) )->line($node->getLine())->build(); } } elseif ($node instanceof Node\Expr\ArrayDimFetch && $node->dim !== null) { + $type = $scope->getType($node->var); $dimType = $scope->getType($node->dim); $hasOffsetValue = $type->hasOffsetValueType($dimType); - if ($type->isOffsetAccessible()->no() || $hasOffsetValue->no()) { - return RuleErrorBuilder::message( + if ($hasOffsetValue->no() || $type->isOffsetAccessible()->no()) { + return $error ?? RuleErrorBuilder::message( sprintf( - 'Coalesce of invalid offset %s on %s.', + '%s of invalid offset %s on %s.', + $action, $dimType->describe(VerbosityLevel::value()), $type->describe(VerbosityLevel::value()) ) @@ -71,17 +79,23 @@ private function canBeCoalesced(Node $node, Scope $scope): ?RuleError return null; } - if ($hasOffsetValue->yes() && $type->isSuperTypeOf(new NullType())->no()) { - return RuleErrorBuilder::message( + // If offset is cannot be null, store this error message and see if one of the earlier offsets is. + // E.g. $array['a']['b']['c'] ?? null; is a valid coalesce if a OR b or C might be null. + if ($hasOffsetValue->yes() && $type->getOffsetValueType($dimType)->isSuperTypeOf(new NullType())->no()) { + $error = RuleErrorBuilder::message( sprintf( - 'Coalesce of offset %s on %s, which cannot be null.', + '%s of offset %s on %s, which cannot be null.', + $action, $dimType->describe(VerbosityLevel::value()), $type->describe(VerbosityLevel::value()) ) )->line($node->getLine())->build(); + + return $this->canBeCoalesced($node->var, $scope, $action, $error); } - return $this->canBeCoalesced($node->var, $scope); + // Has offset, it is nullable + return null; } return null; diff --git a/tests/PHPStan/Rules/Variables/CoalesceRuleTest.php b/tests/PHPStan/Rules/Variables/CoalesceRuleTest.php index 055ae483d7b..21916edf0f0 100644 --- a/tests/PHPStan/Rules/Variables/CoalesceRuleTest.php +++ b/tests/PHPStan/Rules/Variables/CoalesceRuleTest.php @@ -13,29 +13,66 @@ protected function getRule(): \PHPStan\Rules\Rule return new CoalesceRule(); } - public function testUnsetRule(): void + public function testCoalesceRule(): void { - require_once __DIR__ . '/data/coalesce.php'; $this->analyse([__DIR__ . '/data/coalesce.php'], [ [ 'Coalesce of variable $scalar, which cannot be null.', - 7, + 8, ], [ 'Coalesce of invalid offset \'string\' on array(1, 2, 3).', - 11, + 12, ], [ 'Coalesce of invalid offset \'string\' on array(array(1), array(2), array(3)).', - 15, + 16, ], [ 'Coalesce of undefined variable $doesNotExist.', - 17, + 18, ], [ - 'Coalesce of offset \'dim\' on array(\'dim\' => 1), which cannot be null.', - 27, + 'Coalesce of offset \'dim\' on array(\'dim\' => 1, \'dim-null\' => 1|null, \'dim-null-offset\' => array(\'a\' => true|null), \'dim-empty\' => array()), which cannot be null.', + 34, + ], + [ + 'Coalesce of invalid offset \'b\' on array().', + 46, + ], + ]); + } + + public function testCoalesceAssignRule(): void + { + if (PHP_VERSION_ID < 70400) { + $this->markTestSkipped('Test requires PHP 7.4.'); + } + + $this->analyse([__DIR__ . '/data/coalesce-assign.php'], [ + [ + 'Null-coalescing assignment of variable $scalar, which cannot be null.', + 8, + ], + [ + 'Null-coalescing assignment of invalid offset \'string\' on array(1, 2, 3).', + 12, + ], + [ + 'Null-coalescing assignment of invalid offset \'string\' on array(array(1), array(2), array(3)).', + 16, + ], + [ + 'Null-coalescing assignment of undefined variable $doesNotExist.', + 18, + ], + [ + 'Null-coalescing assignment of offset \'dim\' on array(\'dim\' => 1, \'dim-null\' => 1|null, \'dim-null-offset\' => array(\'a\' => true|null), \'dim-empty\' => array()), which cannot be null.', + 34, + ], + [ + 'Null-coalescing assignment of invalid offset \'b\' on array().', + 46, ], ]); } diff --git a/tests/PHPStan/Rules/Variables/data/coalesce-assign.php b/tests/PHPStan/Rules/Variables/data/coalesce-assign.php new file mode 100644 index 00000000000..28e611b04f6 --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/coalesce-assign.php @@ -0,0 +1,55 @@ += 7.4 + +function coalesceAssign () +{ + + $scalar = 3; + + echo $scalar ??= 4; + + $array = [1, 2, 3]; + + echo $array['string'] ??= 0; + + $multiDimArray = [[1], [2], [3]]; + + echo $multiDimArray['string'] ??= 0; + + echo $doesNotExist ??= 0; + + if(rand() > 0.5) { + $maybeVariable = 3; + } + + echo $maybeVariable ??= 0; + + $fixedDimArray = [ + 'dim' => 1, + 'dim-null' => rand() > 0.5 ? null : 1, + 'dim-null-offset' => ['a' => rand() > 0.5 ? true : null], + 'dim-empty' => [] + ]; + + // Always set + echo $fixedDimArray['dim'] ??= 0; + + // Maybe set + echo $fixedDimArray['dim-null'] ??= 0; + + // Never set, then unknown + echo $fixedDimArray['dim-null-not-set']['a'] ??= 0; + + // Always set, then always set + echo $fixedDimArray['dim-null-offset']['a'] ??= 0; + + // Always set, then never set + echo $fixedDimArray['dim-empty']['b'] ??= 0; +} + +/** + * @param array $array + */ +function coalesceAssignStringOffset(array $array) +{ + echo $array['string'] ??= 0; +} diff --git a/tests/PHPStan/Rules/Variables/data/coalesce.php b/tests/PHPStan/Rules/Variables/data/coalesce.php index 59b9cf6e2e6..cc8e29004ea 100644 --- a/tests/PHPStan/Rules/Variables/data/coalesce.php +++ b/tests/PHPStan/Rules/Variables/data/coalesce.php @@ -1,6 +1,7 @@ 1]; + $fixedDimArray = [ + 'dim' => 1, + 'dim-null' => rand() > 0.5 ? null : 1, + 'dim-null-offset' => ['a' => rand() > 0.5 ? true : null], + 'dim-empty' => [] + ]; + // Always set echo $fixedDimArray['dim'] ?? 0; + + // Maybe set + echo $fixedDimArray['dim-null'] ?? 0; + + // Never set, then unknown + echo $fixedDimArray['dim-null-not-set']['a'] ?? 0; + + // Always set, then always set + echo $fixedDimArray['dim-null-offset']['a'] ?? 0; + + // Always set, then never set + echo $fixedDimArray['dim-empty']['b'] ?? 0; } /** From cf7ed83335bbea8065b0f72491100b6d9785026d Mon Sep 17 00:00:00 2001 From: "l.gersen@visymo.com" Date: Wed, 22 Jan 2020 15:23:47 +0100 Subject: [PATCH 4/4] Add support for method calls, PropertyFetch and StaticPropertyFetch --- src/Rules/Variables/CoalesceRule.php | 126 +++++++++++++++--- .../Rules/Variables/CoalesceRuleTest.php | 91 +++++++++++-- .../Rules/Variables/data/coalesce-assign.php | 68 +++++++++- .../PHPStan/Rules/Variables/data/coalesce.php | 64 ++++++++- 4 files changed, 315 insertions(+), 34 deletions(-) diff --git a/src/Rules/Variables/CoalesceRule.php b/src/Rules/Variables/CoalesceRule.php index 83827385062..d8032d45dfd 100644 --- a/src/Rules/Variables/CoalesceRule.php +++ b/src/Rules/Variables/CoalesceRule.php @@ -3,10 +3,14 @@ namespace PHPStan\Rules\Variables; use PhpParser\Node; +use PhpParser\Node\Expr; use PHPStan\Analyser\Scope; +use PHPStan\Rules\Properties\PropertyDescriptor; +use PHPStan\Rules\Properties\PropertyReflectionFinder; use PHPStan\Rules\RuleError; use PHPStan\Rules\RuleErrorBuilder; use PHPStan\Type\NullType; +use PHPStan\Type\Type; use PHPStan\Type\VerbosityLevel; /** @@ -15,6 +19,21 @@ class CoalesceRule implements \PHPStan\Rules\Rule { + /** @var \PHPStan\Rules\Properties\PropertyDescriptor */ + private $propertyDescriptor; + + /** @var \PHPStan\Rules\Properties\PropertyReflectionFinder */ + private $propertyReflectionFinder; + + public function __construct( + PropertyDescriptor $propertyDescriptor, + PropertyReflectionFinder $propertyReflectionFinder + ) + { + $this->propertyDescriptor = $propertyDescriptor; + $this->propertyReflectionFinder = $propertyReflectionFinder; + } + public function getNodeType(): string { return \PhpParser\Node\Expr::class; @@ -39,7 +58,23 @@ public function processNode(Node $node, Scope $scope): array private function canBeCoalesced(Node $node, Scope $scope, string $action, ?RuleError $error = null): ?RuleError { - if ($node instanceof Node\Expr\Variable && is_string($node->name)) { + if ($node instanceof \PhpParser\Node\Expr\FuncCall || + $node instanceof \PhpParser\Node\Expr\MethodCall || + $node instanceof \PhpParser\Node\Expr\StaticCall) { + + if ($node->name instanceof Expr) { + return null; + } + + $resultingType = $scope->getType($node); + + $error = $this->generateError( + $resultingType, + $node, + sprintf('%s of return value for call to \'%s\'', $action, $node->name->toString()) + ); + + } elseif ($node instanceof Node\Expr\Variable && is_string($node->name)) { $hasVariable = $scope->hasVariableType($node->name); @@ -51,11 +86,16 @@ private function canBeCoalesced(Node $node, Scope $scope, string $action, ?RuleE $variableType = $scope->getVariableType($node->name); - // For hasVariable->maybe a coalesce is valid - if ($hasVariable->yes() && $variableType->isSuperTypeOf(new NullType())->no()) { - return $error ?? RuleErrorBuilder::message( - sprintf('%s of variable $%s, which cannot be null.', $action, $node->name) - )->line($node->getLine())->build(); + if ($hasVariable->maybe()) { + return null; + } + + if ($hasVariable->yes()) { + return $error ?? $this->generateError( + $variableType, + $node, + sprintf('%s of variable $%s', $action, $node->name) + ); } } elseif ($node instanceof Node\Expr\ArrayDimFetch && $node->dim !== null) { @@ -81,24 +121,76 @@ private function canBeCoalesced(Node $node, Scope $scope, string $action, ?RuleE // If offset is cannot be null, store this error message and see if one of the earlier offsets is. // E.g. $array['a']['b']['c'] ?? null; is a valid coalesce if a OR b or C might be null. - if ($hasOffsetValue->yes() && $type->getOffsetValueType($dimType)->isSuperTypeOf(new NullType())->no()) { - $error = RuleErrorBuilder::message( - sprintf( - '%s of offset %s on %s, which cannot be null.', - $action, - $dimType->describe(VerbosityLevel::value()), - $type->describe(VerbosityLevel::value()) - ) - )->line($node->getLine())->build(); - - return $this->canBeCoalesced($node->var, $scope, $action, $error); + if ($hasOffsetValue->yes()) { + + $error = $error ?? $this->generateError($type->getOffsetValueType($dimType), $node, sprintf( + '%s of offset %s on %s', + $action, + $dimType->describe(VerbosityLevel::value()), + $type->describe(VerbosityLevel::value()) + )); + + if ($error !== null) { + return $this->canBeCoalesced($node->var, $scope, $action, $error); + } } // Has offset, it is nullable return null; + + } elseif ($node instanceof Node\Expr\PropertyFetch || $node instanceof Node\Expr\StaticPropertyFetch) { + + $propertyReflection = $this->propertyReflectionFinder->findPropertyReflectionFromNode($node, $scope); + + if ($propertyReflection === null) { + return null; + } + + $propertyDescription = $this->propertyDescriptor->describeProperty($propertyReflection, $node); + $propertyType = $propertyReflection->getWritableType(); + + $error = $error ?? $this->generateError( + $propertyReflection->getWritableType(), + $node, + sprintf('%s of %s (%s)', $action, $propertyDescription, $propertyType->describe(VerbosityLevel::typeOnly())) + ); + + if ($error !== null && property_exists($node, 'var')) { + return $this->canBeCoalesced($node->var, $scope, $action, $error); + } + + } + + return $error; + } + + private function generateError(Type $type, Node $node, string $message): ?RuleError + { + $nullType = new NullType(); + + if ($type->equals($nullType)) { + return $this->generateAlwaysNullError($node, $message); + } + + if ($type->isSuperTypeOf($nullType)->no()) { + return $this->generateNeverNullError($node, $message); } return null; } + private function generateAlwaysNullError(Node $node, string $message): RuleError + { + return RuleErrorBuilder::message( + sprintf('%s, which is always null.', $message) + )->line($node->getLine())->build(); + } + + private function generateNeverNullError(Node $node, string $message): RuleError + { + return RuleErrorBuilder::message( + sprintf('%s, which cannot be null.', $message) + )->line($node->getLine())->build(); + } + } diff --git a/tests/PHPStan/Rules/Variables/CoalesceRuleTest.php b/tests/PHPStan/Rules/Variables/CoalesceRuleTest.php index 21916edf0f0..05c57c81362 100644 --- a/tests/PHPStan/Rules/Variables/CoalesceRuleTest.php +++ b/tests/PHPStan/Rules/Variables/CoalesceRuleTest.php @@ -2,6 +2,9 @@ namespace PHPStan\Rules\Variables; +use PHPStan\Rules\Properties\PropertyDescriptor; +use PHPStan\Rules\Properties\PropertyReflectionFinder; + /** * @extends \PHPStan\Testing\RuleTestCase */ @@ -10,35 +13,68 @@ class CoalesceRuleTest extends \PHPStan\Testing\RuleTestCase protected function getRule(): \PHPStan\Rules\Rule { - return new CoalesceRule(); + return new CoalesceRule(new PropertyDescriptor(), new PropertyReflectionFinder()); } public function testCoalesceRule(): void { + require_once __DIR__ . '/data/coalesce.php'; $this->analyse([__DIR__ . '/data/coalesce.php'], [ + [ + 'Coalesce of Property CoalesceRule\FooCoalesce::$string (string), which cannot be null.', + 32, + ], [ 'Coalesce of variable $scalar, which cannot be null.', - 8, + 41, ], [ 'Coalesce of invalid offset \'string\' on array(1, 2, 3).', - 12, + 45, ], [ 'Coalesce of invalid offset \'string\' on array(array(1), array(2), array(3)).', - 16, + 49, ], [ 'Coalesce of undefined variable $doesNotExist.', - 18, + 51, ], [ 'Coalesce of offset \'dim\' on array(\'dim\' => 1, \'dim-null\' => 1|null, \'dim-null-offset\' => array(\'a\' => true|null), \'dim-empty\' => array()), which cannot be null.', - 34, + 67, ], [ 'Coalesce of invalid offset \'b\' on array().', - 46, + 79, + ], + [ + 'Coalesce of return value for call to \'rand\', which cannot be null.', + 81, + ], + [ + 'Coalesce of Property CoalesceRule\FooCoalesce::$string (string), which cannot be null.', + 89, + ], + [ + 'Coalesce of Property CoalesceRule\FooCoalesce::$alwaysNull (null), which is always null.', + 91, + ], + [ + 'Coalesce of Property CoalesceRule\FooCoalesce::$string (string), which cannot be null.', + 93, + ], + [ + 'Coalesce of Static property CoalesceRule\FooCoalesce::$staticString (string), which cannot be null.', + 99, + ], + [ + 'Coalesce of Static property CoalesceRule\FooCoalesce::$staticAlwaysNull (null), which is always null.', + 101, + ], + [ + 'Coalesce of variable $a, which is always null.', + 115, ], ]); } @@ -49,30 +85,59 @@ public function testCoalesceAssignRule(): void $this->markTestSkipped('Test requires PHP 7.4.'); } + require_once __DIR__ . '/data/coalesce-assign.php'; $this->analyse([__DIR__ . '/data/coalesce-assign.php'], [ + [ + 'Null-coalescing assignment of Property CoalesceAssignRule\FooCoalesce::$string (string), which cannot be null.', + 32, + ], [ 'Null-coalescing assignment of variable $scalar, which cannot be null.', - 8, + 41, ], [ 'Null-coalescing assignment of invalid offset \'string\' on array(1, 2, 3).', - 12, + 45, ], [ 'Null-coalescing assignment of invalid offset \'string\' on array(array(1), array(2), array(3)).', - 16, + 49, ], [ 'Null-coalescing assignment of undefined variable $doesNotExist.', - 18, + 51, ], [ 'Null-coalescing assignment of offset \'dim\' on array(\'dim\' => 1, \'dim-null\' => 1|null, \'dim-null-offset\' => array(\'a\' => true|null), \'dim-empty\' => array()), which cannot be null.', - 34, + 67, ], [ 'Null-coalescing assignment of invalid offset \'b\' on array().', - 46, + 79, + ], + [ + 'Null-coalescing assignment of Property CoalesceAssignRule\FooCoalesce::$string (string), which cannot be null.', + 89, + ], + [ + 'Null-coalescing assignment of Property CoalesceAssignRule\FooCoalesce::$alwaysNull (null), which is always null.', + 91, + ], + [ + 'Null-coalescing assignment of Property CoalesceAssignRule\FooCoalesce::$string (string), which cannot be null.', + 93, + ], + [ + 'Null-coalescing assignment of Static property CoalesceAssignRule\FooCoalesce::$staticString (string), which cannot be null.', + 99, + ], + [ + 'Null-coalescing assignment of Static property CoalesceAssignRule\FooCoalesce::$staticAlwaysNull (null), which is always null.', + 101, + ], + [ + 'Null-coalescing assignment of variable $a, which is always null.', + 115, ], ]); } diff --git a/tests/PHPStan/Rules/Variables/data/coalesce-assign.php b/tests/PHPStan/Rules/Variables/data/coalesce-assign.php index 28e611b04f6..df69c39680e 100644 --- a/tests/PHPStan/Rules/Variables/data/coalesce-assign.php +++ b/tests/PHPStan/Rules/Variables/data/coalesce-assign.php @@ -1,6 +1,39 @@ = 7.4 -function coalesceAssign () +namespace CoalesceAssignRule; + +class FooCoalesce +{ + /** @var string|null */ + public static $staticStringOrNull = null; + + /** @var string */ + public static $staticString = ''; + + /** @var null */ + public static $staticAlwaysNull; + + /** @var string|null */ + public $stringOrNull = null; + + /** @var string */ + public $string = ''; + + /** @var null */ + public $alwaysNull; + + /** @var FooCoalesce|null */ + public $fooCoalesceOrNull; + + /** @var FooCoalesce */ + public $fooCoalesce; + + public function thisCoalesce() { + echo $this->string ??= null; + } +} + +function coalesce() { $scalar = 3; @@ -17,7 +50,7 @@ function coalesceAssign () echo $doesNotExist ??= 0; - if(rand() > 0.5) { + if (rand() > 0.5) { $maybeVariable = 3; } @@ -44,12 +77,41 @@ function coalesceAssign () // Always set, then never set echo $fixedDimArray['dim-empty']['b'] ??= 0; + +// echo rand() ??= 0; // not valid for assignment + +// echo preg_replace('', '', '') ??= 0; // not valid for assignment + + $foo = new FooCoalesce(); + + echo $foo->stringOrNull ??= ''; + + echo $foo->string ??= ''; + + echo $foo->alwaysNull ??= ''; + + echo $foo->fooCoalesce->string ??= ''; + + echo $foo->fooCoalesceOrNull->string ??= ''; + + echo FooCoalesce::$staticStringOrNull ??= ''; + + echo FooCoalesce::$staticString ??= ''; + + echo FooCoalesce::$staticAlwaysNull ??= ''; } /** * @param array $array */ -function coalesceAssignStringOffset(array $array) +function coalesceStringOffset(array $array) { echo $array['string'] ??= 0; } + +function alwaysNullCoalesce (?string $a): void +{ + if (!is_string($a)) { + echo $a ??= 'foo'; + } +} diff --git a/tests/PHPStan/Rules/Variables/data/coalesce.php b/tests/PHPStan/Rules/Variables/data/coalesce.php index cc8e29004ea..8f9af52a120 100644 --- a/tests/PHPStan/Rules/Variables/data/coalesce.php +++ b/tests/PHPStan/Rules/Variables/data/coalesce.php @@ -1,5 +1,38 @@ string ?? null; + } +} + function coalesce() { @@ -17,7 +50,7 @@ function coalesce() echo $doesNotExist ?? 0; - if(rand() > 0.5) { + if (rand() > 0.5) { $maybeVariable = 3; } @@ -44,6 +77,28 @@ function coalesce() // Always set, then never set echo $fixedDimArray['dim-empty']['b'] ?? 0; + + echo rand() ?? 0; + + echo preg_replace('', '', '') ?? 0; + + $foo = new FooCoalesce(); + + echo $foo->stringOrNull ?? ''; + + echo $foo->string ?? ''; + + echo $foo->alwaysNull ?? ''; + + echo $foo->fooCoalesce->string ?? ''; + + echo $foo->fooCoalesceOrNull->string ?? ''; + + echo FooCoalesce::$staticStringOrNull ?? ''; + + echo FooCoalesce::$staticString ?? ''; + + echo FooCoalesce::$staticAlwaysNull ?? ''; } /** @@ -53,3 +108,10 @@ function coalesceStringOffset(array $array) { echo $array['string'] ?? 0; } + +function alwaysNullCoalesce (?string $a): void +{ + if (!is_string($a)) { + echo $a ?? 'foo'; + } +}