Skip to content

Commit

Permalink
[CodeQuality] Add CompleteMissingIfElseBracketRector (#5121)
Browse files Browse the repository at this point in the history
  • Loading branch information
TomasVotruba committed Oct 5, 2023
1 parent 8e06627 commit 3e0132d
Show file tree
Hide file tree
Showing 7 changed files with 224 additions and 17 deletions.
25 changes: 23 additions & 2 deletions build/target-repository/docs/rector_rules_overview.md
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
# 353 Rules Overview
# 354 Rules Overview

<br>

## Categories

- [Arguments](#arguments) (4)

- [CodeQuality](#codequality) (71)
- [CodeQuality](#codequality) (72)

- [CodingStyle](#codingstyle) (29)

Expand Down Expand Up @@ -455,6 +455,27 @@ Add missing dynamic properties

<br>

### CompleteMissingIfElseBracketRector

Complete missing if/else brackets

- class: [`Rector\CodeQuality\Rector\If_\CompleteMissingIfElseBracketRector`](../rules/CodeQuality/Rector/If_/CompleteMissingIfElseBracketRector.php)

```diff
class SomeClass
{
public function run($value)
{
- if ($value)
+ if ($value) {
return 1;
+ }
}
}
```

<br>

### ConsecutiveNullCompareReturnsToNullCoalesceQueueRector

Change multiple null compares to ?? queue
Expand Down
2 changes: 2 additions & 0 deletions config/set/code-quality.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
use Rector\CodeQuality\Rector\Identical\SimplifyConditionsRector;
use Rector\CodeQuality\Rector\Identical\StrlenZeroToIdenticalEmptyStringRector;
use Rector\CodeQuality\Rector\If_\CombineIfRector;
use Rector\CodeQuality\Rector\If_\CompleteMissingIfElseBracketRector;
use Rector\CodeQuality\Rector\If_\ConsecutiveNullCompareReturnsToNullCoalesceQueueRector;
use Rector\CodeQuality\Rector\If_\ExplicitBoolCompareRector;
use Rector\CodeQuality\Rector\If_\ShortenElseIfRector;
Expand Down Expand Up @@ -188,5 +189,6 @@
ConvertStaticPrivateConstantToSelfRector::class,
LocallyCalledStaticMethodToNonStaticRector::class,
NumberCompareToMaxFuncCallRector::class,
CompleteMissingIfElseBracketRector::class,
]);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php

declare(strict_types=1);

namespace Rector\Tests\CodeQuality\Rector\If_\CompleteMissingIfElseBracketRector;

use Iterator;
use PHPUnit\Framework\Attributes\DataProvider;
use Rector\Testing\PHPUnit\AbstractRectorTestCase;

final class CompleteMissingIfElseBracketRectorTest extends AbstractRectorTestCase
{
#[DataProvider('provideData')]
public function test(string $filePath): void
{
$this->doTestFile($filePath);
}

public static function provideData(): Iterator
{
return self::yieldFilesFromDirectory(__DIR__ . '/Fixture');
}

public function provideConfigFilePath(): string
{
return __DIR__ . '/config/configured_rule.php';
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\If_\CompleteMissingIfElseBracketRector\Fixture;

class SomeClass
{
public function run($value)
{
if ($value)
return 1;
}
}

?>
-----
<?php

namespace Rector\Tests\CodeQuality\Rector\If_\CompleteMissingIfElseBracketRector\Fixture;

class SomeClass
{
public function run($value)
{
if ($value) {
return 1;
}
}
}

?>
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php

declare(strict_types=1);

use Rector\CodeQuality\Rector\If_\CompleteMissingIfElseBracketRector;
use Rector\Config\RectorConfig;

return static function (RectorConfig $rectorConfig): void {
$rectorConfig->rule(CompleteMissingIfElseBracketRector::class);
};
116 changes: 116 additions & 0 deletions rules/CodeQuality/Rector/If_/CompleteMissingIfElseBracketRector.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
<?php

declare(strict_types=1);

namespace Rector\CodeQuality\Rector\If_;

use PhpParser\Node;
use PhpParser\Node\Stmt\If_;
use Rector\Core\Rector\AbstractRector;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

/**
* @see \Rector\Tests\CodeQuality\Rector\If_\CompleteMissingIfElseBracketRector\CompleteMissingIfElseBracketRectorTest
*/
final class CompleteMissingIfElseBracketRector extends AbstractRector
{
public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition('Complete missing if/else brackets', [
new CodeSample(
<<<'CODE_SAMPLE'
class SomeClass
{
public function run($value)
{
if ($value)
return 1;
}
}
CODE_SAMPLE

,
<<<'CODE_SAMPLE'
class SomeClass
{
public function run($value)
{
if ($value) {
return 1;
}
}
}
CODE_SAMPLE
),
]);
}

/**
* @return array<class-string<Node>>
*/
public function getNodeTypes(): array
{
return [If_::class];
}

/**
* @param If_ $node
*/
public function refactor(Node $node): ?Node
{
if ($this->isBareNewNode($node)) {
return null;
}

$oldTokens = $this->file->getOldTokens();
if ($this->isIfConditionFollowedByOpeningCurlyBracket($node, $oldTokens)) {
return null;
}

// invoke reprint with brackets
$node->setAttribute(AttributeKey::ORIGINAL_NODE, null);

return $node;
}

/**
* @param mixed[] $oldTokens
*/
private function isIfConditionFollowedByOpeningCurlyBracket(If_ $if, array $oldTokens): bool
{
for ($i = $if->getStartTokenPos(); $i < $if->getEndTokenPos(); ++$i) {
if ($oldTokens[$i] !== ')') {
continue;
}

// first closing bracket must be followed by curly opening brackets
// what is next token?
$nextToken = $oldTokens[$i + 1];

if (is_array($nextToken) && trim((string) $nextToken[1]) === '') {
// next token is whitespace
$nextToken = $oldTokens[$i + 2];
}

if ($nextToken === '{') {
// all good
return true;
}
}

return false;
}

private function isBareNewNode(If_ $if): bool
{
$originalNode = $if->getAttribute(AttributeKey::ORIGINAL_NODE);
if (! $originalNode instanceof Node) {
return true;
}

// not defined, probably new if
return $if->getStartTokenPos() === -1;
}
}
30 changes: 15 additions & 15 deletions src/PhpParser/Node/NodeFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,21 @@ public function createReturnBooleanAnd(array $newNodes): ?Expr
return $this->createBooleanAndFromNodes($newNodes);
}

public function createReprintedExpr(Expr $expr): Expr
{
// reset original node, to allow the printer to re-use the expr
$expr->setAttribute(AttributeKey::ORIGINAL_NODE, null);
$this->simpleCallableNodeTraverser->traverseNodesWithCallable(
$expr,
static function (Node $node): Node {
$node->setAttribute(AttributeKey::ORIGINAL_NODE, null);
return $node;
}
);

return $expr;
}

private function createArrayItem(mixed $item, string | int | null $key = null): ArrayItem
{
$arrayItem = null;
Expand Down Expand Up @@ -435,19 +450,4 @@ private function createMethodCaller(

return $exprOrVariableName;
}

public function createReprintedExpr(Expr $expr): Expr
{
// reset original node, to allow the printer to re-use the expr
$expr->setAttribute(AttributeKey::ORIGINAL_NODE, null);
$this->simpleCallableNodeTraverser->traverseNodesWithCallable(
$expr,
static function (Node $node): Node {
$node->setAttribute(AttributeKey::ORIGINAL_NODE, null);
return $node;
}
);

return $expr;
}
}

0 comments on commit 3e0132d

Please sign in to comment.