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

Nested SoftDelete not detected since 0.7.5 #837

Closed
iamrgroot opened this issue May 27, 2021 · 12 comments
Closed

Nested SoftDelete not detected since 0.7.5 #837

iamrgroot opened this issue May 27, 2021 · 12 comments
Labels
enhancement New feature or request

Comments

@iamrgroot
Copy link
Contributor

iamrgroot commented May 27, 2021

Description

Larastan does not detect a nested SoftDeletes trait after updating from 0.7.4 to 0.7.5 or higher.

Laravel code where the issue was found

class User extends Authenticatable
{
    public function group(): BelongsTo
    {
        return $this->belongsTo(Group::class)->withTrashed(); // Call to an undefined method Illuminate\\Database\\Eloquent\\Relations\\BelongsTo<App\\Group, App\\User>::withoutTrashed().
    }
}

class Group extends Model
{
    use NestedSoftDeletes;
}

trait NestedSoftDeletes
{
    use SoftDeletes;
}
@canvural
Copy link
Collaborator

canvural commented May 27, 2021

Hi,

We are using the PHPStan's MethodReflection method hasTraitUse here to determine if a class uses the given trait. Looks like this needs to recursive. But I don't know if it's possible.

Please feel free to create a PR that fixes it in the code I pointed out. Also you can ask in PHPStan if you need help with how to solve this.

@szepeviktor
Copy link
Collaborator

@ondrejmirtes Could you help us whether hasTraitUse is recursive?

@ondrejmirtes
Copy link
Contributor

Try using ClassReflection::getTraits() instead, or send a fix to PHPStan :)

@canvural canvural added the help wanted Extra attention is needed label May 27, 2021
@iamrgroot
Copy link
Contributor Author

@canvural I've added a function in #838 that collects nested traits as the PHPStan's collectTraits function is private. Let me know if this is acceptable.

@canvural
Copy link
Collaborator

canvural commented Jun 8, 2021

@iamrgroot Did you try ClassReflection::getTraits() first as @ondrejmirtes suggested?

@iamrgroot
Copy link
Contributor Author

@canvural Yes!

ClassReflection::getTraits gives App\Group: {"App\\Traits\\NestedSoftDeletes":{}}

(new) collectTraitNames gives App\Group: ["App\\Traits\\NestedSoftDeletes","Illuminate\\Database\\Eloquent\\SoftDeletes"]

See the ClassReflection::collectTraits private function. One could send a PR to make it public, but until then you would need to use your own function.

@canvural
Copy link
Collaborator

I'm not sure if it's the best thing to duplicate collectTraits method in Larastan. @ondrejmirtes ClassReflection::getTraits() does not look like it's recursive, as @iamrgroot pointed out. Would you be fine with a PR that is making collectTraits public, or making hasTraitUse use collectTraits, or do you have any other idea for a solution?

@ondrejmirtes
Copy link
Contributor

I'm fine with changing getTraits() to be recursive, as getInterfaces already are...

@canvural
Copy link
Collaborator

@iamrgroot You can send a PR to PHPStan repo first that changes getTraits to be recursive. Then we can use it here, to solve the issue.

@szepeviktor
Copy link
Collaborator

szepeviktor commented Jun 18, 2021

https://github.com/nunomaduro/larastan/blob/f74befd87b7278285cbf6f1597509cbfaca46bf8/src/Methods/Pipes/Macros.php#L27

So we should add a true parameter and require PHPStan v0.12.90?

@ondrejmirtes
Copy link
Contributor

PHPStan 0.12.90 has just been released - you can now call ClassReflection::getTraits(true) and get traits recursively.

@canvural
Copy link
Collaborator

Closed with #838

@canvural canvural added enhancement New feature or request and removed Waiting for feedback labels Jun 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants