Skip to content

Conversation

Neol3108
Copy link

This is my attempt at a *DynamicParameterTypeExtension, I'm missing some tests for non-closures/arrow-functions, but I'm not sure how to test this. My knowledge of PHPStan internals is quite limited.

My goal is to "fix" the types dynamically for Laravel's with Builder method. I think this should allow me to do that, I'm unsure as to how to test my new code in my extension, requiring phpstan/phpstan-src (with custom repository to this branch) doesn't seem to work.

@ondrejmirtes
Copy link
Member

I believe we don't need this since we already have an extension for this purpose: https://apiref.phpstan.org/2.1.x/PHPStan.Type.MethodParameterClosureTypeExtension.html

If your use-case is different please describe it.

Also AFAIK Larastan is already attempting to fix these Builder methods.

@Neol3108
Copy link
Author

Neol3108 commented Feb 13, 2025

Hi @ondrejmirtes, please refer to your comment here: phpstan/phpstan#11707 (comment)

The problem with Laravel's with method is that it's an array of closures, instead of a single closure as a parameter. This is a more generic approach to the MethodParameterClosureTypeExtension

@Neol3108
Copy link
Author

I think this might not work after all, added a failing test mirroring what I wanted to fix. If anyone could help out that would be awesome

@calebdw
Copy link
Contributor

calebdw commented Feb 16, 2025

@ondrejmirtes, this is needed in order to replace other parameter types besides Closures (like an array of closures). I had started working on a PR similar to this but then I got busy with other things.

@Neol3108, per the linked comment, the existing *ClosureTypeExtensions should be deprecated. You can also take a look at 2.1.x...calebdw:phpstan-src:dynamic_parameter_extension for the implementation I started

@Neol3108
Copy link
Author

@calebdw yeah I was planning on adding that deprecation but I first wanted to make sure this works. As of now I only got it to work replacing a single closure (not sure how to test non-closure types) not an array of closures. See my last commit for reference.
Did your version work?

@calebdw
Copy link
Contributor

calebdw commented Feb 17, 2025

@Neol3108, it's been a while, but I didn't get general parameter replacement working

@ondrejmirtes, do you have any insight into where general parameter replacement would need to take place?

@Neol3108
Copy link
Author

Closing this as I have no idea how or time to fix this

@Neol3108 Neol3108 closed this Mar 19, 2025
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.

3 participants