Skip to content

Commit

Permalink
[TypeDeclaration] Add WhileNullableToInstanceofRector (#3680)
Browse files Browse the repository at this point in the history
Co-authored-by: GitHub Action <actions@github.com>
  • Loading branch information
2 people authored and samsonasik committed May 8, 2023
1 parent a7e7c24 commit 35ce899
Show file tree
Hide file tree
Showing 20 changed files with 294 additions and 65 deletions.
2 changes: 2 additions & 0 deletions config/set/instanceof.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Rector\TypeDeclaration\Rector\BooleanAnd\BinaryOpNullableToInstanceofRector;
use Rector\TypeDeclaration\Rector\Empty_\EmptyOnNullableObjectToInstanceOfRector;
use Rector\TypeDeclaration\Rector\Ternary\FlipNegatedTernaryInstanceofRector;
use Rector\TypeDeclaration\Rector\While_\WhileNullableToInstanceofRector;

return static function (RectorConfig $rectorConfig): void {
$rectorConfig->rules([
Expand All @@ -22,5 +23,6 @@
RemoveDeadInstanceOfRector::class,
FlipNegatedTernaryInstanceofRector::class,
BinaryOpNullableToInstanceofRector::class,
WhileNullableToInstanceofRector::class,
]);
};
11 changes: 9 additions & 2 deletions packages/Testing/PHPUnit/AbstractRectorTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,9 @@ protected function doTestFile(string $fixtureFilePath): void

if (FixtureSplitter::containsSplit($fixtureFileContents)) {
// changed content
[$inputFileContents, $expectedFileContents] = FixtureSplitter::splitFixtureFileContents($fixtureFileContents);
[$inputFileContents, $expectedFileContents] = FixtureSplitter::splitFixtureFileContents(
$fixtureFileContents
);
} else {
// no change
$inputFileContents = $fixtureFileContents;
Expand All @@ -125,7 +127,12 @@ protected function doTestFile(string $fixtureFilePath): void
// write temp file
FileSystem::write($inputFilePath, $inputFileContents);

$this->doTestFileMatchesExpectedContent($inputFilePath, $inputFileContents, $expectedFileContents, $fixtureFilePath);
$this->doTestFileMatchesExpectedContent(
$inputFilePath,
$inputFileContents,
$expectedFileContents,
$fixtureFilePath
);
}

private function includePreloadFilesAndScoperAutoload(): void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public function getParentClassMethod(ClassMethod $classMethod): ?MethodReflectio
/** @var string $methodName */
$methodName = $this->nodeNameResolver->getName($classMethod);
$parentClassReflection = $classReflection->getParentClass();
while ($parentClassReflection instanceof ClassReflection) {
while ($parentClassReflection instanceof ClassReflection) {
if ($parentClassReflection->hasNativeMethod($methodName)) {
return $parentClassReflection->getNativeMethod($methodName);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php

namespace Rector\Tests\TypeDeclaration\Rector\While_\WhileNullableToInstanceofRector\Fixture;

use Rector\Tests\TypeDeclaration\Rector\While_\WhileNullableToInstanceofRector\Source\CheckedClass;

final class SilentCondition
{
function run(?CheckedClass $someClass)
{
while ($someClass) {
// do something
}
}
}

?>
-----
<?php

namespace Rector\Tests\TypeDeclaration\Rector\While_\WhileNullableToInstanceofRector\Fixture;

use Rector\Tests\TypeDeclaration\Rector\While_\WhileNullableToInstanceofRector\Source\CheckedClass;

final class SilentCondition
{
function run(?CheckedClass $someClass)
{
while ($someClass instanceof \Rector\Tests\TypeDeclaration\Rector\While_\WhileNullableToInstanceofRector\Source\CheckedClass) {
// do something
}
}
}

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

namespace Rector\Tests\TypeDeclaration\Rector\While_\WhileNullableToInstanceofRector\Fixture;

use Rector\Tests\TypeDeclaration\Rector\While_\WhileNullableToInstanceofRector\Source\CheckedClass;

class SomeClass
{
function run(?CheckedClass $someClass)
{
while ($someClass !== null) {
// do something
}
}
}

?>
-----
<?php

namespace Rector\Tests\TypeDeclaration\Rector\While_\WhileNullableToInstanceofRector\Fixture;

use Rector\Tests\TypeDeclaration\Rector\While_\WhileNullableToInstanceofRector\Source\CheckedClass;

class SomeClass
{
function run(?CheckedClass $someClass)
{
while ($someClass instanceof \Rector\Tests\TypeDeclaration\Rector\While_\WhileNullableToInstanceofRector\Source\CheckedClass) {
// do something
}
}
}

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

namespace Rector\Tests\TypeDeclaration\Rector\While_\WhileNullableToInstanceofRector\Source;

class CheckedClass
{

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

declare(strict_types=1);

namespace Rector\Tests\TypeDeclaration\Rector\While_\WhileNullableToInstanceofRector;

use Iterator;
use PHPUnit\Framework\Attributes\DataProvider;
use Rector\Testing\PHPUnit\AbstractRectorTestCase;

final class WhileNullableToInstanceofRectorTest extends AbstractRectorTestCase
{
#[DataProvider('provideData')]
public function test(string $filePath): void
{
$this->doTestFile($filePath);
}

public static function provideData(): Iterator
{
return self::yieldFilesFromDirectory(__DIR__ . '/Fixture');
}

public function provideConfigFilePath(): string
{
return __DIR__ . '/config/configured_rule.php';
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php

declare(strict_types=1);

use Rector\Config\RectorConfig;
use Rector\TypeDeclaration\Rector\While_\WhileNullableToInstanceofRector;

return static function (RectorConfig $rectorConfig): void {
$rectorConfig->rule(WhileNullableToInstanceofRector::class);
};
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,9 @@
use PhpParser\Node\Expr\Instanceof_;
use PhpParser\Node\Name\FullyQualified;
use PHPStan\Type\ObjectType;
use PHPStan\Type\Type;
use PHPStan\Type\TypeCombinator;
use PHPStan\Type\UnionType;
use Rector\Core\Rector\AbstractRector;
use Rector\StaticTypeMapper\ValueObject\Type\ShortenedObjectType;
use Rector\TypeDeclaration\TypeAnalyzer\NullableTypeAnalyzer;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

Expand All @@ -25,9 +23,14 @@
*/
final class FlipTypeControlToUseExclusiveTypeRector extends AbstractRector
{
public function __construct(
private readonly NullableTypeAnalyzer $nullableTypeAnalyzer,
) {
}

public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition('Flip type control from null compare to use exclusive instanceof type', [
return new RuleDefinition('Flip type control from null compare to use exclusive instanceof object', [
new CodeSample(
<<<'CODE_SAMPLE'
function process(?DateTime $dateTime)
Expand Down Expand Up @@ -68,42 +71,20 @@ public function refactor(Node $node): ?Node
return null;
}

$bareType = $this->matchBareNullableType($expr);
if (! $bareType instanceof Type) {
$nullableObjectType = $this->nullableTypeAnalyzer->resolveNullableObjectType($expr);
if (! $nullableObjectType instanceof ObjectType) {
return null;
}

return $this->processConvertToExclusiveType($bareType, $expr, $node);
}

private function matchBareNullableType(Expr $expr): ?Type
{
$exprType = $this->getType($expr);
if (! $exprType instanceof UnionType) {
return null;
}

if (! TypeCombinator::containsNull($exprType)) {
return null;
}

if (count($exprType->getTypes()) !== 2) {
return null;
}

return TypeCombinator::removeNull($exprType);
return $this->processConvertToExclusiveType($nullableObjectType, $expr, $node);
}

private function processConvertToExclusiveType(
Type $type,
ObjectType $objectType,
Expr $expr,
Identical|NotIdentical $binaryOp
): BooleanNot|Instanceof_|null {
if (! $type instanceof ObjectType) {
return null;
}

$fullyQualifiedType = $type instanceof ShortenedObjectType ? $type->getFullyQualifiedName() : $type->getClassName();
): BooleanNot|Instanceof_ {
$fullyQualifiedType = $objectType instanceof ShortenedObjectType ? $objectType->getFullyQualifiedName() : $objectType->getClassName();

$instanceof = new Instanceof_($expr, new FullyQualified($fullyQualifiedType));
if ($binaryOp instanceof NotIdentical) {
Expand Down
2 changes: 1 addition & 1 deletion rules/Php70/Rector/FuncCall/MultiDirnameRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public function refactor(Node $node): ?Node
$activeFuncCallNode = $node;
$lastFuncCallNode = $node;

while ($activeFuncCallNode = $this->matchNestedDirnameFuncCall($activeFuncCallNode)) {
while (($activeFuncCallNode = $this->matchNestedDirnameFuncCall($activeFuncCallNode)) instanceof FuncCall) {
$lastFuncCallNode = $activeFuncCallNode;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
use PhpParser\Node\Expr\Instanceof_;
use PhpParser\Node\Name\FullyQualified;
use PHPStan\Type\ObjectType;
use PHPStan\Type\TypeCombinator;
use Rector\Core\Rector\AbstractRector;
use Rector\TypeDeclaration\TypeAnalyzer\NullableTypeAnalyzer;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

Expand All @@ -22,6 +22,11 @@
*/
final class BinaryOpNullableToInstanceofRector extends AbstractRector
{
public function __construct(
private readonly NullableTypeAnalyzer $nullableTypeAnalyzer,
) {
}

public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition('Change && and || between nullable objects to instanceof compares', [
Expand Down Expand Up @@ -74,7 +79,7 @@ public function refactor(Node $node): ?Node

private function processsNullableInstance(BooleanAnd|BooleanOr $node): null|BooleanAnd|BooleanOr
{
$nullableObjectType = $this->returnNullableObjectType($node->left);
$nullableObjectType = $this->nullableTypeAnalyzer->resolveNullableObjectType($node->left);

$hasChanged = false;
if ($nullableObjectType instanceof ObjectType) {
Expand All @@ -83,7 +88,7 @@ private function processsNullableInstance(BooleanAnd|BooleanOr $node): null|Bool
$hasChanged = true;
}

$nullableObjectType = $this->returnNullableObjectType($node->right);
$nullableObjectType = $this->nullableTypeAnalyzer->resolveNullableObjectType($node->right);

if ($nullableObjectType instanceof ObjectType) {
$node->right = $this->createExprInstanceof($node->right, $nullableObjectType);
Expand All @@ -102,7 +107,7 @@ private function processNegationBooleanOr(BooleanOr $booleanOr): ?BooleanOr
{
$hasChanged = false;
if ($booleanOr->left instanceof BooleanNot) {
$nullableObjectType = $this->returnNullableObjectType($booleanOr->left->expr);
$nullableObjectType = $this->nullableTypeAnalyzer->resolveNullableObjectType($booleanOr->left->expr);

if ($nullableObjectType instanceof ObjectType) {
$booleanOr->left->expr = $this->createExprInstanceof($booleanOr->left->expr, $nullableObjectType);
Expand All @@ -111,7 +116,7 @@ private function processNegationBooleanOr(BooleanOr $booleanOr): ?BooleanOr
}

if ($booleanOr->right instanceof BooleanNot) {
$nullableObjectType = $this->returnNullableObjectType($booleanOr->right->expr);
$nullableObjectType = $this->nullableTypeAnalyzer->resolveNullableObjectType($booleanOr->right->expr);

if ($nullableObjectType instanceof ObjectType) {
$booleanOr->right->expr = $this->createExprInstanceof($booleanOr->right->expr, $nullableObjectType);
Expand All @@ -129,18 +134,6 @@ private function processNegationBooleanOr(BooleanOr $booleanOr): ?BooleanOr
return $result;
}

private function returnNullableObjectType(Expr $expr): ObjectType|null
{
$exprType = $this->getType($expr);

$baseType = TypeCombinator::removeNull($exprType);
if (! $baseType instanceof ObjectType) {
return null;
}

return $baseType;
}

private function createExprInstanceof(Expr $expr, ObjectType $objectType): Instanceof_
{
$fullyQualified = new FullyQualified($objectType->getClassName());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,6 @@ private function updateParamTagsIfRequired(
$paramTagValueNodes = $phpDocNode->getParamTagValues();
$paramTagWasUpdated = false;
foreach ($paramTagValueNodes as $paramTagValueNode) {
if ($paramTagValueNode->type === null) {
continue;
}

$param = $this->paramAnalyzer->getParamByName($paramTagValueNode->parameterName, $node);
if (! $param instanceof Param) {
continue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,6 @@ public function refactor(Node $node): ?Node
return null;
}

if ($varTagValueNode->type === null) {
return null;
}

$docType = $this->staticTypeMapper->mapPHPStanPhpDocTypeNodeToPHPStanType($varTagValueNode->type, $node);

$updatedPhpDocType = $this->phpDocNullableTypeHelper->resolveUpdatedPhpDocTypeFromPhpDocTypeAndPhpParserType(
Expand Down

0 comments on commit 35ce899

Please sign in to comment.