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

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

Closed
2 tasks
jrfnl opened this issue Oct 25, 2018 · 4 comments
Closed
2 tasks

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

jrfnl opened this issue Oct 25, 2018 · 4 comments

Comments

@jrfnl
Copy link
Member

jrfnl commented Oct 25, 2018

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:

  • Check all sniffs which deal with the T_FUNCTION token/ which deal with function declarations to see:
    • If they correctly deal with abstract methods / interface methods.
    • Whether the sniff code could be simplified by using the new has_body key in the output from the File::getMethodProperties() method.
@jrfnl jrfnl added Type: Chores/Cleanup Status: Requirement not met On hold until WPCS supports a particular version of a dependency (PHP, PHP_CodeSniffer, etc.). labels Oct 25, 2018
@jrfnl jrfnl added this to the Future Release milestone Oct 25, 2018
@jrfnl jrfnl removed this from the Future Release milestone Mar 27, 2019
@jrfnl jrfnl added this to the 3.0.0 milestone Apr 1, 2020
@jrfnl jrfnl removed the Status: Requirement not met On hold until WPCS supports a particular version of a dependency (PHP, PHP_CodeSniffer, etc.). label Jun 30, 2020
@vdwijngaert
Copy link
Member

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
Copy link
Member Author

jrfnl commented Dec 21, 2022

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

@GaryJones
Copy link
Member

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
Copy link
Member Author

jrfnl commented Jul 6, 2023

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.

@jrfnl jrfnl closed this as not planned Won't fix, can't repro, duplicate, stale Jul 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants