Skip to content

Commit

Permalink
Remove removeNode() from couple rules (#4083)
Browse files Browse the repository at this point in the history
  • Loading branch information
TomasVotruba committed Jun 5, 2023
1 parent 1fa8672 commit a61fbf2
Show file tree
Hide file tree
Showing 9 changed files with 64 additions and 159 deletions.
11 changes: 2 additions & 9 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -684,12 +684,6 @@ parameters:
message: '#Method "renameVariableInFunctionLike\(\)" returns bool type, so the name should start with is/has/was#'
path: rules/Naming/VariableRenamer.php

# next to resolve
- '#Cognitive complexity for "Rector\\CodeQuality\\Rector\\Foreach_\\SimplifyForeachToCoalescingRector\:\:refactor\(\)" is 16, keep it under 11#'
- '#Cognitive complexity for "Rector\\CodeQuality\\Rector\\Foreach_\\ForeachToInArrayRector\:\:refactor\(\)" is \d+, keep it under 11#'
- '#Cognitive complexity for "Rector\\CodeQuality\\Rector\\If_\\SimplifyIfNullableReturnRector\:\:refactor\(\)" is 15, keep it under 11#'
- '#Cognitive complexity for "Rector\\CodeQuality\\Rector\\Isset_\\IssetOnPropertyObjectToPropertyExistsRector\:\:refactor\(\)" is 14, keep it under 11#'

# resolve continually
- '#Property Rector\\Core\\Contract\\PhpParser\\Node\\StmtsAwareInterface\:\:\$stmts \(array<PhpParser\\Node\\Stmt>\|null\) does not accept array<PhpParser\\Node\\Stmt\|null>#'

Expand Down Expand Up @@ -731,9 +725,8 @@ parameters:
path: packages/BetterPhpDocParser/PhpDoc/StringNode.php

# resolve later
- '#Cognitive complexity for "Rector\\CodeQuality\\Rector\\Class_\\InlineConstructorDefaultToPropertyRector\:\:refactor\(\)" is 20, keep it under 11#'
- '#Cognitive complexity for "Rector\\DeadCode\\Rector\\For_\\RemoveDeadIfForeachForRector\:\:refactor\(\)" is 16, keep it under 11#'
- '#Cognitive complexity for "Rector\\DeadCode\\Rector\\MethodCall\\RemoveEmptyMethodCallRector\:\:refactor\(\)" is 18, keep it under 11#'
- '#Cognitive complexity for "Rector\\(.*?)\:\:refactor\(\)" is (1[0-9]|20), keep it under 11#'
- '#Cognitive complexity for "Rector\\DeadCode\\Rector\\StaticCall\\RemoveParentCallWithoutParentRector\:\:refactor\(\)" is 26, keep it under 11#'

- '#Return type (.*?) should be covariant with return type \(1\|2\|3\|4\|array<PhpParser\\Node>\|PhpParser\\Node\|null\) of method Rector\\Core\\Contract\\Rector(.*?)\:\:(.*?)#'

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php

namespace Rector\StaticCall\RemoveParentCallWithoutParentRector\Source;

class SomeParentMethod
{
public function run()
{
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ function multipleDefaults2()
case 'A3':
$format = array(841.89,1190.55);
break;
case 'A4': default: $format = array(595.28,841.89);
default: $format = array(595.28,841.89);
break;
case 'A5':
$format = array(419.53,595.28);
Expand All @@ -33,9 +33,6 @@ function multipleDefaults2()
case 'A3':
$format = array(841.89,1190.55);
break;
case 'A4':
$format = array(595.28,841.89);
break;
case 'A5':
$format = array(419.53,595.28);
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\Else_;
use PhpParser\Node\Stmt\If_;
use PhpParser\NodeTraverser;
use Rector\Core\Rector\AbstractRector;
use Rector\DeadCode\ConditionEvaluator;
use Rector\DeadCode\ConditionResolver;
Expand Down Expand Up @@ -62,9 +63,9 @@ public function getNodeTypes(): array

/**
* @param If_ $node
* @return Stmt[]|null
* @return Stmt[]|null|int
*/
public function refactor(Node $node): ?array
public function refactor(Node $node): array|int|null
{
if ($node->elseifs !== []) {
return null;
Expand Down Expand Up @@ -105,14 +106,13 @@ private function refactorIsMatch(If_ $if): ?array
}

/**
* @return Stmt[]|null
* @return Stmt[]|int
*/
private function refactorIsNotMatch(If_ $if): ?array
private function refactorIsNotMatch(If_ $if): array|int
{
// no else → just remove the node
if (! $if->else instanceof Else_) {
$this->removeNode($if);
return null;
return NodeTraverser::REMOVE_NODE;
}

// else is always used
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
use PhpParser\Node\Name\FullyQualified;
use PhpParser\Node\Stmt\Class_;
use PhpParser\Node\Stmt\Expression;
use PhpParser\NodeTraverser;
use PHPStan\Reflection\ReflectionProvider;
use Rector\Core\Enum\ObjectReference;
use Rector\Core\NodeAnalyzer\ClassAnalyzer;
Expand Down Expand Up @@ -79,47 +78,49 @@ public function refactor(Node $node): ?Node
return null;
}

$class = $node;
$hasChanged = false;

foreach ($node->getMethods() as $classMethod) {
$this->traverseNodesWithCallable($classMethod, function (Node $node) use ($class): int|Assign|null {
// skip nested anonmyous class
if ($node instanceof Class_) {
return NodeTraverser::STOP_TRAVERSAL;
}
if ($classMethod->stmts === null) {
continue;
}

if ($node instanceof Assign) {
return $this->refactorAssign($node, $class);
foreach ($classMethod->stmts as $key => $stmt) {
if (! $stmt instanceof Expression) {
continue;
}

if ($node instanceof Expression) {
$this->refactorExpression($node, $class);
return null;
if ($stmt->expr instanceof StaticCall && $this->isParentStaticCall($stmt->expr)) {
if ($this->doesCalledMethodExistInParent($stmt->expr, $node)) {
continue;
}

unset($classMethod->stmts[$key]);
$hasChanged = true;
}

return null;
});
}
if ($stmt->expr instanceof Assign) {
$assign = $stmt->expr;
if ($assign->expr instanceof StaticCall && $this->isParentStaticCall($assign->expr)) {
$staticCall = $assign->expr;

return null;
}
// is valid call
if ($this->doesCalledMethodExistInParent($staticCall, $node)) {
continue;
}

public function refactorAssign(Assign $assign, Class_ $class): ?Assign
{
if (! $this->isParentStaticCall($assign->expr)) {
return null;
$assign->expr = $this->nodeFactory->createNull();
$hasChanged = true;
}
}
}
}

/** @var StaticCall $staticCall */
$staticCall = $assign->expr;

// is valid call
if ($this->doesCalledMethodExistInParent($staticCall, $class)) {
return null;
if ($hasChanged) {
return $node;
}

$assign->expr = $this->nodeFactory->createNull();
return $assign;
return null;
}

private function isParentStaticCall(Expr $expr): bool
Expand All @@ -128,15 +129,12 @@ private function isParentStaticCall(Expr $expr): bool
return false;
}

if (! $expr->class instanceof Name) {
return false;
}

return $this->isName($expr->class, ObjectReference::PARENT);
}

private function shouldSkipClass(Class_ $class): bool
{
// skip cases when parent class reflection is not found
if ($class->extends instanceof FullyQualified && ! $this->reflectionProvider->hasClass(
$class->extends->toString()
)) {
Expand All @@ -160,22 +158,4 @@ private function doesCalledMethodExistInParent(StaticCall $staticCall, Class_ $c

return $this->classMethodManipulator->hasParentMethodOrInterfaceMethod($class, $calledMethodName);
}

private function refactorExpression(Expression $expression, Class_ $class): void
{
if (! $expression->expr instanceof StaticCall) {
return;
}

if (! $this->isParentStaticCall($expression->expr)) {
return;
}

// is valid call
if ($this->doesCalledMethodExistInParent($expression->expr, $class)) {
return;
}

$this->removeNode($expression);
}
}
46 changes: 12 additions & 34 deletions rules/Php70/Rector/Switch_/ReduceMultipleDefaultSwitchRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

use PhpParser\Node;
use PhpParser\Node\Expr;
use PhpParser\Node\Stmt\Case_;
use PhpParser\Node\Stmt\Switch_;
use Rector\Core\Rector\AbstractRector;
use Rector\Core\ValueObject\PhpVersionFeature;
Expand Down Expand Up @@ -79,44 +78,23 @@ public function refactor(Node $node): ?Node
$defaultCases[$key] = $case;
}

if (count($defaultCases) < 2) {
$defaultCaseCount = count($defaultCases);
if ($defaultCaseCount < 2) {
return null;
}

$this->removeExtraDefaultCases($node->cases, $defaultCases);

return $node;
}

/**
* @param Case_[] $cases
* @param Case_[] $defaultCases
*/
private function removeExtraDefaultCases(array $cases, array $defaultCases): void
{
// keep only last
array_pop($defaultCases);

foreach ($defaultCases as $key => $defaultCase) {
$this->keepStatementsToParentCase($cases, $defaultCase, $key);
$this->removeNode($defaultCase);
}
}
foreach ($node->cases as $key => $case) {
if ($case->cond instanceof Expr) {
continue;
}

/**
* @param Case_[] $cases
*/
private function keepStatementsToParentCase(array $cases, Case_ $case, int $key): void
{
if (! isset($cases[$key - 1])) {
return;
// remove previous default cases
if ($defaultCaseCount > 1) {
unset($node->cases[$key]);
--$defaultCaseCount;
}
}

$previousCase = $cases[$key - 1];

if ($previousCase->stmts === []) {
$previousCase->stmts = $case->stmts;
$case->stmts = [];
}
return $node;
}
}
2 changes: 1 addition & 1 deletion src/NodeManipulator/ClassMethodManipulator.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ final class ClassMethodManipulator
{
public function __construct(
private readonly NodeNameResolver $nodeNameResolver,
private readonly ReflectionResolver $reflectionResolver
private readonly ReflectionResolver $reflectionResolver,
) {
}

Expand Down
3 changes: 1 addition & 2 deletions src/Rector/AbstractRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ abstract class AbstractRector extends NodeVisitorAbstract implements PhpRectorIn
B) Remove the Node:
\$this->removeNode(\$node);
return null;
return NodeTraverser::REMOVE_NODE;
CODE_SAMPLE;

protected NodeNameResolver $nodeNameResolver;
Expand Down

0 comments on commit a61fbf2

Please sign in to comment.