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

Class level template on static method ignored? #10513

Closed
RobertMe opened this issue Jan 30, 2024 · 7 comments
Closed

Class level template on static method ignored? #10513

RobertMe opened this issue Jan 30, 2024 · 7 comments

Comments

@RobertMe
Copy link

Bug report

It seems like class level template declarations are ignored on static methods. See the linked snippet for an example. I would expect the dumped type to be GenericItem<TestItem> (based on the type of the $item argument), while it just uses the parameter type? Template constraint type? (i.e. ItemInterface as generic type)

Testing the same snippet with Psalm does show there is an error with the usage of TItem on the static method:

ERROR: UndefinedDocblockClass - 18:12 - Docblock-defined class, interface or enum named TItem does not exist

ERROR: MismatchingDocblockParamType - 18:12 - Parameter $item has wrong type 'TItem', should be 'ItemInterface'

And while I don't really get why this wouldn't work, fixing it, does fix the issue in PHPStan as well: https://phpstan.org/r/196645b0-ac23-4299-8d33-1536fad66aa8 .
But when changing the generic type to use an unknown name it does properly error. See here where I dropped the @template T on the static, while still using T for the param and return type. This properly reports "has invalid (return) type T".

Code snippet that reproduces the problem

https://phpstan.org/r/6b06b2b0-b234-4b9e-afac-ff0e694419b2

Expected output

Preferably, "it just works", i.e. the original snippet results in GenericItem<TestItem>.

If there is some limitation / good reason why this wouldn't (always) work an error should be reported that TItem is an unknown type / is a class level template which can't be used on a static method / ... (IMO the Psalm error isn't really clear either, because the template type is actually defined, but ignore because it is on the class and not used on the static method).

Did PHPStan help you today? Did it make you happy in any way?

No response

@ondrejmirtes
Copy link
Member

Class-level @template used in a static method signature does not make much sense.

When you're calling an instance method, you're doing so on an instance of type GenericItem<TestItem> for example, so PHPStan knows what's TItem is - it's TestItem.

But when you're calling a static method, you're calling GenericItem::create() so PHPStan doesn't know what TItem is - it's not defined in this static call.

But I think the signature in your code snippet is just misunderstanding, and this is what you wanted to to do instead - introduce a method-level @template to use in @param and @return: https://phpstan.org/r/46f8ac50-23ff-4065-9f95-8d3a452c4fcb

@RobertMe
Copy link
Author

While I understand the argument for not supporting it, then it also shouldn't read it. I.e.: the original snippet should result in an error that type TItem doesn't exist, as Psalm does (even better would be detecting there is a class level template with this name and explicitly naming that this template type isn't used). And showing an error when the type isn't defined at all is already present, so IMO it should be modified / fixed to ignore these class level template types when inspecting a static method.

And for what it's worth, in C# class level generic types can be used by static methods as well, but the type needs to be explicitly named, so GenericItem<TestItem>.Create(new TestItem());. But in PHP you obviously can't do this as you can't specify the generic type in the call / actual code.

@ondrejmirtes
Copy link
Member

We've discussed it and decided against it for now. See: phpstan/phpstan-src#2075

@ondrejmirtes
Copy link
Member

One more link phpstan/phpstan-src#2232

@RobertMe
Copy link
Author

Just checking, how do these tickets relate? Is that in response to possibly supporting class level template types? Or to (at least) error that an undefined / unknown (class) type name is used?

I think the second one, phpstan/phpstan-src#2232, would change it such that an error is reported that TItem is of an unknown type, correct? So would be nice if at least that would be the case. As I now had to resort to Psalm to at least find out that the template type might be ignored, by PHPStan as well.
So to clarify, once more: not actually supporting class level template types on static methods is fine by me. But when they are used, an error describing some problem with the method signature (i.e. "invalid (return) type TItem" in the example) would be very helpful. So it at least shows some information that something is wrong with the declaration. Instead of using some "random" type and then showing an error that the "random" type is incorrect when it actually is used.

@ondrejmirtes
Copy link
Member

The linked pull requests are about this exact topic.

What we realize in the discussions there is we can't stop taking class-level @template completely because of use cases that involve @extends and @implements.

Copy link

github-actions bot commented Mar 2, 2024

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 Mar 2, 2024
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