Skip to content
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

fix: Fixed missing return statement #350

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

PHPGangsta
Copy link
Contributor

Fixes:
127 Method HTMLPurifier_VarParser::parse() should return string but return statement is missing

Fixes:
127 Method HTMLPurifier_VarParser::parse() should return string but return statement is missing
@PHPGangsta PHPGangsta changed the title Fixed missing return statement fix: Fixed missing return statement Nov 18, 2022
@ezyang
Copy link
Owner

ezyang commented Nov 19, 2022

Is there a way to suppress the warning with a noreturn annotation on the method call that would be preferred

@PHPGangsta
Copy link
Contributor Author

If there is no return statement, the function returns "void", correct? If you store the return value to a variable, it's stored as "null".

"void" as a return type is possible since PHP 7.1.0.

So you would prefer something like
@return string|void
in the PHPDoc, if that's possible and fixes the phpstan warning, right?

@ezyang
Copy link
Owner

ezyang commented Nov 19, 2022

Not necessarily! If you raise an exception you don't need to return

@PHPGangsta
Copy link
Contributor Author

Oh, you are right! $this->errorGeneric() always throws an Exception. Maybe the function errorGeneric() should have a PHPDoc of
@throws HTMLPurifier_VarParserException
then phpstan might not report the missing return statement anymore.

I was also thinking about changing the PHPDoc of function parse() to
@return string|null
because it can return null in line 72.

@PHPGangsta
Copy link
Contributor Author

If I set
@return string|null
the error in phpstan is gone, but I still get the error in PHPStorm "Missing 'return' statement":
image

If I set it to
@return string|null|void
both errors are gone.

How do we proceed?

@ezyang
Copy link
Owner

ezyang commented Nov 22, 2022

If there is no way to indicate to PHPStorm that errorGeneric always throws an exception, refactor the code so that the helper function returns the exception object, and throw it at the call site.

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.

2 participants