diff --git a/conf/config.level1.neon b/conf/config.level1.neon index 769c4922cf..54d25c8a43 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 new file mode 100644 index 0000000000..d8032d45df --- /dev/null +++ b/src/Rules/Variables/CoalesceRule.php @@ -0,0 +1,196 @@ + + */ +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; + } + + public function processNode(Node $node, Scope $scope): array + { + 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 []; + } + + return [$error]; + } + + private function canBeCoalesced(Node $node, Scope $scope, string $action, ?RuleError $error = null): ?RuleError + { + 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); + + if ($hasVariable->no()) { + return $error ?? RuleErrorBuilder::message( + sprintf('%s of undefined variable $%s.', $action, $node->name) + )->line($node->getLine())->build(); + } + + $variableType = $scope->getVariableType($node->name); + + 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) { + + $type = $scope->getType($node->var); + $dimType = $scope->getType($node->dim); + $hasOffsetValue = $type->hasOffsetValueType($dimType); + + if ($hasOffsetValue->no() || $type->isOffsetAccessible()->no()) { + return $error ?? RuleErrorBuilder::message( + sprintf( + '%s of invalid offset %s on %s.', + $action, + $dimType->describe(VerbosityLevel::value()), + $type->describe(VerbosityLevel::value()) + ) + )->line($node->getLine())->build(); + } + + if ($hasOffsetValue->maybe()) { + return null; + } + + // 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()) { + + $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 new file mode 100644 index 0000000000..05c57c8136 --- /dev/null +++ b/tests/PHPStan/Rules/Variables/CoalesceRuleTest.php @@ -0,0 +1,145 @@ + + */ +class CoalesceRuleTest extends \PHPStan\Testing\RuleTestCase +{ + + protected function getRule(): \PHPStan\Rules\Rule + { + 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.', + 41, + ], + [ + 'Coalesce of invalid offset \'string\' on array(1, 2, 3).', + 45, + ], + [ + 'Coalesce of invalid offset \'string\' on array(array(1), array(2), array(3)).', + 49, + ], + [ + 'Coalesce of undefined variable $doesNotExist.', + 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.', + 67, + ], + [ + 'Coalesce of invalid offset \'b\' on array().', + 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, + ], + ]); + } + + public function testCoalesceAssignRule(): void + { + if (PHP_VERSION_ID < 70400) { + $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.', + 41, + ], + [ + 'Null-coalescing assignment of invalid offset \'string\' on array(1, 2, 3).', + 45, + ], + [ + 'Null-coalescing assignment of invalid offset \'string\' on array(array(1), array(2), array(3)).', + 49, + ], + [ + 'Null-coalescing assignment of undefined variable $doesNotExist.', + 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.', + 67, + ], + [ + 'Null-coalescing assignment of invalid offset \'b\' on array().', + 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 new file mode 100644 index 0000000000..df69c39680 --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/coalesce-assign.php @@ -0,0 +1,117 @@ += 7.4 + +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; + + 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; + +// 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 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 new file mode 100644 index 0000000000..8f9af52a12 --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/coalesce.php @@ -0,0 +1,117 @@ +string ?? null; + } +} + +function coalesce() +{ + + $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; + + 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 ?? ''; +} + +/** + * @param array $array + */ +function coalesceStringOffset(array $array) +{ + echo $array['string'] ?? 0; +} + +function alwaysNullCoalesce (?string $a): void +{ + if (!is_string($a)) { + echo $a ?? 'foo'; + } +}