-
Notifications
You must be signed in to change notification settings - Fork 103
feat: add rule for Laravel Timestamps property to attribute #491
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace RectorLaravel\Rector\Class_; | ||
|
|
||
| use PhpParser\Node; | ||
| use PhpParser\Node\Attribute; | ||
| use PhpParser\Node\AttributeGroup; | ||
| use PhpParser\Node\Expr\ConstFetch; | ||
| use PhpParser\Node\Name\FullyQualified; | ||
| use PhpParser\Node\Stmt\Class_; | ||
| use PhpParser\Node\Stmt\Property; | ||
| use PHPStan\Type\ObjectType; | ||
| use Rector\Php80\NodeAnalyzer\PhpAttributeAnalyzer; | ||
| use Rector\Rector\AbstractRector; | ||
| use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; | ||
| use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; | ||
|
|
||
| final class TimestampsPropertyToTimestampsAttributeRector extends AbstractRector | ||
| { | ||
| public function __construct( | ||
| private readonly PhpAttributeAnalyzer $phpAttributeAnalyzer, | ||
| ) {} | ||
|
|
||
| public function getRuleDefinition(): RuleDefinition | ||
| { | ||
| return new RuleDefinition( | ||
| 'Replace $timestamps = false property with #[WithoutTimestamps] attribute in Eloquent Models', | ||
| [ | ||
| new CodeSample( | ||
| <<<'PHP' | ||
| use Illuminate\Database\Eloquent\Model; | ||
|
|
||
| class User extends Model | ||
| { | ||
| public $timestamps = false; | ||
| } | ||
| PHP, | ||
| <<<'PHP' | ||
| use Illuminate\Database\Eloquent\Attributes\WithoutTimestamps; | ||
| use Illuminate\Database\Eloquent\Model; | ||
|
|
||
| #[WithoutTimestamps] | ||
| class User extends Model | ||
| { | ||
| } | ||
| PHP, | ||
| ), | ||
| ], | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * @return array<class-string<Node>> | ||
| */ | ||
| public function getNodeTypes(): array | ||
| { | ||
| return [Class_::class]; | ||
| } | ||
|
|
||
| /** | ||
| * @param Class_ $node | ||
| */ | ||
| public function refactor(Node $node): ?Node | ||
| { | ||
| if (!$this->isAnEloquentModelClass($node)) { | ||
| return null; | ||
| } | ||
|
|
||
| if ($this->phpAttributeAnalyzer->hasPhpAttribute($node, 'Illuminate\Database\Eloquent\Attributes\WithoutTimestamps')) { | ||
| return null; | ||
| } | ||
|
|
||
| $timestampsProperty = $node->getProperty('timestamps'); | ||
| if ($timestampsProperty === null){ | ||
| return null; | ||
| } | ||
|
|
||
| $this->removePropertyFromClass($node, $timestampsProperty); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems wrong and should only happen if the attribute is added because the value is false. It just causes weird edge cases and extends the purepose of the rule beyond it's description. |
||
| if($this->isFalseProperty($timestampsProperty)){ | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Equally if you have the property you should be able to do |
||
| $this->addWithoutTimestampsAttribute($node); | ||
| } | ||
|
|
||
| return $node; | ||
| } | ||
|
|
||
| private function isAnEloquentModelClass(Class_ $class): bool | ||
| { | ||
| if ($class->extends === null) { | ||
| return false; | ||
| } | ||
|
|
||
| return $this->isName($class->extends, 'Illuminate\Database\Eloquent\Model') || $this->isName($class->extends, 'Model'); | ||
| } | ||
|
Comment on lines
+88
to
+95
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this can be replaced by a simple |
||
|
|
||
| private function isFalseProperty(Property $timestampsProperty): bool | ||
| { | ||
| if (count($timestampsProperty->props) === 0) { | ||
| return false; | ||
| } | ||
|
|
||
| $default = $timestampsProperty->props[0]->default; | ||
| if ($default === null) { | ||
| return false; | ||
| } | ||
|
|
||
| return $default instanceof ConstFetch && $this->isName($default->name, 'false'); | ||
| } | ||
|
|
||
| private function removePropertyFromClass(Class_ $class, Property $timestampsProperty): void | ||
| { | ||
| foreach ($class->stmts as $key => $stmt) { | ||
| if ($stmt === $timestampsProperty) { | ||
| unset($class->stmts[$key]); | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private function addWithoutTimestampsAttribute(Class_ $class): void | ||
| { | ||
| $class->attrGroups[] = new AttributeGroup([ | ||
| new Attribute(new FullyQualified('Illuminate\Database\Eloquent\Attributes\WithoutTimestamps')), | ||
| ]); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| <?php | ||
|
|
||
| namespace RectorLaravel\Tests\Rector\Class_\TimestampsPropertyToTimestampsAttributeRector\Fixture; | ||
|
|
||
| use Illuminate\Database\Eloquent\Model; | ||
|
|
||
| class TimestampsFalse extends Model | ||
| { | ||
| public $timestamps = false; | ||
| } | ||
|
|
||
| ?> | ||
| ----- | ||
| <?php | ||
|
|
||
| namespace RectorLaravel\Tests\Rector\Class_\TimestampsPropertyToTimestampsAttributeRector\Fixture; | ||
|
|
||
| use Illuminate\Database\Eloquent\Model; | ||
|
|
||
| #[\Illuminate\Database\Eloquent\Attributes\WithoutTimestamps] | ||
| class TimestampsFalse extends Model | ||
| { | ||
| } | ||
|
|
||
| ?> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| <?php | ||
|
|
||
| namespace RectorLaravel\Tests\Rector\Class_\TimestampsPropertyToTimestampsAttributeRector\Fixture; | ||
|
|
||
| use Illuminate\Database\Eloquent\Model; | ||
|
|
||
| class TimestampsTrue extends Model | ||
| { | ||
| public $timestamps = true; | ||
| } | ||
|
|
||
| ?> | ||
| ----- | ||
| <?php | ||
|
|
||
| namespace RectorLaravel\Tests\Rector\Class_\TimestampsPropertyToTimestampsAttributeRector\Fixture; | ||
|
|
||
| use Illuminate\Database\Eloquent\Model; | ||
|
|
||
| class TimestampsTrue extends Model | ||
| { | ||
| } | ||
|
|
||
| ?> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| <?php | ||
|
|
||
| namespace RectorLaravel\Tests\Rector\Class_\TimestampsPropertyToTimestampsAttributeRector\Fixture; | ||
|
|
||
| use Illuminate\Database\Eloquent\Model; | ||
|
|
||
| #[Illuminate\Database\Eloquent\Attributes\WithoutTimestamps] | ||
| class ExistingAttribute extends Model | ||
| { | ||
| } | ||
|
|
||
| ?> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| <?php | ||
|
|
||
| namespace RectorLaravel\Tests\Rector\Class_\TimestampsPropertyToTimestampsAttributeRector\Fixture; | ||
|
|
||
| use Illuminate\Database\Eloquent\Attributes\WithoutTimestamps; | ||
| use Illuminate\Database\Eloquent\Model; | ||
|
|
||
| #[WithoutTimestamps] | ||
| class ExistingFqnAttribute extends Model | ||
| { | ||
| } | ||
|
|
||
| ?> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| <?php | ||
|
|
||
| namespace RectorLaravel\Tests\Rector\Class_\TimestampsPropertyToTimestampsAttributeRector\Fixture; | ||
|
|
||
| class NotAModel | ||
| { | ||
| public $timestamps = true; | ||
| } | ||
|
|
||
| ?> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace PlayPlay\Rector\Tests\Rule\TimestampsPropertyToTimestampsAttributeRector; | ||
|
|
||
| use Iterator; | ||
| use PHPUnit\Framework\Attributes\DataProvider; | ||
| use Rector\Testing\PHPUnit\AbstractRectorTestCase; | ||
|
|
||
| final class TimestampsPropertyToTimestampsAttributeRectorTest extends AbstractRectorTestCase | ||
| { | ||
| public static function provideData(): Iterator | ||
| { | ||
| return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); | ||
| } | ||
|
|
||
| /** | ||
| * @test | ||
| */ | ||
| #[DataProvider('provideData')] | ||
| public function test(string $filePath): void | ||
| { | ||
| $this->doTestFile($filePath); | ||
| } | ||
|
|
||
| public function provideConfigFilePath(): string | ||
| { | ||
| return __DIR__ . '/config/configured_rule.php'; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| use Rector\Config\RectorConfig; | ||
| use RectorLaravel\Rector\Class_\TimestampsPropertyToTimestampsAttributeRector; | ||
|
|
||
| return static function (RectorConfig $rectorConfig): void { | ||
| $rectorConfig->rules([ | ||
| TimestampsPropertyToTimestampsAttributeRector::class, | ||
| ]); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also changed these to live in their own config instead of laravel130.php