Improve type inference for stateful container implementations #210
Conversation
Adds templates to the container and abstract placeholder view helper classes and documents the types used by each implementing view helper. A lot of incorrect docblocks have been fixed, additional magic methods documented and a decent reduction in the baseline. Several implementations store values as plain objects and were in places (parameter) type hinting on `stdClass`. All references to `stdClass` have been replaced with `object` and `instanceof` conditions replaced with `is_object`. There's no BC break because `object` is more general than `stdClass`. The main purpose here is that it's not possible to express object shapes with `stdClass` which is central to the significant improvements in type inference. Various public and protected methods have been marked `@internal` so that they can be made private in the next major, and there are a handful of deprecations for unused, un-documented and un-tested public methods. Signed-off-by: George Steel <george@net-glue.co.uk>
| * @return stdClass|false Returns false if stylesheet is a duplicate | ||
| * @return object|false Returns false if stylesheet is a duplicate |
There was a problem hiding this comment.
IMO we should hold this back a bit - this change is potentially breaking
There was a problem hiding this comment.
That's a deprecated, undocumented and internally unused method and it still actually returns a stdClass. For HeadLink specifically, the internal type is just (object) array<string, mixed>, in other implementations, documented return types have been changed to Psalm object shapes. I can't see any practical way that BC is broken here.
Returning object here just satisfies the template for TValue.
Can you explain how this might be a BC break?
There was a problem hiding this comment.
The expectation downstream is stdClass, not object, heh :D
Thanks for looking at it @Ocramius |
Also further type inference improvements and psalm fixes Signed-off-by: George Steel <george@net-glue.co.uk>
Signed-off-by: George Steel <george@net-glue.co.uk>
…here possible Signed-off-by: George Steel <george@net-glue.co.uk>
|
Nice improvements BTW!
|
|
Why was |
|
@FabianKoestring we'll make many classes |
Adds templates to the container and abstract placeholder view helper classes and documents the types used by each implementing view helper.
A lot of incorrect docblocks have been fixed, additional magic methods documented and a decent reduction in the baseline.
Several implementations store values as plain objects and were in places (parameter) type hinting onstdClass. All references tostdClasshave been replaced withobjectandinstanceofconditions replaced withis_object. There's no BC break becauseobjectis more general thanstdClass. The main purpose here is that it's not possible to express object shapes withstdClasswhich is central to the significant improvements in type inference.Methods that expect
stdClasshave been annotated to acceptobject{foo:string}which results in a lot fewer psalm issues and better inference than the handful ofArgumentTypeCoercionandMismatchingDocblockParamTypeadded to the baseline.Various public and protected methods have been marked
@internalso that they can be made private in the next major, and there are a handful of deprecations for unused, un-documented and un-tested public methods.