Skip to content

Commit

Permalink
[NodeTypeResolver] Remove parent attribute on PHPStanNodeScopeResolve…
Browse files Browse the repository at this point in the history
…r for after UnreachableStatementNode detector (#4415)

* [NodeTypeResolver] Remove parent attribute on PHPStanNodeScopeResolver for after UnreachableStatementNodeVisitor detector

* inject the visitor

* [ci-review] Rector Rectify

* clean up set child

* remove manual set parent

* remove manual set parent

* property fix

* stop on unreachable

* fix phpstan

* scope fix

* fix

* Update UnreachableStatementNodeVisitor.php

* [ci-review] Rector Rectify

* clean up

* clean up

---------

Co-authored-by: GitHub Action <actions@github.com>
  • Loading branch information
samsonasik and actions-user committed Jul 4, 2023
1 parent 708cc4f commit 93a4b2b
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 71 deletions.
5 changes: 0 additions & 5 deletions config/phpstan/static-reflection.neon
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,10 @@ parameters:
featureToggles:
disableRuntimeReflectionProvider: false

conditionalTags:
PhpParser\NodeVisitor\ParentConnectingVisitor:
phpstan.parser.richParserNodeVisitor: true

services:
- Rector\NodeTypeResolver\Reflection\BetterReflection\RectorBetterReflectionSourceLocatorFactory
- Rector\NodeTypeResolver\Reflection\BetterReflection\SourceLocator\IntermediateSourceLocator
- Rector\NodeTypeResolver\Reflection\BetterReflection\SourceLocatorProvider\DynamicSourceLocatorProvider
- PhpParser\NodeVisitor\ParentConnectingVisitor

# basically decorates native PHPStan source locator with a dynamic source locator that is also available in Rector DI
betterReflectionSourceLocator:
Expand Down
4 changes: 4 additions & 0 deletions packages/NodeTypeResolver/NodeScopeAndMetadataDecorator.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use PhpParser\NodeVisitor\CloningVisitor;
use PhpParser\NodeVisitor\ParentConnectingVisitor;
use Rector\Core\PhpParser\NodeTraverser\FileWithoutNamespaceNodeTraverser;
use Rector\Core\PHPStan\NodeVisitor\UnreachableStatementNodeVisitor;
use Rector\Core\ValueObject\Application\File;
use Rector\NodeTypeResolver\NodeVisitor\FunctionLikeParamArgPositionNodeVisitor;
use Rector\NodeTypeResolver\PHPStan\Scope\PHPStanNodeScopeResolver;
Expand All @@ -22,6 +23,7 @@ public function __construct(
private readonly PHPStanNodeScopeResolver $phpStanNodeScopeResolver,
ParentConnectingVisitor $parentConnectingVisitor,
FunctionLikeParamArgPositionNodeVisitor $functionLikeParamArgPositionNodeVisitor,
UnreachableStatementNodeVisitor $unreachableStatementNodeVisitor,
private readonly FileWithoutNamespaceNodeTraverser $fileWithoutNamespaceNodeTraverser
) {
$this->nodeTraverser = new NodeTraverser();
Expand All @@ -33,6 +35,8 @@ public function __construct(
$this->nodeTraverser->addVisitor($parentConnectingVisitor);

$this->nodeTraverser->addVisitor($functionLikeParamArgPositionNodeVisitor);

$this->nodeTraverser->addVisitor($unreachableStatementNodeVisitor);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@
use PhpParser\Node\NullableType;
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\Class_;
use PhpParser\Node\Stmt\ClassLike;
use PhpParser\Node\Stmt\Declare_;
use PhpParser\Node\Stmt\Enum_;
use PhpParser\Node\Stmt\EnumCase;
use PhpParser\Node\Stmt\Expression;
Expand Down Expand Up @@ -65,7 +63,6 @@
use PHPStan\Type\TypeCombinator;
use Rector\Caching\Detector\ChangedFilesDetector;
use Rector\Caching\FileSystem\DependencyResolver;
use Rector\Core\Contract\PhpParser\Node\StmtsAwareInterface;
use Rector\Core\Exception\ShouldNotHappenException;
use Rector\Core\NodeAnalyzer\ClassAnalyzer;
use Rector\Core\PhpParser\Node\CustomNode\FileWithoutNamespace;
Expand Down Expand Up @@ -334,10 +331,6 @@ public function processNodes(
}
}

if ($node instanceof Stmt) {
$this->setChildOfUnreachableStatementNodeAttribute($node, $mutatingScope);
}

// special case for unreachable nodes
if ($node instanceof UnreachableStatementNode) {
$this->processUnreachableStatementNode($node, $filePath, $mutatingScope);
Expand Down Expand Up @@ -418,26 +411,6 @@ private function processAssign(Assign|AssignOp $assign, MutatingScope $mutatingS
}
}

private function setChildOfUnreachableStatementNodeAttribute(Stmt $stmt, MutatingScope $mutatingScope): void
{
if (! $stmt instanceof StmtsAwareInterface && ! $stmt instanceof ClassLike && ! $stmt instanceof Declare_) {
return;
}

if ($stmt->getAttribute(AttributeKey::IS_UNREACHABLE) !== true) {
return;
}

if ($stmt->stmts === null) {
return;
}

foreach ($stmt->stmts as $childStmt) {
$childStmt->setAttribute(AttributeKey::IS_UNREACHABLE, true);
$childStmt->setAttribute(AttributeKey::SCOPE, $mutatingScope);
}
}

private function processArray(Array_ $array, MutatingScope $mutatingScope): void
{
foreach ($array->items as $arrayItem) {
Expand Down Expand Up @@ -511,23 +484,6 @@ private function processUnreachableStatementNode(
$originalStmt->setAttribute(AttributeKey::SCOPE, $mutatingScope);

$this->processNodes([$originalStmt], $filePath, $mutatingScope);

$parentNode = $unreachableStatementNode->getAttribute(AttributeKey::PARENT_NODE);
if (! $parentNode instanceof StmtsAwareInterface && ! $parentNode instanceof ClassLike && ! $parentNode instanceof Declare_) {
return;
}

$stmtKey = $unreachableStatementNode->getAttribute(AttributeKey::STMT_KEY);
$totalKeys = $parentNode->stmts === null ? 0 : count($parentNode->stmts);

for ($key = $stmtKey + 1; $key < $totalKeys; ++$key) {
if (! isset($parentNode->stmts[$key])) {
continue;
}

$parentNode->stmts[$key]->setAttribute(AttributeKey::IS_UNREACHABLE, true);
$this->processNodes([$parentNode->stmts[$key]], $filePath, $mutatingScope);
}
}

private function processProperty(Property $property, MutatingScope $mutatingScope): void
Expand Down
38 changes: 21 additions & 17 deletions rules/Php56/NodeAnalyzer/UndefinedVariableResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,17 +61,17 @@ public function resolve(ClassMethod | Function_ | Closure $node): array
return NodeTraverser::DONT_TRAVERSE_CURRENT_AND_CHILDREN;
}

if ($node instanceof Foreach_) {
if ($node instanceof Foreach_ || $node instanceof Case_) {
// handled above
return NodeTraverser::DONT_TRAVERSE_CURRENT_AND_CHILDREN;
}

if ($node instanceof Case_) {
return NodeTraverser::DONT_TRAVERSE_CURRENT_AND_CHILDREN;
}

if ($node instanceof Stmt) {
$currentStmt = $node;

if ($currentStmt->getAttribute(AttributeKey::IS_UNREACHABLE) === true) {
return NodeTraverser::STOP_TRAVERSAL;
}
}

if (! $node instanceof Variable) {
Expand All @@ -84,16 +84,8 @@ public function resolve(ClassMethod | Function_ | Closure $node): array
return NodeTraverser::STOP_TRAVERSAL;
}

if ($node->getAttribute(AttributeKey::IS_BEING_ASSIGNED) === true) {
return null;
}

$variableName = (string) $this->nodeNameResolver->getName($node);
if ($this->shouldSkipVariable($node, $variableName, $checkedVariables)) {
return null;
}

if ($this->hasVariableTypeOrCurrentStmtUnreachable($node, $variableName, $currentStmt)) {
if ($this->shouldSkipVariable($node, $variableName, $checkedVariables, $currentStmt)) {
return null;
}

Expand Down Expand Up @@ -205,8 +197,12 @@ private function hasVariableTypeOrCurrentStmtUnreachable(
/**
* @param string[] $checkedVariables
*/
private function shouldSkipVariable(Variable $variable, string $variableName, array &$checkedVariables): bool
{
private function shouldSkipVariable(
Variable $variable,
string $variableName,
array &$checkedVariables,
?Stmt $currentStmt
): bool {
$variableName = $this->nodeNameResolver->getName($variable);

// skip $this, as probably in outer scope
Expand All @@ -226,7 +222,15 @@ private function shouldSkipVariable(Variable $variable, string $variableName, ar
return true;
}

return in_array($variableName, $checkedVariables, true);
if (in_array($variableName, $checkedVariables, true)) {
return true;
}

if ($variable->getAttribute(AttributeKey::IS_BEING_ASSIGNED) === true) {
return true;
}

return $this->hasVariableTypeOrCurrentStmtUnreachable($variable, $variableName, $currentStmt);
}

private function isDifferentWithOriginalNodeOrNoScope(Variable $variable): bool
Expand Down
66 changes: 66 additions & 0 deletions src/PHPStan/NodeVisitor/UnreachableStatementNodeVisitor.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
<?php

declare(strict_types=1);

namespace Rector\Core\PHPStan\NodeVisitor;

use PhpParser\Node;
use PhpParser\Node\Stmt\ClassLike;
use PhpParser\Node\Stmt\Declare_;
use PhpParser\NodeVisitorAbstract;
use PHPStan\Analyser\MutatingScope;
use Rector\Core\Contract\PhpParser\Node\StmtsAwareInterface;
use Rector\Core\Provider\CurrentFileProvider;
use Rector\Core\ValueObject\Application\File;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\NodeTypeResolver\PHPStan\Scope\PHPStanNodeScopeResolver;
use Rector\NodeTypeResolver\PHPStan\Scope\ScopeFactory;

final class UnreachableStatementNodeVisitor extends NodeVisitorAbstract
{
public function __construct(
private readonly CurrentFileProvider $currentFileProvider,
private readonly PHPStanNodeScopeResolver $phpStanNodeScopeResolver,
private readonly ScopeFactory $scopeFactory
) {
}

public function enterNode(Node $node): ?Node
{
if (! $node instanceof StmtsAwareInterface && ! $node instanceof ClassLike && ! $node instanceof Declare_) {
return null;
}

if ($node->stmts === null) {
return null;
}

$file = $this->currentFileProvider->getFile();
if (! $file instanceof File) {
return null;
}

$filePath = $file->getFilePath();
$isPassedUnreachableStmt = false;

$mutatingScope = $node->getAttribute(AttributeKey::SCOPE);
$mutatingScope = $mutatingScope instanceof MutatingScope ? $mutatingScope : $this->scopeFactory->createFromFile(
$filePath
);

foreach ($node->stmts as $stmt) {
if ($stmt->getAttribute(AttributeKey::IS_UNREACHABLE) === true) {
$isPassedUnreachableStmt = true;
continue;
}

if ($isPassedUnreachableStmt) {
$stmt->setAttribute(AttributeKey::IS_UNREACHABLE, true);
$stmt->setAttribute(AttributeKey::SCOPE, $mutatingScope);
$this->phpStanNodeScopeResolver->processNodes([$stmt], $filePath, $mutatingScope);
}
}

return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
use PhpParser\Node\Stmt\Namespace_;
use PhpParser\NodeTraverser;
use Rector\Core\PhpParser\Node\CustomNode\FileWithoutNamespace;
use Rector\NodeTypeResolver\Node\AttributeKey;

final class FileWithoutNamespaceNodeTraverser extends NodeTraverser
{
Expand All @@ -26,10 +25,6 @@ public function traverse(array $nodes): array
}

$fileWithoutNamespace = new FileWithoutNamespace($nodes);
foreach ($nodes as $node) {
$node->setAttribute(AttributeKey::PARENT_NODE, $fileWithoutNamespace);
}

return [$fileWithoutNamespace];
}
}

0 comments on commit 93a4b2b

Please sign in to comment.