WordPress / WordPress-Coding-Standards

PHP_CodeSniffer rules (sniffs) to enforce WordPress coding conventions
MIT License
2.54k stars 479 forks source link

PHPCS 3.4.0: use File::getMethodProperties() / 'has_body' #1514

Closed jrfnl closed 1 year ago

jrfnl commented 5 years ago

Rationale

WPCS includes a number of sniffs which examine function declarations.

Functions can be declared with or without body:

// Standard, with body:
function something( $param ) {
    /* function body */
}

// Abstract or stub without body:
class Foo {
    abstract function Bar( $param );
}

interface Foo {
    function Bar( $param );
}

In PHPCS 3.4.0, the output of the upstream File::getMethodProperties() method has been enhanced to contain a 'has_body' array key.

It should be examined if existing code in WPCS:

  1. Takes function declarations without body well enough into account.
  2. Could be simplified by using the enhanced upstream method.

References

Action Checklist

Once WPCS ups the minimum required PHPCS version to 3.4.0:

vdwijngaert commented 1 year ago

I can confirm that for the WordPress.WhiteSpace.ControlStructureSpacing sniff, the NoSpaceAfterOpenParenthesis and NoSpaceBeforeCloseParenthesis violations are not picked up for abstract methods and interface methods. Same thing for T_FN aka arrow functions.

The spacing issue in the following code does not get reported:

// Abstract or stub without body:
class Foo {
    abstract function Bar( $param);
// ------------------------------^
}

interface Foo {
    function Bar( $param);
// ---------------------^
}

$bar = fn( $param) => $param;
// --------------^

As far as I can tell, the reason for this, is that in ControlStructureSpacingSniff, we do an early return if we cannot find a scope closer.

Having something like Sniff::is_method_with_body() might help, but there's probably a ton of more code that can be simplified thanks to these new additions...

    /**
     * @param int $stackPtr The index of the token in the stack. This must point to
     *                      either a T_FUNCTION, T_CLOSURE, T_USE, or T_FN token.
     * 
     * @since 3.0.0
     *
     * @return bool Whether the token is a function call with a body.
     */
    protected function is_method_with_body( $stackPtr ) {
        try {
            $method_parameters = $this->phpcsFile->getMethodParameters( $stackPtr );

            return $method_parameters['has_body'] === true;
        } catch( \PHP_CodeSniffer\Exceptions\RuntimeException $e ) {
            return false;
        }
    }
jrfnl commented 1 year ago

@vdwijngaert Your issue sounds more closely related to #1101...

GaryJones commented 1 year ago

Once WPCS ups the minimum required PHPCS version to 3.4.0

WPCS 3.0 will have PHPCS 3.7.2 as the minimum.

jrfnl commented 1 year ago

I think we can actually close this issue. I just double-checked, but WPCS 3.0.0 won't contain any sniffs which would benefit from that key.