Skip to content

Commit

Permalink
[CodeQuality] Add NumberCompareToMaxFuncCallRector (#4914)
Browse files Browse the repository at this point in the history
  • Loading branch information
TomasVotruba authored Sep 5, 2023
1 parent 8c5cfaf commit 093bcb6
Show file tree
Hide file tree
Showing 13 changed files with 315 additions and 17 deletions.
54 changes: 48 additions & 6 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
# 355 Rules Overview

<br>

## Categories

- [Arguments](#arguments) (4)

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

- [CodingStyle](#codingstyle) (30)

Expand Down Expand Up @@ -54,7 +54,7 @@

- [Transform](#transform) (22)

- [TypeDeclaration](#typedeclaration) (41)
- [TypeDeclaration](#typedeclaration) (42)

- [Visibility](#visibility) (3)

Expand Down Expand Up @@ -862,6 +862,25 @@ Change unsafe new `static()` to new `self()`

<br>

### 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);
}
}
```

<br>

### OptionalParametersAfterRequiredRector

Move required parameters after optional ones
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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;
```

<br>
Expand Down Expand Up @@ -6738,6 +6761,25 @@ Add strict type declaration based on returned constants

<br>

### 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;
}
}
```

<br>

### ReturnTypeFromStrictNativeCallRector

Add strict return type based native function or class method return
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 @@ -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;
Expand Down Expand Up @@ -186,5 +187,6 @@
DisallowedEmptyRuleFixerRector::class,
ConvertStaticPrivateConstantToSelfRector::class,
LocallyCalledStaticMethodToNonStaticRector::class,
NumberCompareToMaxFuncCallRector::class,
]);
};
6 changes: 4 additions & 2 deletions packages/Testing/PHPUnit/AbstractRectorTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\Ternary\NumberCompareToMaxFuncCallRector\Fixture;

final class AlsoTheOtherSide
{
public function run(int $value)
{
return 100 < $value ? $value : 100;
}
}

?>
-----
<?php

namespace Rector\Tests\CodeQuality\Rector\Ternary\NumberCompareToMaxFuncCallRector\Fixture;

final class AlsoTheOtherSide
{
public function run(int $value)
{
return max($value, 100);
}
}

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

namespace Rector\Tests\CodeQuality\Rector\Ternary\NumberCompareToMaxFuncCallRector\Fixture;

final class IncludeGreaterOrEqual
{
public function run(int $value)
{
return $value >= 100 ? $value : 100;
}
}

?>
-----
<?php

namespace Rector\Tests\CodeQuality\Rector\Ternary\NumberCompareToMaxFuncCallRector\Fixture;

final class IncludeGreaterOrEqual
{
public function run(int $value)
{
return max($value, 100);
}
}

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

namespace Rector\Tests\CodeQuality\Rector\Ternary\NumberCompareToMaxFuncCallRector\Fixture;

final class SkipElseAnotherValue
{
public function run(int $value)
{
return $value > 100 ? $value : 55;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\Ternary\NumberCompareToMaxFuncCallRector\Fixture;

final class SkipNonNumber
{
public function run($value)
{
return 100 < $value ? $value : 100;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\Ternary\NumberCompareToMaxFuncCallRector\Fixture;

class SomeClass
{
public function run(int $value)
{
return $value > 100 ? $value : 100;
}
}

?>
-----
<?php

namespace Rector\Tests\CodeQuality\Rector\Ternary\NumberCompareToMaxFuncCallRector\Fixture;

class SomeClass
{
public function run(int $value)
{
return max($value, 100);
}
}

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

declare(strict_types=1);

namespace Rector\Tests\CodeQuality\Rector\Ternary\NumberCompareToMaxFuncCallRector;

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

final class NumberCompareToMaxFuncCallRectorTest 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,10 @@
<?php

declare(strict_types=1);

use Rector\CodeQuality\Rector\Ternary\NumberCompareToMaxFuncCallRector;
use Rector\Config\RectorConfig;

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

declare(strict_types=1);

namespace Rector\CodeQuality\Rector\Ternary;

use PhpParser\Node\Expr\BinaryOp\GreaterOrEqual;
use PhpParser\Node;
use PhpParser\Node\Expr\BinaryOp;
use PhpParser\Node\Expr\BinaryOp\Greater;
use PhpParser\Node\Expr\BinaryOp\Smaller;
use PhpParser\Node\Expr\BinaryOp\SmallerOrEqual;
use PhpParser\Node\Expr\Ternary;
use Rector\Core\Rector\AbstractRector;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

/**
* @see \Rector\Tests\CodeQuality\Rector\Ternary\NumberCompareToMaxFuncCallRector\NumberCompareToMaxFuncCallRectorTest
*/
final class NumberCompareToMaxFuncCallRector extends AbstractRector
{
public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition('Ternary number compare to max() call', [
new CodeSample(
<<<'CODE_SAMPLE'
class SomeClass
{
public function run($value)
{
return $value > 100 ? $value : 100;
}
}
CODE_SAMPLE

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

/**
* @return array<class-string<Node>>
*/
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();
}
}
Loading

0 comments on commit 093bcb6

Please sign in to comment.