From 3e0132d324099a3d45e1f8f0d28be76d310ee0eb Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Thu, 5 Oct 2023 16:35:15 +0200 Subject: [PATCH] [CodeQuality] Add CompleteMissingIfElseBracketRector (#5121) --- .../docs/rector_rules_overview.md | 25 +++- config/set/code-quality.php | 2 + ...CompleteMissingIfElseBracketRectorTest.php | 28 +++++ .../Fixture/some_class.php.inc | 30 +++++ .../config/configured_rule.php | 10 ++ .../CompleteMissingIfElseBracketRector.php | 116 ++++++++++++++++++ src/PhpParser/Node/NodeFactory.php | 30 ++--- 7 files changed, 224 insertions(+), 17 deletions(-) create mode 100644 rules-tests/CodeQuality/Rector/If_/CompleteMissingIfElseBracketRector/CompleteMissingIfElseBracketRectorTest.php create mode 100644 rules-tests/CodeQuality/Rector/If_/CompleteMissingIfElseBracketRector/Fixture/some_class.php.inc create mode 100644 rules-tests/CodeQuality/Rector/If_/CompleteMissingIfElseBracketRector/config/configured_rule.php create mode 100644 rules/CodeQuality/Rector/If_/CompleteMissingIfElseBracketRector.php diff --git a/build/target-repository/docs/rector_rules_overview.md b/build/target-repository/docs/rector_rules_overview.md index aee03445ba2..3070aa41987 100644 --- a/build/target-repository/docs/rector_rules_overview.md +++ b/build/target-repository/docs/rector_rules_overview.md @@ -1,4 +1,4 @@ -# 353 Rules Overview +# 354 Rules Overview
@@ -6,7 +6,7 @@ - [Arguments](#arguments) (4) -- [CodeQuality](#codequality) (71) +- [CodeQuality](#codequality) (72) - [CodingStyle](#codingstyle) (29) @@ -455,6 +455,27 @@ Add missing dynamic properties
+### 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; ++ } + } + } +``` + +
+ ### ConsecutiveNullCompareReturnsToNullCoalesceQueueRector Change multiple null compares to ?? queue diff --git a/config/set/code-quality.php b/config/set/code-quality.php index 230d8076591..5738ab15b81 100644 --- a/config/set/code-quality.php +++ b/config/set/code-quality.php @@ -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; @@ -188,5 +189,6 @@ ConvertStaticPrivateConstantToSelfRector::class, LocallyCalledStaticMethodToNonStaticRector::class, NumberCompareToMaxFuncCallRector::class, + CompleteMissingIfElseBracketRector::class, ]); }; diff --git a/rules-tests/CodeQuality/Rector/If_/CompleteMissingIfElseBracketRector/CompleteMissingIfElseBracketRectorTest.php b/rules-tests/CodeQuality/Rector/If_/CompleteMissingIfElseBracketRector/CompleteMissingIfElseBracketRectorTest.php new file mode 100644 index 00000000000..57e1da3e82f --- /dev/null +++ b/rules-tests/CodeQuality/Rector/If_/CompleteMissingIfElseBracketRector/CompleteMissingIfElseBracketRectorTest.php @@ -0,0 +1,28 @@ +doTestFile($filePath); + } + + public static function provideData(): Iterator + { + return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/configured_rule.php'; + } +} diff --git a/rules-tests/CodeQuality/Rector/If_/CompleteMissingIfElseBracketRector/Fixture/some_class.php.inc b/rules-tests/CodeQuality/Rector/If_/CompleteMissingIfElseBracketRector/Fixture/some_class.php.inc new file mode 100644 index 00000000000..bac57f6b257 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/If_/CompleteMissingIfElseBracketRector/Fixture/some_class.php.inc @@ -0,0 +1,30 @@ + +----- + diff --git a/rules-tests/CodeQuality/Rector/If_/CompleteMissingIfElseBracketRector/config/configured_rule.php b/rules-tests/CodeQuality/Rector/If_/CompleteMissingIfElseBracketRector/config/configured_rule.php new file mode 100644 index 00000000000..41b2fe41666 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/If_/CompleteMissingIfElseBracketRector/config/configured_rule.php @@ -0,0 +1,10 @@ +rule(CompleteMissingIfElseBracketRector::class); +}; diff --git a/rules/CodeQuality/Rector/If_/CompleteMissingIfElseBracketRector.php b/rules/CodeQuality/Rector/If_/CompleteMissingIfElseBracketRector.php new file mode 100644 index 00000000000..c974486032c --- /dev/null +++ b/rules/CodeQuality/Rector/If_/CompleteMissingIfElseBracketRector.php @@ -0,0 +1,116 @@ +> + */ + 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; + } +} diff --git a/src/PhpParser/Node/NodeFactory.php b/src/PhpParser/Node/NodeFactory.php index 8f810b1c9cb..96770d4b011 100644 --- a/src/PhpParser/Node/NodeFactory.php +++ b/src/PhpParser/Node/NodeFactory.php @@ -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; @@ -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; - } }