adds strict type and returns for all php files in module #336
Conversation
ekes
left a comment
ekes
left a comment
There was a problem hiding this comment.
The only one that cause an error is processInbound processOutbound. Documentation says it always gets passed a string, but that's not enforced on the interface at the moment.
The rest are nit-picks, but then it's a standards patch :-)
| */ | ||
| public function settingsForm(array $form, FormStateInterface $form_state) { | ||
| public function settingsForm(array $form, FormStateInterface $form_state): array { | ||
| $elements['title'] = [ |
There was a problem hiding this comment.
If we're being strict here we should also declare $elements as an array before accessing it as such.
$elements = [];
| * {@inheritdoc} | ||
| */ | ||
| public function processInbound($path, Request $request) { | ||
| public function processInbound($path, Request $request): string { |
There was a problem hiding this comment.
If $path is always a string on return, then it must be a string on being called ie: processInbound(string $path, Request $request). I guess that is the case? Is it? If not then it can't guarantee that it'll not return NULL or whatever passed in as $path.
There was a problem hiding this comment.
So at the moment we technically can't guarantee that a string is returned, unless we cast $path to a string. So maybe we should wait for the interface to be updated?
| */ | ||
| public function processOutbound($path, &$options = [], ?Request $request = NULL, ?BubbleableMetadata $bubbleableMetadata = NULL) { | ||
| public function processOutbound($path, &$options = [], ?Request $request = NULL, ?BubbleableMetadata $bubbleableMetadata = NULL): string { | ||
| assert($this->pathProcessor instanceof OutboundPathProcessorInterface); |
There was a problem hiding this comment.
Same as processInbound except even more so as we run substr on $path.
| * | ||
| * @return array | ||
| * Array of item variables to render in Twig template. | ||
| * Array of status objects. |
There was a problem hiding this comment.
It's returning an array from ::getStatusUpdates() and that is returning a render array, not objects.
Closes #253
Thanks to Big Blue Door for sponsoring my time to work on this.