Squiz/BlockComment: fix fixer conflict and more #1717
Merged
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
jrfnl
force-pushed
the
feature/fix-squiz-blockcomment
branch
from
75add08
to
ffa334c
Compare
|
Rebased for merge conflicts & updated code style... |
jrfnl
force-pushed
the
feature/fix-squiz-blockcomment
branch
from
ffa334c
to
a1c96de
Compare
This fixes a fixer conflict when a second block comment is directly after a first.
If an empty comment was indented with tabs instead of spaces, it would not be recognized as empty.
…ultiple of tab width
Don't duplicate function calls already made.
This was causing fixer conflicts with other sniffs dealing with code whitespace rules.
jrfnl
force-pushed
the
feature/fix-squiz-blockcomment
branch
from
a1c96de
to
d99e8c6
Compare
…ent closer to be misidentified. Whether something was identified as a comment line was based on the token code being the same. This did not allow for PHPCS annotations to be included in block comments as those have a different token code. This resulted in comments being "cut short", i.e. the line above a PHPCS annotation being seen as the last line in a comment and subsequent comment lines being ignored, resulting in incorrect `Comment closer must be on a new line` and `Empty line required after block comment` notices. Fixes 1918
jrfnl
force-pushed
the
feature/fix-squiz-blockcomment
branch
2 times, most recently
from
37e5b0b
to
ce2b0fe
Compare
|
Thanks a lot for all these fixes and improvements. |
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) )
Ninth fix in a series to fix the issues found.
To start with, this PR fixes a fixer conflict where the
Squiz.Commenting.BlockCommentsniff conflicts with itself.To reproduce, run the following against the first commit (additional unit tests):
phpcbf -p -s ./src/Standards/Squiz/Tests/Commenting/BlockCommentUnitTest.inc --standard=Squiz --sniffs=Squiz.Commenting.BlockComment.The fixer conflict is caused by the sniff seeing the second block comment of the additional unit test as both:
a) an individual block comment and
b) as part of the first block comment, which leads to conflicting directives on what the indent should be.
Fixed by the second commit.
While working on this I noticed a number of other things which could use some love:
tab-widthwould always be 4. This would cause a fixer conflict with itself iftab-widthwas set to, for instance, 3. The fixer made the presumption that the expected indent would always be cleanly dividable by thetab-widthand would end up continuously making the same fix as the indent used by the fixer would not be the same as the expected indent. Fixed in the fifth commit.LastLineIndentwas missing while all other line indent error codes did have a fixer attached to it. Added in the sixth commit.As a final commit I've made some minor efficiency fixes, reducing the number of (duplicate) function calls the sniff was making.
Updated
I've added one more commit to fix another fixer conflict.
To reproduce, run the following against
master:phpcbf -p -s ./src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.inc --standard=SquizNow the sniff will check whether a one-line
/* */comment is the last content on a line and only auto-fix it if it is. If not, it will throw an error, but won't auto-fix.Updated (again)
I've added one more commit to fix a problem the sniff was having with the new PHPCS annotations.
See the commit message and the issue in which I reported the problem for more details.
I've (of course) added unit tests to demonstrate the issue and safeguard against it in the future.
For the builds to pass, I've had to rebase the PR to take advantage of fixes to the tokenizing of PHPCS annotations added between when this PR was originally pulled and now (PR #1901 and #1827).
Fixes #1918