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

Squiz.WhiteSpace.OperatorSpacing is not checking spacing on either side of a short ternary operator #1454

Merged

Conversation

gmponos
Copy link
Contributor

@gmponos gmponos commented May 7, 2017

If the sniff has found a ternary statement it would return to quickly and so it would avoid doing some checks.

You can check the tests to see what I am talking about.

@gsherwood
Copy link
Member

The original commit for that skipping code is here: 17a1653

Looking at that change, the intention does seem to be that no space is checked between the ? and : operators, although the change also stops spacing being checked before and after ?:. So I think the change in this PR corrects the intention of the sniff even though it's now checking spacing where it hasn't been checked in 3.5 years.

@gsherwood gsherwood changed the title OperatorSpacingSniff returned quick on Ternary Squiz.WhiteSpace.OperatorSpacing is not checking spacing on either side of a short ternary operator May 7, 2017
@gsherwood gsherwood merged commit 3fc2b51 into squizlabs:master May 7, 2017
gsherwood added a commit that referenced this pull request May 7, 2017
@gsherwood
Copy link
Member

Thanks a lot for finding and fixing this issue - and especially for the tests!

@gmponos gmponos deleted the fix_inline_if_spaces_for_ternary branch May 8, 2017 18:42
@gsherwood gsherwood added this to the 3.0.1 milestone Jun 11, 2017
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.

2 participants