Detect loops with no body

[gmponos]

Follow up of the comment here #2994 (comment)

I also discovered that the foreach-loop can also have a semicolon at the end.

[gmponos]

Not sure what's wrong with the failing build :/

[jrfnl]

Not sure what's wrong with the failing build :/

@gmponos As per: https://travis-ci.org/github/squizlabs/PHP_CodeSniffer/jobs/701538216#L544-L549 you didn't add the new files to the package.xml file which means they would not be included in the PEAR package (which is why that check is in place).

[jrfnl]

May I suggest adding an additional check for there being a semi-colon being after the parenthesis_closer and a open curly after that ?

The sniff would now also trigger an error on legitimate uses of control structures without body.

Note: the only control structures which can be declared without a body are: while, for and declare. All other control structures need a body with or without the curly braces or the alternative syntax with a colon and "end...".

[gmponos]

May I suggest adding an additional check for there being a semi-colon being after the parenthesis_closer and a open curly after that ?

Don't quite get that... Is this the case you want to cover?

for($i =0; $i<10; $i++); {
}

If not can you provide an example similar to the above...

the only control structures which can be declared without a body are: while, for and declare.

And foreach... https://3v4l.org/LHkDc

In anyway I must add while here as well.. https://3v4l.org/gZ64v

I don't think we should include declare in the current sniff.. It is specific that the sniff is only about loops.

[jrfnl]

And foreach... https://3v4l.org/LHkDc

Fair enough, but using a foreach without body doesn't really do anything, so there wouldn't be any point doing that. The one exception being when using references, but that's a whole other snakepit.

Regarding my suggestion:

IMO, the sniff should never trigger on clearly deliberate use of a control structure without body:

for($i =0; $i<10; $i++);
echo $i;

Only when it could conceivably be a mistake:

for($i =0; $i<10; $i++); {
}

[gmponos]

IMO, the sniff should never trigger on clearly deliberate use of a control structure without body

Nowdays due to PSR2/12 it is common to add braces to loops...
but not all standards use braces for control structures..

So IMHO both of the cases above should be reported.

[jrfnl]

@gmponos This is not about braces or no braces. My example is 100% valid code without any mistakes.

And just like you say, as it is now common to use braces, the sniff should not trigger on intentional use of control structures without body.

This:

for($i =0; $i<10; $i++);
echo $i; // 10

Is something completely different from:

for($i =0; $i<10; $i++)
    echo $i; // 12345678910

or from this - which is what you stated you were targetting:

for($i =0; $i<10; $i++);
{
    echo $i; // 10, but was intended to be 12345678910
}

[jrfnl]

At the very least, please consider using a different error code for with/without curly brace as that way, people who use the short notation without body could still use the sniff, but exclude the "without braces" error code.

[gsherwood]

Sniffs typically use the code Found so the message code reads better - Generic.CodeAnalysis.LoopWithNoBody.Found

I'd also strtolower the token content because you never know what case it will be defined in.

[gsherwood]

You mentioned on the original issue that you thought this should be a warning, but you are generating an error here. Was that an intentional change or do you think warning is still the correct severity for this?