Merge pull request #1291 from rectorphp/double-neg

[CodeQuality] prevent double negative in SimplifyBoolIdenticalTrueRector
This commit is contained in:
Tomáš Votruba 2019-04-03 21:56:00 +02:00 committed by GitHub
commit b19b6ea434
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 295 additions and 38 deletions

View File

@ -116,12 +116,12 @@
"@phpstan",
"@update-docs"
],
"check-cs": "vendor/bin/ecs check bin packages src tests utils",
"check-cs": "vendor/bin/ecs check bin packages src tests utils --ansi",
"fix-cs": [
"vendor/bin/ecs check bin packages src tests utils --fix",
"vendor/bin/ecs check bin packages src tests utils --fix --ansi",
"bin/clean_trailing_spaces.sh"
],
"phpstan": "vendor/bin/phpstan analyse packages src tests --error-format symplify",
"phpstan": "vendor/bin/phpstan analyse packages src tests --error-format symplify --ansi",
"docs": [
"bin/rector dump-rectors -o markdown > docs/AllRectorsOverview.md",
"bin/rector dump-nodes -o markdown > docs/NodesOverview.md"

View File

@ -1,2 +1,3 @@
services:
Rector\Symfony\Rector\BinaryOp\ResponseStatusCodeRector: ~
Rector\CodeQuality\Rector\Identical\BooleanNotIdenticalToNotIdenticalRector: ~

View File

@ -2714,7 +2714,7 @@ Changes rand, srand and getrandmax by new md_* alternatives.
- class: `Rector\Php\Rector\FuncCall\PregReplaceEModifierRector`
The /e modifier is no longer supported, use preg_replace_callback instead
The /e modifier is no longer supported, use preg_replace_callback instead
```diff
class SomeClass

View File

@ -910,7 +910,7 @@ if (true) {
```php
?>
<strong>feel</strong><?php
<strong>feel</strong><?php
```
<br>

View File

@ -0,0 +1,105 @@
<?php declare(strict_types=1);
namespace Rector\CodeQuality\Rector\Identical;
use PhpParser\Node;
use PhpParser\Node\Expr\BinaryOp\Identical;
use Rector\Rector\AbstractRector;
use Rector\RectorDefinition\CodeSample;
use Rector\RectorDefinition\RectorDefinition;
/**
* @see https://3v4l.org/GoEPq
*/
final class BooleanNotIdenticalToNotIdenticalRector extends AbstractRector
{
public function getDefinition(): RectorDefinition
{
return new RectorDefinition(
'Negated identical boolean compare to not identical compare (does not apply to non-bool values)',
[
new CodeSample(
<<<'CODE_SAMPLE'
class SomeClass
{
public function run()
{
$a = true;
$b = false;
var_dump(! $a === $b); // true
var_dump(! ($a === $b)); // true
var_dump($a !== $b); // true
}
}
CODE_SAMPLE
,
<<<'CODE_SAMPLE'
class SomeClass
{
public function run()
{
$a = true;
$b = false;
var_dump($a !== $b); // true
var_dump($a !== $b); // true
var_dump($a !== $b); // true
}
}
CODE_SAMPLE
),
]
);
}
/**
* @return string[]
*/
public function getNodeTypes(): array
{
return [Identical::class, Node\Expr\BooleanNot::class];
}
/**
* @param Identical|Node\Expr\BooleanNot $node
*/
public function refactor(Node $node): ?Node
{
if ($node instanceof Identical) {
return $this->processIdentical($node);
}
if ($node->expr instanceof Identical) {
$identical = $node->expr;
if (! $this->isBoolType($identical->left)) {
return null;
}
if (! $this->isBoolType($identical->right)) {
return null;
}
return new Node\Expr\BinaryOp\NotIdentical($identical->left, $identical->right);
}
return null;
}
private function processIdentical(Identical $identical): ?Node\Expr\BinaryOp\NotIdentical
{
if (! $this->isBoolType($identical->left)) {
return null;
}
if (! $this->isBoolType($identical->right)) {
return null;
}
if ($identical->left instanceof Node\Expr\BooleanNot) {
return new Node\Expr\BinaryOp\NotIdentical($identical->left->expr, $identical->right);
}
return null;
}
}

View File

@ -55,46 +55,40 @@ CODE_SAMPLE
public function refactor(Node $node): ?Node
{
if ($this->isBoolType($node->left) && ! $this->isBool($node->left)) {
if ($node instanceof Identical) {
if ($this->isTrue($node->right)) {
return $node->left;
}
if ($this->isFalse($node->right)) {
return new BooleanNot($node->left);
}
}
if ($node instanceof NotIdentical) {
if ($this->isFalse($node->right)) {
return $node->left;
}
if ($this->isTrue($node->right)) {
return new BooleanNot($node->left);
}
}
return $this->processBoolTypeToNotBool($node, $node->left, $node->right);
}
if ($this->isBoolType($node->right) && ! $this->isBool($node->right)) {
if ($node instanceof Identical) {
if ($this->isTrue($node->left)) {
return $node->right;
}
return $this->processBoolTypeToNotBool($node, $node->right, $node->left);
}
if ($this->isFalse($node->left)) {
return new BooleanNot($node->right);
}
return null;
}
private function processBoolTypeToNotBool(Node $node, Node\Expr $leftExpr, Node\Expr $rightExpr): ?Node\Expr
{
if ($node instanceof Identical) {
if ($this->isTrue($rightExpr)) {
return $leftExpr;
}
if ($node instanceof NotIdentical) {
if ($this->isFalse($node->left)) {
return $node->right;
if ($this->isFalse($rightExpr)) {
// prevent !!
if ($leftExpr instanceof BooleanNot) {
return $leftExpr->expr;
}
if ($this->isTrue($node->left)) {
return new BooleanNot($node->right);
}
return new BooleanNot($leftExpr);
}
}
if ($node instanceof NotIdentical) {
if ($this->isFalse($rightExpr)) {
return $leftExpr;
}
if ($this->isTrue($rightExpr)) {
return new BooleanNot($leftExpr);
}
}

View File

@ -0,0 +1,23 @@
<?php declare(strict_types=1);
namespace Rector\CodeQuality\Tests\Rector\Identical\BooleanNotIdenticalToNotIdenticalRector;
use Rector\CodeQuality\Rector\Identical\BooleanNotIdenticalToNotIdenticalRector;
use Rector\Testing\PHPUnit\AbstractRectorTestCase;
final class BooleanNotIdenticalToNotIdenticalRectorTest extends AbstractRectorTestCase
{
public function test(): void
{
$this->doTestFiles([
__DIR__ . '/Fixture/fixture.php.inc',
__DIR__ . '/Fixture/just_first.php.inc',
__DIR__ . '/Fixture/keep.php.inc',
]);
}
protected function getRectorClass(): string
{
return BooleanNotIdenticalToNotIdenticalRector::class;
}
}

View File

@ -0,0 +1,37 @@
<?php
namespace Rector\CodeQuality\Tests\Rector\Identical\BooleanNotIdenticalToNotIdenticalRector\Fixture;
class SomeClass
{
public function run()
{
$a = true;
$b = false;
var_dump(! $a === $b); // true
var_dump(! ($a === $b)); // true
var_dump($a !== $b); // true
}
}
?>
-----
<?php
namespace Rector\CodeQuality\Tests\Rector\Identical\BooleanNotIdenticalToNotIdenticalRector\Fixture;
class SomeClass
{
public function run()
{
$a = true;
$b = false;
var_dump($a !== $b); // true
var_dump($a !== $b); // true
var_dump($a !== $b); // true
}
}
?>

View File

@ -0,0 +1,37 @@
<?php
namespace Rector\CodeQuality\Tests\Rector\Identical\BooleanNotIdenticalToNotIdenticalRector\Fixture;
class JustFirst
{
public function run()
{
$a = 'true';
$b = false;
var_dump(! $a === $b); // true
var_dump(! ($a === $b)); // true
var_dump($a !== $b); // true
}
}
?>
-----
<?php
namespace Rector\CodeQuality\Tests\Rector\Identical\BooleanNotIdenticalToNotIdenticalRector\Fixture;
class JustFirst
{
public function run()
{
$a = 'true';
$b = false;
var_dump($a !== $b); // true
var_dump(! ($a === $b)); // true
var_dump($a !== $b); // true
}
}
?>

View File

@ -0,0 +1,25 @@
<?php
namespace Rector\CodeQuality\Tests\Rector\Identical\BooleanNotIdenticalToNotIdenticalRector\Fixture;
class Keep
{
public function run()
{
$a = 'true';
$b = false;
var_dump(! ($a === $b)); // true
var_dump($a !== $b); // true
}
public function again()
{
$a = true;
$b = 'false';
var_dump(! $a === $b); // true
var_dump(! ($a === $b)); // true
var_dump($a !== $b); // true
}
}

View File

@ -0,0 +1,31 @@
<?php
namespace Rector\CodingStyle\Tests\Rector\Identical\IdenticalFalseToBooleanNotRector\Fixture;
class DoubleNegate
{
public function run()
{
if (! true === false) {
return 'yes';
}
}
}
?>
-----
<?php
namespace Rector\CodingStyle\Tests\Rector\Identical\IdenticalFalseToBooleanNotRector\Fixture;
class DoubleNegate
{
public function run()
{
if (true) {
return 'yes';
}
}
}
?>

View File

@ -9,7 +9,11 @@ final class SimplifyBoolIdenticalTrueRectorTest extends AbstractRectorTestCase
{
public function test(): void
{
$this->doTestFiles([__DIR__ . '/Fixture/directly.php.inc', __DIR__ . '/Fixture/negate.php.inc']);
$this->doTestFiles([
__DIR__ . '/Fixture/directly.php.inc',
__DIR__ . '/Fixture/negate.php.inc',
__DIR__ . '/Fixture/double_negate.php.inc',
]);
}
protected function getRectorClass(): string