Support AssignOp\Coalesce in CoalesceRule, add test cases for ArrayDi…

…mFetch
phpstan · Feb 13, 2020 · b098255 · b098255
1 parent c98b3fb
commit b098255
Show file tree

Hide file tree

Showing 4 changed files with 150 additions and 25 deletions.
diff --git 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,36 +37,38 @@ 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();
 			}
 
 			$variableType = $scope->getVariableType($node->name);
 
 			// 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
@@ -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
@@ -0,0 +1,55 @@
+<?php // lint >= 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<string, int> $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
@@ -1,6 +1,7 @@
 <?php
 
-function coalesce () {
+function coalesce()
+{
 
 	$scalar = 3;
 
@@ -22,9 +23,27 @@ function coalesce () {
 
 	echo $maybeVariable ?? 0;
 
-	$fixedDimArray = ['dim' => 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;
 }
 
 /**