Rule for checking null-coalescing operator (level 1) - bleeding edge

phpstan · Feb 29, 2020 · 3608ded · 3608ded
1 parent a68512e
commit 3608ded
Show file tree

Hide file tree

Showing 7 changed files with 609 additions and 1 deletion.
diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon
@@ -3,3 +3,4 @@ parameters:
 		closureUsesThis: true
 		randomIntParameters: true
 		resultCache: true
+		nullCoalesce: true
diff --git a/conf/config.level1.neon b/conf/config.level1.neon
@@ -7,8 +7,16 @@ parameters:
 	reportMagicMethods: true
 	reportMagicProperties: true
 
+conditionalTags:
+	PHPStan\Rules\Variables\NullCoalesceRule:
+		phpstan.rules.rule: %featureToggles.nullCoalesce%
+
 rules:
 	- PHPStan\Rules\Classes\UnusedConstructorParametersRule
 	- PHPStan\Rules\Constants\ConstantRule
 	- PHPStan\Rules\Functions\UnusedClosureUsesRule
 	- PHPStan\Rules\Variables\VariableCertaintyInIssetRule
+
+services:
+	-
+		class: PHPStan\Rules\Variables\NullCoalesceRule
diff --git a/conf/config.neon b/conf/config.neon
@@ -13,6 +13,7 @@ parameters:
 		resultCache: false
 		closureUsesThis: false
 		randomIntParameters: false
+		nullCoalesce: false
 	fileExtensions:
 		- php
 	checkAlwaysTrueCheckTypeFunctionCall: false
@@ -134,7 +135,8 @@ parametersSchema:
 		enableScanningPaths: bool(),
 		resultCache: bool(),
 		closureUsesThis: bool(),
-		randomIntParameters: bool()
+		randomIntParameters: bool(),
+		nullCoalesce: bool()
 	])
 	fileExtensions: listOf(string())
 	checkAlwaysTrueCheckTypeFunctionCall: bool()

diff --git a/src/Rules/Variables/NullCoalesceRule.php b/src/Rules/Variables/NullCoalesceRule.php
@@ -0,0 +1,178 @@
+<?php declare(strict_types = 1);
+
+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;
+
+/**
+ * @implements \PHPStan\Rules\Rule<\PhpParser\Node\Expr>
+ */
+class NullCoalesceRule 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, '??');
+		} elseif ($node instanceof Node\Expr\AssignOp\Coalesce) {
+			$error = $this->canBeCoalesced($node->var, $scope, '??=');
+		} else {
+			return [];
+		}
+
+		if ($error === null) {
+			return [];
+		}
+
+		return [$error];
+	}
+
+	private function canBeCoalesced(Expr $expr, Scope $scope, string $action, ?RuleError $error = null): ?RuleError
+	{
+		if ($expr instanceof Node\Expr\Variable && is_string($expr->name)) {
+
+			$hasVariable = $scope->hasVariableType($expr->name);
+
+			if ($hasVariable->no()) {
+				return $error ?? RuleErrorBuilder::message(
+					sprintf('Variable $%s on left side of %s is never defined.', $expr->name, $action)
+				)->build();
+			}
+
+			$variableType = $scope->getType($expr);
+
+			if ($hasVariable->maybe()) {
+				return null;
+			}
+
+			if ($hasVariable->yes()) {
+				return $error ?? $this->generateError(
+					$variableType,
+					sprintf('Variable $%s on left side of %s always exists and', $expr->name, $action)
+				);
+			}
+
+		} elseif ($expr instanceof Node\Expr\ArrayDimFetch && $expr->dim !== null) {
+
+			$type = $scope->getType($expr->var);
+			$dimType = $scope->getType($expr->dim);
+			$hasOffsetValue = $type->hasOffsetValueType($dimType);
+			if (!$type->isOffsetAccessible()->yes()) {
+				return $error;
+			}
+
+			if ($hasOffsetValue->no()) {
+				return $error ?? RuleErrorBuilder::message(
+					sprintf(
+						'Offset %s on %s on left side of %s does not exist.',
+						$dimType->describe(VerbosityLevel::value()),
+						$type->describe(VerbosityLevel::value()),
+						$action
+					)
+				)->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), sprintf(
+					'Offset %s on %s on left side of %s always exists and',
+					$dimType->describe(VerbosityLevel::value()),
+					$type->describe(VerbosityLevel::value()),
+					$action
+				));
+
+				if ($error !== null) {
+					return $this->canBeCoalesced($expr->var, $scope, $action, $error);
+				}
+			}
+
+			// Has offset, it is nullable
+			return null;
+
+		} elseif ($expr instanceof Node\Expr\PropertyFetch || $expr instanceof Node\Expr\StaticPropertyFetch) {
+
+			$propertyReflection = $this->propertyReflectionFinder->findPropertyReflectionFromNode($expr, $scope);
+
+			if ($propertyReflection === null) {
+				return null;
+			}
+
+			$propertyDescription = $this->propertyDescriptor->describeProperty($propertyReflection, $expr);
+			$propertyType = $propertyReflection->getWritableType();
+
+			$error = $error ?? $this->generateError(
+				$propertyReflection->getWritableType(),
+				sprintf('%s (%s) on left side of %s', $propertyDescription, $propertyType->describe(VerbosityLevel::typeOnly()), $action)
+			);
+
+			if ($error !== null) {
+				if ($expr instanceof Node\Expr\PropertyFetch) {
+					return $this->canBeCoalesced($expr->var, $scope, $action, $error);
+				}
+
+				if ($expr->class instanceof Expr) {
+					return $this->canBeCoalesced($expr->class, $scope, $action, $error);
+				}
+			}
+
+			return $error;
+		}
+
+		return $error ?? $this->generateError($scope->getType($expr), sprintf('Left side of %s', $action));
+	}
+
+	private function generateError(Type $type, string $message): ?RuleError
+	{
+		$nullType = new NullType();
+
+		if ($type->equals($nullType)) {
+			return RuleErrorBuilder::message(
+				sprintf('%s is always null.', $message)
+			)->build();
+		}
+
+		if ($type->isSuperTypeOf($nullType)->no()) {
+			return RuleErrorBuilder::message(
+				sprintf('%s is not nullable.', $message)
+			)->build();
+		}
+
+		return null;
+	}
+
+}
diff --git a/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php b/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php
@@ -0,0 +1,169 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Variables;
+
+use PHPStan\Rules\Properties\PropertyDescriptor;
+use PHPStan\Rules\Properties\PropertyReflectionFinder;
+
+/**
+ * @extends \PHPStan\Testing\RuleTestCase<NullCoalesceRule>
+ */
+class NullCoalesceRuleTest extends \PHPStan\Testing\RuleTestCase
+{
+
+	protected function getRule(): \PHPStan\Rules\Rule
+	{
+		return new NullCoalesceRule(new PropertyDescriptor(), new PropertyReflectionFinder());
+	}
+
+	public function testCoalesceRule(): void
+	{
+		require_once __DIR__ . '/data/null-coalesce.php';
+		$this->analyse([__DIR__ . '/data/null-coalesce.php'], [
+			[
+				'Property CoalesceRule\FooCoalesce::$string (string) on left side of ?? is not nullable.',
+				32,
+			],
+			[
+				'Variable $scalar on left side of ?? always exists and is not nullable.',
+				41,
+			],
+			[
+				'Offset \'string\' on array(1, 2, 3) on left side of ?? does not exist.',
+				45,
+			],
+			[
+				'Offset \'string\' on array(array(1), array(2), array(3)) on left side of ?? does not exist.',
+				49,
+			],
+			[
+				'Variable $doesNotExist on left side of ?? is never defined.',
+				51,
+			],
+			[
+				'Offset \'dim\' on array(\'dim\' => 1, \'dim-null\' => 1|null, \'dim-null-offset\' => array(\'a\' => true|null), \'dim-empty\' => array()) on left side of ?? always exists and is not nullable.',
+				67,
+			],
+			[
+				'Offset \'b\' on array() on left side of ?? does not exist.',
+				79,
+			],
+			[
+				'Left side of ?? is not nullable.',
+				81,
+			],
+			[
+				'Property CoalesceRule\FooCoalesce::$string (string) on left side of ?? is not nullable.',
+				89,
+			],
+			[
+				'Property CoalesceRule\FooCoalesce::$alwaysNull (null) on left side of ?? is always null.',
+				91,
+			],
+			[
+				'Property CoalesceRule\FooCoalesce::$string (string) on left side of ?? is not nullable.',
+				93,
+			],
+			[
+				'Static property CoalesceRule\FooCoalesce::$staticString (string) on left side of ?? is not nullable.',
+				99,
+			],
+			[
+				'Static property CoalesceRule\FooCoalesce::$staticAlwaysNull (null) on left side of ?? is always null.',
+				101,
+			],
+			[
+				'Variable $a on left side of ?? always exists and is always null.',
+				115,
+			],
+			[
+				'Property CoalesceRule\FooCoalesce::$string (string) on left side of ?? is not nullable.',
+				120,
+			],
+			[
+				'Property CoalesceRule\FooCoalesce::$alwaysNull (null) on left side of ?? is always null.',
+				122,
+			],
+			[
+				'Left side of ?? is not nullable.',
+				124,
+			],
+			[
+				'Left side of ?? is always null.',
+				125,
+			],
+			[
+				'Static property CoalesceRule\FooCoalesce::$staticAlwaysNull (null) on left side of ?? is always null.',
+				130,
+			],
+			[
+				'Static property CoalesceRule\FooCoalesce::$staticString (string) on left side of ?? is not nullable.',
+				131,
+			],
+		]);
+	}
+
+	public function testCoalesceAssignRule(): void
+	{
+		if (PHP_VERSION_ID < 70400) {
+			$this->markTestSkipped('Test requires PHP 7.4.');
+		}
+
+		require_once __DIR__ . '/data/null-coalesce-assign.php';
+		$this->analyse([__DIR__ . '/data/null-coalesce-assign.php'], [
+			[
+				'Property CoalesceAssignRule\FooCoalesce::$string (string) on left side of ??= is not nullable.',
+				32,
+			],
+			[
+				'Variable $scalar on left side of ??= always exists and is not nullable.',
+				41,
+			],
+			[
+				'Offset \'string\' on array(1, 2, 3) on left side of ??= does not exist.',
+				45,
+			],
+			[
+				'Offset \'string\' on array(array(1), array(2), array(3)) on left side of ??= does not exist.',
+				49,
+			],
+			[
+				'Variable $doesNotExist on left side of ??= is never defined.',
+				51,
+			],
+			[
+				'Offset \'dim\' on array(\'dim\' => 1, \'dim-null\' => 1|null, \'dim-null-offset\' => array(\'a\' => true|null), \'dim-empty\' => array()) on left side of ??= always exists and is not nullable.',
+				67,
+			],
+			[
+				'Offset \'b\' on array() on left side of ??= does not exist.',
+				79,
+			],
+			[
+				'Property CoalesceAssignRule\FooCoalesce::$string (string) on left side of ??= is not nullable.',
+				89,
+			],
+			[
+				'Property CoalesceAssignRule\FooCoalesce::$alwaysNull (null) on left side of ??= is always null.',
+				91,
+			],
+			[
+				'Property CoalesceAssignRule\FooCoalesce::$string (string) on left side of ??= is not nullable.',
+				93,
+			],
+			[
+				'Static property CoalesceAssignRule\FooCoalesce::$staticString (string) on left side of ??= is not nullable.',
+				99,
+			],
+			[
+				'Static property CoalesceAssignRule\FooCoalesce::$staticAlwaysNull (null) on left side of ??= is always null.',
+				101,
+			],
+			[
+				'Variable $a on left side of ??= always exists and is always null.',
+				115,
+			],
+		]);
+	}
+
+}