Add sniff for consistent function spacing to extra ruleset.

[jrfnl]

See #590 (comment)

Please note:

This rule does conflict with the WordPress.WhiteSpace.ControlStructureSpacing.BlankLineAfterEnd rule for the last method in a class if the function does not have a } // end myfunction() comment.
Still - the fact that it throws a message Blank line found after control structure when finding a blank line after the last method in a class might actually be a bug....

[jrfnl]

FYI: PR #597 fixes the conflict which looked to be a bug.

[JDGrimes]

Still - the fact that it throws a message Blank line found after control structure when finding a blank line after the last method in a class might actually be a bug....

IMHO there should never be a blank line after the last method in a class. The next line should have the class's closing brace on it. AFAIK that is the style that all of WordPress core's code follows (could be wrong though). Maybe that wasn't the best sniff to check for that, but at the very least I think it should be possible to configure WPCS so that it does flag blank lines after the last method in a class.

So, I'm not saying that #597 has to be reverted, but maybe we should let that last-method check be toggled. And I wouldn't want to merge this until it was possible to have it not force a blank line after the last method in a class.

[jrfnl]

So, I'm not saying that #597 has to be reverted, but maybe we should let that last-method check be toggled.

As the sniff #597 relates to is about control structures, I don't think that should be solved there.

Rather it should possibly be solved in this PR by either extending or copying the sniff this PR would add and changing it's behaviour for the last method of a class.

All the same, personally I don't agree with the no blank line after last method in class. There generally is a blank line after the class opener, so for symmetry it would only make sense to also have one before the class closer.

[GaryJones]

There generally is a blank line after the class opener

There doesn't need to be - the indentation of the first method (DocBlock) does the same job as a Blank line. I never have closing blank line at the start or end of classes.
Regardless, I don't think is consistently used either way to make it worthwhile for 80% of users.

[jrfnl]

So... shall we just add this to the ruleset used for WPCS itself ? and close this PR/not add it to the WPCS extra ruleset ?

[JDGrimes]

So... shall we just add this to the ruleset used for WPCS itself ? and close this PR/not add it to the WPCS extra ruleset ?

I think that there is some agreement on the "1 blank line between functions" rule, but the "before the first" and "after the last" part doesn't have a consensus, and I guess isn't currently address by WordPress's coding standards. So I guess unless that upstream sniff can be configured to only check between functions/methods, we'll close this PR.

[jrfnl]

I agree with closing this for WPCS, but what about this:

So... shall we just add this to the ruleset used for WPCS itself ?

I know we don't all agree with it, but it is how I've done it (for now) in the clean up. The reason why I'm asking is that with the Theme Check sniffs being created now, it would be good to have them auto-checked for some consistency in the function spacing rather than leaving that up to the individual developer.

What do think ? Shall I open a separate PR for just adding it to the WPCS "own" ruleset ?

[JDGrimes]

What do think ? Shall I open a separate PR for just adding it to the WPCS "own" ruleset ?

I lean towards not, but if others want to, I'm indifferent. 😄

[jrfnl]

FYI: there is already an open PR at PHPCS to remove the check after the last function/method in a file: squizlabs/PHP_CodeSniffer#1041

[jrfnl]

Suggest closing for lack of activity/interest.

[JDGrimes]

Suggest closing for lack of activity/interest.