Add TableLocator get by FQCN #18957
Add TableLocator get by FQCN #18957
Conversation
|
Forgot to run phpcs, making fix |
| { | ||
| // phpcs:ignore | ||
| /** @var T */ | ||
| return $this->getTableLocator()->get($class, $options); |
There was a problem hiding this comment.
Do we really need a new method? Can't use conditional return type with phpdoc?
There was a problem hiding this comment.
Tried it but it didnt work or phpstan was throwing errors
There was a problem hiding this comment.
Does PHPstan not have a way to do method signature overloads?
There was a problem hiding this comment.
I tried it in the single method for 2 hours, but i cannot figure it out.
There was a problem hiding this comment.
maybe something like this https://phpstan.org/r/e39d9eaf-51f5-4b6b-a5dc-b6511808d8a2
There was a problem hiding this comment.
Didnt know IDEs have support for ternary operators in return. I think it is working. Thanks for help :)
| * @param array<string, mixed> $options The options you want to build the table with. | ||
| * If a table has already been loaded the registry options will be ignored. | ||
| * @return \Cake\ORM\Table | ||
| * @return ($alias is class-string<T> ? T : \Cake\ORM\Table) |
There was a problem hiding this comment.
I wonder if this should be moved into phpstan-* tags instead of having a precedence on using these non-standard ones directly on param/return.
There was a problem hiding this comment.
When i move this to phpstan-* the feature stops working for VSCode
There was a problem hiding this comment.
I didnt know IDEs support this, that was main reason i did it in separate function
|
Now that this contains only annotation changes, it can go in |
dereuromark
left a comment
There was a problem hiding this comment.
I tested in PHPStorm, also seems to work fine out of the box.
I wonder if there are other similar cases where we could apply this.
|
In vscode it works too |
|
Thank you 🎉 |
* Add TableLocator get by FQCN * Add missing docblock, add phpcs ignore * Fix whitespace * Revert TableLocator, Add fetchTableByClass * Merge fetchTable and fetchTableByClass --------- Co-authored-by: Adam Halfar <halfar@seonet.cz>
Resolves #18954