Squiz/InlineComment: fix fixer conflict when comment found at end of function #1710
Conversation
This file contains hidden or 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
jrfnl
force-pushed
the
feature/fix-1709-squiz-inline-comment
branch
2 times, most recently
from
2351657 to
327f389
Compare
|
Rebased for merge conflicts & updated code style... |
jrfnl
force-pushed
the
feature/fix-1709-squiz-inline-comment
branch
from
327f389 to
2292c82
Compare
…function When an inline comment is found at the end of a function, removing a blank line after it conflicts with the `Squiz.WhiteSpace.FunctionClosingBraceSpace` sniff which demands a blank line at the end of a function. I have chosen to fix this by adding a specific error code for that situation and excluding that error code from the `Squiz` ruleset. This way, the backward-compatibility break for other standards using the `Squiz.Commenting.InlineComment` sniff will be smallest. Only if a standard explicitly in/excluded the `Squiz.Commenting.InlineComment.SpacingAfter` errorcode will this have any effect on them. The unit tests already contain a test covering this. As the errorcode is excluded via the ruleset, the results of the unit tests do not change. Fixes 1709
jrfnl
force-pushed
the
feature/fix-1709-squiz-inline-comment
branch
from
2292c82 to
dcd89b7
Compare
|
Rebased (again) for merge conflicts... |
|
Your solution seems like a good one to me as well. Thanks for this. |
|
You're welcome 😃 FYI: I'm just now doing a new fixer conflict run to see where we're at. Initial findings:
Current status based on running
(MySource can not be tested as the ruleset includes an PR #1813 will solve five of these. See: https://travis-ci.org/jrfnl/PHP_CodeSniffer/builds/352204624 |
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.
Came across this error when running fixer conflict checks for the various standards. (See #1645 (comment) )
Fourth fix in a series to fix the issues found.
When an inline comment is found at the end of a function, removing a blank line after it conflicts with the
Squiz.WhiteSpace.FunctionClosingBraceSpacesniff which demands a blank line at the end of a function.To reproduce the issue, run (against
master):phpcbf -p -s --standard=Squiz ./src/Standards/Squiz/Tests/Commenting/InlineCommentUnitTest.inc -vvI have chosen to fix this by adding a specific error code for that situation and excluding that error code from the
Squizruleset.This way, the backward-compatibility break for other standards using the
Squiz.Commenting.InlineCommentsniff will be smallest.Only if a standard explicitly in/excluded the
Squiz.Commenting.InlineComment.SpacingAftererrorcode will this have any effect on them.The unit tests already contain a test covering this. As the errorcode is excluded via the ruleset, the results of the unit tests do not change.
Fixes #1709