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

afterCreating method does not accept Closure(Model) #1131

Closed
Batisska opened this issue Feb 15, 2022 · 9 comments · Fixed by #1195
Closed

afterCreating method does not accept Closure(Model) #1131

Batisska opened this issue Feb 15, 2022 · 9 comments · Fixed by #1195

Comments

@Batisska
Copy link

Batisska commented Feb 15, 2022

  • Larastan Version: 2.0.0
  • --level used: 8

Description

Every time I call afterMaking or afterCreating phpstan crashes with an error.

Laravel code where the issue was found

    /**
     * @return TaskFactory
     */
    public function configure(): TaskFactory
    {
        return $this->afterCreating(function (Task $task) {
            $task->assigned()->attach(User::factory()->count(5)->create());
            $task->files()->attach(File::factory()->count(5)->create());
        });
    }
Parameter #1 $callback of method Illuminate\Database\Eloquent\Factories\Factory<Illuminate\Database\Eloquent\Model>::afterCreating() expects 
    Closure(Illuminate\Database\Eloquent\Model): mixed,
    Closure(App\Data\Models\Task): void given.    
@Batisska Batisska changed the title Phpstan crashes afterCreation in factory Phpstan found error in factory Feb 15, 2022
@szepeviktor
Copy link
Collaborator

Hello Сергей! 👋

Thank you for your report.
I see two possible solutions.
Does Task extend Model?
What if you return something?

@szepeviktor szepeviktor changed the title Phpstan found error in factory afterCreating method does not accept Closure(Model) Feb 15, 2022
@Batisska
Copy link
Author

Batisska commented Feb 15, 2022

Yes Task extends the Model.

If you return the Task, the same error.

@szepeviktor
Copy link
Collaborator

Thank you!
It is a bug.

@szepeviktor szepeviktor added bug Something isn't working and removed bug Something isn't working labels Feb 15, 2022
@szepeviktor
Copy link
Collaborator

szepeviktor commented Feb 15, 2022

No. We need generics!

/**
 * @extends \Illuminate\Database\Eloquent\Factories\Factory<\App\Data\Models\Task>
 */
class TaskFactory extends Factory
{

But I cannot get it working.

@Batisska
Copy link
Author

Batisska commented Feb 15, 2022

No. We need generics!

/**
 * @extends \Illuminate\Database\Eloquent\Factories\Factory<\App\Data\Models\Task>
 */
class TaskFactory extends Factory
{

But I cannot get it working.

What information can I provide you? To understand where the bug crept in? Thank you.

Generic did not help.

@szepeviktor
Copy link
Collaborator

@canvural Can you help us? We are a bit lost.

@canvural
Copy link
Collaborator

Hello,

The types are coming from Laravel itself. So I suggest opening an issue there.

A solution would be to change the type here to just \Closure(TModel): mixed Since TModel is bound to EloquentModel anyway, there is no need of an union type. I think that would resolve the issue.

Also instead of an issue you can directly create a PR there. Feel free to link this issue or this comment.

Thanks.

@mdpoulter
Copy link
Contributor

@canvural It looks like Laravel won't be making that change in the near future, as the delay is IDE support (laravel/framework#41286). Is this something which Larastan can support in the meantime?

@canvural
Copy link
Collaborator

@mdpoulter Yeah, I guess we can do that. It should be added to this stub file.

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 a pull request may close this issue.

4 participants