Skip to content

Commit

Permalink
Remove null coalesce for methods with default argument (#268)
Browse files Browse the repository at this point in the history
* basic mvp

* Working in all scenarios

* All positive test cases

* Improvement

* Working fully

* fixes

* simplify the rule configuration

* improved

* Fix removal of default

* Docs and added to CodeQuality set

* add env function

* use data_get and position argument

* cs fix

* Update rector_rules_overview.md
  • Loading branch information
peterfox authored Nov 5, 2024
1 parent 65c14c5 commit f8d624d
Show file tree
Hide file tree
Showing 10 changed files with 288 additions and 1 deletion.
2 changes: 2 additions & 0 deletions config/sets/laravel-code-quality.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@

use Rector\Config\RectorConfig;
use RectorLaravel\Rector\Assign\CallOnAppArrayAccessToStandaloneAssignRector;
use RectorLaravel\Rector\Coalesce\ApplyDefaultInsteadOfNullCoalesceRector;
use RectorLaravel\Rector\MethodCall\ReverseConditionableMethodCallRector;

return static function (RectorConfig $rectorConfig): void {
$rectorConfig->import(__DIR__ . '/../config.php');
$rectorConfig->rule(CallOnAppArrayAccessToStandaloneAssignRector::class);
$rectorConfig->rule(ReverseConditionableMethodCallRector::class);
$rectorConfig->rule(ApplyDefaultInsteadOfNullCoalesceRector::class);
};
17 changes: 16 additions & 1 deletion docs/rector_rules_overview.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# 71 Rules Overview
# 72 Rules Overview

## AbortIfRector

Expand Down Expand Up @@ -221,6 +221,21 @@ Replace `$app->environment() === 'local'` with `$app->environment('local')`

<br>

## ApplyDefaultInsteadOfNullCoalesceRector

Apply default instead of null coalesce

:wrench: **configure it!**

- class: [`RectorLaravel\Rector\Coalesce\ApplyDefaultInsteadOfNullCoalesceRector`](../src/Rector/Coalesce/ApplyDefaultInsteadOfNullCoalesceRector.php)

```diff
-custom_helper('app.name') ?? 'Laravel';
+custom_helper('app.name', 'Laravel');
```

<br>

## ArgumentFuncCallToMethodCallRector

Move help facade-like function calls to constructor injection
Expand Down
136 changes: 136 additions & 0 deletions src/Rector/Coalesce/ApplyDefaultInsteadOfNullCoalesceRector.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
<?php

namespace RectorLaravel\Rector\Coalesce;

use PhpParser\Node;
use PhpParser\Node\Arg;
use PhpParser\Node\Expr\BinaryOp\Coalesce;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Expr\StaticCall;
use PHPStan\Type\ObjectType;
use Rector\Contract\Rector\ConfigurableRectorInterface;
use Rector\Rector\AbstractRector;
use RectorLaravel\ValueObject\ApplyDefaultInsteadOfNullCoalesce;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\ConfiguredCodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
use Webmozart\Assert\Assert;

/**
* @see \RectorLaravel\Tests\Rector\Coalesce\ApplyDefaultInsteadOfNullCoalesceRector\ApplyDefaultInsteadOfNullCoalesceRectorTest
*/
final class ApplyDefaultInsteadOfNullCoalesceRector extends AbstractRector implements ConfigurableRectorInterface
{
/**
* @var ApplyDefaultInsteadOfNullCoalesce[]
*/
private array $applyDefaultWith;

public function __construct()
{
$this->applyDefaultWith = self::defaultLaravelMethods();
}

/**
* @return ApplyDefaultInsteadOfNullCoalesce[]
*/
public static function defaultLaravelMethods(): array
{
return [
new ApplyDefaultInsteadOfNullCoalesce('config'),
new ApplyDefaultInsteadOfNullCoalesce('env'),
new ApplyDefaultInsteadOfNullCoalesce('data_get', argumentPosition: 2),
new ApplyDefaultInsteadOfNullCoalesce('input', new ObjectType('Illuminate\Http\Request')),
new ApplyDefaultInsteadOfNullCoalesce('get', new ObjectType('Illuminate\Support\Env')),
];
}

public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition(
'Apply default instead of null coalesce',
[
new ConfiguredCodeSample(
<<<'CODE_SAMPLE'
custom_helper('app.name') ?? 'Laravel';
CODE_SAMPLE,
<<<'CODE_SAMPLE'
custom_helper('app.name', 'Laravel');
CODE_SAMPLE
,
[
...self::defaultLaravelMethods(),
new ApplyDefaultInsteadOfNullCoalesce('custom_helper'),
],
),
]
);
}

public function getNodeTypes(): array
{
return [Coalesce::class];
}

/**
* @param Coalesce $node
*/
public function refactor(Node $node): MethodCall|StaticCall|FuncCall|null
{
if (! $node->left instanceof FuncCall &&
! $node->left instanceof MethodCall &&
! $node->left instanceof StaticCall
) {
return null;
}

if ($node->left->isFirstClassCallable()) {
return null;
}

$call = $node->left;

foreach ($this->applyDefaultWith as $applyDefaultWith) {
$valid = false;

$objectType = $call->var ?? $call->class ?? null;

if (
$applyDefaultWith->getObjectType() instanceof ObjectType &&
$objectType !== null &&
$this->isObjectType(
$objectType,
$applyDefaultWith->getObjectType()) &&
$this->isName($call->name, $applyDefaultWith->getMethodName())
) {
$valid = true;
} elseif (
$applyDefaultWith->getObjectType() === null &&
$this->isName($call->name, $applyDefaultWith->getMethodName())
) {
$valid = true;
}

if (! $valid) {
continue;
}

if (count($call->args) === $applyDefaultWith->getArgumentPosition()) {
if ($this->getType($node->right)->isNull()->yes()) {
return $call;
}
$call->args[count($call->args)] = new Arg($node->right);

return $call;
}
}

return null;
}

public function configure(array $configuration): void
{
Assert::allIsInstanceOf($configuration, ApplyDefaultInsteadOfNullCoalesce::class);
$this->applyDefaultWith = $configuration;
}
}
27 changes: 27 additions & 0 deletions src/ValueObject/ApplyDefaultInsteadOfNullCoalesce.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

namespace RectorLaravel\ValueObject;

use PHPStan\Type\ObjectType;

final readonly class ApplyDefaultInsteadOfNullCoalesce
{
public function __construct(private string $methodName, private ?ObjectType $objectType = null, private int $argumentPosition = 1)
{
}

public function getObjectType(): ?ObjectType
{
return $this->objectType;
}

public function getMethodName(): string
{
return $this->methodName;
}

public function getArgumentPosition(): int
{
return $this->argumentPosition;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

declare(strict_types=1);

namespace RectorLaravel\Tests\Rector\Coalesce\ApplyDefaultInsteadOfNullCoalesceRector;

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

final class ApplyDefaultInsteadOfNullCoalesceRectorTest extends AbstractRectorTestCase
{
public static function provideData(): Iterator
{
return self::yieldFilesFromDirectory(__DIR__ . '/Fixture');
}

/**
* @test
*/
#[DataProvider('provideData')]
public function test(string $filePath): void
{
$this->doTestFile($filePath);
}

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

namespace RectorLaravel\Tests\Rector\Coalesce\ApplyDefaultInsteadOfNullCoalesceRector\Fixture;

data_get([], 'key') ?? false;

?>
-----
<?php

namespace RectorLaravel\Tests\Rector\Coalesce\ApplyDefaultInsteadOfNullCoalesceRector\Fixture;

data_get([], 'key', false);

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

namespace RectorLaravel\Tests\Rector\Coalesce\ApplyDefaultInsteadOfNullCoalesceRector\Fixture;

config('app.name') ?? false;
config('app.name') ?? null;

(new \Illuminate\Http\Request())->input('value') ?? '';

\Illuminate\Support\Env::get('APP_NAME') ?? '';

?>
-----
<?php

namespace RectorLaravel\Tests\Rector\Coalesce\ApplyDefaultInsteadOfNullCoalesceRector\Fixture;

config('app.name', false);
config('app.name');

(new \Illuminate\Http\Request())->input('value', '');

\Illuminate\Support\Env::get('APP_NAME', '');

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

namespace RectorLaravel\Tests\Rector\Coalesce\ApplyDefaultInsteadOfNullCoalesceRector\Fixture;

config('app.name', false) ?? false;

(new \Illuminate\Http\Request())->input('value', '') ?? '';

\Illuminate\Support\Env::get('APP_NAME', '') ?? '';

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

namespace RectorLaravel\Tests\Rector\Coalesce\ApplyDefaultInsteadOfNullCoalesceRector\Fixture;

config1('app.name') ?? false;

(new \Illuminate\Http\Request())->inputOf('value') ?? '';
(new \Illuminate\Http\RequestElse())->input('value') ?? '';

\Illuminate\Support\Env::getAgain('APP_NAME') ?? '';
\Illuminate\Support\EnvUtil::get('APP_NAME') ?? '';

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

declare(strict_types=1);

use Rector\Config\RectorConfig;
use RectorLaravel\Rector\Coalesce\ApplyDefaultInsteadOfNullCoalesceRector;

return static function (RectorConfig $rectorConfig): void {
$rectorConfig->import(__DIR__ . '/../../../../../config/config.php');

$rectorConfig->rule(ApplyDefaultInsteadOfNullCoalesceRector::class);
};

0 comments on commit f8d624d

Please sign in to comment.