From 2526a6a75803c5d53864cfc1e764731785e9b087 Mon Sep 17 00:00:00 2001 From: Liam Hammett Date: Sat, 4 Oct 2025 20:56:23 +0200 Subject: [PATCH 1/3] Refactor `abort_if()` calls to `Gate::denyIf()` when a user is concerned --- .../FuncCall/AbortIfToGateDenyIfRector.php | 112 ++++++++++++++++++ .../AbortIfToGateDenyIfRectorTest.php | 31 +++++ .../Fixture/fixture.php.inc | 27 +++++ .../Fixture/skip_non_user_condition.php.inc | 13 ++ .../Fixture/with_auth_helper.php.inc | 27 +++++ .../Fixture/with_multiple_args.php.inc | 27 +++++ .../config/configured_rule.php | 12 ++ 7 files changed, 249 insertions(+) create mode 100644 src/Rector/FuncCall/AbortIfToGateDenyIfRector.php create mode 100644 tests/Rector/FuncCall/AbortIfToGateDenyIfRector/AbortIfToGateDenyIfRectorTest.php create mode 100644 tests/Rector/FuncCall/AbortIfToGateDenyIfRector/Fixture/fixture.php.inc create mode 100644 tests/Rector/FuncCall/AbortIfToGateDenyIfRector/Fixture/skip_non_user_condition.php.inc create mode 100644 tests/Rector/FuncCall/AbortIfToGateDenyIfRector/Fixture/with_auth_helper.php.inc create mode 100644 tests/Rector/FuncCall/AbortIfToGateDenyIfRector/Fixture/with_multiple_args.php.inc create mode 100644 tests/Rector/FuncCall/AbortIfToGateDenyIfRector/config/configured_rule.php diff --git a/src/Rector/FuncCall/AbortIfToGateDenyIfRector.php b/src/Rector/FuncCall/AbortIfToGateDenyIfRector.php new file mode 100644 index 000000000..af47a4915 --- /dev/null +++ b/src/Rector/FuncCall/AbortIfToGateDenyIfRector.php @@ -0,0 +1,112 @@ +id === $post->user_id); +abort_if($post->user()->is($user)); +CODE_SAMPLE + , + <<<'CODE_SAMPLE' +\Illuminate\Support\Facades\Gate::denyIf($user->id === $post->user_id); +\Illuminate\Support\Facades\Gate::denyIf($post->user()->is($user)); +CODE_SAMPLE + ), + ]); + } + + /** + * @return array> + */ + public function getNodeTypes(): array + { + return [FuncCall::class]; + } + + /** + * @param FuncCall $node + */ + public function refactor(Node $node): ?Node + { + if (! $this->isName($node->name, 'abort_if')) { + return null; + } + + if (! isset($node->args[0])) { + return null; + } + + $condition = $node->args[0]->value; + + if (! $this->containsUserRelatedCheck($condition)) { + return null; + } + + return new StaticCall( + new FullyQualified('Illuminate\Support\Facades\Gate'), + 'denyIf', + $node->args + ); + } + + /** + * Check if the expression contains user-related references + */ + private function containsUserRelatedCheck(Node $node): bool + { + $hasUserReference = false; + + $this->traverseNodesWithCallable($node, function (Node $subNode) use (&$hasUserReference): ?int { + if ($subNode instanceof Variable && is_string($subNode->name)) { + if (str_contains(strtolower($subNode->name), 'user')) { + $hasUserReference = true; + return null; + } + } + + if ($subNode instanceof PropertyFetch) { + $propertyName = $this->getName($subNode->name); + if ($propertyName && str_contains(strtolower($propertyName), 'user')) { + $hasUserReference = true; + return null; + } + } + + if ($subNode instanceof FuncCall && $this->isName($subNode->name, 'auth')) { + $hasUserReference = true; + return null; + } + + if ($subNode instanceof StaticCall && $this->isName($subNode->class, 'Auth')) { + $hasUserReference = true; + return null; + } + + return null; + }); + + return $hasUserReference; + } +} diff --git a/tests/Rector/FuncCall/AbortIfToGateDenyIfRector/AbortIfToGateDenyIfRectorTest.php b/tests/Rector/FuncCall/AbortIfToGateDenyIfRector/AbortIfToGateDenyIfRectorTest.php new file mode 100644 index 000000000..74140f68c --- /dev/null +++ b/tests/Rector/FuncCall/AbortIfToGateDenyIfRector/AbortIfToGateDenyIfRectorTest.php @@ -0,0 +1,31 @@ +doTestFile($filePath); + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/configured_rule.php'; + } +} diff --git a/tests/Rector/FuncCall/AbortIfToGateDenyIfRector/Fixture/fixture.php.inc b/tests/Rector/FuncCall/AbortIfToGateDenyIfRector/Fixture/fixture.php.inc new file mode 100644 index 000000000..5078b1df8 --- /dev/null +++ b/tests/Rector/FuncCall/AbortIfToGateDenyIfRector/Fixture/fixture.php.inc @@ -0,0 +1,27 @@ +id === $post->user_id); + } +} + +?> +----- +id === $post->user_id); + } +} + +?> diff --git a/tests/Rector/FuncCall/AbortIfToGateDenyIfRector/Fixture/skip_non_user_condition.php.inc b/tests/Rector/FuncCall/AbortIfToGateDenyIfRector/Fixture/skip_non_user_condition.php.inc new file mode 100644 index 000000000..599f8b305 --- /dev/null +++ b/tests/Rector/FuncCall/AbortIfToGateDenyIfRector/Fixture/skip_non_user_condition.php.inc @@ -0,0 +1,13 @@ + diff --git a/tests/Rector/FuncCall/AbortIfToGateDenyIfRector/Fixture/with_auth_helper.php.inc b/tests/Rector/FuncCall/AbortIfToGateDenyIfRector/Fixture/with_auth_helper.php.inc new file mode 100644 index 000000000..654a60bc7 --- /dev/null +++ b/tests/Rector/FuncCall/AbortIfToGateDenyIfRector/Fixture/with_auth_helper.php.inc @@ -0,0 +1,27 @@ +id() !== $post->author_id, 403); + } +} + +?> +----- +id() !== $post->author_id, 403); + } +} + +?> diff --git a/tests/Rector/FuncCall/AbortIfToGateDenyIfRector/Fixture/with_multiple_args.php.inc b/tests/Rector/FuncCall/AbortIfToGateDenyIfRector/Fixture/with_multiple_args.php.inc new file mode 100644 index 000000000..b7b3e14ca --- /dev/null +++ b/tests/Rector/FuncCall/AbortIfToGateDenyIfRector/Fixture/with_multiple_args.php.inc @@ -0,0 +1,27 @@ +user()->is(auth()->user()), 403, 'Unauthorized'); + } +} + +?> +----- +user()->is(auth()->user()), 403, 'Unauthorized'); + } +} + +?> diff --git a/tests/Rector/FuncCall/AbortIfToGateDenyIfRector/config/configured_rule.php b/tests/Rector/FuncCall/AbortIfToGateDenyIfRector/config/configured_rule.php new file mode 100644 index 000000000..04ec91ec4 --- /dev/null +++ b/tests/Rector/FuncCall/AbortIfToGateDenyIfRector/config/configured_rule.php @@ -0,0 +1,12 @@ +import(__DIR__ . '/../../../../../config/config.php'); + + $rectorConfig->rule(AbortIfToGateDenyIfRector::class); +}; From 67d00cfc7c12e72c306c62f93d9a7e655b055249 Mon Sep 17 00:00:00 2001 From: Liam Hammett Date: Sat, 4 Oct 2025 21:01:02 +0200 Subject: [PATCH 2/3] Update docs --- docs/rector_rules_overview.md | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/docs/rector_rules_overview.md b/docs/rector_rules_overview.md index 9c9c85011..b319699bc 100644 --- a/docs/rector_rules_overview.md +++ b/docs/rector_rules_overview.md @@ -1,4 +1,4 @@ -# 86 Rules Overview +# 87 Rules Overview ## AbortIfRector @@ -19,6 +19,21 @@ Change if abort to abort_if
+## AbortIfToGateDenyIfRector + +Change `abort_if()` to `Gate::denyIf()` when condition is user-related + +- class: [`RectorLaravel\Rector\FuncCall\AbortIfToGateDenyIfRector`](../src/Rector/FuncCall/AbortIfToGateDenyIfRector.php) + +```diff +-abort_if($user->id === $post->user_id); +-abort_if($post->user()->is($user)); ++\Illuminate\Support\Facades\Gate::denyIf($user->id === $post->user_id); ++\Illuminate\Support\Facades\Gate::denyIf($post->user()->is($user)); +``` + +
+ ## AddArgumentDefaultValueRector Adds default value for arguments in defined methods. From 7c4043283021f95a81b4ddf5f59ec5ad7762f150 Mon Sep 17 00:00:00 2001 From: Liam Hammett Date: Mon, 6 Oct 2025 12:22:58 +0200 Subject: [PATCH 3/3] Code style/linting fixes --- .../FuncCall/AbortIfToGateDenyIfRector.php | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/Rector/FuncCall/AbortIfToGateDenyIfRector.php b/src/Rector/FuncCall/AbortIfToGateDenyIfRector.php index af47a4915..35941cb7a 100644 --- a/src/Rector/FuncCall/AbortIfToGateDenyIfRector.php +++ b/src/Rector/FuncCall/AbortIfToGateDenyIfRector.php @@ -5,8 +5,8 @@ namespace RectorLaravel\Rector\FuncCall; use PhpParser\Node; +use PhpParser\Node\Arg; use PhpParser\Node\Expr\FuncCall; -use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Expr\PropertyFetch; use PhpParser\Node\Expr\StaticCall; use PhpParser\Node\Expr\Variable; @@ -58,7 +58,12 @@ public function refactor(Node $node): ?Node return null; } - $condition = $node->args[0]->value; + $firstArg = $node->args[0]; + if (! $firstArg instanceof Arg) { + return null; + } + + $condition = $firstArg->value; if (! $this->containsUserRelatedCheck($condition)) { return null; @@ -79,28 +84,30 @@ private function containsUserRelatedCheck(Node $node): bool $hasUserReference = false; $this->traverseNodesWithCallable($node, function (Node $subNode) use (&$hasUserReference): ?int { - if ($subNode instanceof Variable && is_string($subNode->name)) { - if (str_contains(strtolower($subNode->name), 'user')) { - $hasUserReference = true; - return null; - } + if ($subNode instanceof Variable && is_string($subNode->name) && str_contains(strtolower($subNode->name), 'user')) { + $hasUserReference = true; + + return null; } if ($subNode instanceof PropertyFetch) { $propertyName = $this->getName($subNode->name); - if ($propertyName && str_contains(strtolower($propertyName), 'user')) { + if ($propertyName !== null && str_contains(strtolower($propertyName), 'user')) { $hasUserReference = true; + return null; } } if ($subNode instanceof FuncCall && $this->isName($subNode->name, 'auth')) { $hasUserReference = true; + return null; } if ($subNode instanceof StaticCall && $this->isName($subNode->class, 'Auth')) { $hasUserReference = true; + return null; }