Add support for generic CallableType. #2938
Conversation
| You've opened the pull request against the latest branch 1.11.x. If your code is relevant on 1.10.x and you want it to be released sooner, please rebase your pull request and change its target to 1.10.x. |
There was a problem hiding this comment.
not sure at all about this but wasn't sure of an alternative
There was a problem hiding this comment.
Yeah, I'd do the same thing here, if it doesn't work you have to debug why :)
The transformation happens in CallableType::traverse(). So basically it should be automatic.
You should also pass TemplateTypeMap into ClosureType.
As an exercise you should make the CallableType constructor parameters required, and make sure they're passed in everywhere it's relevant. Most importantly in all new self(...) instances inside the class.
What gives me a little pause is $this->resolvedTemplateTypeMap. I'm not sure what should happen there. Maybe we can just continue using createEmpty there.
| The main workflow that influences generics is through ParametersAcceptorSelector, GenericParametersAcceptorResolver specifically. So the code that resolves these types will always go through there, so you can debug what's going on. |
| I realized that ClosureType already supports generics (https://phpstan.org/r/eebaf81b-333c-4e96-97fb-801436690236) so if something does not work for CallableType you can debug what's going on in ClosureType and get inspired by that :) |
mad-briller force-pushed the generic-callables branch from abaea93 to f5d6c34 Compare 
| thanks for your wisdom there @ondrejmirtes it guided me to where to begin looking, and great tip about |
| This pull request has been marked as ready for review. |
mad-briller force-pushed the generic-callables branch from f5d6c34 to e663838 Compare There was a problem hiding this comment.
We also need to catch the things MethodTagValueNode catches for these types in these places:
- Method parameters
- Method
@param-out - Method return types
- Function parameters
- Function
@param-out - Function return types
- Property types
- Inline
@vars - All the class-level PHPDocs like
@methodand@property
You can write the common logic in a rule helper that you'll inject into these rules:
* IncompatiblePhpDocTypeRule (covers everything for functions and methods)
- IncompatiblePropertyPhpDocTypeRule
- IncompatibleSelfOutTypeRule
- InvalidPhpDocVarTagTypeRule
- MethodTagTemplateTypeRule (maybe rename this one)
- a new rule for property tags - nothing is currently checked about them!
This could also be an opportunity to check more about @method parameters and return types, like classes that do not exist (this is typically done in ExistingClassesIn* rules), unresolvable types etc.
Do you want to do this here or in a follow-up PR? :) Thanks.
| Also please inspect the failures in https://github.com/phpstan/phpstan-src/actions/runs/8031291020, they look very interesting! |
| Oh sorry, most of them are fault of a previous PR :) |
yeah i think the rector ones are caused by this pr so i'll look into them when i get more time; working on this has cooked my brain so i need a break! |
mad-briller force-pushed the generic-callables branch from e663838 to 3ba9e29 Compare 
mad-briller force-pushed the generic-callables branch from 3ba9e29 to b31a746 Compare | The rector failures are caused by the action installing https://github.com/rectorphp/rector-src/blob/main/composer.json#L26 is it worth me handling this by adding a |
| */ | ||
| function testNestedClosures(Closure $closure, string $str, int $int): void | ||
| { | ||
| assertType('Closure<TRetFirst of mixed>(TRetFirst): (Closure<TRetSecond of mixed>(TRetSecond $valtwo): (TRetFirst|TRetSecond))', $closure); |
There was a problem hiding this comment.
the return type Closure does not have the parameter names removed as the removal is done in describe() not toPhpDocNode(), and describe is only called for the top-level. not sure if this is intended or not so thought i'd mention it
| i can investigate adding these in a follow up pr if you'd like, though i'm a bit overwhelmed by the list 😅 |
| This pull request has been marked as ready for review. |
| About #2938 (review) - yeah, sure, this can be addressed in multiple small follow-up PRs :) I'd really appreciate if you continued working on this :) (I have even more similar stuff for you if you want :)) |
| Thank you! |
3 participants
Trying to follow up the
phpdoc-parserpr with implementing generic callables and closures. About as far as i can get tonight so putting this up to get some feedback; not sure if this is the right direction at all, just trying to apply what i learned from the other prs.Right now the test is outputting:
which seems somewhat familiar to the
$nameScopething you mentioned in the generic@methodpr, but i am calling->withTemplateTypeMap()like you suggested over there.One major thing that i can't seem to find is where the template types are replaced with the actual real types; as i feel like with the addition of
$resolvedTemplateTypeMaponCallableTypei'm going to have to do that somewhere to then pass in the resolved type map, but where i should do that escapes me.