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

Attempt to fix #6937 #6963

Merged
merged 1 commit into from Nov 23, 2021
Merged

Attempt to fix #6937 #6963

merged 1 commit into from Nov 23, 2021

Conversation

ptomulik
Copy link
Contributor

@ptomulik ptomulik commented Nov 22, 2021

Just a blind shot, prehaps completely wrong. But maybe a good starting point for discussion.

@orklah orklah linked an issue Nov 22, 2021 that may be closed by this pull request
@orklah
Copy link
Collaborator

orklah commented Nov 22, 2021

Seems like it solves the issue without adding more, that's already a huge win!

I don't have much experience in traits handling with Psalm, mind explaining a little what you did in there so we can have an idea of what's going on?

@orklah orklah added the release:fix label Nov 22, 2021
@ptomulik ptomulik force-pushed the issue-6937 branch 2 times, most recently from f191558 to 4bc2628 Compare Nov 22, 2021
@ptomulik
Copy link
Contributor Author

ptomulik commented Nov 22, 2021

The patch is more an effect of experimenting with the example from the original issue than a consciously elaborated solution. I've observed, than for the class Test involving two traits as follows

class Test {
    /** @template-use AggregateTrait<int> */
    use AggregateTrait; // implements getValue()
    
    /** @template-use AnotherTrait<int> */
    use AnotherTrait; // declares abstract getValue()
    
    public function __construct() {
        $this->value = 123;
    }

the method compareMethodDocblockReturnTypes gets called twice. In the first call

$implementer_classlike_storage->name is "AggregateTrait"
$guide_classlike_storage->name is "AnotherTrait"
$implementer_classlike_storage->is_trait is true

And this passes without any problems. But in the second call

$implementer_classlike_storage->name is "Test"
$guide_classlike_storage->name is "AnotherTrait"
$implementer_classlike_storage->is_trait is false

and this fails. It seems that in the second call psalm treated our getValue() method as it was implemented by Test class, not by AggregateTrait. The template parameters were not applied (transformed) and the return type of getValue() was inferred to be a generic T:AggregateTrait instead of int.

What I proposed here is to apply the template parameters (int) to use AggregateTrait in the second call, when the "implementer classlike" is not a trait.

@orklah
Copy link
Collaborator

orklah commented Nov 23, 2021

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants