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

Static call to instance method error should not happen if class implements __callStatic method #3641

Closed
canvural opened this issue Jul 20, 2020 · 6 comments

Comments

@canvural
Copy link
Contributor

Feature request

For the given code:

<?php declare(strict_types = 1);

class Foo
{
	public function bar(): int
	{
		return 5;
	}
}

/**
 * @mixin Foo
 */
class Bar
{
	/**
     * @param  mixed[]  $args
     * @return mixed
     */
	public static function __callStatic(string $method, $args)
    {
        $instance = new Foo;

        return $instance->$method(...$args);
    }
}

Bar::bar();

https://phpstan.org/r/856294ef-b9e3-4d5f-94d9-2dcae85f72cb

I'd expect no errors. (https://3v4l.org/MRhFu)

@ondrejmirtes
Copy link
Member

Hi, this is some really crazy code. I'm reluctant to just if this situation out in the CallMethodsRule because the existence of __callStatic doesn't guarantee the method can be called - it can be there just to give a nicer exception, like here:

https://github.com/consistence/consistence/blob/49786347a96696337591f14f26dabf46d1e69fd8/src/Type/ObjectMixinTrait.php#L24-L36

I'd advise you to fix your code so that it doesn't rely on this hack, or just @phpstan-ignore-line this.

@canvural
Copy link
Contributor Author

Hi, this is some really crazy code.

It's from Laravel 😅 This kind of pattern is popular in Laravel. Calling static methods on facades, and then facade forwards the call to the underlying instance via __callStatic

because the existence of __callStatic doesn't guarantee the method can be called

It's not just about the __callStatic In my example, Bar class has mixin Foo. So CallStaticMethodsRule knows this method exists via a mixin, and it can check if __callStatic is implemented.

I'm trying to add support to some facades in Larastan. And just creating a stub for the facade, and adding the correct @mixin annotation to it, would solve the problem easily. Otherwise, I have to create an extension for every facade.

@ondrejmirtes ondrejmirtes reopened this Jul 21, 2020
@ondrejmirtes
Copy link
Member

Oh, I looked at the example more closely and realized I misread it earlier today. I thought the problem was that there’s both method “bar()” and “__callStatic” on Bar, that would be far more problematic.

The current code example can probably be solved. I’d just prefer if there was a different a different annotation like @staticMixin that’s supposed to be used with __callStatic()...

@ondrejmirtes
Copy link
Member

Fixed: phpstan/phpstan-src@aa38695

Please test the dev-master of phpstan/phpstan in 15 minutes to verify that this works in Laravel :) Thanks!

@canvural
Copy link
Contributor Author

Yes, it works now! Thank you!

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants