Skip to content

Commit

Permalink
[CodeQuality] Add new rule - ExplicitReturnNullRector (#5753)
Browse files Browse the repository at this point in the history
* [CodeQuality] Add new rule - ExplicitReturnNullRector

* bump docs

* register in set
  • Loading branch information
TomasVotruba committed Mar 21, 2024
1 parent f32dff3 commit dc69b1a
Show file tree
Hide file tree
Showing 11 changed files with 303 additions and 7 deletions.
53 changes: 49 additions & 4 deletions build/target-repository/docs/rector_rules_overview.md
@@ -1,16 +1,16 @@
# 366 Rules Overview
# 368 Rules Overview

<br>

## Categories

- [Arguments](#arguments) (4)

- [CodeQuality](#codequality) (74)
- [CodeQuality](#codequality) (75)

- [CodingStyle](#codingstyle) (28)

- [DeadCode](#deadcode) (42)
- [DeadCode](#deadcode) (43)

- [EarlyReturn](#earlyreturn) (9)

Expand Down Expand Up @@ -549,6 +549,28 @@ Make if conditions more explicit

<br>

### ExplicitReturnNullRector

Add explicit return null to method/function that returns a value, but missed main return

- class: [`Rector\CodeQuality\Rector\ClassMethod\ExplicitReturnNullRector`](../rules/CodeQuality/Rector/ClassMethod/ExplicitReturnNullRector.php)

```diff
class SomeClass
{
public function run(int $number)
{
if ($number > 50) {
return 'yes';
}
+
+ return null;
}
}
```

<br>

### FlipTypeControlToUseExclusiveTypeRector

Flip type control from null compare to use exclusive instanceof object
Expand Down Expand Up @@ -2157,6 +2179,29 @@ Removes recasting of the same type

<br>

### ReduceAlwaysFalseIfOrRector

Reduce always false in a if ( || ) condition

- class: [`Rector\DeadCode\Rector\If_\ReduceAlwaysFalseIfOrRector`](../rules/DeadCode/Rector/If_/ReduceAlwaysFalseIfOrRector.php)

```diff
class SomeClass
{
public function run(int $number)
{
- if (! is_int($number) || $number > 50) {
+ if ($number > 50) {
return 'yes';
}

return 'no';
}
}
```

<br>

### RemoveAlwaysTrueIfConditionRector

Remove if condition that is always true
Expand Down Expand Up @@ -6681,7 +6726,7 @@ Change return type based on strict returns type operations

### ChildDoctrineRepositoryClassTypeRector

Add return type to classes that extend `Doctrine\ORM\EntityRepository`
Add return type to classes that extend `Doctrine\ORM\EntityRepository` based on return Doctrine method names

- class: [`Rector\TypeDeclaration\Rector\Class_\ChildDoctrineRepositoryClassTypeRector`](../rules/TypeDeclaration/Rector/Class_/ChildDoctrineRepositoryClassTypeRector.php)

Expand Down
2 changes: 2 additions & 0 deletions config/set/code-quality.php
Expand Up @@ -13,6 +13,7 @@
use Rector\CodeQuality\Rector\Class_\InlineConstructorDefaultToPropertyRector;
use Rector\CodeQuality\Rector\Class_\StaticToSelfStaticMethodCallOnFinalClassRector;
use Rector\CodeQuality\Rector\ClassConstFetch\ConvertStaticPrivateConstantToSelfRector;
use Rector\CodeQuality\Rector\ClassMethod\ExplicitReturnNullRector;
use Rector\CodeQuality\Rector\ClassMethod\InlineArrayReturnAssignRector;
use Rector\CodeQuality\Rector\ClassMethod\LocallyCalledStaticMethodToNonStaticRector;
use Rector\CodeQuality\Rector\ClassMethod\OptionalParametersAfterRequiredRector;
Expand Down Expand Up @@ -147,6 +148,7 @@
ThrowWithPreviousExceptionRector::class,
RemoveSoleValueSprintfRector::class,
ShortenElseIfRector::class,
ExplicitReturnNullRector::class,
ArrayMergeOfNonArraysToSimpleArrayRector::class,
ArrayKeyExistsTernaryThenValueToCoalescingRector::class,
AbsolutizeRequireAndIncludePathRector::class,
Expand Down
@@ -0,0 +1,28 @@
<?php

declare(strict_types=1);

namespace Rector\Tests\CodeQuality\Rector\ClassMethod\ExplicitReturnNullRector;

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

final class ExplicitReturnNullRectorTest 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';
}
}
@@ -0,0 +1,15 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\ClassMethod\ExplicitReturnNullRector\Fixture;

final class SkipAlreadyReturn
{
public function run(int $number)
{
if ($number > 50) {
return 'yes';
}

return 100;
}
}
@@ -0,0 +1,13 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\ClassMethod\ExplicitReturnNullRector\Fixture;

final class SkipNestedReturn
{
public function run(int $number)
{
$result = function () {
return 100;
};
}
}
@@ -0,0 +1,17 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\ClassMethod\ExplicitReturnNullRector\Fixture;

final class SkipReturnNoValue
{
public function run(int $number)
{
if ($number > 50) {
return;
}

if ($number < 50) {
return 100;
}
}
}
@@ -0,0 +1,32 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\ClassMethod\ExplicitReturnNullRector\Fixture;

class SomeClass
{
public function run(int $number)
{
if ($number > 50) {
return 'yes';
}
}
}

?>
-----
<?php

namespace Rector\Tests\CodeQuality\Rector\ClassMethod\ExplicitReturnNullRector\Fixture;

class SomeClass
{
public function run(int $number)
{
if ($number > 50) {
return 'yes';
}
return null;
}
}

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

declare(strict_types=1);

use Rector\CodeQuality\Rector\ClassMethod\ExplicitReturnNullRector;
use Rector\Config\RectorConfig;

return static function (RectorConfig $rectorConfig): void {
$rectorConfig->rule(ExplicitReturnNullRector::class);
};
135 changes: 135 additions & 0 deletions rules/CodeQuality/Rector/ClassMethod/ExplicitReturnNullRector.php
@@ -0,0 +1,135 @@
<?php

declare(strict_types=1);

namespace Rector\CodeQuality\Rector\ClassMethod;

use PhpParser\Node;
use PhpParser\Node\Expr\ConstFetch;
use PhpParser\Node\Expr\Yield_;
use PhpParser\Node\Expr\YieldFrom;
use PhpParser\Node\Name;
use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node\Stmt\Return_;
use PhpParser\Node\Stmt\Throw_;
use Rector\PhpParser\Node\BetterNodeFinder;
use Rector\Rector\AbstractRector;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

/**
* @see \Rector\Tests\CodeQuality\Rector\ClassMethod\ExplicitReturnNullRector\ExplicitReturnNullRectorTest
*/
final class ExplicitReturnNullRector extends AbstractRector
{
public function __construct(
private readonly BetterNodeFinder $betterNodeFinder,
) {
}

public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition(
'Add explicit return null to method/function that returns a value, but missed main return',
[
new CodeSample(
<<<'CODE_SAMPLE'
class SomeClass
{
public function run(int $number)
{
if ($number > 50) {
return 'yes';
}
}
}
CODE_SAMPLE

,
<<<'CODE_SAMPLE'
class SomeClass
{
public function run(int $number)
{
if ($number > 50) {
return 'yes';
}
return null;
}
}
CODE_SAMPLE
),

]
);
}

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

/**
* @param ClassMethod $node
*/
public function refactor(Node $node): ?Node
{
// known return type, nothing to improve
if ($node->returnType instanceof Node) {
return null;
}

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

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

// it has at least some return value in it
if (! $this->hasReturnsWithValues($node)) {
return null;
}

$node->stmts[] = new Return_(new ConstFetch(new Name('null')));

return $node;
}

private function hasRootLevelReturn(ClassMethod $classMethod): bool
{
foreach ((array) $classMethod->stmts as $stmt) {
if ($stmt instanceof Return_) {
return true;
}
}

return false;
}

private function containsYieldOrThrow(ClassMethod $classMethod): bool
{
return (bool) $this->betterNodeFinder->findInstancesOf(
$classMethod,
[Yield_::class, Throw_::class, Node\Expr\Throw_::class, YieldFrom::class]
);
}

private function hasReturnsWithValues(ClassMethod $classMethod): bool
{
/** @var Return_[] $returns */
$returns = $this->betterNodeFinder->findInstancesOfInFunctionLikeScoped($classMethod, Return_::class);
foreach ($returns as $return) {
if (! $return->expr instanceof Node) {
return false;
}
}

return $returns !== [];
}
}
3 changes: 1 addition & 2 deletions rules/DeadCode/Rector/If_/ReduceAlwaysFalseIfOrRector.php
Expand Up @@ -23,8 +23,7 @@ final class ReduceAlwaysFalseIfOrRector extends AbstractRector
public function __construct(
private readonly BetterNodeFinder $betterNodeFinder,
private readonly ExprAnalyzer $exprAnalyzer
)
{
) {
}

public function getRuleDefinition(): RuleDefinition
Expand Down
Expand Up @@ -182,7 +182,7 @@ private function refactorIfWithBooleanAnd(If_ $if): ?If_
return null;
}

if (!$leftType->getValue()) {
if (! $leftType->getValue()) {
return null;
}

Expand Down

0 comments on commit dc69b1a

Please sign in to comment.