Skip to content

apply parameter closure type extension in FunctionCallParametersCheck #3347

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

Conversation

schlndh
Copy link
Contributor

@schlndh schlndh commented Aug 25, 2024

@ondrejmirtes
Copy link
Member

I thought about this. I thought I wanted a different solution using virtual nodes but it'd need a lot of refactoring. And maybe it'd still be very hard.

For similar reasons I already added $scope->pushInFunctionCall() in the same Check class.

So feel free to pursue the same direction you started 😊

@schlndh schlndh force-pushed the feature-applyParameterClosureTypeExtensionInFuncCallCheck branch 2 times, most recently from ee278fc to 3d593f5 Compare August 26, 2024 16:03
@schlndh schlndh force-pushed the feature-applyParameterClosureTypeExtensionInFuncCallCheck branch from 74e9ac6 to 1665c48 Compare August 30, 2024 10:38
@schlndh schlndh changed the base branch from 1.11.x to 1.12.x August 30, 2024 10:38
@schlndh schlndh marked this pull request as ready for review August 30, 2024 11:03
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@schlndh schlndh force-pushed the feature-applyParameterClosureTypeExtensionInFuncCallCheck branch from 7fda20c to f63df3b Compare September 13, 2024 10:19
@ondrejmirtes ondrejmirtes force-pushed the feature-applyParameterClosureTypeExtensionInFuncCallCheck branch from f63df3b to b308008 Compare May 5, 2025 08:44
@ondrejmirtes
Copy link
Member

I've looked at this and here are my thoughts: I don't like that FunctionCallParametersCheck::check() gains full reflection as another parameter. This isn't how I would design this. There are enough things already passed from reflection that this change really feels like it's bolted on to achieve a single goal.

Since the goal is to basically change $parameter from already passed $parametersAcceptor, I feel like we should modify $parametersAcceptor before it's passed to the check() method.

So the rules that call FunctionCallParametersCheck should modify ParametersAcceptor before it's passed there.

@ondrejmirtes
Copy link
Member

Alright, I see this is just an alternative to #3282. I will merge that instead. Thank you.

@schlndh schlndh deleted the feature-applyParameterClosureTypeExtensionInFuncCallCheck branch May 5, 2025 12:57
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.

preg_replace_callback doesn't account for flags in callback signature
3 participants