Skip to content

Commit

Permalink
Decouple AddFunctionVoidReturnTypeWhereNoReturnRector to allow leveli…
Browse files Browse the repository at this point in the history
…ng by simple node first (#5563)
  • Loading branch information
TomasVotruba authored Feb 5, 2024
1 parent 56c2507 commit 09398c3
Show file tree
Hide file tree
Showing 7 changed files with 129 additions and 18 deletions.
2 changes: 2 additions & 0 deletions config/set/type-declaration.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
use Rector\TypeDeclaration\Rector\ClassMethod\StrictStringParamConcatRector;
use Rector\TypeDeclaration\Rector\Closure\AddClosureVoidReturnTypeWhereNoReturnRector;
use Rector\TypeDeclaration\Rector\Empty_\EmptyOnNullableObjectToInstanceOfRector;
use Rector\TypeDeclaration\Rector\Function_\AddFunctionVoidReturnTypeWhereNoReturnRector;
use Rector\TypeDeclaration\Rector\FunctionLike\AddParamTypeSplFixedArrayRector;
use Rector\TypeDeclaration\Rector\FunctionLike\AddReturnTypeDeclarationFromYieldsRector;
use Rector\TypeDeclaration\Rector\Property\TypedPropertyFromAssignsRector;
Expand All @@ -48,6 +49,7 @@
ReturnTypeFromStrictTypedPropertyRector::class,
TypedPropertyFromStrictConstructorRector::class,
AddClosureVoidReturnTypeWhereNoReturnRector::class,
AddFunctionVoidReturnTypeWhereNoReturnRector::class,
AddVoidReturnTypeWhereNoReturnRector::class,
ReturnTypeFromStrictFluentReturnRector::class,
ReturnTypeFromReturnNewRector::class,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php

declare(strict_types=1);

namespace Rector\Tests\TypeDeclaration\Rector\Function_\AddFunctionVoidReturnTypeWhereNoReturnRector;

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

final class AddFunctionVoidReturnTypeWhereNoReturnRectorTest 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\Function_\AddFunctionVoidReturnTypeWhereNoReturnRector;

return static function (RectorConfig $rectorConfig): void {
$rectorConfig->rule(AddFunctionVoidReturnTypeWhereNoReturnRector::class);
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,8 @@
namespace Rector\TypeDeclaration\Rector\ClassMethod;

use PhpParser\Node;
use PhpParser\Node\Expr\Closure;
use PhpParser\Node\Identifier;
use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node\Stmt\Function_;
use PhpParser\Node\Stmt\Throw_;
use Rector\NodeAnalyzer\MagicClassMethodAnalyzer;
use Rector\Rector\AbstractRector;
Expand Down Expand Up @@ -68,11 +66,11 @@ public function getValues(): void
*/
public function getNodeTypes(): array
{
return [ClassMethod::class, Function_::class, Closure::class];
return [ClassMethod::class];
}

/**
* @param ClassMethod|Function_|Closure $node
* @param ClassMethod $node
*/
public function refactor(Node $node): ?Node
{
Expand All @@ -89,7 +87,7 @@ public function refactor(Node $node): ?Node
return null;
}

if ($node instanceof ClassMethod && $this->classMethodReturnVendorLockResolver->isVendorLocked($node)) {
if ($this->classMethodReturnVendorLockResolver->isVendorLocked($node)) {
return null;
}

Expand All @@ -102,35 +100,31 @@ public function provideMinPhpVersion(): int
return PhpVersionFeature::VOID_TYPE;
}

private function shouldSkipClassMethod(ClassMethod|Function_|Closure $functionLike): bool
private function shouldSkipClassMethod(ClassMethod $classMethod): bool
{
if (! $functionLike instanceof ClassMethod) {
return false;
}

if ($this->magicClassMethodAnalyzer->isUnsafeOverridden($functionLike)) {
if ($this->magicClassMethodAnalyzer->isUnsafeOverridden($classMethod)) {
return true;
}

if ($functionLike->isAbstract()) {
if ($classMethod->isAbstract()) {
return true;
}

// is not final and has only exception? possibly implemented by child
if ($this->isNotFinalAndHasExceptionOnly($functionLike)) {
if ($this->isNotFinalAndHasExceptionOnly($classMethod)) {
return true;
}

// possibly required by child implementation
if ($this->isNotFinalAndEmpty($functionLike)) {
if ($this->isNotFinalAndEmpty($classMethod)) {
return true;
}

if ($functionLike->isProtected()) {
return ! $this->classModifierChecker->isInsideFinalClass($functionLike);
if ($classMethod->isProtected()) {
return ! $this->classModifierChecker->isInsideFinalClass($classMethod);
}

return $this->classModifierChecker->isInsideAbstractClass($functionLike) && $functionLike->getStmts() === [];
return $this->classModifierChecker->isInsideAbstractClass($classMethod) && $classMethod->getStmts() === [];
}

private function isNotFinalAndHasExceptionOnly(ClassMethod $classMethod): bool
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
<?php

declare(strict_types=1);

namespace Rector\TypeDeclaration\Rector\Function_;

use PhpParser\Node;
use PhpParser\Node\Identifier;
use PhpParser\Node\Stmt\Function_;
use Rector\Rector\AbstractRector;
use Rector\TypeDeclaration\TypeInferer\SilentVoidResolver;
use Rector\ValueObject\PhpVersionFeature;
use Rector\VersionBonding\Contract\MinPhpVersionInterface;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

/**
* @see \Rector\Tests\TypeDeclaration\Rector\Function_\AddFunctionVoidReturnTypeWhereNoReturnRector\AddFunctionVoidReturnTypeWhereNoReturnRectorTest
*/
final class AddFunctionVoidReturnTypeWhereNoReturnRector extends AbstractRector implements MinPhpVersionInterface
{
public function __construct(
private readonly SilentVoidResolver $silentVoidResolver,
) {
}

public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition('Add function return type void if there is no return', [
new CodeSample(
<<<'CODE_SAMPLE'
function restore() {
}
CODE_SAMPLE

,
<<<'CODE_SAMPLE'
function restore(): void {
}
CODE_SAMPLE
),
]);
}

/**
* @return array<class-string<Node>>
*/
public function getNodeTypes(): array
{
return [Function_::class];
}

/**
* @param Function_ $node
*/
public function refactor(Node $node): ?Node
{
// already has return type → skip
if ($node->returnType instanceof Node) {
return null;
}

if (! $this->silentVoidResolver->hasExclusiveVoid($node)) {
return null;
}

$node->returnType = new Identifier('void');
return $node;
}

public function provideMinPhpVersion(): int
{
return PhpVersionFeature::VOID_TYPE;
}
}
4 changes: 3 additions & 1 deletion src/Configuration/Levels/TypeCoverageLevel.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
use Rector\TypeDeclaration\Rector\ClassMethod\StrictStringParamConcatRector;
use Rector\TypeDeclaration\Rector\Closure\AddClosureVoidReturnTypeWhereNoReturnRector;
use Rector\TypeDeclaration\Rector\Empty_\EmptyOnNullableObjectToInstanceOfRector;
use Rector\TypeDeclaration\Rector\Function_\AddFunctionVoidReturnTypeWhereNoReturnRector;
use Rector\TypeDeclaration\Rector\FunctionLike\AddParamTypeSplFixedArrayRector;
use Rector\TypeDeclaration\Rector\FunctionLike\AddReturnTypeDeclarationFromYieldsRector;
use Rector\TypeDeclaration\Rector\Property\TypedPropertyFromStrictConstructorRector;
Expand Down Expand Up @@ -56,7 +57,8 @@ final class TypeCoverageLevel
// php 7.0
// start with closure first, as safest
AddClosureVoidReturnTypeWhereNoReturnRector::class,
// @todo continue with functions
AddFunctionVoidReturnTypeWhereNoReturnRector::class,

AddVoidReturnTypeWhereNoReturnRector::class,

// php 7.4
Expand Down

0 comments on commit 09398c3

Please sign in to comment.