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.

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

feat: adjust AddGenericReturnTypeToRelationsRector rule to be able to generate new generic style code #263

Merged
merged 2 commits into from
Oct 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 19 additions & 0 deletions docs/rector_rules_overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ Adds the `@extends` annotation to Factories.

Add generic return type to relations in child of `Illuminate\Database\Eloquent\Model`

:wrench: **configure it!**

- class: [`RectorLaravel\Rector\ClassMethod\AddGenericReturnTypeToRelationsRector`](../src/Rector/ClassMethod/AddGenericReturnTypeToRelationsRector.php)

```diff
Expand All @@ -82,6 +84,23 @@ Add generic return type to relations in child of `Illuminate\Database\Eloquent\M

<br>

```diff
use App\Account;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\HasMany;

class User extends Model
{
+ /** @return HasMany<Account, $this> */
public function accounts(): HasMany
{
return $this->hasMany(Account::class);
}
}
```

<br>

## AddGuardToLoginEventRector

Add new `$guard` argument to Illuminate\Auth\Events\Login
Expand Down
168 changes: 152 additions & 16 deletions src/Rector/ClassMethod/AddGenericReturnTypeToRelationsRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,23 +12,30 @@
use PHPStan\Analyser\Scope;
use PHPStan\PhpDocParser\Ast\PhpDoc\ReturnTagValueNode;
use PHPStan\PhpDocParser\Ast\Type\GenericTypeNode;
use PHPStan\PhpDocParser\Ast\Type\IdentifierTypeNode;
use PHPStan\Reflection\ClassReflection;
use PHPStan\Type\Constant\ConstantStringType;
use PHPStan\Type\Generic\GenericClassStringType;
use PHPStan\Type\Generic\GenericObjectType;
use PHPStan\Type\ObjectType;
use PHPStan\Type\ThisType;
use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory;
use Rector\BetterPhpDocParser\ValueObject\Type\FullyQualifiedIdentifierTypeNode;
use Rector\Comments\NodeDocBlock\DocBlockUpdater;
use Rector\Contract\Rector\ConfigurableRectorInterface;
use Rector\NodeTypeResolver\TypeComparator\TypeComparator;
use Rector\PhpParser\Node\BetterNodeFinder;
use Rector\Rector\AbstractScopeAwareRector;
use Rector\StaticTypeMapper\StaticTypeMapper;
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\ClassMethod\AddGenericReturnTypeToRelationsRector\AddGenericReturnTypeToRelationsRectorTest */
class AddGenericReturnTypeToRelationsRector extends AbstractScopeAwareRector
/**
* @see \RectorLaravel\Tests\Rector\ClassMethod\AddGenericReturnTypeToRelationsRector\AddGenericReturnTypeToRelationsRectorNewGenericsTest
* @see \RectorLaravel\Tests\Rector\ClassMethod\AddGenericReturnTypeToRelationsRector\AddGenericReturnTypeToRelationsRectorOldGenericsTest
*/
class AddGenericReturnTypeToRelationsRector extends AbstractScopeAwareRector implements ConfigurableRectorInterface
{
// Relation methods which are supported by this Rector.
private const RELATION_METHODS = [
Expand All @@ -41,6 +48,11 @@ class AddGenericReturnTypeToRelationsRector extends AbstractScopeAwareRector
// Relation methods which need the class as TChildModel.
private const RELATION_WITH_CHILD_METHODS = ['belongsTo', 'morphTo'];

// Relation methods which need the class as TIntermediateModel.
private const RELATION_WITH_INTERMEDIATE_METHODS = ['hasManyThrough', 'hasOneThrough'];

private bool $shouldUseNewGenerics = false;

public function __construct(
private readonly TypeComparator $typeComparator,
private readonly DocBlockUpdater $docBlockUpdater,
Expand All @@ -55,7 +67,7 @@ public function getRuleDefinition(): RuleDefinition
return new RuleDefinition(
'Add generic return type to relations in child of Illuminate\Database\Eloquent\Model',
[
new CodeSample(
new ConfiguredCodeSample(
<<<'CODE_SAMPLE'
use App\Account;
use Illuminate\Database\Eloquent\Model;
Expand Down Expand Up @@ -84,8 +96,39 @@ public function accounts(): HasMany
return $this->hasMany(Account::class);
}
}
CODE_SAMPLE,
['shouldUseNewGenerics' => false]),
new ConfiguredCodeSample(
<<<'CODE_SAMPLE'
use App\Account;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\HasMany;

class User extends Model
{
public function accounts(): HasMany
{
return $this->hasMany(Account::class);
}
}
CODE_SAMPLE
),

,
<<<'CODE_SAMPLE'
use App\Account;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\HasMany;

class User extends Model
{
/** @return HasMany<Account, $this> */
public function accounts(): HasMany
{
return $this->hasMany(Account::class);
}
}
CODE_SAMPLE,
['shouldUseNewGenerics' => true]),
]
);
}
Expand Down Expand Up @@ -154,6 +197,7 @@ public function refactorWithScope(Node $node, Scope $scope): ?Node
}

$classForChildGeneric = $this->getClassForChildGeneric($scope, $relationMethodCall);
$classForIntermediateGeneric = $this->getClassForIntermediateGeneric($relationMethodCall);

// Don't update the docblock if return type already contains the correct generics. This avoids overwriting
// non-FQCN with our fully qualified ones.
Expand All @@ -163,15 +207,16 @@ public function refactorWithScope(Node $node, Scope $scope): ?Node
$node,
$phpDocInfo->getReturnTagValue(),
$relatedClass,
$classForChildGeneric
$classForChildGeneric,
$classForIntermediateGeneric
)
) {
return null;
}

$genericTypeNode = new GenericTypeNode(
new FullyQualifiedIdentifierTypeNode($methodReturnTypeName),
$this->getGenericTypes($relatedClass, $classForChildGeneric),
$this->getGenericTypes($relatedClass, $classForChildGeneric, $classForIntermediateGeneric),
);

// Update or add return tag
Expand All @@ -187,6 +232,24 @@ public function refactorWithScope(Node $node, Scope $scope): ?Node
return $node;
}

/**
* {@inheritDoc}
*/
public function configure(array $configuration): void
{
if ($configuration === []) {
$this->shouldUseNewGenerics = false;

return;
}

Assert::count($configuration, 1);
Assert::keyExists($configuration, 'shouldUseNewGenerics');
Assert::boolean($configuration['shouldUseNewGenerics']);

$this->shouldUseNewGenerics = $configuration['shouldUseNewGenerics'];
}

private function getRelatedModelClassFromMethodCall(MethodCall $methodCall): ?string
{
$argType = $this->getType($methodCall->getArgs()[0]->value);
Expand Down Expand Up @@ -243,6 +306,10 @@ private function getRelationMethodCall(ClassMethod $classMethod): ?MethodCall
*/
private function getClassForChildGeneric(Scope $scope, MethodCall $methodCall): ?string
{
if ($this->shouldUseNewGenerics) {
return null;
}

if (! $this->doesMethodHasName($methodCall, self::RELATION_WITH_CHILD_METHODS)) {
return null;
}
Expand All @@ -252,6 +319,45 @@ private function getClassForChildGeneric(Scope $scope, MethodCall $methodCall):
return $classReflection?->getName();
}

/**
* We need the intermediate class for generics which need a TIntermediateModel.
* This is the case for *through relations
*/
private function getClassForIntermediateGeneric(MethodCall $methodCall): ?string
{
if (! $this->shouldUseNewGenerics) {
return null;
}

if (! $this->doesMethodHasName($methodCall, self::RELATION_WITH_INTERMEDIATE_METHODS)) {
return null;
}

$args = $methodCall->getArgs();

if (count($args) < 2) {
return null;
}

$argType = $this->getType($args[1]->value);

if ($argType instanceof ConstantStringType && $argType->isClassStringType()->yes()) {
return $argType->getValue();
}

if (! $argType instanceof GenericClassStringType) {
return null;
}

$modelType = $argType->getGenericType();

if (! $modelType instanceof ObjectType) {
return null;
}

return $modelType->getClassName();
}

private function areNativeTypeAndPhpDocReturnTypeEqual(
ClassMethod $classMethod,
Node $node,
Expand Down Expand Up @@ -279,7 +385,8 @@ private function areGenericTypesEqual(
Node $node,
ReturnTagValueNode $returnTagValueNode,
string $relatedClass,
?string $classForChildGeneric
?string $classForChildGeneric,
?string $classForIntermediateGeneric
): bool {
$phpDocPHPStanType = $this->staticTypeMapper->mapPHPStanPhpDocTypeNodeToPHPStanType(
$returnTagValueNode->type,
Expand All @@ -299,16 +406,37 @@ private function areGenericTypesEqual(
return false;
}

$phpDocHasChildGeneric = count($phpDocTypes) === 2;
if ($classForChildGeneric === null && ! $phpDocHasChildGeneric) {
return true;
if (! $this->shouldUseNewGenerics) {
$phpDocHasChildGeneric = count($phpDocTypes) === 2;

if ($classForChildGeneric === null && ! $phpDocHasChildGeneric) {
return true;
}

if ($classForChildGeneric === null || ! $phpDocHasChildGeneric) {
return false;
}

return $this->typeComparator->areTypesEqual($phpDocTypes[1], new ObjectType($classForChildGeneric));
}

$phpDocHasIntermediateGeneric = count($phpDocTypes) === 3;

if ($classForIntermediateGeneric === null && ! $phpDocHasIntermediateGeneric) {
// If there is only one generic, it means method is using the old format. We should update it.
if (count($phpDocTypes) === 1) {
return false;
}

// We want to convert the existing relationship definition to use `$this` as the second generic
return $phpDocTypes[1] instanceof ThisType;
}

if ($classForChildGeneric === null || ! $phpDocHasChildGeneric) {
if ($classForIntermediateGeneric === null || ! $phpDocHasIntermediateGeneric) {
return false;
}

return $this->typeComparator->areTypesEqual($phpDocTypes[1], new ObjectType($classForChildGeneric));
return $this->typeComparator->areTypesEqual($phpDocTypes[1], new ObjectType($classForIntermediateGeneric));
}

private function shouldSkipNode(ClassMethod $classMethod, Scope $scope): bool
Expand Down Expand Up @@ -341,16 +469,24 @@ private function doesMethodHasName(MethodCall $methodCall, array $methodNames):
}

/**
* @return FullyQualifiedIdentifierTypeNode[]
* @return IdentifierTypeNode[]
*/
private function getGenericTypes(string $relatedClass, ?string $childClass): array
private function getGenericTypes(string $relatedClass, ?string $childClass, ?string $intermediateClass): array
{
$generics = [new FullyQualifiedIdentifierTypeNode($relatedClass)];

if ($childClass !== null) {
if (! $this->shouldUseNewGenerics && $childClass !== null) {
$generics[] = new FullyQualifiedIdentifierTypeNode($childClass);
}

if ($this->shouldUseNewGenerics) {
if ($intermediateClass !== null) {
$generics[] = new FullyQualifiedIdentifierTypeNode($intermediateClass);
}

$generics[] = new IdentifierTypeNode('$this');
}

return $generics;
}
}
11 changes: 11 additions & 0 deletions stubs/Illuminate/Database/Eloquent/Relations/HasOneThrough.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

namespace Illuminate\Database\Eloquent\Relations;

if (class_exists('Illuminate\Database\Eloquent\Relations\HasOneThrough')) {
return;
}

class HasOneThrough extends Relation
{
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php

declare(strict_types=1);

namespace RectorLaravel\Tests\Rector\ClassMethod\AddGenericReturnTypeToRelationsRector;

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

final class AddGenericReturnTypeToRelationsRectorNewGenericsTest extends AbstractRectorTestCase
{
public static function provideData(): Iterator
{
// yield [__DIR__ . '/Fixture/NewGenerics/has-one-through.php.inc'];
return self::yieldFilesFromDirectory(__DIR__ . '/Fixture/NewGenerics');
}

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

public function provideConfigFilePath(): string
{
return __DIR__ . '/config/use_new_generics_configured_rule.php';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@
use PHPUnit\Framework\Attributes\DataProvider;
use Rector\Testing\PHPUnit\AbstractRectorTestCase;

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

/**
Expand All @@ -26,6 +26,6 @@ public function test(string $filePath): void

public function provideConfigFilePath(): string
{
return __DIR__ . '/config/configured_rule.php';
return __DIR__ . '/config/use_old_generics_configured_rule.php';
}
}
Loading
Loading