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

Add generic support to @method definitions #6371

Closed
allejo opened this issue Jan 15, 2022 · 9 comments
Closed

Add generic support to @method definitions #6371

allejo opened this issue Jan 15, 2022 · 9 comments

Comments

@allejo
Copy link
Contributor

allejo commented Jan 15, 2022

Feature request

Would it be possible to add generic support to functions that are defined via @method docs?

<?php declare(strict_types = 1);

/**
 * @template T
 * @template K
 *
 * @method bool compare<T, K>(T $t, K $k) Compares two objects magically somehow
 * @method bool dumbCompare($t, $k) Compares without generics
 */
class HelloWorld
{
	public function __call(): mixed {
		return true;
	}
}

$h = new HelloWorld();
$h->compare(1, 4);
$h->dumbCompare(5, 3);

https://phpstan.org/r/1da86817-e322-401b-938d-6272e51d85e0

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

Yes! It's helped me a lot with avoiding errors in my projects because I would mess up types. Especially generic support, which is incredibly useful ❤️

@ondrejmirtes
Copy link
Member

Thank you for your kind words! :)

This isn't hard to implement, the main work needs to be done in https://github.com/phpstan/phpdoc-parser

Don't forget to also support bounds like: @method bool compare<T of Foo, K of Bar>(T $t, K $k)

After that's done and released, it's really easy to propagate that info here, right now the "template type map" in @method representation is simply empty: https://github.com/phpstan/phpstan-src/blob/c8a3ab49f3f20d09f25d85404266567bf3532328/src/Reflection/Annotations/AnnotationMethodReflection.php#L72

@phpstan-bot
Copy link
Contributor

@allejo After the latest push in 1.9.x, PHPStan now reports different result with your code snippet:

@@ @@
-10: PHPDoc tag @method has invalid value (bool compare<T, K>(T $t, K $k) Compares two objects magically somehow): Unexpected token "<", expected '(' at offset 60
-18: Call to an undefined method HelloWorld<mixed, mixed>::compare().
+No errors

@ondrejmirtes
Copy link
Member

This isn't fully implemented yet, only in phpdoc-parser.

@mad-briller
Copy link
Contributor

@ondrejmirtes i've been trying to implement this and have been struggling unfortunately;

i think i lack some understanding of what the TemplateTypeMap here represents. Is it supposed to represent the already resolved templates, so name => resolved type?

would that be resolved and passed in here?

i've tried looking around the codebase to see if there are any examples similar to what i'm trying to implement, but i can't seem to find anything despite knowing there must be one, so even just pointing at that could be a big help, thanks for your time

@ondrejmirtes
Copy link
Member

FunctionVariant has two template type maps: the "original declaration" one and the "resolved" one. You should only put in the "original declaration" one that basically represents how @templates are declared, or in case of @method, the types between < and > (@method bool compare<T, K>(...)) because that's what the @templates are for these @method declarations.

The "resolved" template type map can stay null in AnnotationMethodReflection.

I think that once you convert the $templateTypes from MethodTagValueNode into TemplateTypeMap and pass it to the FunctionVariant in AnnotationMethodReflection, it should just start working.

@mad-briller
Copy link
Contributor

mad-briller commented Feb 22, 2024

and just to confirm, in TemplateTypeMap the Type part of array<string, Type> should be the bound or MixedType if there is no bound?

@ondrejmirtes
Copy link
Member

No, the Type part is going to be TemplateType instance.

You can look for example at ClassReflection::getTemplateTypeMap to see how an array of TemplateTags becomes TemplateTypeMap.

@ondrejmirtes
Copy link
Member

Implemented: phpstan/phpstan-src#2931

FYI @allejo Your example didn't even require generics support for @method tags as only the class is generic, not the magic method: https://phpstan.org/r/21547a9f-9e51-4773-bdd8-3a8ec31570ab

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

4 participants