| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
On second thought maybe it should be it's own rule, with in the bleeding edge. Because Psalm version of the tag is already in use. I'll change it. |
Sorry, something went wrong.
|
I think it's not necessary, it's going to be really rare. I've also enabled conditional return types related rules for everyone :) Some ideas (I haven't seen the code yet):
|
Sorry, something went wrong.
Yes, that's the goal from phpstan/phpstan#5512 I was planning to do that in another PR. But sure it can be part of this one.
If the logic will be in IncompatiblePhpDocTypeRule it's not gonna be a new rule, is it? Do you mean have a parameter for bleeding edge toggle, and do the checks according to that? |
Sorry, something went wrong.
|
Yes, I often add a boolean parameter in bleedingEdge that changes behaviour of existing rules 😊 But if you find the implementation inelegant, certainly feel free to implement it in a separate rule. For one, it only applies to methods in classes and not functions, so IncompatiblePhpDocTypeRule might not be great after all. |
Sorry, something went wrong.
|
I added a commit to correctly infer static<...> types, if the class has the tag. I think static<...> only makes sense for return types. So I didn't add it to property or method argument types. And about that, should IncompatiblePhpDocTypeRule give error if someone uses static<..> in property or argument? (that's a separate PR) I'll implement the second part about the IncompatiblePhpDocTypeRule later. |
Sorry, something went wrong.
|
Some ideas about validating static<...> I got meanwhile
|
Sorry, something went wrong.
|
It's funny a little because one of the places where GenericObjectTypeCheck is called from is IncompatiblePhpDocTypeRule :) So I guess the changes should be tested in IncompatiblePhpDocTypeRuleTest by writing some new sample classes. |
Sorry, something went wrong.
|
The current problem I'm having is that static<...> return type tag is already resolved to GenericObjectType when docblock is resolved in IncompatiblePhpDocTypeRule Should I inspect the $docComment->getText() to figure out if static<...> is used? Do you have any idea of the top of your head? |
Sorry, something went wrong.
|
@canvural Yeah, I realized that we might need a new GenericStaticObjectType for this so that the information is kept when going out from TypeNodeResolver::resolveGenericTypeNode(). Same as GenericObjectType extends ObjectType, GenericStaticObjectType should extend StaticType. It's also useful because we should be able to validate that static<X, Y> is truly returned from a method that typehints @return static<X, Y>. static works by preserving StaticType when we're inside the class and work mainly with $this, but is converted to the final type (specific subclass) when a method is called from the outside. The hardest part is going to be to figure out the correct relationships between ObjectType, StaticType, GenericObjectType, GenericStaticType when asked about isSuperTypeOf() about each other. But there's already some precedens about this from the current types. |
Sorry, something went wrong.
|
Before PHPStan 1.7.0 is out I'll take a look at the current state of this PR and decide what we can cut from my demands to make it part of that release :) |
Sorry, something went wrong.
|
@ondrejmirtes I actually implemented all the rule changes and GenericStaticObjectType (maybe with some errors), but couldn't find time to clean it up and push :/ Was planning to do it tomorrow since I'll have some free time. |
Sorry, something went wrong.
|
Looking forward to it 😊 |
Sorry, something went wrong.
|
@ondrejmirtes I pushed the changes. I did the rule changes from here. But did not implement isSuperTypeOf for GenericStaticObjectType Because I'm not sure where that check would be used. What kind of test case would use isSuperTypeOf on GenericStaticObjectType? Other than that, I guess this is ready for a review. |
Sorry, something went wrong.
|
@canvural Type::isSuperTypeOf() is the most useful and important methods of all :) See https://phpstan.org/developing-extensions/type-system#querying-a-specific-type for more details. It describes the relationship between types. The best way to test it is through type normalization (https://phpstan.org/developing-extensions/type-system#type-normalization) which means to add test cases in TypeCombinatorTest::dataUnion() and TypeCombinatorTest::dataIntersection(). In case of union, the more general type wins and in case of incompatible types, they remain as a union (like string|int). In case of intersection, the more specific type wins and in case of incompatible types, the result is NeverType. I'd expect GenericStaticObjectType to be tested against ObjectWithoutClassType, ObjectType (compatible class), ObjectType (incompatible class), StaticType, GenericObjectType, and GenericStaticObjectType. |
Sorry, something went wrong.
|
@ondrejmirtes Thank you for the detailed explanation. But I already knew all that. I can implement that, and add the tests. But my question was more specific to in which scenarios the isSuperTypeOf would be called for GenericStaticObjectType? Currently it uses the parent's (StaticType) implementation and it seems to be fine. Also GenericStaticObjectType resolves to GenericObjectType as soon as the method is called outside of the class context. All that said, I'll probably continue to work on this next week. Because tomorrow morning I'm leaving for the phpday! I'll make sure to say hi to you! 😊 |
Sorry, something went wrong.
|
Oh wow, I'm looking forward to that a lot! I kind of hoped to see more familiar faces aside from other speakers so it's great my wishes are gonna be fulfilled 😊 |
Sorry, something went wrong.
|
And about your question:
You're in a class method like and have code like this: if (rand(0, 1)) {
$a = $this->doFoo(); // returns static<T, U>
} else {
$a = $this->doBar(); // returns static<Foo, Bar>
}
// $a is union of $a from if+else branchesGenericStaticObjectType remains GenericStaticObjectType here but we need to union the types from if and else. Of course you can come up with other (basically an infinite number) of examples. The relationship between GenericStaticObjectType and StaticType is the same as GenericObjectType and ObjectType, so you can draw some inspiration from there. Also it just occured to me that GenericStaticObjectType should probably be called just GenericStaticType :) |
Sorry, something went wrong.
|
PHPStan 1.7.0 is on its way to be released, I think we can put this one even in 1.7.x or maybe in 1.8.0 depending on when you find time to finish this :) Thanks. |
Sorry, something went wrong.
|
It's fine by me. It's not urgent to get this merged. But I guess I'll spend some time on this this week. Then we'll see. |
Sorry, something went wrong.
|
I can provide another example if needed: https://phpstan.org/r/5b601b92-3518-4f51-9de6-0387b28ebfd5 as discussed in phpstan/phpstan#9975 |
Sorry, something went wrong.
|
Just bumped into this issue with my generic collection class as well. |
Sorry, something went wrong.
|
any hope for making it into 2.x? |
Sorry, something went wrong.
|
Superseded by: 0c1f22a I realized we don't need to enforce @phpstan-consistent-templates for generic static type to work. I couldn't have done it without your PR, it saved me a lot of time figuring out how the static type even works, so thank you! |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
This PR adds a new tag @phpstan-consistent-templates and new checks for it. Fixes one part of phpstan/phpstan#5512
There are some parts I am not sure of.
- Naming of the hasConsistentTemplates Everything else, like internal, final, has naming with isXX. But isConsistentTemplates did not make sense to me.
- Wording of the error messages. I am open to suggestions to improve/change the error messages.
- Currently all variants of the tag is supported. Is this good?
- Currently it is implemented in a way that the errors will only appear if the class has @extends or @implements tag. Just class Foo extends Bar will not produce these errors even Bar has @phpstan-consistent-templates
Lastly, there is this edge case.Basically if a class both extends and implements something, and if those parent classes have different number of template types, the child class can never satisfy both.