Skip to content

Commit

Permalink
Rewrite of expression to previous statement
Browse files Browse the repository at this point in the history
  • Loading branch information
jeroensmit committed Oct 27, 2019
1 parent 878a575 commit 10391ae
Show file tree
Hide file tree
Showing 18 changed files with 175 additions and 102 deletions.
8 changes: 4 additions & 4 deletions .phpstorm.meta.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
\Rector\NodeTypeResolver\Node\AttributeKey::PARENT_CLASS_NAME,
\Rector\NodeTypeResolver\Node\AttributeKey::NEXT_NODE,
\Rector\NodeTypeResolver\Node\AttributeKey::PREVIOUS_NODE,
\Rector\NodeTypeResolver\Node\AttributeKey::CURRENT_EXPRESSION,
\Rector\NodeTypeResolver\Node\AttributeKey::PREVIOUS_EXPRESSION,
\Rector\NodeTypeResolver\Node\AttributeKey::CURRENT_STATEMENT,
\Rector\NodeTypeResolver\Node\AttributeKey::PREVIOUS_STATEMENT,
\Rector\NodeTypeResolver\Node\AttributeKey::USE_NODES,
\Rector\NodeTypeResolver\Node\AttributeKey::START_TOKEN_POSITION,
\Rector\NodeTypeResolver\Node\AttributeKey::ORIGINAL_NODE,
Expand All @@ -55,8 +55,8 @@
\Rector\NodeTypeResolver\Node\AttributeKey::PARENT_CLASS_NAME,
\Rector\NodeTypeResolver\Node\AttributeKey::NEXT_NODE,
\Rector\NodeTypeResolver\Node\AttributeKey::PREVIOUS_NODE,
\Rector\NodeTypeResolver\Node\AttributeKey::CURRENT_EXPRESSION,
\Rector\NodeTypeResolver\Node\AttributeKey::PREVIOUS_EXPRESSION,
\Rector\NodeTypeResolver\Node\AttributeKey::CURRENT_STATEMENT,
\Rector\NodeTypeResolver\Node\AttributeKey::PREVIOUS_STATEMENT,
\Rector\NodeTypeResolver\Node\AttributeKey::USE_NODES,
\Rector\NodeTypeResolver\Node\AttributeKey::START_TOKEN_POSITION,
\Rector\NodeTypeResolver\Node\AttributeKey::ORIGINAL_NODE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ private function shouldSkip(Return_ $returnNode): bool
*/
private function isPreviousExpressionVisuallySimilar(Expression $previousExpression, Node $previousNode): bool
{
$prePreviousExpression = $previousExpression->getAttribute(AttributeKey::PREVIOUS_EXPRESSION);
$prePreviousExpression = $previousExpression->getAttribute(AttributeKey::PREVIOUS_STATEMENT);
if ($prePreviousExpression instanceof Expression && $prePreviousExpression->expr instanceof AssignOp) {
if ($this->areNodesEqual($prePreviousExpression->expr->var, $previousNode->var)) {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use PhpParser\Node\Stmt\Do_;
use PhpParser\Node\Stmt\Else_;
use PhpParser\Node\Stmt\ElseIf_;
use PhpParser\Node\Stmt\Expression;
use PhpParser\Node\Stmt\Foreach_;
use PhpParser\Node\Stmt\If_;
use PhpParser\Node\Stmt\While_;
Expand Down Expand Up @@ -75,8 +76,8 @@ public function refactor(Node $node): ?Node
return null;
}

$previousExpression = $node->getAttribute(AttributeKey::PREVIOUS_EXPRESSION);
if ($previousExpression === null) {
$previousExpression = $node->getAttribute(AttributeKey::PREVIOUS_STATEMENT);
if (!$previousExpression instanceof Expression) {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use PhpParser\Node\Stmt\If_;
use PhpParser\Node\Stmt\Namespace_;
use PhpParser\Node\Stmt\Nop;
use PhpParser\NodeDumper;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\Rector\AbstractRector;
use Rector\RectorDefinition\CodeSample;
Expand Down Expand Up @@ -71,14 +72,13 @@ public function refactor(Node $node): ?Node
return null;
}

if (! $this->isUnreachable($node)) {
return null;
// might be PHPStan false positive, better skip
$previousStatement = $node->getAttribute(AttributeKey::PREVIOUS_STATEMENT);
if ($previousStatement instanceof If_) {
$node->setAttribute(AttributeKey::IS_UNREACHABLE, $previousStatement->getAttribute(AttributeKey::IS_UNREACHABLE));
}

// might be PHPStan false positive, better skip
$previousNode = $node->getAttribute(AttributeKey::PREVIOUS_NODE);
if ($previousNode instanceof If_) {
$node->setAttribute(AttributeKey::IS_UNREACHABLE, false);
if (! $this->isUnreachable($node)) {
return null;
}

Expand All @@ -100,15 +100,15 @@ private function isUnreachable(Node $node): bool
}

// traverse up for unreachable node in the same scope
$previousNode = $node->getAttribute(AttributeKey::PREVIOUS_NODE);
$previousNode = $node->getAttribute(AttributeKey::PREVIOUS_STATEMENT);

while ($previousNode instanceof Node && ! $this->isBreakingScopeNode($node)) {
while ($previousNode instanceof Node && ! $this->isBreakingScopeNode($previousNode)) {
$isUnreachable = $previousNode->getAttribute(AttributeKey::IS_UNREACHABLE);
if ($isUnreachable === true) {
return true;
}

$previousNode = $previousNode->getAttribute(AttributeKey::PREVIOUS_NODE);
$previousNode = $previousNode->getAttribute(AttributeKey::PREVIOUS_STATEMENT);
}

return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ private function processMysqlFetchField(Assign $assign, FuncCall $funcCall): Ass
'stmts' => [new Expression($funcCall)],
]);

$previousExpression = $assign->getAttribute(AttributeKey::PREVIOUS_EXPRESSION);
$previousExpression = $assign->getAttribute(AttributeKey::PREVIOUS_STATEMENT);
if ($previousExpression === null) {
throw new ShouldNotHappenException();
}
Expand Down
8 changes: 6 additions & 2 deletions packages/NodeTypeResolver/src/Node/AttributeKey.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,12 @@ final class AttributeKey
/**
* @var string
*/
public const PREVIOUS_EXPRESSION = 'previousExpression';
public const PREVIOUS_STATEMENT = 'previousExpression';

/**
* @var string
*/
public const CURRENT_EXPRESSION = 'currentExpression';
public const CURRENT_STATEMENT = 'currentExpression';

/**
* @var string
Expand All @@ -113,4 +113,8 @@ final class AttributeKey
* @var string
*/
public const IS_UNREACHABLE = 'is_unreachable';

public const FUNCTION_NAME = 'function_name';

public const FUNCTION_NODE = 'function_node';
}
12 changes: 6 additions & 6 deletions packages/NodeTypeResolver/src/NodeScopeAndMetadataDecorator.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
use PhpParser\NodeVisitor\CloningVisitor;
use PhpParser\NodeVisitor\NameResolver;
use Rector\Configuration\Configuration;
use Rector\NodeTypeResolver\NodeVisitor\ClassAndMethodNodeVisitor;
use Rector\NodeTypeResolver\NodeVisitor\ExpressionNodeVisitor;
use Rector\NodeTypeResolver\NodeVisitor\FunctionMethodAndClassNodeVisitor;
use Rector\NodeTypeResolver\NodeVisitor\StatementNodeVisitor;
use Rector\NodeTypeResolver\NodeVisitor\FileInfoNodeVisitor;
use Rector\NodeTypeResolver\NodeVisitor\NamespaceNodeVisitor;
use Rector\NodeTypeResolver\NodeVisitor\NodeCollectorNodeVisitor;
Expand All @@ -35,7 +35,7 @@ final class NodeScopeAndMetadataDecorator
private $parentAndNextNodeVisitor;

/**
* @var ClassAndMethodNodeVisitor
* @var FunctionMethodAndClassNodeVisitor
*/
private $classAndMethodNodeVisitor;

Expand All @@ -45,7 +45,7 @@ final class NodeScopeAndMetadataDecorator
private $namespaceNodeVisitor;

/**
* @var ExpressionNodeVisitor
* @var StatementNodeVisitor
*/
private $expressionNodeVisitor;

Expand All @@ -68,9 +68,9 @@ public function __construct(
NodeScopeResolver $nodeScopeResolver,
ParentAndNextNodeVisitor $parentAndNextNodeVisitor,
CloningVisitor $cloningVisitor,
ClassAndMethodNodeVisitor $classAndMethodNodeVisitor,
FunctionMethodAndClassNodeVisitor $classAndMethodNodeVisitor,
NamespaceNodeVisitor $namespaceNodeVisitor,
ExpressionNodeVisitor $expressionNodeVisitor,
StatementNodeVisitor $expressionNodeVisitor,
FileInfoNodeVisitor $fileInfoNodeVisitor,
NodeCollectorNodeVisitor $nodeCollectorNodeVisitor,
Configuration $configuration
Expand Down
71 changes: 53 additions & 18 deletions packages/NodeTypeResolver/src/NodeTypeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\NodeTypeResolver\PHPStan\Type\TypeFactory;
use Rector\NodeTypeResolver\Reflection\ClassReflectionTypesResolver;
use Rector\PhpParser\Node\BetterNodeFinder;
use Rector\PhpParser\Node\Resolver\NameResolver;
use Rector\PhpParser\NodeTraverser\CallableNodeTraverser;
use Rector\PhpParser\Printer\BetterStandardPrinter;
Expand Down Expand Up @@ -101,6 +102,10 @@ final class NodeTypeResolver
* @var ObjectTypeSpecifier
*/
private $objectTypeSpecifier;
/**
* @var BetterNodeFinder
*/
private $betterNodeFinder;

/**
* @param PerNodeTypeResolverInterface[] $perNodeTypeResolvers
Expand All @@ -113,6 +118,7 @@ public function __construct(
Broker $broker,
TypeFactory $typeFactory,
ObjectTypeSpecifier $objectTypeSpecifier,
BetterNodeFinder $betterNodeFinder,
array $perNodeTypeResolvers
) {
$this->betterStandardPrinter = $betterStandardPrinter;
Expand All @@ -127,6 +133,7 @@ public function __construct(
$this->broker = $broker;
$this->typeFactory = $typeFactory;
$this->objectTypeSpecifier = $objectTypeSpecifier;
$this->betterNodeFinder = $betterNodeFinder;
}

/**
Expand Down Expand Up @@ -498,35 +505,63 @@ private function resolveStaticCallType(StaticCall $staticCall): Type
*/
private function correctPregMatchType(Node $node, Type $originalType): Type
{
/** @var Expression|null $previousExpression */
$previousExpression = $node->getAttribute(AttributeKey::CURRENT_EXPRESSION);
if ($previousExpression === null) {
if (!$node instanceof Variable) {
return $originalType;
}

if (! $previousExpression->expr instanceof FuncCall) {
if ($originalType instanceof ArrayType) {
return $originalType;
}

$funcCallNode = $previousExpression->expr;
if (! $this->nameResolver->isNames($funcCallNode, ['preg_match', 'preg_match_all'])) {
return $originalType;
}
foreach ($this->getVariableUsages($node) as $usage) {
/** @var Expression|null $previousExpression */
$possiblyArg = $usage->getAttribute(AttributeKey::PARENT_NODE);
if (!$possiblyArg instanceof Arg) {
continue;
}

if (! isset($funcCallNode->args[2])) {
return $originalType;
}
$funcCallNode = $possiblyArg->getAttribute(AttributeKey::PARENT_NODE);
if (!$funcCallNode instanceof FuncCall) {
continue;
}

// are the same variables
if (! $this->betterStandardPrinter->areNodesEqual($funcCallNode->args[2]->value, $node)) {
return $originalType;
}
if (! $this->nameResolver->isNames($funcCallNode, ['preg_match', 'preg_match_all'])) {
continue;
}

if ($originalType instanceof ArrayType) {
return $originalType;
if (! isset($funcCallNode->args[2])) {
continue;
}

// are the same variables
if (! $this->betterStandardPrinter->areNodesEqual($funcCallNode->args[2]->value, $node)) {
continue;
}
return new ArrayType(new MixedType(), new MixedType());
}
return $originalType;
}

private function getScopeNode(Node $node) : Node
{
return $node->getAttribute(AttributeKey::METHOD_NODE)
?? $node->getAttribute(AttributeKey::FUNCTION_NODE)
?? $node->getAttribute( AttributeKey::NAMESPACE_NODE);
}

return new ArrayType(new MixedType(), new MixedType());
/**
* @return Node[]
*/
private function getVariableUsages(Variable $variable) : array
{
$scope = $this->getScopeNode($variable);

return $this->betterNodeFinder->find((array) $scope->stmts, function (Node $node) use (
$variable
): bool {
return $node instanceof Variable && $node->name === $variable->name;
return $this->betterStandardPrinter->areNodesEqual($node, $variable);
});
}

private function isIntersectionArrayType(Type $nodeType): bool
Expand Down

This file was deleted.

Loading

0 comments on commit 10391ae

Please sign in to comment.