Generic.CodeAnalysis.EmptyStatement Add option to allow only comments in statements #2240
Conversation
| ($token['scope_opener'] + 1), | ||
| ($token['scope_closer'] - 1), | ||
| $token['scope_closer'], |
There was a problem hiding this comment.
Wasn't expecting to have to change this, but skips the last token otherwise.
There was a problem hiding this comment.
This is a known issue, see #1319
| */ | ||
| } elseif ($baz) { | ||
| // phpcs:set Generic.CodeAnalysis.EmptyStatement somethingMadeUp true |
There was a problem hiding this comment.
To prevent confusion between real test related PHPCS annotations and annotations which are part of the test, it's better to use something like //phpcs:ignore Standard.Category.SniffName -- for reasons here.
| } else { | ||
|
|
||
| } |
There was a problem hiding this comment.
You should probably reset the changed property to the default at the end of this test to prevent issues with future test cases being added.
|
Just thinking out loud: is a property the way to go here or should this be solved via error codes, i.e. having a different error code for a completely empty statement vs an empty statement only containing a comment ? That way, you could, for instance, exclude |
|
I like the idea of more granular errors. Might need to go for something like |
|
Error codes are not supposed to contain Adding additional error codes will be BC-break, but only a quite minor one in this case, so I suspect it would be an acceptable one, though @gsherwood can give you a more definite answer. |
|
I prefer the new error codes over the sniff property as it is more flexible, but this BC break will silently stop existing include and exclude rules from working. This sniff was changed to use new error codes (issues #314) so that it could be customised more easily, so I assume people are doing that today. What PHPCS probably needs is the ability to provide aliases for message codes to allow for these sort of deprecations to occur. For example, we throw a |
|
PR #3409 was marked as a duplicate of this. |
There are cases where it's considered valid to only have a comment in a statement, this adds an option to allow it (refs libero/php-coding-standard#34 (comment)).