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

[TypeDeclaration] Add WhileNullableToInstanceofRector #3680

Merged
merged 5 commits into from
Apr 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 23 additions & 2 deletions build/target-repository/docs/rector_rules_overview.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# 419 Rules Overview
# 420 Rules Overview

<br>

Expand Down Expand Up @@ -64,7 +64,7 @@

- [Transform](#transform) (34)

- [TypeDeclaration](#typedeclaration) (39)
- [TypeDeclaration](#typedeclaration) (40)

- [Visibility](#visibility) (3)

Expand Down Expand Up @@ -10109,6 +10109,27 @@ Add or remove null type from `@var` phpdoc typehint based on php property type d

<br>

### WhileNullableToInstanceofRector

Change while null compare to strict instanceof check

- class: [`Rector\TypeDeclaration\Rector\While_\WhileNullableToInstanceofRector`](../rules/TypeDeclaration/Rector/While_/WhileNullableToInstanceofRector.php)

```diff
final class SomeClass
{
public function run(?SomeClass $someClass)
{
- while ($someClass !== null) {
+ while ($someClass instanceof SomeClass) {
// do something
}
}
}
```

<br>

## Visibility

### ChangeConstantVisibilityRector
Expand Down
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