Automattic / phpcs-neutron-standard

A set of phpcs sniffs for PHP >7 development
MIT License
94 stars 7 forks source link

Problem in return point detection #40

Closed gmazzap closed 6 years ago

gmazzap commented 6 years ago

There are 2 issues with TypeHintSniff when detecting return points.

First issue

The first problem regards a code like this:

<?php
function returnZero() : int
{
    return 0;
}

That even if fine, raises an error like:

---------------------------------------------------------------------------------------------
WARNING | Return type with no return (NeutronStandard.Functions.TypeHint.UnusedReturnType)
---------------------------------------------------------------------------------------------

This is not caused in the TypeHintSniff.php file itself, but in SniffHelpers::isReturnValueVoid() where a there's the line:

return ! (
    $returnValue
    && $returnValue['content']   // <-- this is problematic 
    && $returnValue['code'] !== 'PHPCS_T_SEMICOLON'
);

this is problematic because when $returnValue['content'] is '0' or '' (empty string) the return value is still non-void but $returnValue['content'] will evaluate as false.

Considering that when the token is not a whitespace and is not a semicolon it can be safely considered as non-void there's no need to check the "content" attribute.


Second issue

The second problem regards a code like this:

function test(bool $condition): int
{
   if ($condition) {
      return 1;
   }

   return;
}

Which is clearly wrong and could cause a fatal error, but is not recognized as problematic, because currently if any non-void return point is found and the function as a non-void return type no error is raised.

This could be fixed counting all the void and non-void return token and then apply proper logic based on the count found.


One more thing

Related to this, another issue IMO is that UnusedReturnType and IncorrectVoidReturnType should be errors, and not warnings, because executing the code where those are detected will cause fatal errors.

gmazzap commented 6 years ago

PR #41 solve all above issues. I also updated/added tests for them.