Skip to content

Commit

Permalink
Bleeding edge - empty() rule
Browse files Browse the repository at this point in the history
  • Loading branch information
ondrejmirtes committed Sep 10, 2021
1 parent 1ef74b7 commit 601460c
Show file tree
Hide file tree
Showing 7 changed files with 228 additions and 25 deletions.
5 changes: 5 additions & 0 deletions conf/config.level4.neon
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ conditionalTags:
phpstan.rules.rule: %featureToggles.unusedClassElements%
PHPStan\Rules\DeadCode\UnusedPrivatePropertyRule:
phpstan.rules.rule: %featureToggles.unusedClassElements%
PHPStan\Rules\Variables\EmptyRule:
phpstan.rules.rule: %featureToggles.nullCoalesce%
PHPStan\Rules\Variables\IssetRule:
phpstan.rules.rule: %featureToggles.nullCoalesce%
PHPStan\Rules\Variables\NullCoalesceRule:
Expand Down Expand Up @@ -164,6 +166,9 @@ services:
tags:
- phpstan.rules.rule

-
class: PHPStan\Rules\Variables\EmptyRule

-
class: PHPStan\Rules\Variables\IssetRule

Expand Down
43 changes: 21 additions & 22 deletions src/Rules/IssetCheck.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
use PHPStan\Rules\Properties\PropertyDescriptor;
use PHPStan\Rules\Properties\PropertyReflectionFinder;
use PHPStan\Type\MixedType;
use PHPStan\Type\NullType;
use PHPStan\Type\Type;
use PHPStan\Type\VerbosityLevel;

Expand All @@ -28,7 +27,10 @@ public function __construct(
$this->propertyReflectionFinder = $propertyReflectionFinder;
}

public function check(Expr $expr, Scope $scope, string $operatorDescription, ?RuleError $error = null): ?RuleError
/**
* @param callable(Type): ?string $typeMessageCallback
*/
public function check(Expr $expr, Scope $scope, string $operatorDescription, callable $typeMessageCallback, ?RuleError $error = null): ?RuleError
{
if ($expr instanceof Node\Expr\Variable && is_string($expr->name)) {
$hasVariable = $scope->hasVariableType($expr->name);
Expand Down Expand Up @@ -70,10 +72,10 @@ public function check(Expr $expr, Scope $scope, string $operatorDescription, ?Ru
$dimType->describe(VerbosityLevel::value()),
$type->describe(VerbosityLevel::value()),
$operatorDescription
));
), $typeMessageCallback);

if ($error !== null) {
return $this->check($expr->var, $scope, $operatorDescription, $error);
return $this->check($expr->var, $scope, $operatorDescription, $typeMessageCallback, $error);
}
}

Expand Down Expand Up @@ -104,42 +106,39 @@ public function check(Expr $expr, Scope $scope, string $operatorDescription, ?Ru

$error = $error ?? $this->generateError(
$propertyReflection->getWritableType(),
sprintf('%s (%s) %s', $propertyDescription, $propertyType->describe(VerbosityLevel::typeOnly()), $operatorDescription)
sprintf('%s (%s) %s', $propertyDescription, $propertyType->describe(VerbosityLevel::typeOnly()), $operatorDescription),
$typeMessageCallback
);

if ($error !== null) {
if ($expr instanceof Node\Expr\PropertyFetch) {
return $this->check($expr->var, $scope, $operatorDescription, $error);
return $this->check($expr->var, $scope, $operatorDescription, $typeMessageCallback, $error);
}

if ($expr->class instanceof Expr) {
return $this->check($expr->class, $scope, $operatorDescription, $error);
return $this->check($expr->class, $scope, $operatorDescription, $typeMessageCallback, $error);
}
}

return $error;
}

return $error ?? $this->generateError($scope->getType($expr), sprintf('Expression %s', $operatorDescription));
return $error ?? $this->generateError($scope->getType($expr), sprintf('Expression %s', $operatorDescription), $typeMessageCallback);
}

private function generateError(Type $type, string $message): ?RuleError
/**
* @param callable(Type): ?string $typeMessageCallback
*/
private function generateError(Type $type, string $message, callable $typeMessageCallback): ?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();
$typeMessage = $typeMessageCallback($type);
if ($typeMessage === null) {
return null;
}

return null;
return RuleErrorBuilder::message(
sprintf('%s %s.', $message, $typeMessage)
)->build();
}

}
78 changes: 78 additions & 0 deletions src/Rules/Variables/EmptyRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Variables;

use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\IssetCheck;
use PHPStan\Type\Constant\ConstantBooleanType;
use PHPStan\Type\ErrorType;
use PHPStan\Type\NullType;
use PHPStan\Type\Type;

/**
* @implements \PHPStan\Rules\Rule<Node\Expr\Empty_>
*/
class EmptyRule implements \PHPStan\Rules\Rule
{

private IssetCheck $issetCheck;

public function __construct(IssetCheck $issetCheck)
{
$this->issetCheck = $issetCheck;
}

public function getNodeType(): string
{
return Node\Expr\Empty_::class;
}

public function processNode(Node $node, Scope $scope): array
{
$error = $this->issetCheck->check($node->expr, $scope, 'in empty()', static function (Type $type): ?string {
$isNull = (new NullType())->isSuperTypeOf($type);
$isFalsey = (new ConstantBooleanType(false))->isSuperTypeOf($type->toBoolean());
if ($isNull->maybe()) {
return null;
}
if ($isFalsey->maybe()) {
return null;
}

if ($isNull->yes()) {
if ($isFalsey->yes()) {
return 'is always falsy';
}
if ($isFalsey->no()) {
return 'is not falsy';
}

return 'is always null';
}

if ($isFalsey->yes()) {
return 'is always falsy';
}

if ($isFalsey->no()) {
return 'is not falsy';
}

return 'is not nullable';
});
if ($error === null) {
return [];
}

$exprType = $scope->getType($node->expr);
$exprBooleanType = $exprType->toBoolean();
$isFalse = (new ConstantBooleanType(false))->isSuperTypeOf($exprBooleanType);
if (!$exprType instanceof ErrorType && $isFalse->maybe()) {
return [];
}

return [$error];
}

}
15 changes: 14 additions & 1 deletion src/Rules/Variables/IssetRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\IssetCheck;
use PHPStan\Type\NullType;
use PHPStan\Type\Type;

/**
* @implements \PHPStan\Rules\Rule<Node\Expr\Isset_>
Expand All @@ -28,7 +30,18 @@ public function processNode(Node $node, Scope $scope): array
{
$messages = [];
foreach ($node->vars as $var) {
$error = $this->issetCheck->check($var, $scope, 'in isset()');
$error = $this->issetCheck->check($var, $scope, 'in isset()', static function (Type $type): ?string {
$isNull = (new NullType())->isSuperTypeOf($type);
if ($isNull->maybe()) {
return null;
}

if ($isNull->yes()) {
return 'is always null';
}

return 'is not nullable';
});
if ($error === null) {
continue;
}
Expand Down
19 changes: 17 additions & 2 deletions src/Rules/Variables/NullCoalesceRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\IssetCheck;
use PHPStan\Type\NullType;
use PHPStan\Type\Type;

/**
* @implements \PHPStan\Rules\Rule<\PhpParser\Node\Expr>
Expand All @@ -26,10 +28,23 @@ public function getNodeType(): string

public function processNode(Node $node, Scope $scope): array
{
$typeMessageCallback = static function (Type $type): ?string {
$isNull = (new NullType())->isSuperTypeOf($type);
if ($isNull->maybe()) {
return null;
}

if ($isNull->yes()) {
return 'is always null';
}

return 'is not nullable';
};

if ($node instanceof Node\Expr\BinaryOp\Coalesce) {
$error = $this->issetCheck->check($node->left, $scope, 'on left side of ??');
$error = $this->issetCheck->check($node->left, $scope, 'on left side of ??', $typeMessageCallback);
} elseif ($node instanceof Node\Expr\AssignOp\Coalesce) {
$error = $this->issetCheck->check($node->var, $scope, 'on left side of ??=');
$error = $this->issetCheck->check($node->var, $scope, 'on left side of ??=', $typeMessageCallback);
} else {
return [];
}
Expand Down
51 changes: 51 additions & 0 deletions tests/PHPStan/Rules/Variables/EmptyRuleTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Variables;

use PHPStan\Rules\IssetCheck;
use PHPStan\Rules\Properties\PropertyDescriptor;
use PHPStan\Rules\Properties\PropertyReflectionFinder;
use PHPStan\Testing\RuleTestCase;

/**
* @extends RuleTestCase<EmptyRule>
*/
class EmptyRuleTest extends RuleTestCase
{

protected function getRule(): \PHPStan\Rules\Rule
{
return new EmptyRule(new IssetCheck(new PropertyDescriptor(), new PropertyReflectionFinder()));
}

public function testRule(): void
{
$this->analyse([__DIR__ . '/data/empty-rule.php'], [
[
'Offset \'nonexistent\' on array(?0 => bool, ?1 => false, 2 => bool, 3 => false, 4 => true) in empty() does not exist.',
22,
],
[
'Offset 3 on array(?0 => bool, ?1 => false, 2 => bool, 3 => false, 4 => true) in empty() always exists and is always falsy.',
24,
],
[
'Offset 4 on array(?0 => bool, ?1 => false, 2 => bool, 3 => false, 4 => true) in empty() always exists and is not falsy.',
25,
],
[
'Offset 0 on array(\'\', \'0\', \'foo\', \'\'|\'foo\') in empty() always exists and is always falsy.',
36,
],
[
'Offset 1 on array(\'\', \'0\', \'foo\', \'\'|\'foo\') in empty() always exists and is always falsy.',
37,
],
[
'Offset 2 on array(\'\', \'0\', \'foo\', \'\'|\'foo\') in empty() always exists and is not falsy.',
38,
],
]);
}

}
42 changes: 42 additions & 0 deletions tests/PHPStan/Rules/Variables/data/empty-rule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<?php

namespace EmptyRule;

class Foo
{

public function doFoo()
{
$a = [];
if (rand(0, 1)) {
$a[0] = rand(0,1) ? true : false;
$a[1] = false;
}

$a[2] = rand(0,1) ? true : false;
$a[3] = false;
$a[4] = true;

empty($a[0]);
empty($a[1]);
empty($a['nonexistent']);
empty($a[2]);
empty($a[3]);
empty($a[4]);
}

public function doBar()
{
$a = [
'',
'0',
'foo',
rand(0, 1) ? '' : 'foo',
];
empty($a[0]);
empty($a[1]);
empty($a[2]);
empty($a[3]);
}

}

0 comments on commit 601460c

Please sign in to comment.