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

Disallow extra parameters for callables #2397

Draft
wants to merge 1 commit into
base: 1.10.x
Choose a base branch
from

Conversation

enumag
Copy link
Contributor

@enumag enumag commented May 9, 2023

Attempting to fix phpstan/phpstan#9248

Should this be bleeding-edge only error for now?

@phpstan-bot
Copy link
Collaborator

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.

@enumag enumag changed the base branch from 1.11.x to 1.10.x May 9, 2023 07:27
@ondrejmirtes
Copy link
Member

As you can see from the CI results this isn't the right fix. This is totally OK to do for situations where it doesn't cause fatal errors. A callable might be called with a parameter but the passed callable might not be interested in that parameter.

Instead, we need to somehow remember when a callable is created from an internal function affected by this behaviour. This includes strlen(...) (first-class callable - ClosureType) and also 'strlen' - ConstantStringType. And then use this information in CallableTypeHelper.

We might need to require ParametersAcceptorWithPhpDocs instead of ParametersAcceptor in CallableTypeHelper. Think of ParametersAcceptorWithPhpDocs as ExtendedParametersAcceptor, similar to ExtendedMethodReflection. And add a relevant method on ParametersAcceptorWithPhpDocs so that we can ask about this.

@enumag
Copy link
Contributor Author

enumag commented May 9, 2023

Yeah, I expected something like that. That's why I marked it as a draft.

CallableTypeHelper is used in CallableType and ClosureType.

I believe I only need ParametersAcceptorWithPhpDocs for the $theirs argument which is the passed callback function. But how can I obtain ParametersAcceptorWithPhpDocs implementation instead of ParametersAcceptor in those two places?

EDIT: It seems like both ClosureType and CallableType would need to implement ParametersAcceptorWithPhpDocs which won't be easy considering all the places where they can be created.

@ondrejmirtes
Copy link
Member

Your EDIT is right.

which won't be easy considering all the places where they can be created

Exactly, these are the places that need to pass in the information about the function being internal or not.

@enumag
Copy link
Contributor Author

enumag commented May 9, 2023

Exactly, these are the places that need to pass in the information about the function being internal or not.

Right now I'm more concerned about how to correctly handle the extra data which ParametersAcceptorWithPhpDocs already has since correctly handling that is a pre-requisite:

  • more restrictive getParameters()
  • getPhpDocReturnType()
  • getNativeReturnType()

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.

Unreported fatal error with first-class callable
3 participants