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

Fix bug in control structures whitespace sniff triggered on last method of class #597

Merged

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Jul 14, 2016

As reported here:

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....

Added separate commits for unit test and fix to let travis demonstrate the bug and the fix.

@jrfnl
Copy link
Member Author

jrfnl commented Jul 14, 2016

Freaky weird that the bug is not triggered by the unit test when running this on travis. I can consistently reproduce this on my machine on several different PHP versions.

Edit: even weirder - it did trigger in the travis build for my fork: https://travis-ci.org/jrfnl/WordPress-Coding-Standards/builds/144630156

@jrfnl jrfnl force-pushed the WPCS/feature/fix-bug-last-in-class branch from cffb1d7 to df2c5e3 Compare July 14, 2016 17:57
@jrfnl
Copy link
Member Author

jrfnl commented Jul 14, 2016

Rebased for merge conflict.

@jrfnl jrfnl force-pushed the WPCS/feature/fix-bug-last-in-class branch from df2c5e3 to eee2279 Compare July 14, 2016 18:06
@westonruter westonruter merged commit a2b0383 into WordPress:develop Jul 15, 2016
@jrfnl jrfnl deleted the WPCS/feature/fix-bug-last-in-class branch July 15, 2016 17:51
@jrfnl jrfnl added this to the 0.10.0 milestone Aug 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants