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

AddReturnTypeDeclarationBasedOnParentClassMethodRector #2666

Merged
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
21 changes: 20 additions & 1 deletion packages/VendorLocker/ParentClassMethodTypeOverrideGuard.php
Expand Up @@ -15,6 +15,8 @@
use Rector\Core\Reflection\ReflectionResolver;
use Rector\Core\ValueObject\MethodName;
use Rector\NodeNameResolver\NodeNameResolver;
use Rector\NodeTypeResolver\TypeComparator\TypeComparator;
use Rector\StaticTypeMapper\StaticTypeMapper;
use Rector\TypeDeclaration\TypeInferer\ParamTypeInferer;
use Symplify\SmartFileSystem\Normalizer\PathNormalizer;

Expand All @@ -25,7 +27,9 @@ public function __construct(
private readonly PathNormalizer $pathNormalizer,
private readonly AstResolver $astResolver,
private readonly ParamTypeInferer $paramTypeInferer,
private readonly ReflectionResolver $reflectionResolver
private readonly ReflectionResolver $reflectionResolver,
private readonly TypeComparator $typeComparator,
private readonly StaticTypeMapper $staticTypeMapper
) {
}

Expand Down Expand Up @@ -138,4 +142,19 @@ public function getParentClassMethod(ClassMethod $classMethod): ?MethodReflectio

return null;
}

public function shouldSkipReturnTypeChange(ClassMethod $classMethod, Type $parentType): bool
{
if ($classMethod->returnType === null) {
return false;
}

$currentReturnType = $this->staticTypeMapper->mapPhpParserNodePHPStanType($classMethod->returnType);

if ($this->typeComparator->isSubtype($currentReturnType, $parentType)) {
return true;
}

return $this->typeComparator->areTypesEqual($currentReturnType, $parentType);
}
}
@@ -0,0 +1,33 @@
<?php

declare(strict_types=1);

namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationBasedOnParentClassMethodRector;

use Iterator;
use Rector\Testing\PHPUnit\AbstractRectorTestCase;
use Symplify\SmartFileSystem\SmartFileInfo;

final class AddReturnTypeDeclarationBasedOnParentClassMethodRectorTest extends AbstractRectorTestCase
{
/**
* @dataProvider provideData()
*/
public function test(SmartFileInfo $fileInfo): void
{
$this->doTestFileInfo($fileInfo);
}

/**
* @return Iterator<SmartFileInfo>
*/
public function provideData(): Iterator
{
return $this->yieldFilesFromDirectory(__DIR__ . '/Fixture');
}

public function provideConfigFilePath(): string
{
return __DIR__ . '/config/configured_rule.php';
}
}
@@ -0,0 +1,29 @@
<?php

namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationBasedOnParentClassMethodRector\Fixture;

use Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationBasedOnParentClassMethodRector\Source\SomeClassWithReturnType;

class MyClass extends SomeClassWithReturnType
{
public function run()
{
}
}

?>
-----
<?php

namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationBasedOnParentClassMethodRector\Fixture;

use Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationBasedOnParentClassMethodRector\Source\SomeClassWithReturnType;

class MyClass extends SomeClassWithReturnType
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. The namespace is missing here, should be completed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So add to all fixtures

namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationBasedOnParentClassMethodRector\Fixture;

and remove that line from composer.json?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While the namespace should be added in Fixture files, the issue is about resolving Source directory, like other registration in composer.json.

But, that's worth a try

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK namespaces added and line drom composer.json removed. So we will see.

I also rebased on current master. But there are still some PHPStan errors not related to this PR - it seems they are caused by new rules added to symplify/phpstan-rules.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test seems failure https://github.com/rectorphp/rector-src/runs/7468797088?check_suite_focus=true#step:5:84

please add new commit instead of squash commit so we can see the different between before and after registered in composer.json

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@samsonasik I noticed it too and it is fixed now. But i squashed commits before I read your message. But it seems that Ci pases now even without change in composer.json.

{
public function run(): int
{
}
}

?>
@@ -0,0 +1,29 @@
<?php

namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationBasedOnParentClassMethodRector\Fixture;

use Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationBasedOnParentClassMethodRector\Source\SomeClassWithReturnMixed;

class MyClass extends SomeClassWithReturnMixed
{
public function run()
{
}
}

?>
-----
<?php

namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationBasedOnParentClassMethodRector\Fixture;

use Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationBasedOnParentClassMethodRector\Source\SomeClassWithReturnMixed;

class MyClass extends SomeClassWithReturnMixed
{
public function run(): mixed
{
}
}

?>
@@ -0,0 +1,29 @@
<?php

namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationBasedOnParentClassMethodRector\Fixture;

use Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationBasedOnParentClassMethodRector\Source\SomeClassWithoutReturnType;

class MyClass extends SomeClassWithoutReturnType
{
public function run()
{
}
}

?>
-----
<?php

namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationBasedOnParentClassMethodRector\Fixture;

use Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationBasedOnParentClassMethodRector\Source\SomeClassWithoutReturnType;

class MyClass extends SomeClassWithoutReturnType
{
public function run()
{
}
}

?>
@@ -0,0 +1,29 @@
<?php

namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationBasedOnParentClassMethodRector\Fixture;

use Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationBasedOnParentClassMethodRector\Source\SomeInterfaceWithReturnType;

abstract class MyClass implements SomeInterfaceWithReturnType
{
public function run()
{
}
}

?>
-----
<?php

namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationBasedOnParentClassMethodRector\Fixture;

use Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationBasedOnParentClassMethodRector\Source\SomeInterfaceWithReturnType;

abstract class MyClass implements SomeInterfaceWithReturnType
{
public function run(): string
{
}
}

?>
@@ -0,0 +1,14 @@
<?php

namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationBasedOnParentClassMethodRector\Fixture;

use Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationBasedOnParentClassMethodRector\Source\SomeClassWithReturnMixed;

class MyClass extends SomeClassWithReturnMixed
{
public function run(): string
{
}
}

?>
@@ -0,0 +1,13 @@
<?php

declare(strict_types=1);

namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationBasedOnParentClassMethodRector\Source;

class SomeClassWithReturnMixed
{
public function run(): mixed
{
return 5;
}
}
@@ -0,0 +1,13 @@
<?php

declare(strict_types=1);

namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationBasedOnParentClassMethodRector\Source;

class SomeClassWithReturnType
{
public function run(): int
{
return 5;
}
}
@@ -0,0 +1,13 @@
<?php

declare(strict_types=1);

namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationBasedOnParentClassMethodRector\Source;

class SomeClassWithoutReturnType
{
public function run()
{
return 5;
}
}
@@ -0,0 +1,10 @@
<?php

declare(strict_types=1);

namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationBasedOnParentClassMethodRector\Source;

interface SomeInterfaceWithReturnType
{
public function run(): string;
}
@@ -0,0 +1,13 @@
<?php

declare(strict_types=1);

use Rector\Config\RectorConfig;
use Rector\Core\ValueObject\PhpVersionFeature;
use Rector\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationBasedOnParentClassMethodRector;

return static function (RectorConfig $rectorConfig): void {
$rectorConfig->rule(AddReturnTypeDeclarationBasedOnParentClassMethodRector::class);

$rectorConfig->phpVersion(PhpVersionFeature::MIXED_TYPE);
};