Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\BooleanAnd\RepeatedAndNotEqualToNotInArrayRector\Fixture;

final class CompareSameVariable
{
public function demo($someVariable): bool
{
if ($someVariable !== 'a' && $someVariable !== 'b' && $someVariable !== 'c') {
return true;
}

return false;
}
}

?>
-----
<?php

namespace Rector\Tests\CodeQuality\Rector\BooleanAnd\RepeatedAndNotEqualToNotInArrayRector\Fixture;

final class CompareSameVariable
{
public function demo($someVariable): bool
{
if (!in_array($someVariable, ['a', 'b', 'c'], true)) {
return true;
}

return false;
}
}

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

declare(strict_types=1);

namespace Rector\Tests\CodeQuality\Rector\BooleanAnd\RepeatedAndNotEqualToNotInArrayRector;

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

final class RepeatedAndNotEqualToNotInArrayRectorTest 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,9 @@
<?php

declare(strict_types=1);

use Rector\CodeQuality\Rector\BooleanAnd\RepeatedAndNotEqualToNotInArrayRector;
use Rector\Config\RectorConfig;

return RectorConfig::configure()
->withRules([RepeatedAndNotEqualToNotInArrayRector::class]);
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

namespace Rector\Tests\DeadCode\Rector\If_\RemoveUnusedNonEmptyArrayBeforeForeachRector\Fixture;

class NotEqual
final class NotEqualCondition
{
public function run()
{
Expand All @@ -21,7 +21,7 @@ class NotEqual

namespace Rector\Tests\DeadCode\Rector\If_\RemoveUnusedNonEmptyArrayBeforeForeachRector\Fixture;

class NotEqual
final class NotEqualCondition
{
public function run()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,195 @@
<?php

declare(strict_types=1);

namespace Rector\CodeQuality\Rector\BooleanAnd;

use PhpParser\Node;
use PhpParser\Node\Arg;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\BinaryOp\BooleanAnd;
use PhpParser\Node\Expr\BinaryOp\NotEqual;
use PhpParser\Node\Expr\BinaryOp\NotIdentical;
use PhpParser\Node\Expr\BooleanNot;
use PhpParser\Node\Expr\ConstFetch;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Name;
use Rector\CodeQuality\ValueObject\ComparedExprAndValueExpr;
use Rector\PhpParser\Node\BetterNodeFinder;
use Rector\Rector\AbstractRector;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

/**
* @see \Rector\Tests\CodeQuality\Rector\BooleanAnd\RepeatedAndNotEqualToNotInArrayRector\RepeatedAndNotEqualToNotInArrayRectorTest
*/
final class RepeatedAndNotEqualToNotInArrayRector extends AbstractRector
{
public function __construct(
private readonly BetterNodeFinder $betterNodeFinder,
) {
}

public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition(
'Simplify repeated && compare of same value, to ! in_array() call',
[
new CodeSample(
<<<'CODE_SAMPLE'
if ($value !== 10 && $value !== 20 && $value !== 30) {
// ...
}

CODE_SAMPLE
,
<<<'CODE_SAMPLE'
if (! in_array($value, [10, 20, 30], true)) {
// ...
}
CODE_SAMPLE
),
]
);
}

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

/**
* @param BooleanAnd $node
*/
public function refactor(Node $node): ?BooleanNot
{
if (! $this->isNotEqualOrNotIdentical($node->right)) {
return null;
}

// match compared variable and expr
if (! $node->left instanceof BooleanAnd && ! $this->isNotEqualOrNotIdentical($node->left)) {
return null;
}

$comparedExprAndValueExprs = $this->matchComparedAndDesiredValues($node);
if ($comparedExprAndValueExprs === null) {
return null;
}

if (count($comparedExprAndValueExprs) < 3) {
return null;
}

// ensure all compared expr are the same
$valueExprs = $this->resolveValueExprs($comparedExprAndValueExprs);

/** @var ComparedExprAndValueExpr $firstComparedExprAndValue */
$firstComparedExprAndValue = array_pop($comparedExprAndValueExprs);

// all compared expr must be equal
foreach ($comparedExprAndValueExprs as $comparedExprAndValueExpr) {
if (! $this->nodeComparator->areNodesEqual(
$firstComparedExprAndValue->getComparedExpr(),
$comparedExprAndValueExpr->getComparedExpr()
)) {
return null;
}
}

$array = $this->nodeFactory->createArray($valueExprs);

$args = $this->nodeFactory->createArgs([$firstComparedExprAndValue->getComparedExpr(), $array]);

if ($this->isStrictComparison($node)) {
$args[] = new Arg(new ConstFetch(new Name('true')));
}

$inArrayFuncCall = new FuncCall(new Name('in_array'), $args);

return new BooleanNot($inArrayFuncCall);
}

private function isNotEqualOrNotIdentical(Expr $expr): bool
{
if ($expr instanceof NotIdentical) {
return true;
}

return $expr instanceof NotEqual;
}

private function matchComparedExprAndValueExpr(NotIdentical|NotEqual $expr): ComparedExprAndValueExpr
{
return new ComparedExprAndValueExpr($expr->left, $expr->right);
}

/**
* @param ComparedExprAndValueExpr[] $comparedExprAndValueExprs
* @return Expr[]
*/
private function resolveValueExprs(array $comparedExprAndValueExprs): array
{
$valueExprs = [];

foreach ($comparedExprAndValueExprs as $comparedExprAndValueExpr) {
$valueExprs[] = $comparedExprAndValueExpr->getValueExpr();
}

return $valueExprs;
}

/**
* @return null|ComparedExprAndValueExpr[]
*/
private function matchComparedAndDesiredValues(BooleanAnd $booleanAnd): ?array
{
/** @var NotIdentical|NotEqual $rightCompare */
$rightCompare = $booleanAnd->right;

// match compared expr and desired value
$comparedExprAndValueExprs = [$this->matchComparedExprAndValueExpr($rightCompare)];

$currentBooleanAnd = $booleanAnd;

while ($currentBooleanAnd->left instanceof BooleanAnd) {
if (! $this->isNotEqualOrNotIdentical($currentBooleanAnd->left->right)) {
return null;
}

/** @var NotIdentical|NotEqual $leftRight */
$leftRight = $currentBooleanAnd->left->right;
$comparedExprAndValueExprs[] = $this->matchComparedExprAndValueExpr($leftRight);
$currentBooleanAnd = $currentBooleanAnd->left;
}

if (! $this->isNotEqualOrNotIdentical($currentBooleanAnd->left)) {
return null;
}

/** @var NotIdentical|NotEqual $leftCompare */
$leftCompare = $currentBooleanAnd->left;

$comparedExprAndValueExprs[] = $this->matchComparedExprAndValueExpr($leftCompare);

// keep original natural order, as left/right goes from bottom up
return array_reverse($comparedExprAndValueExprs);
}

private function isStrictComparison(BooleanAnd $booleanAnd): bool
{
$notIdenticals = $this->betterNodeFinder->findInstanceOf($booleanAnd, NotIdentical::class);
$notEquals = $this->betterNodeFinder->findInstanceOf($booleanAnd, NotEqual::class);

if ($notIdenticals !== []) {
// mix not identical and not equals, keep as is
// @see https://3v4l.org/2SoHZ
return $notEquals === [];
}

return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public function __construct(
public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition(
'Simplify repeated || compare of same value, to in_array() class',
'Simplify repeated || compare of same value, to in_array() call',
[
new CodeSample(
<<<'CODE_SAMPLE'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ private function replaceVisibilityFlag(ClassMethod | Property | ClassConst | Par
$this->makeNonStatic($node);
}

if ($visibility !== Visibility::STATIC && $visibility !== Visibility::ABSTRACT && $visibility !== Visibility::FINAL) {
if (! in_array($visibility, [Visibility::STATIC, Visibility::ABSTRACT, Visibility::FINAL], true)) {
$this->removeVisibility($node);
}

Expand Down
2 changes: 2 additions & 0 deletions src/Config/Level/CodeQualityLevel.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use Rector\CodeQuality\Rector\Assign\CombinedAssignRector;
use Rector\CodeQuality\Rector\BooleanAnd\RemoveUselessIsObjectCheckRector;
use Rector\CodeQuality\Rector\BooleanAnd\RepeatedAndNotEqualToNotInArrayRector;
use Rector\CodeQuality\Rector\BooleanAnd\SimplifyEmptyArrayCheckRector;
use Rector\CodeQuality\Rector\BooleanNot\ReplaceMultipleBooleanNotRector;
use Rector\CodeQuality\Rector\BooleanNot\SimplifyDeMorganBinaryRector;
Expand Down Expand Up @@ -107,6 +108,7 @@ final class CodeQualityLevel
ReplaceMultipleBooleanNotRector::class,
ForeachToInArrayRector::class,
RepeatedOrEqualToInArrayRector::class,
RepeatedAndNotEqualToNotInArrayRector::class,
SimplifyForeachToCoalescingRector::class,
SimplifyFuncGetArgsCountRector::class,
SimplifyInArrayValuesRector::class,
Expand Down
3 changes: 2 additions & 1 deletion src/ValueObject/Error/SystemError.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ public function getRectorClass(): ?string
public function getRectorShortClass(): ?string
{
$rectorClass = $this->rectorClass;
if ($rectorClass !== null && $rectorClass !== '' && $rectorClass !== '0') {

if (! in_array($rectorClass, [null, ''], true)) {
return (string) Strings::after($rectorClass, '\\', -1);
}

Expand Down