diff --git a/docs/rector_rules_overview.md b/docs/rector_rules_overview.md index 9c9c85011..5757445b4 100644 --- a/docs/rector_rules_overview.md +++ b/docs/rector_rules_overview.md @@ -1020,6 +1020,21 @@ Refactor Model `$casts` property with `casts()` method
+## ModelComparisonToIsMethodRector + +Convert model ID comparisons to use the `is()` method + +- class: [`RectorLaravel\Rector\Expr\ModelComparisonToIsMethodRector`](../src/Rector/Expr/ModelComparisonToIsMethodRector.php) + +```diff +-$team->user_id === $user->id; +-$post->author_id === $author->id; ++$team->user()->is($user); ++$post->author()->is($author); +``` + +
+ ## NotFilledBlankFuncCallToBlankFilledFuncCallRector Swap the use of NotBooleans used with `filled()` and `blank()` to the correct helper. diff --git a/src/Rector/Expr/ModelComparisonToIsMethodRector.php b/src/Rector/Expr/ModelComparisonToIsMethodRector.php new file mode 100644 index 000000000..c7e539a70 --- /dev/null +++ b/src/Rector/Expr/ModelComparisonToIsMethodRector.php @@ -0,0 +1,150 @@ +user_id === $user->id; +$post->author_id === $author->id; +CODE_SAMPLE, + <<<'CODE_SAMPLE' +$team->user()->is($user); +$post->author()->is($author); +CODE_SAMPLE + )] + ); + } + + /** + * @return array> + */ + public function getNodeTypes(): array + { + return [Equal::class, Identical::class]; + } + + /** + * @param Equal|Identical $node + */ + public function refactor(Node $node): ?Node + { + if (! $node instanceof Equal && ! $node instanceof Identical) { + return null; + } + + $result = $this->matchModelComparison($node); + if ($result === null) { + return null; + } + + [$leftVar, $relationshipName, $rightVar] = $result; + + if (! $this->couldBeModel($leftVar) || ! $this->couldBeModel($rightVar)) { + return null; + } + + $methodCall = new MethodCall($leftVar, new Identifier($relationshipName)); + + return new MethodCall($methodCall, new Identifier('is'), [new Arg($rightVar)]); + } + + /** + * @return array{Expr, string, Expr}|null + */ + private function matchModelComparison(Equal|Identical $node): ?array + { + $left = $node->left; + $right = $node->right; + + if (! $left instanceof PropertyFetch || ! $right instanceof PropertyFetch) { + return null; + } + + $leftProperty = $left->name; + $rightProperty = $right->name; + + if (! $leftProperty instanceof Identifier || ! $rightProperty instanceof Identifier) { + return null; + } + + $leftPropertyName = $leftProperty->name; + $rightPropertyName = $rightProperty->name; + + if ($this->isForeignKeyToIdPattern($leftPropertyName, $rightPropertyName)) { + // $model->foreign_key_id == $otherModel->id + $relationshipName = $this->extractRelationshipName($leftPropertyName); + + return [$left->var, $relationshipName, $right->var]; + } + + if ($this->isForeignKeyToIdPattern($rightPropertyName, $leftPropertyName)) { + // $otherModel->id == $model->foreign_key_id + $relationshipName = $this->extractRelationshipName($rightPropertyName); + + return [$right->var, $relationshipName, $left->var]; + } + + return null; + } + + private function isForeignKeyToIdPattern(string $leftProperty, string $rightProperty): bool + { + return str_ends_with($leftProperty, '_id') && $rightProperty === 'id'; + } + + private function extractRelationshipName(string $foreignKeyProperty): string + { + return substr($foreignKeyProperty, 0, -3); + } + + private function couldBeModel(Expr $expr): bool + { + $objectType = new ObjectType('Illuminate\Database\Eloquent\Model'); + + if ($expr instanceof PropertyFetch) { + $varType = $this->getType($expr->var); + if ($this->isObjectType($expr->var, $objectType)) { + return true; + } + + return $varType instanceof MixedType; + } + + if ($expr instanceof Variable) { + $varType = $this->getType($expr); + if ($this->isObjectType($expr, $objectType)) { + return true; + } + + return $varType instanceof MixedType; + } + + return false; + } +} diff --git a/tests/Rector/Expr/ModelComparisonToIsMethodRector/Fixture/complex_relationships.php.inc b/tests/Rector/Expr/ModelComparisonToIsMethodRector/Fixture/complex_relationships.php.inc new file mode 100644 index 000000000..866811c27 --- /dev/null +++ b/tests/Rector/Expr/ModelComparisonToIsMethodRector/Fixture/complex_relationships.php.inc @@ -0,0 +1,89 @@ +belongsTo(Post::class); + } + + public function author() + { + return $this->belongsTo(User::class); + } +} + +class Post extends Model +{ + public $id; + public $author_id; + + public function author() + { + return $this->belongsTo(User::class); + } +} + +class User extends Model +{ + public $id; +} + +// Complex relationship comparisons +$comment->post_id === $post->id; +$comment->author_id === $author->id; +$post->author_id === $user->id; + +?> +----- +belongsTo(Post::class); + } + + public function author() + { + return $this->belongsTo(User::class); + } +} + +class Post extends Model +{ + public $id; + public $author_id; + + public function author() + { + return $this->belongsTo(User::class); + } +} + +class User extends Model +{ + public $id; +} + +// Complex relationship comparisons +$comment->post()->is($post); +$comment->author()->is($author); +$post->author()->is($user); + +?> diff --git a/tests/Rector/Expr/ModelComparisonToIsMethodRector/Fixture/fixture.php.inc b/tests/Rector/Expr/ModelComparisonToIsMethodRector/Fixture/fixture.php.inc new file mode 100644 index 000000000..32747c84d --- /dev/null +++ b/tests/Rector/Expr/ModelComparisonToIsMethodRector/Fixture/fixture.php.inc @@ -0,0 +1,85 @@ +belongsTo(User::class); + } +} + +class Post extends Model +{ + public $author_id; + + public function author() + { + return $this->belongsTo(User::class); + } +} + +// Basic comparison cases +$team->user_id === $user->id; +$team->user_id === $user->id; +$post->author_id === $author->id; +$post->author_id === $author->id; + +// Reversed comparison +$user->id === $team->user_id; +$author->id === $post->author_id; + +?> +----- +belongsTo(User::class); + } +} + +class Post extends Model +{ + public $author_id; + + public function author() + { + return $this->belongsTo(User::class); + } +} + +// Basic comparison cases +$team->user()->is($user); +$team->user()->is($user); +$post->author()->is($author); +$post->author()->is($author); + +// Reversed comparison +$team->user()->is($user); +$post->author()->is($author); + +?> diff --git a/tests/Rector/Expr/ModelComparisonToIsMethodRector/Fixture/multiple_relationships.php.inc b/tests/Rector/Expr/ModelComparisonToIsMethodRector/Fixture/multiple_relationships.php.inc new file mode 100644 index 000000000..d3d47facd --- /dev/null +++ b/tests/Rector/Expr/ModelComparisonToIsMethodRector/Fixture/multiple_relationships.php.inc @@ -0,0 +1,103 @@ +belongsTo(User::class); + } + + public function editor() + { + return $this->belongsTo(User::class); + } +} + +class Comment +{ + public $post_id; + public $author_id; + + public function post() + { + return $this->belongsTo(Post::class); + } + + public function author() + { + return $this->belongsTo(User::class); + } +} + +// Multiple relationships in one function +function checkOwnership($post, $comment, $user) { + if ($post->author_id === $user->id && $comment->author_id === $user->id) { + return true; + } + + return $post->editor_id === $user->id || $comment->post_id === $post->id; +} + +?> +----- +belongsTo(User::class); + } + + public function editor() + { + return $this->belongsTo(User::class); + } +} + +class Comment +{ + public $post_id; + public $author_id; + + public function post() + { + return $this->belongsTo(Post::class); + } + + public function author() + { + return $this->belongsTo(User::class); + } +} + +// Multiple relationships in one function +function checkOwnership($post, $comment, $user) { + if ($post->author()->is($user) && $comment->author()->is($user)) { + return true; + } + + return $post->editor()->is($user) || $comment->post()->is($post); +} + +?> diff --git a/tests/Rector/Expr/ModelComparisonToIsMethodRector/Fixture/skip_invalid_patterns.php.inc b/tests/Rector/Expr/ModelComparisonToIsMethodRector/Fixture/skip_invalid_patterns.php.inc new file mode 100644 index 000000000..a5b89cfc4 --- /dev/null +++ b/tests/Rector/Expr/ModelComparisonToIsMethodRector/Fixture/skip_invalid_patterns.php.inc @@ -0,0 +1,49 @@ +name === $user->email; +$team->user_id === $user->email; +$team->name === $user->id; +$someVar === $user->id; +$team->user_id === $someVar; + +?> +----- +name === $user->email; +$team->user_id === $user->email; +$team->name === $user->id; +$someVar === $user->id; +$team->user_id === $someVar; + +?> diff --git a/tests/Rector/Expr/ModelComparisonToIsMethodRector/Fixture/skip_non_model_variables.php.inc b/tests/Rector/Expr/ModelComparisonToIsMethodRector/Fixture/skip_non_model_variables.php.inc new file mode 100644 index 000000000..0266eefa8 --- /dev/null +++ b/tests/Rector/Expr/ModelComparisonToIsMethodRector/Fixture/skip_non_model_variables.php.inc @@ -0,0 +1,61 @@ +belongsTo(User::class); + } +} + +// Should NOT be transformed - non-model variables +$team = new Team(); +$user = new User(); +$index = 0; +$data = ['key' => 'value']; + +$team->user_id = $user->id; +$team->user_id === $index; +$post->author_id == $data; + +?> +----- +belongsTo(User::class); + } +} + +// Should NOT be transformed - non-model variables +$team = new Team(); +$user = new User(); +$index = 0; +$data = ['key' => 'value']; + +$team->user_id = $user->id; +$team->user_id === $index; +$post->author_id == $data; + +?> diff --git a/tests/Rector/Expr/ModelComparisonToIsMethodRector/ModelComparisonToIsMethodRectorTest.php b/tests/Rector/Expr/ModelComparisonToIsMethodRector/ModelComparisonToIsMethodRectorTest.php new file mode 100644 index 000000000..9f9066c64 --- /dev/null +++ b/tests/Rector/Expr/ModelComparisonToIsMethodRector/ModelComparisonToIsMethodRectorTest.php @@ -0,0 +1,31 @@ +doTestFile($filePath); + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/configured_rule.php'; + } +} diff --git a/tests/Rector/Expr/ModelComparisonToIsMethodRector/config/configured_rule.php b/tests/Rector/Expr/ModelComparisonToIsMethodRector/config/configured_rule.php new file mode 100644 index 000000000..35e94ad69 --- /dev/null +++ b/tests/Rector/Expr/ModelComparisonToIsMethodRector/config/configured_rule.php @@ -0,0 +1,12 @@ +import(__DIR__ . '/../../../../../config/config.php'); + + $rectorConfig->rule(ModelComparisonToIsMethodRector::class); +};