ezyang / htmlpurifier

Standards compliant HTML filter written in PHP
http://htmlpurifier.org
GNU Lesser General Public License v2.1
3.03k stars 323 forks source link

fix: Fixed missing return statement #350

Open PHPGangsta opened 1 year ago

PHPGangsta commented 1 year ago

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

ezyang commented 1 year ago

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

PHPGangsta commented 1 year ago

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 commented 1 year ago

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

PHPGangsta commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.