Add ability to type cast objects.

[Fuco1]

Hi.

In languages like Java you can cast objects at runtime with the "usual" cast syntax ClassA a = (ClassA) new ClassB(); This syntax is not recognized in php, but sometimes runtime casts are necessary when the system can't properly figure out the types.

One instance where this is extra painful is the (arguably bad) service locator pattern. Often you have some "module" hierarchy where every manager/worker class extends some interface or abstract class and the best you can do with the "getService" method on the service locator is to return that interface... but it is never the correct type!

Example code:

class Service { }

class A extends Service { }

class B extends Service { }

class Code {

    public function main() {
        $service = $this->getService('A'); // returns 'Service'
        $this->test($service); // call fails to check even if $service is actually 'A'
    }

    /**
     * @return Service superclass
     */
    public function getService($type) { return new $type(); }

    public function test(A $a) { }
}

I propose an extension to syntax like this

$super = /*(A)*/ $this->getService('A');

The "commented cast" will alter the type of the following expression, in this case the method call.

This is unsafe (the sevice returned might not be A and it is impossible to know), so in case of e.g. Java a warning is generated by the compiler which has to be suppressed manually. Here it is not required because this is a deliberate annotation and so the programmer probably knows why they put it in.

This is very helpful on older projects littered with explicit calls to service locators or containers (I happen to work on such a codebase) and would allow tons of type checking which is simply impossible now.

I've tried to hack a bit on Scope.php's method getType to add the support and it doesn't seem that difficult (basically check any Expr to see if it has a comment attached with this particular syntax and then return the type directly), but I'm failing to figure out how to do namespace/uses resolution so that I don't have to always provide the full type.

Here is the code I've put in and it works, but as you can see I check for MethodCall explicitly (which is only one of many Exprs). This is because php-parser attaches the comment to all subnodes and not just the parent node, which in this case would for example also cast $this to A and then complain A has no such method as test). There is an issue opened for that: nikic/PHP-Parser#253.

if ($node instanceof Expr\MethodCall && isset($node->getAttributes()['comments'])) {
    $name = $node->getAttributes()['comments'][0]->getText();
    if (preg_match('#/\*\(.*?\)\*/#', $name)) {
        $type = substr($name, 3, -3);
        // this is not correct, I guess there is some "resolver" from string to type
        return new ObjectType($type, false);
    }
}

I will happily hack on this if someone can provide guidance on how to resolve the types/namespaces/renames.

[hkdobrev]

The syntax accepted by IDEs and other tools is the following:

/** @var FooBar */
$instance = $a->getFoo();

Not entirely sure if PHPStan accepts this notation, but it might.

[ondrejmirtes]

Yes, that's what I wanted to write - specifying types and casting to different types can be currently done with @var. All following syntax is supported:

/** @var Foo $foo */
$foo = doFoo();

/** @var $foo Foo */
$foo = doFoo();

/* @var Foo $foo */
$foo = doFoo();

/* @var $foo Foo */
$foo = doFoo();

The only limitation is that the doc comment has to be written above the assignment of the same variable.

[hkdobrev]

@ondrejmirtes I thought the name of the variable should be omitted in inline comments like this.

[Fuco1]

This only works for assignment though, not arbitrary expressions.

Does PHPStan recognize the assignment syntax above already? We tried it and it didn't work, but we're not on the bleeding edge.

[ondrejmirtes]

@Fuco1 The code example I've written in my previous response works for a few major versions already. As for omitting the variable name, I looked into the phpDocumentor documentation and there's this:

The @var tag MUST contain the name of the element it documents. An exception to this is when property declarations only refer to a single property. In this case the name of the property MAY be omitted.

I suppose this means when you're assigning only a single variable, I could use @var type info even without the variable name. I'll look into that for the next version.

[Fuco1]

Another issue I found is when the variable you assign to is a property

$this->db = $servicelocator->getService('db');

The above solution doesn't work for me, one has to write

/* @var $db CDb */
$db = $servicelocator->getService('db');
$this->db = $db;

So I think an ability to typecast an expression would still be valuable.

[ondrejmirtes]

You can write a dynamic return type extension for the getService method that would look up the correct type of the service.

[Fuco1]

Writing an extension for my particular software would only solve my problem, it would hardly be usable for others.

I'm getting a vibe you don't particularly like this proposal, which I don't quite understand. Would you mind sharing some thought behind this? Is it simply too difficult/unfeasable to implement or is there some deeper reason why we should not want this?

[ondrejmirtes]

@Fuco1 OK, so this requires full-fledged response :)

My responses are often short because they are typed on a smartphone keyboard while I'm on the go. I prioritize giving you a response as quickly as possible so your problem is resolved and you can do your next steps :) If it sounded rude to you, that wasn't absolutely my intent. I'm trying to handle all feedback exemplarily as I'd like other OSS projects to handle my feedback. If you'd rather want a longer response but later, well, that's what I'm doing now :)

I told you that you should write dynamic return type extension for your getService locator call, bercause, in my opinion, that's the right solution and these extensions were designed exactly for this scenario. Using this extension would automatically cover your whole codebase without the need to annotate each call with a @var, including chained calls like $serviceLocator->getService('foo')->doFoo(). I think that annotating and recasting should be used only as a last resort if there's no other option.

BTW: Besides the dynamic return type extension, you could also write a separate check to check if all the getService calls are valid and point to existign services :)

would only solve my problem, it would hardly be usable for others.

But this is the point of extensions. PHPStan core should only cover describing behaviour of PHP language that is common for all its users. You should write an extension because getService call is specific only to your codebase or to codebases that use some specific DI container. I'm publishing extensions specific to frameworks under phpstan organization, see: Doctrine, Nette. But I've written a dozen of extensions that do not make sense to be shared publicly, because they describe some magic behaviour relevant only in our company's codebase. The possibility to do this is PHPStan's strength and big "selling" goal.

Also, what I should have mentioned is that supporting @var when assigning to a property in a method also isn't something I'd vote for, because you should declare the property type when declaring the property:

/** @var Foo */
private $foo;

That works since the first version. It allows to all accessors of the property to know the type, not just the code in a single method where you'd write @var.

As for the original proposal, I'm keeping this issue open because I'd like to implement the @var syntax without the variable name for variable assignments. That's quite straightforward and will be in next (0.7) version.

As for the custom casting syntax $super = /*(A)*/ $this->getService('A');, I'm not a fan of this because no existing PHP code looks like this so it would be PHPStan-specific syntax and that doesn't sit well with me for the future and spreading usage of the tool.

Uff, I hope you're satisfied with the answer :)

[Fuco1]

Thanks for the reply, I took no offense, no worries.

I come from another world (c#, Java...) where casting is "common" and sometimes necessary (well, not so much these days with automatic type inference) so a functionality like this seems a "no brainer", but I understand if this doesn't fit well into PHP or would just add cryptic rarely-used nonsense. I suspect further versions of PHP will add something like this anyway.

Also, what I should have mentioned is that supporting @var when assigning to a property in a method also isn't something I'd vote for, because you should declare the property type when declaring the property:

I have done that, the protected property was annotated with the type of the service, but because the return type of the getModule doesn't match that (it's a superclass to subclass assignment -> invalid) I get an error on level 3:

(the error is printed in the minibuffer at the bottom)

[ondrejmirtes]

I feel the need to break down the differences between Java (and similar) and PHP.

In Java, you have generics, so for some cases, you don't need casting:

Article a = entityManager.find(Article.class, id);

But in different cases, mostly with a "stringly typed" code, you need it:

Foo f = (Foo) serviceLocator.get("foo");

You need to cast the type because you need to make the compiler happy so it knows about the right type.

In PHP, you don't have to cast anything:

$article = $entityManager->find(Article::class, $id);
$foo = $serviceLocator->get('foo');

Because there's no compiler and everything is resolved at runtime (and a variable can be of a different type every time the program runs).

You're adding @var annotations to help your IDE and other tools to know what type your variable has. But in PHPStan, you can define with your own custom logic what does the method return. And it's even more powerful than generics because I can define what is returned even in the second case ($serviceLocator.get('foo')). The dynamic return type extension can for example look like this.

My opinion is that in object-oriented languages, the need for using casting and instanceof is a feedback that the code could be designed in a better way. Every time I have to use them, I think it thoroughly if there's some better way how to design the interfaces, return types. etc.

So I think you shouldn't need casting that much, just a way to figure out the return type based on function arguments :) And that's what dynamic return type extensions are for.