Skip to content

Commit

Permalink
Add support for method calls, PropertyFetch and StaticPropertyFetch
Browse files Browse the repository at this point in the history
  • Loading branch information
@leongersen
leongersen committed Feb 13, 2020
1 parent b098255 commit 2b50d56
Show file tree
Hide file tree
Showing 4 changed files with 315 additions and 34 deletions.
126 changes: 109 additions & 17 deletions src/Rules/Variables/CoalesceRule.php
View file
Expand Up @@ -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;

/**
Expand All @@ -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;
Expand All @@ -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);

Expand All @@ -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) {
Expand All @@ -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();
}

}
91 changes: 78 additions & 13 deletions tests/PHPStan/Rules/Variables/CoalesceRuleTest.php
View file
Expand Up @@ -2,6 +2,9 @@

namespace PHPStan\Rules\Variables;

use PHPStan\Rules\Properties\PropertyDescriptor;
use PHPStan\Rules\Properties\PropertyReflectionFinder;

/**
* @extends \PHPStan\Testing\RuleTestCase<CoalesceRule>
*/
Expand All @@ -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,
],
]);
}
Expand All @@ -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,
],
]);
}
Expand Down

0 comments on commit 2b50d56

Please sign in to comment.