Skip to content

[12.x] Refactor Unify DocBlock return types for functions with possib… #56323

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

Closed
wants to merge 1 commit into from

Conversation

mohsenetm
Copy link
Contributor

Some functions in Laravel have a return type of mixed, while the function may also work as void. To make this clearer, I changed the Doc Blocks to be more explicit. Although I have seen that in one place in the framework this situation is written as mixed|null, in my opinion it is better to have a consistent style everywhere, meaning either use mixed|void or mixed|null.
If this style is not correct, please let me know so I can change it to something else.

Sample One:

/**
* Execute a given callback while advancing a progress bar.
*
* @param iterable|int $totalSteps
* @param \Closure $callback
* @return mixed|void
*/
public function withProgressBar($totalSteps, Closure $callback)
{
$bar = $this->output->createProgressBar(
is_iterable($totalSteps) ? count($totalSteps) : $totalSteps
);
$bar->start();
if (is_iterable($totalSteps)) {
foreach ($totalSteps as $key => $value) {
$callback($value, $bar, $key);
$bar->advance();
}
} else {
$callback($bar);
}
$bar->finish();
if (is_iterable($totalSteps)) {
return $totalSteps;
}
}

Sample Two:

/**
* Fire a custom model event for the given event.
*
* @param string $event
* @param string $method
* @return mixed|null
*/
protected function fireCustomModelEvent($event, $method)
{
if (! isset($this->dispatchesEvents[$event])) {
return;
}
$result = static::$dispatcher->$method(new $this->dispatchesEvents[$event]($this));
if (! is_null($result)) {
return $result;
}
}

…le void returns

Some functions in Laravel have a return type of mixed, while the function may also work as void. To make this clearer, I changed the Doc Blocks to be more explicit. Although I have seen that in one place in the framework this situation is written as mixed|null, in my opinion it is better to have a consistent style everywhere, meaning either use mixed|void or mixed|null. If this style is not correct, please let me know so I can change it to something else.
@crynobone
Copy link
Member

mixed already includes null: https://php.watch/versions/8.0/mixed-type

@mohsenetm
Copy link
Contributor Author

@crynobone
Currently, it seems that two different conventions are being used simultaneously in the project. If the correct approach is to avoid using null or void altogether, it might be a good idea to review and update the parts that use them to ensure greater consistency in the codebase.
I have used mixed|void as I believe this approach is more appropriate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants