Skip to content

Commit

Permalink
Add failing test fixture for RemoveParentCallWithoutParentRector (#711)
Browse files Browse the repository at this point in the history
* Add failing test fixture for RemoveParentCallWithoutParentRector

* Add test for PHP 7.4

* Try to fix.

* Skip anonymous class

* Fix circular mixin

* Use ClassAnalyzer service for detecting anonymous class

* Revert "Use ClassAnalyzer service for detecting anonymous class"

This reverts commit c2ed3c2.

* improve performance
  • Loading branch information
zingimmick committed Aug 19, 2021
1 parent 7095dca commit 9f311dc
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
use PHPStan\Node\UnreachableStatementNode;
use PHPStan\Reflection\ClassReflection;
use PHPStan\Reflection\ReflectionProvider;
use PHPStan\Type\ObjectType;
use Rector\Caching\Detector\ChangedFilesDetector;
use Rector\Caching\FileSystem\DependencyResolver;
use Rector\Core\Exception\ShouldNotHappenException;
Expand Down Expand Up @@ -127,18 +128,12 @@ private function processNodesWithMixinHandling(
MutatingScope $mutatingScope,
callable $nodeCallback
): array {
$contents = $smartFileInfo->getContents();

// avoid crash on class with @mixin @see https://github.com/rectorphp/rector-src/pull/688
if (! Strings::match($contents, self::MIXIN_REGEX)) {
// avoid crash on class with @mixin in source @see https://github.com/rectorphp/rector-src/pull/689
if ($this->isMixinInSource($nodes, $smartFileInfo)) {
return $nodes;
}

$this->nodeScopeResolver->processNodes($nodes, $mutatingScope, $nodeCallback);
if ($this->isMixinInSource($nodes)) {
return $nodes;
}

$this->nodeScopeResolver->processNodes($nodes, $mutatingScope, $nodeCallback);

$this->resolveAndSaveDependentFiles($nodes, $mutatingScope, $smartFileInfo);

return $nodes;
Expand All @@ -147,44 +142,58 @@ private function processNodesWithMixinHandling(
/**
* @param Node[] $nodes
*/
private function isMixinInSource(array $nodes, SmartFileInfo $smartFileInfo): bool
private function isMixinInSource(array $nodes): bool
{
$realPath = $smartFileInfo->getRealPath();
return (bool) $this->betterNodeFinder->findFirst($nodes, function (Node $node) use ($realPath): bool {
if (! $node instanceof FullyQualified) {
return (bool) $this->betterNodeFinder->findFirst($nodes, function (Node $node): bool {
if (! $node instanceof FullyQualified && ! $node instanceof Class_) {
return false;
}

$className = $node->toString();

// fix error in parallel test
// use function_exists on purpose as using reflectionProvider broke the test in parallel
if (function_exists($className)) {
if ($node instanceof Class_ && $node->isAnonymous()) {
return false;
}

$hasClass = $this->reflectionProvider->hasClass($className);
$className = $node instanceof FullyQualified ? $node->toString() : $node->namespacedName->toString();

if (! $hasClass) {
return false;
}
return $this->isCircularMixin($className);
});
}

$classReflection = $this->reflectionProvider->getClass($className);
if ($classReflection->isBuiltIn()) {
private function isCircularMixin(string $className): bool
{
// fix error in parallel test
// use function_exists on purpose as using reflectionProvider broke the test in parallel
if (function_exists($className)) {
return false;
}

$hasClass = $this->reflectionProvider->hasClass($className);

if (! $hasClass) {
return false;
}

$classReflection = $this->reflectionProvider->getClass($className);
if ($classReflection->isBuiltIn()) {
return false;
}

foreach ($classReflection->getMixinTags() as $mixinTag) {
$type = $mixinTag->getType();
if (!$type instanceof ObjectType){
return false;
}

$fileName = $classReflection->getFileName();
if ($fileName === false) {
return false;
if ($type->getClassName() === $className) {
return true;
}

if ($fileName === $realPath) {
return false;
if ($this->isCircularMixin($type->getClassName())){
return true;
}
}

return $classReflection->getMixinTags() !== [];
});
return false;
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

namespace Rector\Tests\DeadCode\Rector\StaticCall\RemoveParentCallWithoutParentRector\Fixture;

class ParentWithReturnAndUsedInMixin extends SomeClass
{
public function getIncrementing()
{
return parent::getIncrementing();
}
}

class SomeClass
{
public function getIncrementing()
{
return true;
}
}

/**
* @mixin SomeClass
*/
class SomeUnrelatedClass
{
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php

declare(strict_types=1);

namespace Rector\Tests\DeadCode\Rector\StaticCall\RemoveParentCallWithoutParentRector;

use Iterator;
use Rector\Testing\PHPUnit\AbstractRectorTestCase;
use Symplify\SmartFileSystem\SmartFileInfo;

final class Php74Test extends AbstractRectorTestCase
{
/**
* @dataProvider provideData()
*/
public function test(SmartFileInfo $fileInfo): void
{
$this->doTestFileInfo($fileInfo);
}

/**
* @return Iterator<SmartFileInfo>
*/
public function provideData(): Iterator
{
return $this->yieldFilesFromDirectory(__DIR__ . '/FixturePhp74');
}

public function provideConfigFilePath(): string
{
return __DIR__ . '/config/php74.php';
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

declare(strict_types=1);

use Rector\Core\Configuration\Option;
use Rector\Core\ValueObject\PhpVersion;
use Rector\DeadCode\Rector\StaticCall\RemoveParentCallWithoutParentRector;
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;

return static function (ContainerConfigurator $containerConfigurator): void {
$parameters = $containerConfigurator->parameters();
$parameters->set(Option::PHP_VERSION_FEATURES, PhpVersion::PHP_74);

$services = $containerConfigurator->services();
$services->set(RemoveParentCallWithoutParentRector::class);
};

0 comments on commit 9f311dc

Please sign in to comment.