Skip to content

Commit

Permalink
Merge branch 'main' into feature/remove-null-coalesce-for-default
Browse files Browse the repository at this point in the history
  • Loading branch information
peterfox authored Nov 5, 2024
2 parents c20f49a + 65c14c5 commit 303817a
Show file tree
Hide file tree
Showing 31 changed files with 750 additions and 43 deletions.
19 changes: 18 additions & 1 deletion docs/rector_rules_overview.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# 72 Rules Overview
# 73 Rules Overview

## AbortIfRector

Expand Down Expand Up @@ -82,6 +82,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
3 changes: 3 additions & 0 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,6 @@ parameters:
- '#Parameter \#1 \$node (.*?) of method RectorLaravel\\(.*?)\:\:(refactor|refactorWithScope)\(\) should be contravariant with parameter \$node \(PhpParser\\Node\) of method Rector\\Contract\\Rector\\RectorInterface\:\:refactor\(\)#'

- '#Parameter \#1 \$className of method Rector\\Reflection\\ReflectionResolver\:\:resolveMethodReflection\(\) expects class\-string, string given#'

# Laravel Container not being recognized properly in some of the tests
- '#Call to method needs\(\) on an unknown class Illuminate\\Contracts\\Container\\ContextualBindingBuilder#'
154 changes: 142 additions & 12 deletions src/Rector/ClassMethod/AddGenericReturnTypeToRelationsRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,28 @@
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\NodeTypeResolver\TypeComparator\TypeComparator;
use Rector\PhpParser\Node\BetterNodeFinder;
use Rector\Rector\AbstractScopeAwareRector;
use Rector\StaticTypeMapper\StaticTypeMapper;
use ReflectionClassConstant;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

/** @see \RectorLaravel\Tests\Rector\ClassMethod\AddGenericReturnTypeToRelationsRector\AddGenericReturnTypeToRelationsRectorTest */
/**
* @see \RectorLaravel\Tests\Rector\ClassMethod\AddGenericReturnTypeToRelationsRector\AddGenericReturnTypeToRelationsRectorNewGenericsTest
* @see \RectorLaravel\Tests\Rector\ClassMethod\AddGenericReturnTypeToRelationsRector\AddGenericReturnTypeToRelationsRectorOldGenericsTest
*/
class AddGenericReturnTypeToRelationsRector extends AbstractScopeAwareRector
{
// Relation methods which are supported by this Rector.
Expand All @@ -41,12 +47,18 @@ 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,
private readonly PhpDocInfoFactory $phpDocInfoFactory,
private readonly BetterNodeFinder $betterNodeFinder,
private readonly StaticTypeMapper $staticTypeMapper,
private readonly string $applicationClass = 'Illuminate\Foundation\Application',
) {
}

Expand Down Expand Up @@ -84,6 +96,37 @@ public function accounts(): HasMany
return $this->hasMany(Account::class);
}
}
CODE_SAMPLE
),
new CodeSample(
<<<'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
),
]
Expand Down Expand Up @@ -153,7 +196,11 @@ public function refactorWithScope(Node $node, Scope $scope): ?Node
return null;
}

// Put here to make the check as late as possible
$this->setShouldUseNewGenerics();

$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 +210,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 Down Expand Up @@ -243,6 +291,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 +304,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 +370,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 +391,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 +454,33 @@ 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;
}

private function setShouldUseNewGenerics(): void
{
$reflectionClassConstant = new ReflectionClassConstant($this->applicationClass, 'VERSION');

if (is_string($reflectionClassConstant->getValue())) {
$this->shouldUseNewGenerics = version_compare($reflectionClassConstant->getValue(), '11.15.0', '>=');
}
}
}
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
Expand Up @@ -8,11 +8,11 @@
use PHPUnit\Framework\Attributes\DataProvider;
use Rector\Testing\PHPUnit\AbstractRectorTestCase;

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

/**
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_new_generics_configured_rule.php';
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?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 AddGenericReturnTypeToRelationsRectorOldGenericsTest extends AbstractRectorTestCase
{
public static function provideData(): Iterator
{
return self::yieldFilesFromDirectory(__DIR__ . '/Fixture/OldGenerics');
}

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

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

0 comments on commit 303817a

Please sign in to comment.