Skip to content

Conversation

VincentLanglet
Copy link
Contributor

@VincentLanglet VincentLanglet commented Jul 21, 2021

Without the fix I get

------ ----------------------------------------------------------------------------------------------------------------------------------------------------- 
  Line   bug-5336.php                                                                                                                                         
 ------ ----------------------------------------------------------------------------------------------------------------------------------------------------- 
  23     Property Bug5336\Pager::$query is never read, only written.                                                                                          
  50     Property Bug5336\Test::$pager (Bug5336\Pager<Bug5336\ProxyQueryInterface&Bug5336\Stub>) does not accept Bug5336\Pager<Bug5336\ProxyQueryInterface>.  
  51     Expected type Bug5336\Pager<Bug5336\ProxyQueryInterface&Bug5336\Stub>, actual: Bug5336\Pager<Bug5336\ProxyQueryInterface>                            
 ------ ----------------------------------------------------------------------------------------------------------------------------------------------------- 

on the file.

With the fix I get

------ ------------------------------------------------------------- 
  Line   bug-5336.php                                                 
 ------ ------------------------------------------------------------- 
  23     Property Bug5336\Pager::$query is never read, only written.  
 ------ ------------------------------------------------------------- 

The fix I made in IntersectionType::inferTemplateTypesOn might be improved...
More important, I don't know if the fix should be added to IntersectionType::inferTemplateTypes too (and what's the diff between both methods).

Close phpstan/phpstan#5336

@VincentLanglet
Copy link
Contributor Author

I rebased on master @ondrejmirtes :)

@VincentLanglet
Copy link
Contributor Author

Friendly ping @ondrejmirtes

@ondrejmirtes
Copy link
Member

Hi, I don't feel like this fix is right, the $inferred->isEmpty() condition is weird and I don't know how it's justified. But for me I'd have to think long and hard about this and it's not in my current priorities. Thanks for understanding.

@VincentLanglet
Copy link
Contributor Author

Hi, I don't feel like this fix is right, the $inferred->isEmpty() condition is weird and I don't know how it's justified.

There is an intersection type

Bug5336\ProxyQueryInterface & Bug5336\Stub

on which the call

inferTemplateTypesOn

is made with

T of Bug5336\ProxyQueryInterface (class Bug5336\Pager, parameter)

This call is doing

$templateType->inferTemplateTypes($type)

on each type of the intersection.

Then

(T of Bug5336\ProxyQueryInterface)->inferTemplateTypes(Bug5336\ProxyQueryInterface) 

is Bug5336\ProxyQueryInterface, but

(T of Bug5336\ProxyQueryInterface)->inferTemplateTypes(Bug5336\Stub) 

Is empty

This is the empty check I do. In order to add again the type Bug5336\Stub.
When a type has nothing in common with the template type, it just need to be fully keep as an extra condition.

@arnaud-lb
Copy link
Contributor

@VincentLanglet this is what I'm seeing as well: The intersection members that are not a sub type of the template bound are rejected by T->inferTemplateTypes(), so we end up with just ProxyQueryInterface.

I agree that it would be useful to not reject the extra types.

I can see the following issues with the current fix:

@ondrejmirtes
Copy link
Member

To me it also looked like the current logic in this PR isn't sufficient, thanks.

@VincentLanglet
Copy link
Contributor Author

To me it also looked like the current logic in this PR isn't sufficient, thanks.

No problem.

I can see the following issues with the current fix:

I think that there are cases where isEmpty() will not be enough to detect this. I was not able to find a case demonstrating that, though.
If the intersection does not contain any compatible type, it may result in invalid type inference: https://phpstan.org/r/babfd6ef-222c-4ff5-bd97-7f3d2ec02ef6
It doesn't handle nested template types: https://phpstan.org/r/15e4e15e-0999-409c-ba64-8728468c2cce

Any idea what would be the right fix @arnaud-lb ?

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.

Issue with generics and intersection
3 participants