Skip to content

Fix tests broken in https://github.com/phpstan/phpstan-src/pull/231#236

Closed
rainbow-alex wants to merge 1 commit intophpstan:masterfrom
rainbow-alex:fix/PR231-tests
Closed

Fix tests broken in https://github.com/phpstan/phpstan-src/pull/231#236
rainbow-alex wants to merge 1 commit intophpstan:masterfrom
rainbow-alex:fix/PR231-tests

Conversation

@rainbow-alex
Copy link
Copy Markdown
Contributor

These make the tests green, but I'm not sure you want to fix it this way.

@ondrejmirtes
Copy link
Copy Markdown
Member

We probably need to continue to support ['Foo', 'doBar'] style of callables :/ I'l look into a possible fix of your issue, probably by modifying DependencyResolver.

Could you try to revert all your changes from #231 and modify DependencyResolver around line 160 so that it calls ->isCallable() only when the first array item is ClassConstFetch with ::class? Thanks.

@rainbow-alex
Copy link
Copy Markdown
Contributor Author

Yeah, I think there are probably still a lot of legit 'Class\\String' literals out there. But if we change DependencyResolver, won't it just move the problem? I don't understand the internals as well as you do, but I'm guessing something is still going to use that resolved type and also expect ['Closure', 'bind'] to be callable, right?

@ondrejmirtes
Copy link
Copy Markdown
Member

If someone else asks about the type being isCallable() then yes but at that point you're probably trying to use the array as an actual callable.

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.

2 participants