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

Remove the model property from Factories and add extends annotation #128

Conversation

GeniJaho
Copy link
Collaborator

@GeniJaho GeniJaho commented Aug 18, 2023

The $model property is not needed anymore starting from Laravel 8 laravel/framework#39310, as the Factory can find the name of the Model to create in other ways.

In Laravel 9, however, they made the factories generic laravel/framework#39169. It allows for much better static analysis and autocompletion in tests.

This PR adds a new rule to the Laravel 8.0 set that removes the $model property from all factories and adds the @extends annotation to the Factory with the removed $model value. Users can only reap the benefits of the annotation starting from Laravel 9.0.

The reason I did not add an extra rule to add the annotation separately is that we don't have a way to correctly determine the name of the model to use. We can add the annotation safely only if we get it from the user-defined $model value.


Update

Removing the $model property actually works only if the Models are in the App\Models or App namespace, so removing them by default doesn't make much sense. I will make two separate rules in this case:

  • Rule to remove the $model property, outside of any set list.
  • Rule to add the @extends annotation if there is a $model property defined, which will be part of the Laravel 9 set list.

@GeniJaho GeniJaho changed the title Remove the model property and add the extends annotation with the model name Remove the model property from Factories and add extends annotation Aug 18, 2023
@GeniJaho GeniJaho force-pushed the move-factory-model-property-to-extends-annotation branch from 41c18d7 to a66d7fe Compare August 18, 2023 19:12
@driftingly driftingly merged commit e4a442e into driftingly:main Aug 20, 2023
4 checks passed
@GeniJaho GeniJaho deleted the move-factory-model-property-to-extends-annotation branch April 11, 2024 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants