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.

Sign up for GitHub

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

Jump to bottom

Allow configuring EloquentOrderByToLatestOrOldestRector #148

Merged
merged 5 commits into from
Dec 8, 2023
Merged

Allow configuring EloquentOrderByToLatestOrOldestRector #148

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
77888c7
Allow configuring EloquentOrderByToLatestOrOldestRector
johnbacon Oct 12, 2023
2ae2ef1
Update Rector rules overview markdown
johnbacon Oct 13, 2023
67c53ce
Ensure all `ALLOWED_PATTERNS` values are strings
johnbacon Oct 16, 2023
711110f
Merge branch 'driftingly:main' into 147-eloquentorderbytolatestorolde…
johnbacon Dec 7, 2023
2a32580
Move query statements to `.php.inc` file
johnbacon Dec 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 33 additions & 2 deletions docs/rector_rules_overview.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,17 +490,48 @@ The EloquentMagicMethodToQueryBuilderRule is designed to automatically transform

Changes `orderBy()` to `latest()` or `oldest()`

:wrench: **configure it!**

- class: [`RectorLaravel\Rector\MethodCall\EloquentOrderByToLatestOrOldestRector`](../src/Rector/MethodCall/EloquentOrderByToLatestOrOldestRector.php)

```php
<?php

declare(strict_types=1);

use RectorLaravel\Rector\MethodCall\EloquentOrderByToLatestOrOldestRector;
use Rector\Config\RectorConfig;

return static function (RectorConfig $rectorConfig): void {
$rectorConfig->ruleWithConfiguration(EloquentOrderByToLatestOrOldestRector::class, [
EloquentOrderByToLatestOrOldestRector::ALLOWED_PATTERNS => [
'submitted_a*',
'*tested_at',
'$allowed_variable_name',
],
]);
};
```

↓

```diff
use Illuminate\Database\Eloquent\Builder;

$column = 'tested_at';

-$builder->orderBy('created_at');
-$builder->orderBy('created_at', 'desc');
-$builder->orderBy('deleted_at');
-$builder->orderBy('submitted_at');
-$builder->orderByDesc('submitted_at');
-$builder->orderBy($allowed_variable_name);
+$builder->oldest();
+$builder->latest();
+$builder->oldest('deleted_at');
+$builder->oldest('submitted_at');
+$builder->latest('submitted_at');
+$builder->oldest($allowed_variable_name);
$builder->orderBy($unallowed_variable_name);
$builder->orderBy('unallowed_column_name');
```

<br>
Expand Down
90 changes: 80 additions & 10 deletions src/Rector/MethodCall/EloquentOrderByToLatestOrOldestRector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,38 +7,63 @@
use PhpParser\Node;
use PhpParser\Node\Expr\MethodCall;
use PHPStan\Type\ObjectType;
use Rector\Core\Contract\Rector\ConfigurableRectorInterface;
use Rector\Core\Rector\AbstractRector;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\ConfiguredCodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
use Webmozart\Assert\Assert;

/**
* @see \RectorLaravel\Tests\Rector\MethodCall\EloquentOrderByToLatestOrOldestRector\EloquentOrderByToLatestOrOldestRectorTest
*/
class EloquentOrderByToLatestOrOldestRector extends AbstractRector
class EloquentOrderByToLatestOrOldestRector extends AbstractRector implements ConfigurableRectorInterface
{
/**
* @var string
*/
final public const ALLOWED_PATTERNS = 'allowed_patterns';

/**
* @var string[]
*/
private array $allowedPatterns = [];

public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition(
'Changes orderBy() to latest() or oldest()',
[
new CodeSample(
<<<'CODE_SAMPLE'
new ConfiguredCodeSample(<<<'CODE_SAMPLE'
use Illuminate\Database\Eloquent\Builder;

$column = 'tested_at';

$builder->orderBy('created_at');
$builder->orderBy('created_at', 'desc');
$builder->orderBy('deleted_at');
$builder->orderBy('submitted_at');
$builder->orderByDesc('submitted_at');
$builder->orderBy($allowed_variable_name);
$builder->orderBy($unallowed_variable_name);
$builder->orderBy('unallowed_column_name');
CODE_SAMPLE
,
<<<'CODE_SAMPLE'
,<<<'CODE_SAMPLE'
use Illuminate\Database\Eloquent\Builder;

$column = 'tested_at';

$builder->oldest();
$builder->latest();
$builder->oldest('deleted_at');
$builder->oldest('submitted_at');
$builder->latest('submitted_at');
$builder->oldest($allowed_variable_name);
$builder->orderBy($unallowed_variable_name);
$builder->orderBy('unallowed_column_name');
CODE_SAMPLE
,
),
, [self::ALLOWED_PATTERNS => [
'submitted_a*',
'*tested_at',
'$allowed_variable_name',]]),
]
);
}
Expand All @@ -54,7 +79,7 @@ public function refactor(Node $node): ?Node
return null;
}

if ($this->isOrderByMethodCall($node)) {
if ($this->isOrderByMethodCall($node) && $this->isAllowedPattern($node)) {
return $this->convertOrderByToLatest($node);
}

Expand All @@ -71,6 +96,39 @@ private function isOrderByMethodCall(MethodCall $methodCall): bool
&& count($methodCall->args) > 0;
}

private function isAllowedPattern(MethodCall $methodCall): bool
{
$columnArg = $methodCall->args[0]->value ?? null;

// If no patterns are specified, consider all column names as matching
if ($this->allowedPatterns === []) {
return true;
}

if ($columnArg instanceof Node\Scalar\String_) {
$columnName = $columnArg->value;

// If specified, only allow certain patterns
foreach ($this->allowedPatterns as $pattern) {
if (fnmatch($pattern, $columnName)) {
return true;
}
}
}

if ($columnArg instanceof Node\Expr\Variable && is_string($columnArg->name)) {
// Check against allowed patterns
foreach ($this->allowedPatterns as $pattern) {
if (fnmatch(ltrim($pattern, '$'), $columnArg->name)) {
return true;
}
}
}


return false;
}

private function convertOrderByToLatest(MethodCall $methodCall): MethodCall
{
if (! isset($methodCall->args[0]) && ! $methodCall->args[0] instanceof Node\VariadicPlaceholder) {
Expand Down Expand Up @@ -107,4 +165,16 @@ private function convertOrderByToLatest(MethodCall $methodCall): MethodCall

return $methodCall;
}


/**
* @param mixed[] $configuration
*/
public function configure(array $configuration): void
{
$allowedPatterns = $configuration[self::ALLOWED_PATTERNS] ?? [];
Assert::isArray($allowedPatterns);
johnbacon marked this conversation as resolved.
Show resolved Hide resolved

$this->allowedPatterns = $allowedPatterns;
}
}
8 changes: 6 additions & 2 deletions tests/Rector/MethodCall/EloquentOrderByToLatestOrOldestRector/Fixture/fixture.php.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ $query->orderBy('created_at');
$query->orderBy('created_at', 'desc');
$query->orderBy('submitted_at');
$query->orderByDesc('submitted_at');
$query->orderBy($column);
$query->orderBy($allowed_variable_name);
$query->orderBy($unallowed_variable_name);
$query->orderBy('unallowed_column_name');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would probably be better to exist in their own file like skip_if_not_eloquent_builder.php.inc

// skip_if_column_not_allowed.php.inc
<?php

namespace RectorLaravel\Tests\Rector\Cast\DatabaseExpressionCastsToMethodCall\Fixture;

$query->orderBy($unallowed_variable_name);
$query->orderBy('unallowed_column_name');

?>

Copy link
Contributor Author

@johnbacon johnbacon Oct 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I can do this, but I thought these would be unallowed not because they weren't an instance of Builder but because they weren't desired to be automatically refactored. Does it still make sense to move to a different file? I'm not familiar with best practices in this regard.

Copy link
Owner

Choose a reason for hiding this comment

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

I think @peterfox is saying to create a file called skip_if_column_not_allowed.php.inc or skip_if_column_not_configured.php.inc similar to how he did with with the EloquentOrderByToLatestOrOldestRector fixtures.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it! At least... I think I got it. I don't have any experience with this at this point, besides following orders. Please let me know if I need to do something else!


?>
-----
Expand All @@ -28,6 +30,8 @@ $query->oldest();
$query->latest();
$query->oldest('submitted_at');
$query->latest('submitted_at');
$query->oldest($column);
$query->oldest($allowed_variable_name);
$query->orderBy($unallowed_variable_name);
$query->orderBy('unallowed_column_name');

?>
12 changes: 11 additions & 1 deletion tests/Rector/MethodCall/EloquentOrderByToLatestOrOldestRector/config/configured_rule.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,15 @@
return static function (RectorConfig $rectorConfig): void {
$rectorConfig->import(__DIR__ . '/../../../../../config/config.php');

$rectorConfig->rule(EloquentOrderByToLatestOrOldestRector::class);
$rectorConfig->ruleWithConfiguration(
EloquentOrderByToLatestOrOldestRector::class,
[
EloquentOrderByToLatestOrOldestRector::ALLOWED_PATTERNS => [
'created_at',
'submitted_a*',
'*tested_at',
'$allowed_variable_name',
],
],
);
};