Skip to content

Conversation

@samsonasik
Copy link
Member

Description

Applied ExplicitReturnNullRector to add explicit return null to method/function that returns a value, but missed main return.

void in union should not happen.

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@samsonasik
Copy link
Member Author

samsonasik commented Dec 10, 2024

Ready for review 👍 , while doesn't change native type, this can goes to 4.6 branch if needed since it change docblock returns from allowed union void to nullable union and may cause phpstan notice on user that override it.

@neznaika0
Copy link
Contributor

In Filters, you can remove PHPDoc @return. It is inherited from the interface and is superfluous.

@samsonasik
Copy link
Member Author

On child classes, more specific returns is ok, the parent interface allow @return RequestInterface|ResponseInterface|string|null on before() method.

@paulbalandan
Copy link
Member

Why are we now returning $response in the after() of the filters?

@samsonasik samsonasik changed the base branch from develop to 4.6 December 10, 2024 14:27
@samsonasik samsonasik force-pushed the refactor-enable-code-quality-level-34-for-rector branch from dcf235e to 6538738 Compare December 10, 2024 14:29
@michalsn michalsn added the 4.6 label Dec 10, 2024
@neznaika0
Copy link
Contributor

Why are we now returning $response in the after() of the filters?

Response return in system/CodeIgniter.php and other places:

$possibleResponse = $this->runRequiredBeforeFilters($filters);

@paulbalandan paulbalandan force-pushed the refactor-enable-code-quality-level-34-for-rector branch from 00b3b1b to 88862ad Compare December 21, 2024 11:23
@paulbalandan paulbalandan merged commit 345e1c3 into codeigniter4:4.6 Dec 21, 2024
49 checks passed
@samsonasik samsonasik deleted the refactor-enable-code-quality-level-34-for-rector branch December 21, 2024 11:49
@paulbalandan paulbalandan mentioned this pull request Jan 19, 2025
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.

4 participants