Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rule checking coalesces #36

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions conf/config.level1.neon
Expand Up @@ -12,3 +12,4 @@ rules:
- PHPStan\Rules\Constants\ConstantRule
- PHPStan\Rules\Functions\UnusedClosureUsesRule
- PHPStan\Rules\Variables\VariableCertaintyInIssetRule
- PHPStan\Rules\Variables\CoalesceRule
196 changes: 196 additions & 0 deletions src/Rules/Variables/CoalesceRule.php
@@ -0,0 +1,196 @@
<?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 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();
}

}
145 changes: 145 additions & 0 deletions tests/PHPStan/Rules/Variables/CoalesceRuleTest.php
@@ -0,0 +1,145 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Variables;

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

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

}