Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DeadCode] Skip RemoveAlwaysTrueIfConditionRector on property use by @var docblock #2717

Merged
merged 13 commits into from Jul 27, 2022
@@ -0,0 +1,25 @@
<?php

namespace Rector\Tests\DeadCode\Rector\If_\RemoveAlwaysTrueIfConditionRector\Fixture;

class SkipPropertyByVarDoc
{
/** @var \DateTime */
private $property;

public function __construct()
{
if (rand(0, 1)) {
$this->property = new \DateTime('now');
}
}

public function verify()
{
if ($this->property instanceof \DateTime) {
return true;
}

return false;
}
}
@@ -0,0 +1,18 @@
<?php

namespace Rector\Tests\DeadCode\Rector\If_\RemoveAlwaysTrueIfConditionRector\Fixture;

use Rector\Tests\DeadCode\Rector\If_\RemoveAlwaysTrueIfConditionRector\Source\SomeClassWithPropertyUseVarDocblock;

class SkipPropertyByVarDoc2
{
public function verify()
{
$obj = new SomeClassWithPropertyUseVarDocblock();
if ($obj->property instanceof \DateTime) {
return true;
}

return false;
}
}
@@ -0,0 +1,9 @@
<?php

namespace Rector\Tests\DeadCode\Rector\If_\RemoveAlwaysTrueIfConditionRector\Source;

class SomeClassWithPropertyUseVarDocblock
{
/** @var \DateTime */
public $property;
}
43 changes: 43 additions & 0 deletions rules/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector.php
Expand Up @@ -5,10 +5,15 @@
namespace Rector\DeadCode\Rector\If_;

use PhpParser\Node;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Expr\StaticPropertyFetch;
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\If_;
use PHPStan\Reflection\ClassReflection;
use PHPStan\Type\Constant\ConstantBooleanType;
use Rector\Core\Rector\AbstractRector;
use Rector\Core\Reflection\ReflectionResolver;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

Expand All @@ -17,6 +22,10 @@
*/
final class RemoveAlwaysTrueIfConditionRector extends AbstractRector
{
public function __construct(private readonly ReflectionResolver $reflectionResolver)
{
}

public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition('Remove if condition that is always true', [
Expand Down Expand Up @@ -82,11 +91,45 @@ public function refactor(Node $node): If_|null|array
return null;
}

if ($this->shouldSkipPropertyFetch($node->cond)) {
return null;
}

if ($node->stmts === []) {
$this->removeNode($node);
return null;
}

return $node->stmts;
}

private function shouldSkipPropertyFetch(Expr $expr): bool
{
/** @var PropertyFetch[]|StaticPropertyFetch[] $propertyFetches */
$propertyFetches = $this->betterNodeFinder->findInstancesOf(
$expr,
[PropertyFetch::class, StaticPropertyFetch::class]
);

foreach ($propertyFetches as $propertyFetch) {
$classReflection = $this->reflectionResolver->resolveClassReflectionSourceObject($propertyFetch);

if (! $classReflection instanceof ClassReflection) {
continue;
}

$propertyName = (string) $this->nodeNameResolver->getName($propertyFetch);

if (! $classReflection->hasNativeProperty($propertyName)) {
continue;
}

$nativeProperty = $classReflection->getNativeProperty($propertyName);
if (! $nativeProperty->hasNativeType()) {
return true;
}
}

return false;
}
}
24 changes: 21 additions & 3 deletions src/Reflection/ReflectionResolver.php
Expand Up @@ -84,9 +84,27 @@ public function resolveClassReflection(?Node $node): ?ClassReflection
return $scope->getClassReflection();
}

public function resolveClassReflectionSourceObject(MethodCall|StaticCall $call): ?ClassReflection
{
$classMethod = $this->astResolver->resolveClassMethodFromCall($call);
public function resolveClassReflectionSourceObject(
MethodCall|StaticCall|PropertyFetch|StaticPropertyFetch $node
): ?ClassReflection {
if ($node instanceof PropertyFetch || $node instanceof StaticPropertyFetch) {
$objectType = $node instanceof PropertyFetch
? $this->nodeTypeResolver->getType($node->var)
: $this->nodeTypeResolver->getType($node->class);

if (! $objectType instanceof TypeWithClassName) {
return null;
}

$className = $objectType->getClassName();
if (! $this->reflectionProvider->hasClass($className)) {
return null;
}

return $this->reflectionProvider->getClass($className);
}

$classMethod = $this->astResolver->resolveClassMethodFromCall($node);
return $this->resolveClassReflection($classMethod);
}

Expand Down