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

Generic/InlineControlStructures: sniff does not allow while without body #2822

Closed
jrfnl opened this issue Jan 13, 2020 · 3 comments · Fixed by #2827
Closed

Generic/InlineControlStructures: sniff does not allow while without body #2822

jrfnl opened this issue Jan 13, 2020 · 3 comments · Fixed by #2827
Milestone

Comments

@jrfnl
Copy link
Contributor

jrfnl commented Jan 13, 2020

A while control structure without body is perfectly valid PHP but gets flagged by the Generic.ControlStructures.InlineControlStructures sniff.

$i = 10;
while ($i > 0 && --$i);

// $i is now 0

See: https://3v4l.org/RXNqL

As far as I can see, the sniff is intended to find inline control structures with a body, but without braces and fix those. An inline control structure without a body should be outside the scope of this sniff.

I've got a fix ready for this, but found that a number of existing unit test would start failing (line 42, 43, 226, 228 and 230).

@gsherwood With that in mind, I'd like second opinion on how the sniff should handle while structures without body.

As an alternative to the fix I've got prepared, I could create a fix which would maintain the existing behaviour, but would give while structures without a body a different error code.

Opinions ?

@gsherwood
Copy link
Member

As far as I can see, the sniff is intended to find inline control structures with a body, but without braces and fix those. An inline control structure without a body should be outside the scope of this sniff.

100% agree.

With that in mind, I'd like second opinion on how the sniff should handle while structures without body.

I think the sniff should ignore them. Happy to change the tests to align with this change.

@jrfnl
Copy link
Contributor Author

jrfnl commented Jan 16, 2020

@gsherwood Thanks for the response. PR #2827 should fix it in that case.

@jrfnl
Copy link
Contributor Author

jrfnl commented Jan 30, 2020

Just realized that for statements can also exist without body. I've updated the PR to include a fix for that too.

for ($i = 1, $j = 0; $i <= 10; $j += $i, print $i, $i++);

@gsherwood gsherwood added this to the 3.5.4 milestone Jan 30, 2020
gsherwood added a commit that referenced this issue Jan 30, 2020
tobias-trozowski pushed a commit to tobias-trozowski/PHP_CodeSniffer that referenced this issue Feb 13, 2020
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 a pull request may close this issue.

2 participants