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

calling static methods on strings #504

Closed
SignpostMarv opened this issue Sep 16, 2017 · 7 comments
Closed

calling static methods on strings #504

SignpostMarv opened this issue Sep 16, 2017 · 7 comments
Milestone

Comments

@SignpostMarv
Copy link
Contributor

<?php declare(strict_types = 1);

interface foo
{
	public static function bar();
}

function baz1(string $thing)
{
	$thing::bar();
}

function baz2(string $thing)
{
	if (is_a($thing, foo::class, true)) {
		$thing::bar();
	}
}

function baz3(string $thing)
{
	if (is_a($thing, foo::class, true) === false) {
		throw new Exception('fail');
	}
	
	$thing::bar();
}

ideally, this should only cause an error in the case of baz1, since there's nothing in particular to indicate that $thing is an implementation of foo

@SignpostMarv
Copy link
Contributor Author

the workaround on 74c3cf8 seems to be to @var foo $thing then (string) $foo for any function that requires a string parameter.

@ondrejmirtes
Copy link
Member

There are two things going on at once:

  1. There's currently a bug on dev-master because PHPStan thinks that static methods cannot be called on a string, which isn't true.
  2. There's no type-specifying code when is_a is used, unlike for is_int (and other functions) and instanceof keyword which ARE supported. So PHPStan does not recognize your type check when calling is_a.

About 1), I'm torn whether PHPStan should complain when a method is called on a string without any is_a check, forcing the developer to have a sanity check:

public function doFoo(string $className)
{
    $className::doSomething(); // dangerous, report Cannot call static method doSomething() on string.
}

versus:

public function doFoo(string $className)
{
    if (!is_a($className, DoesSomething::class, true)) {
        throw new \InvalidArgumentException));
    }
    $className::doSomething(); // this is fine
}

It probably should report that, but the error message is not a convenient place to explain that the developer should use is_a check beforehand.

About 2) - I will try to implement this and I hope the current type system will allow this. If not, it will have to be postponed until later. If everything goes well, both 1) and 2) will be adressed at once.

@ondrejmirtes ondrejmirtes added this to the 0.9 milestone Sep 16, 2017
@SignpostMarv
Copy link
Contributor Author

no rush on it, although mildly inellegant, the workaround to (string) $className after * @var SomeClass $className works for now, the static analysis I've got running doesn't complain about typecasting to string something that's already string :)

p.s. see the changes here corresponding to (mostly phpstan) today's update on static analysis.

@ondrejmirtes
Copy link
Member

Fixee the "cannot call on string" problem: a7c931c

I'm removing the "0.9" milestone and leaving this open for the is_a support.

@ondrejmirtes ondrejmirtes removed this from the 0.9 milestone Oct 6, 2017
@JanTvrdik
Copy link
Contributor

@ondrejmirtes Removing the milestone seems unnecessary as is_a is already implement as part of #507

@ondrejmirtes ondrejmirtes added this to the 0.9 milestone Oct 6, 2017
@ondrejmirtes
Copy link
Member

@JanTvrdik Sorry, didn't notice that, the milestone is back :)

@ondrejmirtes
Copy link
Member

#507 got merged so this issue should be completely solved. Feel free to test dev-master.

@lock lock bot locked as resolved and limited conversation to collaborators Dec 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants