Skip to content

Commit

Permalink
[CodeQuality] Add SimplifyFalseComparisonToNegateRector
Browse files Browse the repository at this point in the history
  • Loading branch information
TomasVotruba committed Oct 15, 2018
1 parent 5be9bcc commit a7afe1f
Show file tree
Hide file tree
Showing 9 changed files with 151 additions and 13 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
<?php declare(strict_types=1);

namespace Rector\CodeQuality\Rector\Identical;

use PhpParser\Node;
use PhpParser\Node\Expr\BinaryOp;
use PhpParser\Node\Expr\BinaryOp\Identical;
use PhpParser\Node\Expr\BooleanNot;
use Rector\NodeAnalyzer\ConstFetchAnalyzer;
use Rector\Rector\AbstractRector;
use Rector\RectorDefinition\CodeSample;
use Rector\RectorDefinition\RectorDefinition;

final class SimplifyIdenticalFalseToBooleanNotRector extends AbstractRector

This comment has been minimized.

Copy link
@carusogabriel

carusogabriel Oct 15, 2018

Contributor

This is pure Coding Standards, but okay

This comment has been minimized.

Copy link
@TomasVotruba

TomasVotruba Oct 16, 2018

Author Member

They'll overlap more and more, as tools evolve.

It also solve WTF like this:

-if (!$something === false) {
+if ($something) {

Did you know, that PHP CS Fixer was initiall supposed to be build on php-parser - nikic/PHP-Parser#41 (comment)?

{
/**
* @var ConstFetchAnalyzer
*/
private $constFetchAnalyzer;

public function __construct(ConstFetchAnalyzer $constFetchAnalyzer)
{
$this->constFetchAnalyzer = $constFetchAnalyzer;
}

public function getDefinition(): RectorDefinition
{
return new RectorDefinition('Changes === false to negate !', [
new CodeSample('if ($something === false) {}', 'if (! $something) {}'),
]);
}

/**
* @return string[]
*/
public function getNodeTypes(): array
{
return [Identical::class];
}

/**
* @param Identical $node
*/
public function refactor(Node $node): ?Node
{
if ($this->constFetchAnalyzer->isFalse($node->right)) {
$comparedNode = $node->left;
$shouldUnwrap = $node->left instanceof BooleanNot;
} elseif ($this->constFetchAnalyzer->isFalse($node->left)) {
$comparedNode = $node->right;
$shouldUnwrap = $node->right instanceof BooleanNot;
} else {
return $node;
}

if ($shouldUnwrap) {
/** @var BooleanNot $comparedNode */
$comparedNode = $comparedNode->expr;
if ($this->shouldSkip($comparedNode)) {
return $node;
}

return $comparedNode;
}

if ($this->shouldSkip($comparedNode)) {
return $node;
}

return new BooleanNot($comparedNode);
}

private function shouldSkip(Node $node): bool
{
if ($node instanceof BinaryOp) {
return true;
}

return false;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

if (!$something) {}

if (!$something) {}

if ($something) {}

if ((! $one && $two) === false) {
return null;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php declare(strict_types=1);

namespace Rector\CodeQuality\Tests\Rector\Identical\SimplifyIdenticalFalseToBooleanNotRector;

use Iterator;
use Rector\Testing\PHPUnit\AbstractRectorTestCase;

/**
* @covers \Rector\CodeQuality\Rector\Identical\SimplifyIdenticalFalseToBooleanNotRector
*/
final class SimplifyIdenticalFalseToBooleanNotRectorTest extends AbstractRectorTestCase
{
/**
* @dataProvider provideWrongToFixedFiles()
*/
public function test(string $wrong, string $fixed): void
{
$this->doTestFileMatchesExpectedContent($wrong, $fixed);
}

public function provideWrongToFixedFiles(): Iterator
{
yield [__DIR__ . '/Wrong/wrong.php.inc', __DIR__ . '/Correct/correct.php.inc'];
}

protected function provideConfig(): string
{
return __DIR__ . '/config.yml';
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

if ($something === false) {}

if (false === $something) {}

if (! $something === false) {}

if ((! $one && $two) === false) {
return null;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
services:
Rector\CodeQuality\Rector\Identical\SimplifyIdenticalFalseToBooleanNotRector: ~
22 changes: 12 additions & 10 deletions packages/ContributorTools/src/Command/CreateRectorCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

use Nette\Utils\FileSystem;
use Nette\Utils\Strings;
use PhpParser\Node\Expr\Array_;
use PhpParser\Node\Expr\ArrayItem;
use PhpParser\Node\Expr\ClassConstFetch;
use PhpParser\Node\Name\FullyQualified;
use Rector\Console\ConsoleStyle;
Expand Down Expand Up @@ -92,13 +94,13 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$this->consoleStyle->note(sprintf('New file "%s" was generated', $destination));
}

$this->consoleStyle->success(sprintf('New Rector "%s" is ready!', $configuration->getName()));

if ($testCasePath) {
$this->consoleStyle->note(
sprintf('Now make these tests green:%svendor/bin/phpunit %s', PHP_EOL, $testCasePath)
);
}
$this->consoleStyle->success(sprintf(
'New Rector "%s" is ready!%sNow make these tests green again:%svendor/bin/phpunit %s',
$configuration->getName(),
PHP_EOL . PHP_EOL,
PHP_EOL,
$testCasePath
));

return ShellCode::SUCCESS;
}
Expand All @@ -117,11 +119,11 @@ private function prepareData(Configuration $configuration): array
'_CodeAfter_' => $configuration->getCodeAfter(),
];

$nodeTypesPhp = [];
$arrayNodes = [];
foreach ($configuration->getNodeTypes() as $nodeType) {
$nodeTypesPhp[] = new ClassConstFetch(new FullyQualified($nodeType), 'class');
$arrayNodes[] = new ArrayItem(new ClassConstFetch(new FullyQualified($nodeType), 'class'));
}
$data['_NodeTypes_Php_'] = $this->betterStandardPrinter->prettyPrint($nodeTypesPhp);
$data['_NodeTypes_Php_'] = $this->betterStandardPrinter->prettyPrint([new Array_($arrayNodes)]);

$data['_NodeTypes_Doc_'] = '\\' . implode('|\\', $configuration->getNodeTypes());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,6 @@ private function resolveFullyQualifiedNodeTypes(array $nodeTypes): array
}
}

return $fqnNodeTypes;
return array_unique($fqnNodeTypes);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public function getDefinition(): RectorDefinition
*/
public function getNodeTypes(): array
{
return [_NodeTypes_Php_];
return _NodeTypes_Php_;
}

/**
Expand Down
3 changes: 2 additions & 1 deletion phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,9 @@ parameters:
# Error php-parser token
- '#Access to an undefined property PhpParser\\Node\\Expr\\Error\|PhpParser\\Node\\Expr\\Variable::\$name#'

# false positive, has annotation type above (@todo recheck for possible ignored positives)
# false positive, has annotation type above
- '#Access to an undefined property PhpParser\\Node::\$name#' # 11
- '#Access to an undefined property PhpParser\\Node\\Expr::\$expr#'

# subtype
- '#Property PhpParser\\Node\\Param::\$type \(PhpParser\\Node\\Name|PhpParser\\Node\\NullableType\|string\|null\) does not accept PhpParser\\Node\\Identifier|PhpParser\\Node\\Name\|PhpParser\\Node\\NullableType#' # 3
Expand Down

0 comments on commit a7afe1f

Please sign in to comment.