Generic/DisallowYodaConditions: add code coverage ignore annotation for untestable line #525
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit adds the `@codeCoverageIgnoreStart/@codeCoverageIgnoreEnd` PHPUnit annotations to ignore, for the purposes of code coverage, a line that can't be tested. I considered doing this initially when I worked on improving code coverage for this sniff in PHPCSStandards#465, but I wrongly assumed that was not an option due to the `Squiz.Commenting.InlineComment.InvalidEndChar` error (which is part of the standard used by PHPCS). What I failed to notice back then is that I was adding the annotation below a comment, and this triggers the error: ``` // Shouldn't be possible but may happen if external sniffs are using this method. // @codeCoverageIgnoreStart ``` Adding the annotation before the comment as it is done in this commit does not trigger the error because the `Squiz.Commenting.InlineComment` sniff will only trigger the `InvalidEndChar` error if the first character of the comment is a letter. Using `return true; // @codeCoverageIgnore` is not an option because it triggers `Squiz.Commenting.PostStatementComment.Found`.
jrfnl
reviewed
Jul 17, 2024
There was a problem hiding this comment.
I also considered using return true;
// @codeCoverageIgnore, but this is not an option because it triggersSquiz.Commenting.PostStatementComment.Found.
I'm not too keen on the current change and would rather use the trailing comment version. Maybe we should consider opening an issue to improve the Squiz.Commenting.PostStatementComment first ?
I mean, IIRC that sniff does allow for trailing PHPCS annotations, so maybe it should also allow for trailing annotations related to some other tools ?
jrfnl
added
Status: awaiting feedback
and removed
Standard: Generic
Status: triage
labels
Open
2 tasks
|
Here is the issue: #560 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR adds the
@codeCoverageIgnoreStart/@codeCoverageIgnoreEndPHPUnit annotations to ignore, for the purposes of code coverage, a line that can't be tested. I considered doing this initially when I worked on improving code coverage for this sniff in #465, but I wrongly assumed that was not an option due to theSquiz.Commenting.InlineComment.InvalidEndCharerror (which is part of the standard used by PHPCS).What I failed to notice back then is that I was adding the annotation below a comment, and this triggers the error:
Adding the annotation before the comment as it is done in this PR does not trigger the error because the
Squiz.Commenting.InlineCommentsniff will only trigger theInvalidEndCharerror if the first character of the comment is a letter.I also considered using
return true; // @codeCoverageIgnore, but this is not an option because it triggersSquiz.Commenting.PostStatementComment.Found.Related issues/external references
Part of #146
Types of changes
PR checklist