-
-
Notifications
You must be signed in to change notification settings - Fork 684
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix missing stmt on adding a new node #6383
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ | |
use Rector\ChangesReporting\Collector\RectorChangeCollector; | ||
use Rector\Core\Exception\ShouldNotHappenException; | ||
use Rector\Core\PhpParser\Node\BetterNodeFinder; | ||
use Rector\Core\PhpParser\Printer\BetterStandardPrinter; | ||
use Rector\NodeTypeResolver\Node\AttributeKey; | ||
use Rector\PostRector\Contract\Collector\NodeCollectorInterface; | ||
|
||
|
@@ -37,10 +38,19 @@ final class NodesToAddCollector implements NodeCollectorInterface | |
*/ | ||
private $rectorChangeCollector; | ||
|
||
public function __construct(BetterNodeFinder $betterNodeFinder, RectorChangeCollector $rectorChangeCollector) | ||
{ | ||
/** | ||
* @var BetterStandardPrinter | ||
*/ | ||
private $betterStandardPrinter; | ||
|
||
public function __construct( | ||
BetterNodeFinder $betterNodeFinder, | ||
RectorChangeCollector $rectorChangeCollector, | ||
BetterStandardPrinter $betterStandardPrinter | ||
) { | ||
$this->betterNodeFinder = $betterNodeFinder; | ||
$this->rectorChangeCollector = $rectorChangeCollector; | ||
$this->betterStandardPrinter = $betterStandardPrinter; | ||
} | ||
|
||
public function isActive(): bool | ||
|
@@ -132,14 +142,22 @@ private function resolveNearestExpressionPosition(Node $node): string | |
return spl_object_hash($node); | ||
} | ||
|
||
$currentStmt = $node->getAttribute(AttributeKey::CURRENT_STATEMENT); | ||
if ($currentStmt instanceof Stmt) { | ||
return spl_object_hash($currentStmt); | ||
} | ||
|
||
$parent = $node->getAttribute(AttributeKey::PARENT_NODE); | ||
if ($parent instanceof Return_) { | ||
return spl_object_hash($parent); | ||
} | ||
|
||
$foundNode = $this->betterNodeFinder->findParentTypes($node, [Expression::class, Stmt::class]); | ||
if ($foundNode === null) { | ||
$foundNode = $node; | ||
Comment on lines
-141
to
-142
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure why I put this here (probably not knowing difference between Stmt and Expr in that time), but it was asking for a bug 🤔 If it's |
||
|
||
if (! $foundNode instanceof Stmt) { | ||
$printedNode = $this->betterStandardPrinter->print($node); | ||
$errorMessage = sprintf('Could not find parent Stmt of "%s" node', $printedNode); | ||
throw new ShouldNotHappenException($errorMessage); | ||
} | ||
|
||
return spl_object_hash($foundNode); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Rector\Core\Tests\Issues; | ||
|
||
use Iterator; | ||
use Rector\Testing\PHPUnit\AbstractRectorTestCase; | ||
use Symplify\SmartFileSystem\SmartFileInfo; | ||
|
||
final class CovariantTrioTest extends AbstractRectorTestCase | ||
{ | ||
/** | ||
* @dataProvider provideData() | ||
*/ | ||
public function test(SmartFileInfo $fileInfo): void | ||
{ | ||
$this->doTestFileInfo($fileInfo); | ||
} | ||
|
||
/** | ||
* @return Iterator<SmartFileInfo> | ||
*/ | ||
public function provideData(): Iterator | ||
{ | ||
return $this->yieldFilesFromDirectory(__DIR__ . '/Fixture'); | ||
} | ||
|
||
public function provideConfigFilePath(): string | ||
{ | ||
return __DIR__ . '/config/covariant_trio.php'; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
<?php | ||
|
||
namespace Rector\Core\Tests\Issues\Fixture; | ||
|
||
final class CoriantMixture | ||
{ | ||
private function parseArgument(string $token, array $all) | ||
{ | ||
if (($inputArgument = $all[$key = array_key_first($all)] ?? null) && 'command' === $inputArgument->getName()) { | ||
} | ||
} | ||
} | ||
|
||
?> | ||
----- | ||
<?php | ||
|
||
namespace Rector\Core\Tests\Issues\Fixture; | ||
|
||
final class CoriantMixture | ||
{ | ||
/** | ||
* @param string $token | ||
*/ | ||
private function parseArgument($token, array $all) | ||
{ | ||
reset($all); | ||
if (($inputArgument = isset($all[$key = key($all)]) ? $all[$key = key($all)] : null) && 'command' === $inputArgument->getName()) { | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @samsonasik BTW, today I find one nasty bug in Rector. This code previously failed and missed all the removed types. Causing missmatch: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @TomasVotruba thank you 👍 |
||
} | ||
|
||
?> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
<?php | ||
|
||
class ArgvInput extends Input | ||
{ | ||
/** | ||
* Parses an argument. | ||
* | ||
* @throws RuntimeException When too many arguments are given | ||
*/ | ||
private function parseArgument(string $token, array $all) | ||
{ | ||
if (($inputArgument = $all[$key = array_key_first($all)] ?? null) && 'command' === $inputArgument->getName()) { | ||
} | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function hasParameterOption($values, bool $onlyParams = false) | ||
{ | ||
} | ||
} | ||
|
||
abstract class Input implements InputInterface | ||
{ | ||
|
||
} | ||
|
||
class ArrayInput extends Input | ||
{ | ||
public function hasParameterOption($values, bool $onlyParams = false) | ||
{ | ||
|
||
} | ||
} | ||
|
||
interface InputInterface | ||
{ | ||
public function hasParameterOption($values, bool $onlyParams = false); | ||
} | ||
|
||
?> | ||
----- | ||
<?php | ||
|
||
class ArgvInput extends Input | ||
{ | ||
/** | ||
* Parses an argument. | ||
* | ||
* @throws RuntimeException When too many arguments are given | ||
* @param string $token | ||
*/ | ||
private function parseArgument($token, array $all) | ||
{ | ||
reset($all); | ||
if (($inputArgument = isset($all[$key = key($all)]) ? $all[$key = key($all)] : null) && 'command' === $inputArgument->getName()) { | ||
} | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
* @param bool $onlyParams | ||
*/ | ||
public function hasParameterOption($values, $onlyParams = false) | ||
{ | ||
} | ||
} | ||
|
||
abstract class Input implements InputInterface | ||
{ | ||
|
||
} | ||
|
||
class ArrayInput extends Input | ||
{ | ||
/** | ||
* @param bool $onlyParams | ||
*/ | ||
public function hasParameterOption($values, $onlyParams = false) | ||
{ | ||
|
||
} | ||
} | ||
|
||
interface InputInterface | ||
{ | ||
/** | ||
* @param bool $onlyParams | ||
*/ | ||
public function hasParameterOption($values, $onlyParams = false); | ||
} | ||
|
||
?> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
use Rector\Core\Configuration\Option; | ||
use Rector\Core\ValueObject\PhpVersion; | ||
use Rector\DowngradePhp70\Rector\Coalesce\DowngradeNullCoalesceRector; | ||
use Rector\DowngradePhp70\Rector\FunctionLike\DowngradeTypeDeclarationRector; | ||
use Rector\DowngradePhp73\Rector\FuncCall\DowngradeArrayKeyFirstLastRector; | ||
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; | ||
|
||
return static function (ContainerConfigurator $containerConfigurator): void { | ||
$parameters = $containerConfigurator->parameters(); | ||
$parameters->set(Option::PHP_VERSION_FEATURES, PhpVersion::PHP_70); | ||
|
||
$services = $containerConfigurator->services(); | ||
$services->set(DowngradeArrayKeyFirstLastRector::class); | ||
$services->set(DowngradeTypeDeclarationRector::class); | ||
$services->set(DowngradeNullCoalesceRector::class); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the fix that helped. It might require more parent node/stmt connecting fixes though