Skip to content

Commit

Permalink
[EarlyReturn] Handle crash on RemoveAlwaysElseRector+ReturnEarlyIfVar…
Browse files Browse the repository at this point in the history
…iableRector (#3011)

Co-authored-by: GitHub Action <action@github.com>
  • Loading branch information
samsonasik and actions-user committed Oct 24, 2022
1 parent f915a1f commit 5131abd
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 23 deletions.
36 changes: 13 additions & 23 deletions src/ProcessAnalyzer/RectifiedAnalyzer.php
Original file line number Diff line number Diff line change
Expand Up @@ -92,34 +92,24 @@ private function shouldContinue(
return true;
}

if ($this->isPreviousCreatedByRuleAttributeEquals($rectifiedNodeClass, $rectifiedNodeNode, $node)) {
return true;
}

$startTokenPos = $node->getStartTokenPos();
return $startTokenPos < 0;
}
$parentNode = $node->getAttribute(AttributeKey::PARENT_NODE);

/**
* @param class-string<RectorInterface> $rectifiedNodeClass
*/
private function isPreviousCreatedByRuleAttributeEquals(
string $rectifiedNodeClass,
Node $rectifiedNodeNode,
Node $node
): bool {
/** @var class-string<RectorInterface>[] $createdByRule */
$createdByRule = $node->getAttribute(AttributeKey::CREATED_BY_RULE) ?? [];
$countCreatedByRule = count($createdByRule);
if ($countCreatedByRule !== 1) {
return $countCreatedByRule !== 0;
if (! $parentNode instanceof Node) {
return true;
}

// different rule, allowed
if (current($createdByRule) !== $rectifiedNodeClass) {
$parentOriginalNode = $parentNode->getAttribute(AttributeKey::ORIGINAL_NODE);
if ($parentOriginalNode instanceof Node) {
return true;
}

return $rectifiedNodeNode === $node;
/**
* Start token pos must be < 0 to continue, as the node and parent node just re-printed
*
* - Node's original node is null
* - Parent Node's original node is null
*/
$startTokenPos = $node->getStartTokenPos();
return $startTokenPos < 0;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
<?php

namespace Rector\Core\Tests\Issues\IssueRemoveAlwaysElseReturnEarlyIfVariable\Fixture;

final class Fixture
{
public function get($a, $b = false)
{
if ($b !== true) {
$value = $a * 5;

if ($value === 'hello') {
$value = false;
}

return $value;
} elseif ($a === true) {
return false;
}

return true;
}
}

?>
-----
<?php

namespace Rector\Core\Tests\Issues\IssueRemoveAlwaysElseReturnEarlyIfVariable\Fixture;

final class Fixture
{
public function get($a, $b = false)
{
if ($b !== true) {
$value = $a * 5;
if ($value === 'hello') {
return false;
}
return $value;
}
if ($a === true) {
return false;
}

return true;
}
}

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

declare(strict_types=1);

namespace Rector\Core\Tests\Issues\IssueRemoveAlwaysElseReturnEarlyIfVariable;

use Iterator;
use Rector\Testing\PHPUnit\AbstractRectorTestCase;

final class IssueRemoveAlwaysElseReturnEarlyIfVariableTest extends AbstractRectorTestCase
{
/**
* @dataProvider provideData()
*/
public function test(string $filePath): void
{
$this->doTestFile($filePath);
}

/**
* @return Iterator<array<string>>
*/
public function provideData(): Iterator
{
return $this->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,12 @@
<?php

declare(strict_types=1);

use Rector\Config\RectorConfig;
use Rector\EarlyReturn\Rector\If_\RemoveAlwaysElseRector;
use Rector\EarlyReturn\Rector\StmtsAwareInterface\ReturnEarlyIfVariableRector;

return static function (RectorConfig $rectorConfig): void {
$rectorConfig->rule(RemoveAlwaysElseRector::class);
$rectorConfig->rule(ReturnEarlyIfVariableRector::class);
};

0 comments on commit 5131abd

Please sign in to comment.