-
Notifications
You must be signed in to change notification settings - Fork 545
Implement template default types #3457
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
Conversation
You've opened the pull request against the latest branch 2.0.x. PHPStan 2.0 is not going to be released for months. If your code is relevant on 1.12.x and you want it to be released sooner, please rebase your pull request and change its target to 1.12.x.
6fea880 to
71f6083
Compare
Wow, what a thing to drop on Friday afternoon! :) Do you think you could base it on 1.12.x? IMHO you shouldn't have any conflicts doing that. If it'd be too difficult then I'm happy to get this in 2.0, but please try to rebase it back on 1.12.x :)
71f6083 to
f52900e
Compare
Sure, rebased on 1.12.x :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is realworld example where default type is helpful over default implied maximal type (the type after of)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the discussion at phpstan/phpstan#4801
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue did not answer my question. Default implied maximal type would solve the:
Consider this: If a value passed for $a is null, it's impossible to resolve TResult's type. Then default type should be used. Currently, the default type everywhere is mixed instead.
too and in much safer way without any extra coding needed as long as of type is specified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I don't get it. Please show the example in phpstan/phpstan#4801 and discuss it there. I plan to merge this PR very close to as it is.
Other languages also offer "default types for @template".
I really like this PR! I just checked out the branch and I'm trying to break it locally. So far I found:
- This should report an error (the default in the middle doesn't make sense):
/** * @template T of int * @template U of string = 'foo' * @template V of string */ class Foo { }
-
There are some messages that are not aware of default types. When the generics are missing completely: "Function aaa\foo() has parameter $d with generic class aaa\Foo but does not specify its types: T, U, V" (the message should somehow say that some of these types at the end of the list are optional). See 3) for a suggestion at the end.
-
When the generics are there but not all of the required ones. Let's say I have class Foo with 3
@templateand the last one is optional. HavingFoo<1>leads to error message:Generic type aaa\Foo<int, string> in PHPDoc tag @param for parameter $d does not specify all template types of class aaa\Foo: T, U, V. I'm not even sure why it saysaaa\Foo<int, string>. The original type isFoo<1>. Thestringat the 2nd position looks like a bug.
Personally, I'd like to reuse the same message pattern the function call check has:
phpstan-src/src/Rules/Functions/CallToFunctionParametersRule.php
Lines 53 to 58 in 105b9fa
Generic type aaa\Foo<int> in PHPDoc tag @param for parameter $d does not specify all template types of class aaa\Foo: T, U, V, 2-3 required..
Of course, the part , 2-3 required. should only go in when the class has at least one optional @template, for backward compatibility.
@templatetag with default does not make sense above a function, but leads to no error message:
/** * @template T of Foo = Foo * @param T $d */ function foo(Foo $d): void { }
This should be caught similarly how @template-covariant is caught: "Variance annotation is only allowed for type parameters of classes and interfaces..."
I just checked out the branch and I'm trying to break it locally. So far I found:
Yes please, keep them coming :)
I agree with your suggestions in 1, 2, and 3, and I'm going to take a look at them.
The
stringat the 2nd position looks like a bug.
These messages use the typeOnly verbosity to describe types, so while the template is resolved to its default ('foo'), ConstantStringType then describes itself as string:
@template tag with default does not make sense above a function, but leads to no error message:
Default types are useful even above functions/methods, I'll add a test case :)
I think it may also close phpstan/phpstan#7105
Could this https://phpstan.org/r/b999dd1e-983e-4dd1-b22d-ee5c518cd79a be supported ?
The idea was to copy array behavior which have the "key" template optional.
But it kinda go against
- This should report an error (the default in the middle doesn't make sense):
/** * @template T of int * @template U of string = 'foo' * @template V of string */ class Foo { }
6da89de to
d17d2fe
Compare
d17d2fe to
58e9c41
Compare
It's brilliant, I love it, thank you :)
f9a2648
into
phpstan:1.12.x
I feel like we need to put this somewhere in the docs. In another of endless attempts to explain generics to people, I think we should create a brief "generics reference" in this place https://phpstan.org/writing-php-code/phpdoc-types#generics that would just list all the possible syntax in a dry way.
Could you please try send a PR that merges 1.12.x into 2.0.x? I'd expect the PR to contain a merge commit. It'd help me a lot, thanks!
I think the only thing you should be aware of is that I removed __set_state everywhere.
Also, you can run bin/make-optional-parameters-required.php and commit it for relevant places that you added here. Thanks!
I feel like we need to put this somewhere in the docs. In another of endless attempts to explain generics to people, I think we should create a brief "generics reference" in this place https://phpstan.org/writing-php-code/phpdoc-types#generics that would just list all the possible syntax in a dry way.
Yep, I haven't yet figured out how to explain generics and not lose the audience (or get lost myself) along the way. Perhaps a dry reference could be a good starting point
Could you please try send a PR that merges 1.12.x into 2.0.x?
👉 #3560
Closes phpstan/phpstan#4801
I've finally picked up where @rvanvelzen left off in #1835. I've mainly added checks that the default type is valid and within bounds.