From 093bcb63a19db3fcdab8264897c584212d755463 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 5 Sep 2023 15:17:38 +0200 Subject: [PATCH] [CodeQuality] Add NumberCompareToMaxFuncCallRector (#4914) --- .../docs/rector_rules_overview.md | 54 ++++++++- config/set/code-quality.php | 2 + .../PHPUnit/AbstractRectorTestCase.php | 6 +- .../Fixture/also_the_other_side.php.inc | 27 +++++ .../Fixture/include_greater_or_equal.php.inc | 27 +++++ .../Fixture/skip_else_another_value.php.inc | 11 ++ .../Fixture/skip_non_number.php.inc | 11 ++ .../Fixture/some_class.php.inc | 27 +++++ .../NumberCompareToMaxFuncCallRectorTest.php | 28 +++++ .../config/configured_rule.php | 10 ++ .../NumberCompareToMaxFuncCallRector.php | 111 ++++++++++++++++++ .../MyCLabsMethodCallToEnumConstRector.php | 14 +-- .../StrictArrayParamDimFetchRector.php | 4 +- 13 files changed, 315 insertions(+), 17 deletions(-) create mode 100644 rules-tests/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector/Fixture/also_the_other_side.php.inc create mode 100644 rules-tests/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector/Fixture/include_greater_or_equal.php.inc create mode 100644 rules-tests/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector/Fixture/skip_else_another_value.php.inc create mode 100644 rules-tests/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector/Fixture/skip_non_number.php.inc create mode 100644 rules-tests/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector/Fixture/some_class.php.inc create mode 100644 rules-tests/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector/NumberCompareToMaxFuncCallRectorTest.php create mode 100644 rules-tests/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector/config/configured_rule.php create mode 100644 rules/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector.php diff --git a/build/target-repository/docs/rector_rules_overview.md b/build/target-repository/docs/rector_rules_overview.md index 8b95129f29a..ef1c4daa592 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 +# 355 Rules Overview
@@ -6,7 +6,7 @@ - [Arguments](#arguments) (4) -- [CodeQuality](#codequality) (70) +- [CodeQuality](#codequality) (71) - [CodingStyle](#codingstyle) (30) @@ -54,7 +54,7 @@ - [Transform](#transform) (22) -- [TypeDeclaration](#typedeclaration) (41) +- [TypeDeclaration](#typedeclaration) (42) - [Visibility](#visibility) (3) @@ -862,6 +862,25 @@ Change unsafe new `static()` to new `self()`
+### NumberCompareToMaxFuncCallRector + +Ternary number compare to `max()` call + +- class: [`Rector\CodeQuality\Rector\Ternary\NumberCompareToMaxFuncCallRector`](../rules/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector.php) + +```diff + class SomeClass + { + public function run($value) + { +- return $value > 100 ? $value : 100; ++ return max($value, 100); + } + } +``` + +
+ ### OptionalParametersAfterRequiredRector Move required parameters after optional ones @@ -993,8 +1012,9 @@ Simplify bool value compare to true or false public function run(bool $value, string $items) { - $match = in_array($value, $items, TRUE) === TRUE; -- $match = in_array($value, $items, TRUE) !== FALSE; + $match = in_array($value, $items, TRUE); + +- $match = in_array($value, $items, TRUE) !== FALSE; + $match = in_array($value, $items, TRUE); } } @@ -1714,8 +1734,11 @@ Convert enscaped {$string} to more readable sprintf or concat, if no mask is use - class: [`Rector\CodingStyle\Rector\Encapsed\EncapsedStringsToSprintfRector`](../rules/CodingStyle/Rector/Encapsed/EncapsedStringsToSprintfRector.php) ```diff --echo "Unsupported format {$format}"; -+echo sprintf('Unsupported format %s', $format); +-echo "Unsupported format {$format} - use another"; ++echo sprintf('Unsupported format %s - use another', $format); + +-echo "Try {$allowed}"; ++echo 'Try ' . $allowed; ```
@@ -6738,6 +6761,25 @@ Add strict type declaration based on returned constants
+### ReturnTypeFromStrictFluentReturnRector + +Add return type from strict return `$this` + +- class: [`Rector\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromStrictFluentReturnRector`](../rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictFluentReturnRector.php) + +```diff + final class SomeClass + { +- public function run() ++ public function run(): self + { + return $this; + } + } +``` + +
+ ### ReturnTypeFromStrictNativeCallRector Add strict return type based native function or class method return diff --git a/config/set/code-quality.php b/config/set/code-quality.php index 81cf60aca83..230d8076591 100644 --- a/config/set/code-quality.php +++ b/config/set/code-quality.php @@ -68,6 +68,7 @@ use Rector\CodeQuality\Rector\Switch_\SingularSwitchToIfRector; use Rector\CodeQuality\Rector\Switch_\SwitchTrueToIfRector; use Rector\CodeQuality\Rector\Ternary\ArrayKeyExistsTernaryThenValueToCoalescingRector; +use Rector\CodeQuality\Rector\Ternary\NumberCompareToMaxFuncCallRector; use Rector\CodeQuality\Rector\Ternary\SimplifyTautologyTernaryRector; use Rector\CodeQuality\Rector\Ternary\SwitchNegatedTernaryRector; use Rector\CodeQuality\Rector\Ternary\TernaryEmptyArrayArrayDimFetchToCoalesceRector; @@ -186,5 +187,6 @@ DisallowedEmptyRuleFixerRector::class, ConvertStaticPrivateConstantToSelfRector::class, LocallyCalledStaticMethodToNonStaticRector::class, + NumberCompareToMaxFuncCallRector::class, ]); }; diff --git a/packages/Testing/PHPUnit/AbstractRectorTestCase.php b/packages/Testing/PHPUnit/AbstractRectorTestCase.php index 61daf9503f8..74a474a5e21 100644 --- a/packages/Testing/PHPUnit/AbstractRectorTestCase.php +++ b/packages/Testing/PHPUnit/AbstractRectorTestCase.php @@ -230,7 +230,8 @@ private function doTestFileMatchesExpectedContent( SimpleParameterProvider::setParameter(Option::SOURCE, [$originalFilePath]); $changedContent = $this->processFilePath($originalFilePath); - $originalFileContent = $this->currentFileProvider->getFile()->getOriginalFileContent(); + $originalFileContent = $this->currentFileProvider->getFile() + ->getOriginalFileContent(); $fixtureFilename = basename($fixtureFilePath); $failureMessage = sprintf('Failed on fixture file "%s"', $fixtureFilename); @@ -255,7 +256,8 @@ private function processFilePath(string $filePath): string $this->applicationFileProcessor->processFiles([$filePath], $configuration); - return $this->currentFileProvider->getFile()->getFileContent(); + return $this->currentFileProvider->getFile() + ->getFileContent(); } private function createInputFilePath(string $fixtureFilePath): string diff --git a/rules-tests/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector/Fixture/also_the_other_side.php.inc b/rules-tests/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector/Fixture/also_the_other_side.php.inc new file mode 100644 index 00000000000..0b4bd7c9031 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector/Fixture/also_the_other_side.php.inc @@ -0,0 +1,27 @@ + +----- + diff --git a/rules-tests/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector/Fixture/include_greater_or_equal.php.inc b/rules-tests/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector/Fixture/include_greater_or_equal.php.inc new file mode 100644 index 00000000000..52cd9799966 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector/Fixture/include_greater_or_equal.php.inc @@ -0,0 +1,27 @@ += 100 ? $value : 100; + } +} + +?> +----- + diff --git a/rules-tests/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector/Fixture/skip_else_another_value.php.inc b/rules-tests/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector/Fixture/skip_else_another_value.php.inc new file mode 100644 index 00000000000..afcee30bb7d --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector/Fixture/skip_else_another_value.php.inc @@ -0,0 +1,11 @@ + 100 ? $value : 55; + } +} diff --git a/rules-tests/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector/Fixture/skip_non_number.php.inc b/rules-tests/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector/Fixture/skip_non_number.php.inc new file mode 100644 index 00000000000..d0453079c09 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector/Fixture/skip_non_number.php.inc @@ -0,0 +1,11 @@ + 100 ? $value : 100; + } +} + +?> +----- + diff --git a/rules-tests/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector/NumberCompareToMaxFuncCallRectorTest.php b/rules-tests/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector/NumberCompareToMaxFuncCallRectorTest.php new file mode 100644 index 00000000000..40915ff9736 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector/NumberCompareToMaxFuncCallRectorTest.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/Ternary/NumberCompareToMaxFuncCallRector/config/configured_rule.php b/rules-tests/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector/config/configured_rule.php new file mode 100644 index 00000000000..f92ad062ac4 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector/config/configured_rule.php @@ -0,0 +1,10 @@ +rule(NumberCompareToMaxFuncCallRector::class); +}; diff --git a/rules/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector.php b/rules/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector.php new file mode 100644 index 00000000000..9977a134b80 --- /dev/null +++ b/rules/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector.php @@ -0,0 +1,111 @@ + 100 ? $value : 100; + } +} +CODE_SAMPLE + + , + <<<'CODE_SAMPLE' +class SomeClass +{ + public function run($value) + { + return max($value, 100); + } +} +CODE_SAMPLE + ), + ]); + } + + /** + * @return array> + */ + public function getNodeTypes(): array + { + return [Ternary::class]; + } + + /** + * @param Ternary $node + */ + public function refactor(Node $node): ?Node + { + if (! $node->cond instanceof BinaryOp) { + return null; + } + + $binaryOp = $node->cond; + if (! $this->areIntegersCompared($binaryOp)) { + return null; + } + + if ($binaryOp instanceof Smaller || $binaryOp instanceof SmallerOrEqual) { + if (! $this->nodeComparator->areNodesEqual($binaryOp->left, $node->else)) { + return null; + } + + if (! $this->nodeComparator->areNodesEqual($binaryOp->right, $node->if)) { + return null; + } + + return $this->nodeFactory->createFuncCall('max', [$node->if, $node->else]); + } + + if ($binaryOp instanceof Greater || $binaryOp instanceof GreaterOrEqual) { + if (! $this->nodeComparator->areNodesEqual($binaryOp->left, $node->if)) { + return null; + } + + if (! $this->nodeComparator->areNodesEqual($binaryOp->right, $node->else)) { + return null; + } + + return $this->nodeFactory->createFuncCall('max', [$node->if, $node->else]); + } + + return null; + } + + private function areIntegersCompared(BinaryOp $binaryOp): bool + { + $leftType = $this->getType($binaryOp->left); + if (! $leftType->isInteger()->yes()) { + return false; + } + + $rightType = $this->getType($binaryOp->right); + return $rightType->isInteger() + ->yes(); + } +} diff --git a/rules/Php81/Rector/MethodCall/MyCLabsMethodCallToEnumConstRector.php b/rules/Php81/Rector/MethodCall/MyCLabsMethodCallToEnumConstRector.php index 61fcdea349c..4eb791f5c82 100644 --- a/rules/Php81/Rector/MethodCall/MyCLabsMethodCallToEnumConstRector.php +++ b/rules/Php81/Rector/MethodCall/MyCLabsMethodCallToEnumConstRector.php @@ -101,6 +101,11 @@ public function refactor(Node $node): ?Node return $this->nodeFactory->createClassConstFetch($className, $enumCaseName); } + public function provideMinPhpVersion(): int + { + return PhpVersionFeature::ENUM; + } + private function isEnumConstant(string $className, string $constant): bool { $classReflection = $this->reflectionProvider->getClass($className); @@ -108,11 +113,6 @@ private function isEnumConstant(string $className, string $constant): bool return $classReflection->hasConstant($constant); } - public function provideMinPhpVersion(): int - { - return PhpVersionFeature::ENUM; - } - private function refactorGetKeyMethodCall(MethodCall $methodCall): ?ClassConstFetch { if (! $methodCall->var instanceof StaticCall) { @@ -166,7 +166,7 @@ private function refactorGetValueMethodCall(MethodCall $methodCall): ?PropertyFe private function refactorEqualsMethodCall(MethodCall $methodCall): ?Identical { $expr = $this->getNonEnumReturnTypeExpr($methodCall->var); - if (!$expr instanceof Expr) { + if (! $expr instanceof Expr) { $expr = $this->getValidEnumExpr($methodCall->var); if (! $expr instanceof Expr) { return null; @@ -179,7 +179,7 @@ private function refactorEqualsMethodCall(MethodCall $methodCall): ?Identical } $right = $this->getNonEnumReturnTypeExpr($arg->value); - if (!$right instanceof Expr) { + if (! $right instanceof Expr) { $right = $this->getValidEnumExpr($arg->value); if (! $right instanceof Expr) { return null; diff --git a/rules/TypeDeclaration/Rector/ClassMethod/StrictArrayParamDimFetchRector.php b/rules/TypeDeclaration/Rector/ClassMethod/StrictArrayParamDimFetchRector.php index e654f454c24..d259966e35f 100644 --- a/rules/TypeDeclaration/Rector/ClassMethod/StrictArrayParamDimFetchRector.php +++ b/rules/TypeDeclaration/Rector/ClassMethod/StrictArrayParamDimFetchRector.php @@ -4,20 +4,20 @@ namespace Rector\TypeDeclaration\Rector\ClassMethod; -use PhpParser\Node\Stmt\Expression; -use PhpParser\Node\Expr\MethodCall; use PhpParser\Node; use PhpParser\Node\Expr\ArrayDimFetch; use PhpParser\Node\Expr\AssignOp\Coalesce as AssignOpCoalesce; use PhpParser\Node\Expr\BinaryOp\Coalesce; use PhpParser\Node\Expr\Closure; use PhpParser\Node\Expr\FuncCall; +use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Expr\Variable; use PhpParser\Node\FunctionLike; use PhpParser\Node\Identifier; use PhpParser\Node\Param; use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\ClassMethod; +use PhpParser\Node\Stmt\Expression; use PhpParser\Node\Stmt\Function_; use PhpParser\NodeTraverser; use Rector\Core\Rector\AbstractRector;