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

Add new Yoast.WhiteSpace.FunctionSpacing sniff #86

Merged
merged 1 commit into from
Aug 3, 2018

Conversation

jrfnl
Copy link
Collaborator

@jrfnl jrfnl commented Aug 2, 2018

Turns out the upstream Squiz.WhiteSpace.FunctionSpacing sniff which was added in PR #83, examines all functions and not just the functions in classes and other OO structures.

This is problematic for code which follows a pattern like this:

if ( ! function_exists( 'fname' ) ) {
    /**
     * Docblock.
     */
    function fname() {
    }
}

.. in that the sniff demands a blank line between the if and the docblock, as well as between the function close brace line and the if close brace line.
Especially that last one is problematic as it causes a fixer conflict with the ControlStructureSpacing sniff which demands no blank lines at the end of control structures.

This has now been solved by adding a new Yoast sniff which extends the original upstream sniff, but makes sure that the sniff is only applied to methods within OO-structures.

Functions in the global namespace will no longer be examined by the sniff.

The Yoast version of the sniff overloads the public properties from the parent sniff with the Yoast specific config.
If so desired for individual projects, these public properties can still be overruled by setting them in a custom ruleset.

Includes unit tests for the sniff as well as the fixer.

Turns out the upstream `Squiz.WhiteSpace.FunctionSpacing` sniff which was added in PR 83, examines *all* functions and not just the functions in classes and other OO structures.

This is problematic for code which follows a pattern like this:
```php
if ( ! function_exists( 'fname' ) ) {
    /**
     * Docblock.
     */
    function fname() {
    }
}
```
.. in that the sniff demands a blank line between the `if` and the docblock, as well as between the function close brace line and the `if` close brace line.
Especially that last one is problematic as it causes a fixer conflict with the `ControlStructureSpacing` sniff which demands no blank lines at the end of control structures.

This has now been solved by adding a new Yoast sniff which extends the original upstream sniff, but makes sure that the sniff is only applied to methods within OO-structures.

Functions in the global namespace will no longer be examined by the sniff.

The Yoast version of the sniff overloads the public properties from the parent sniff with the Yoast specific config.
If so desired for individual projects, these public properties can still be overruled by setting them in a custom ruleset.

Includes unit tests for the sniff as well as the fixer.
@jrfnl jrfnl added this to the 1.0.0 milestone Aug 2, 2018
@jrfnl jrfnl requested a review from moorscode August 2, 2018 20:10
@moorscode moorscode merged commit 463c0cd into develop Aug 3, 2018
@moorscode moorscode deleted the JRF/improve-function-spacing-rule branch August 3, 2018 06:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants