Skip to content

Commit

Permalink
[PHPStan 1.0] Remove node naming visitors and re-use PHPStan ones (#1144
Browse files Browse the repository at this point in the history
)
  • Loading branch information
TomasVotruba committed Nov 5, 2021
1 parent 09f82ca commit 2304666
Show file tree
Hide file tree
Showing 23 changed files with 50 additions and 183 deletions.
3 changes: 3 additions & 0 deletions packages/NodeTypeResolver/Node/AttributeKey.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@

use Symplify\SmartFileSystem\SmartFileInfo;

/**
* @enum
*/
final class AttributeKey
{
/**
Expand Down
36 changes: 3 additions & 33 deletions packages/NodeTypeResolver/NodeScopeAndMetadataDecorator.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
use PhpParser\Node\Stmt;
use PhpParser\NodeTraverser;
use PhpParser\NodeVisitor\CloningVisitor;
use PhpParser\NodeVisitor\NameResolver;
use PhpParser\NodeVisitor\NodeConnectingVisitor;
use Rector\Core\ValueObject\Application\File;
use Rector\NodeTypeResolver\NodeVisitor\FunctionLikeParamArgPositionNodeVisitor;
Expand All @@ -18,16 +17,6 @@

final class NodeScopeAndMetadataDecorator
{
/**
* @var string
*/
private const OPTION_PRESERVE_ORIGINAL_NAMES = 'preserveOriginalNames';

/**
* @var string
*/
private const OPTION_REPLACE_NODES = 'replaceNodes';

public function __construct(
private CloningVisitor $cloningVisitor,
private FunctionMethodAndClassNodeVisitor $functionMethodAndClassNodeVisitor,
Expand All @@ -45,35 +34,16 @@ public function __construct(
*/
public function decorateNodesFromFile(File $file, array $stmts): array
{
$nodeTraverser = new NodeTraverser();
$nodeTraverser->addVisitor(new NameResolver(null, [
self::OPTION_PRESERVE_ORIGINAL_NAMES => true,
// required by PHPStan
self::OPTION_REPLACE_NODES => true,
]));

/** @var Stmt[] $stmts */
$stmts = $nodeTraverser->traverse($stmts);

$smartFileInfo = $file->getSmartFileInfo();

$stmts = $this->phpStanNodeScopeResolver->processNodes($stmts, $smartFileInfo);

$nodeTraverserForPreservingName = new NodeTraverser();

$preservingNameResolver = new NameResolver(null, [
self::OPTION_PRESERVE_ORIGINAL_NAMES => true,
// this option would override old non-fqn-namespaced nodes otherwise, so it needs to be disabled
self::OPTION_REPLACE_NODES => false,
]);

$nodeTraverserForPreservingName->addVisitor($preservingNameResolver);
$stmts = $nodeTraverserForPreservingName->traverse($stmts);

$nodeTraverserForFormatPreservePrinting = new NodeTraverser();
// needed also for format preserving printing
$nodeTraverserForFormatPreservePrinting->addVisitor($this->cloningVisitor);

// this one has to be run again to re-connect nodes with new attributes
$nodeTraverserForFormatPreservePrinting->addVisitor($this->nodeConnectingVisitor);

$nodeTraverserForFormatPreservePrinting->addVisitor($this->functionMethodAndClassNodeVisitor);
$nodeTraverserForFormatPreservePrinting->addVisitor($this->namespaceNodeVisitor);
$nodeTraverserForFormatPreservePrinting->addVisitor($this->functionLikeParamArgPositionNodeVisitor);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ declare(strict_types=1);

namespace Rector\Tests\CodingStyle\Rector\Namespace_\ImportFullyQualifiedNamesRector\Fixture;

use Rector\Tests\CodingStyle\Rector\Namespace_\ImportFullyQualifiedNamesRector\Source\Contract\Token;
use Rector\Tests\CodingStyle\Rector\Namespace_\ImportFullyQualifiedNamesRector\Source\Contract;

abstract class KeepStaticMethod
{
public static function decode(string $token): Contract\Token
Expand All @@ -22,7 +25,9 @@ declare(strict_types=1);

namespace Rector\Tests\CodingStyle\Rector\Namespace_\ImportFullyQualifiedNamesRector\Fixture;

use Rector\Tests\CodingStyle\Rector\Namespace_\ImportFullyQualifiedNamesRector\Fixture\Contract\Token;
use Rector\Tests\CodingStyle\Rector\Namespace_\ImportFullyQualifiedNamesRector\Source\Contract\Token;
use Rector\Tests\CodingStyle\Rector\Namespace_\ImportFullyQualifiedNamesRector\Source\Contract;

abstract class KeepStaticMethod
{
public static function decode(string $token): Token
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,3 @@ class SkipAliasedNames
$constraint = Assert\Blank::class;
}
}
?>
-----
<?php
namespace Rector\Tests\CodingStyle\Rector\Namespace_\ImportFullyQualifiedNamesRector\Fixture;

use Symfony\Component\Validator\Constraints\Blank;
use Symfony\Component\Validator\Constraints as Assert;

class SkipAliasedNames
{
public function __construct()
{
$constraint = Blank::class;
}
}
?>
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,3 @@ class SkipSameNamespacedUsedClass
{
}
}
-----
<?php

namespace Rector\Tests\CodingStyle\Rector\Namespace_\ImportFullyQualifiedNamesRector\Fixture;

use Rector\Tests\CodingStyle\Rector\Namespace_\ImportFullyQualifiedNamesRector\Source\SharedShortName;
class SkipSameNamespacedUsedClass
{
/**
* @return SharedShortName
*/
public function run(): SharedShortName
{
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ use PhpSpec\ObjectBehavior;
class RatesTest extends \PHPUnit\Framework\TestCase
{
private \Rector\Tests\PhpSpecToPHPUnit\Rector\Variable\PhpSpecToPHPUnitRector\Fixture\Rates $rates;
private \PHPUnit\Framework\MockObject\MockObject|\spec\Rector\Tests\PhpSpecToPHPUnit\Rector\Variable\PhpSpecToPHPUnitRector\Fixture\Provider $provider;
private \PHPUnit\Framework\MockObject\MockObject|\Provider $provider;
protected function setUp()
{
$this->provider = $this->createMock(Provider::class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class CreateMeTest extends \PHPUnit\Framework\TestCase

public function testCalled()
{
/** @var spec\Rector\Tests\PhpSpecToPHPUnit\Rector\Variable\PhpSpecToPHPUnitRector\Fixture\Cart|\PHPUnit\Framework\MockObject\MockObject $cart */
/** @var Cart|\PHPUnit\Framework\MockObject\MockObject $cart */
$cart = $this->createMock(Cart::class);
$cart->expects($this->atLeastOnce())->method('price')->willReturn(5);
$cart->expects($this->atLeastOnce())->method('shippingAddress')->with($this->isInstanceOf(Address::class));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class BlablaTest extends \PHPUnit\Framework\TestCase
}
public function testMe()
{
/** @var spec\Rector\Tests\PhpSpecToPHPUnit\Rector\Variable\PhpSpecToPHPUnitRector\Fixture\SomeType|\PHPUnit\Framework\MockObject\MockObject $someType */
/** @var SomeType|\PHPUnit\Framework\MockObject\MockObject $someType */
$someType = $this->createMock(SomeType::class);
$assignMe = $someType;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
class PageLinksGeneratorTest extends \PHPUnit\Framework\TestCase
{
private \Rector\Tests\PhpSpecToPHPUnit\Rector\Variable\PhpSpecToPHPUnitRector\Fixture\PageLinksGenerator $pageLinksGenerator;
private \PHPUnit\Framework\MockObject\MockObject|\Symfony\Bundle\FrameworkBundle\Routing\Router $router;
private \PHPUnit\Framework\MockObject\MockObject|\Router $router;
public function testInitializable()
{
$this->assertInstanceOf(PageLinksGenerator::class, $this->pageLinksGenerator);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use PhpSpec\ObjectBehavior;
class CurrencyTest extends \PHPUnit\Framework\TestCase
{
private \Rector\Tests\PhpSpecToPHPUnit\Rector\Variable\PhpSpecToPHPUnitRector\Fixture\Currency $currency;
private \PHPUnit\Framework\MockObject\MockObject|\spec\Rector\Tests\PhpSpecToPHPUnit\Rector\Variable\PhpSpecToPHPUnitRector\Fixture\CurrencyData $data;
private \CurrencyData|\PHPUnit\Framework\MockObject\MockObject $data;
protected function setUp()
{
$this->data = $this->createMock(CurrencyData::class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ class DeliveryTest extends \PHPUnit\Framework\TestCase
private \Rector\Tests\PhpSpecToPHPUnit\Rector\Variable\PhpSpecToPHPUnitRector\Fixture\Delivery $delivery;
protected function setUp()
{
/** @var Rector\Tests\PhpSpecToPHPUnit\Rector\Variable\PhpSpecToPHPUnitRector\Source\DeliveryFactory|\PHPUnit\Framework\MockObject\MockObject $factory */
/** @var DeliveryFactory|\PHPUnit\Framework\MockObject\MockObject $factory */
$factory = $this->createMock(DeliveryFactory::class);
/** @var Rector\Tests\PhpSpecToPHPUnit\Rector\Variable\PhpSpecToPHPUnitRector\Source\ShippingMethod|\PHPUnit\Framework\MockObject\MockObject $shippingMethod */
/** @var ShippingMethod|\PHPUnit\Framework\MockObject\MockObject $shippingMethod */
$shippingMethod = $this->createMock(ShippingMethod::class);
$factory->expects($this->atLeastOnce())
->method('createShippingMethodFor')->with($this->equalTo(5))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use Rector\Tests\PhpSpecToPHPUnit\Rector\Variable\PhpSpecToPHPUnitRector\Source\
class MockPropertiesTest extends \PHPUnit\Framework\TestCase
{
private \Rector\Tests\PhpSpecToPHPUnit\Rector\Variable\PhpSpecToPHPUnitRector\Fixture\MockProperties $mockProperties;
private \PHPUnit\Framework\MockObject\MockObject|\Rector\Tests\PhpSpecToPHPUnit\Rector\Variable\PhpSpecToPHPUnitRector\Source\OrderFactory $factory;
private \OrderFactory|\PHPUnit\Framework\MockObject\MockObject $factory;
protected function setUp()
{
$this->factory = $this->createMock(OrderFactory::class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,22 +39,22 @@ class MockPropertiesNonLocalTest extends \PHPUnit\Framework\TestCase
private \Rector\Tests\PhpSpecToPHPUnit\Rector\Variable\PhpSpecToPHPUnitRector\Fixture\MockPropertiesNonLocal $mockPropertiesNonLocal;
protected function setUp()
{
/** @var spec\Rector\Tests\PhpSpecToPHPUnit\Rector\Variable\PhpSpecToPHPUnitRector\Fixture\OrderFactory|\PHPUnit\Framework\MockObject\MockObject $factory */
/** @var OrderFactory|\PHPUnit\Framework\MockObject\MockObject $factory */
$factory = $this->createMock(OrderFactory::class);
$this->mockPropertiesNonLocal = new \Rector\Tests\PhpSpecToPHPUnit\Rector\Variable\PhpSpecToPHPUnitRector\Fixture\MockPropertiesNonLocal($factory);
}

public function testLetItGo()
{
/** @var Rector\Tests\PhpSpecToPHPUnit\Rector\Variable\PhpSpecToPHPUnitRector\Source\AnotherMock|\PHPUnit\Framework\MockObject\MockObject $anotherMock */
/** @var AnotherMock|\PHPUnit\Framework\MockObject\MockObject $anotherMock */
$anotherMock = $this->createMock(AnotherMock::class);
$anotherMock->setName('Nummy');
$this->mockPropertiesNonLocal->addAnotherMock($anotherMock);
}

public function testLetItGoAgain()
{
/** @var Rector\Tests\PhpSpecToPHPUnit\Rector\Variable\PhpSpecToPHPUnitRector\Source\AnotherMock|\PHPUnit\Framework\MockObject\MockObject $anotherMock */
/** @var AnotherMock|\PHPUnit\Framework\MockObject\MockObject $anotherMock */
$anotherMock = $this->createMock(AnotherMock::class);
$anotherMock->setName('Nummy2');
$this->mockPropertiesNonLocal->addAnotherMock($anotherMock);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ class OrderTest extends \PHPUnit\Framework\TestCase
private \Rector\Tests\PhpSpecToPHPUnit\Rector\Variable\PhpSpecToPHPUnitRector\Fixture\Order $order;
protected function setUp()
{
/** @var spec\Rector\Tests\PhpSpecToPHPUnit\Rector\Variable\PhpSpecToPHPUnitRector\Fixture\OrderFactory|\PHPUnit\Framework\MockObject\MockObject $factory */
/** @var OrderFactory|\PHPUnit\Framework\MockObject\MockObject $factory */
$factory = $this->createMock(OrderFactory::class);
/** @var spec\Rector\Tests\PhpSpecToPHPUnit\Rector\Variable\PhpSpecToPHPUnitRector\Fixture\ShippingMethod|\PHPUnit\Framework\MockObject\MockObject $shippingMethod */
/** @var ShippingMethod|\PHPUnit\Framework\MockObject\MockObject $shippingMethod */
$shippingMethod = $this->createMock(ShippingMethod::class);
$factory->expects($this->atLeastOnce())->method('createShippingMethodFor')->willReturn($shippingMethod);
$factory->expects($this->atLeastOnce())->method('anotherMethod')->with($this->equalTo(25))->willReturn($shippingMethod);
Expand Down
22 changes: 2 additions & 20 deletions rules/DeadCode/NodeAnalyzer/ExprUsedInNextNodeAnalyzer.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@

use PhpParser\Node;
use PhpParser\Node\Expr;
use PhpParser\Node\Name;
use PHPStan\Analyser\Scope;
use Rector\Core\PhpParser\Node\BetterNodeFinder;
use Rector\NodeTypeResolver\Node\AttributeKey;

final class ExprUsedInNextNodeAnalyzer
{
Expand All @@ -19,26 +16,11 @@ public function __construct(
) {
}

/**
* $isCheckNameScope parameter is used to whether to check scope of Name that may be renamed
* @see https://github.com/rectorphp/rector/issues/6675
*/
public function isUsed(Expr $expr, bool $isCheckNameScope = false): bool
public function isUsed(Expr $expr): bool
{
return (bool) $this->betterNodeFinder->findFirstNext(
$expr,
function (Node $node) use ($expr, $isCheckNameScope): bool {
if ($isCheckNameScope && $node instanceof Name) {
$scope = $node->getAttribute(AttributeKey::SCOPE);
$resolvedName = $node->getAttribute(AttributeKey::RESOLVED_NAME);

if (! $scope instanceof Scope && ! $resolvedName instanceof Name) {
return true;
}
}

return $this->exprUsedInNodeAnalyzer->isUsed($node, $expr);
}
fn (Node $node): bool => $this->exprUsedInNodeAnalyzer->isUsed($node, $expr)
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public function refactor(Node $node): ?Node
return null;
}

if ($this->exprUsedInNextNodeAnalyzer->isUsed($node->var, true)) {
if ($this->exprUsedInNextNodeAnalyzer->isUsed($node->var)) {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,9 @@

use PhpParser\Node;
use PhpParser\Node\Stmt\Class_;
use PhpParser\Node\Stmt\Interface_;
use PhpParser\Node\Stmt\Property;
use PhpParser\Node\Stmt\Trait_;
use Rector\Core\NodeManipulator\PropertyManipulator;
use Rector\Core\Rector\AbstractRector;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\Removing\NodeManipulator\ComplexNodeRemover;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
Expand Down Expand Up @@ -83,8 +80,7 @@ private function shouldSkipProperty(Property $property): bool
return true;
}

/** @var Class_|Interface_|Trait_|null $classLike */
$classLike = $property->getAttribute(AttributeKey::CLASS_NODE);
return ! $classLike instanceof Class_;
$class = $this->betterNodeFinder->findParentType($property, Class_::class);
return ! $class instanceof Class_;
}
}
3 changes: 1 addition & 2 deletions src/NodeManipulator/PropertyManipulator.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,8 @@ public function isPropertyUsedInReadContext(Property | Param $propertyOrPromoted
return true;
}

// $propertyOrPromotedParam->attrGroups

$privatePropertyFetches = $this->propertyFetchFinder->findPrivatePropertyFetches($propertyOrPromotedParam);

foreach ($privatePropertyFetches as $privatePropertyFetch) {
if ($this->readWritePropertyAnalyzer->isRead($privatePropertyFetch)) {
return true;
Expand Down

0 comments on commit 2304666

Please sign in to comment.