Skip to content

Commit 5b597d8

Browse files
rodrigoprimojrfnl
authored andcommitted
Squiz/ClosingDeclarationCommentSniff: prevent non-problematic bug
This commit fixes a non-problematic bug on line 96. The code `if (rtrim($tokens[$next]['content']) === $comment)` was not taking into account that `$next` can be false. To fix it, a `$next !== false` check was added to the if condition. This problem did not cause any issues because when `$next` is `false`, the code would check the content of the first token (`$tokens[0]['content']`) and this token can only be the PHP open tag or inline HTML. The code would only produce false positives if the content of the first token would be `// end ...` which is unlikely and would be invalid HTML. A test was added exercising the code path where `$next` is `false`. And another tests which actually hits the bug and safeguards against potential regressions for the non-problematic bug fix.
1 parent 579f081 commit 5b597d8

6 files changed

+46
-2
lines changed

src/Standards/Squiz/Sniffs/Commenting/ClosingDeclarationCommentSniff.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ public function process(File $phpcsFile, $stackPtr)
8888
$data = [$comment];
8989
if (isset($tokens[($closingBracket + 1)]) === false || $tokens[($closingBracket + 1)]['code'] !== T_COMMENT) {
9090
$next = $phpcsFile->findNext(T_WHITESPACE, ($closingBracket + 1), null, true);
91-
if (rtrim($tokens[$next]['content']) === $comment) {
91+
if ($next !== false && rtrim($tokens[$next]['content']) === $comment) {
9292
// The comment isn't really missing; it is just in the wrong place.
9393
$fix = $phpcsFile->addFixableError('Expected %s directly after closing brace', $closingBracket, 'Misplaced', $data);
9494
if ($fix === true) {
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
3+
// This should be the only test in this file.
4+
// Testing that the sniff is triggered when the closing bracket is
5+
// the last character in the file (no newline after it).
6+
7+
function closingBraceAtEndOfFile() {
8+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
3+
// This should be the only test in this file.
4+
// Testing that the sniff is triggered when the closing bracket is
5+
// the last character in the file (no newline after it).
6+
7+
function closingBraceAtEndOfFile() {
8+
}//end closingBraceAtEndOfFile()
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
//end closingBraceAtEndOfFileMissingComment()
2+
3+
<?php
4+
5+
// This should be the only test in this file.
6+
// Testing that the sniff is triggered and the fixer works when the closing bracket is
7+
// the last character in the file (no newline after it) and the content of the first token
8+
// is a "//end closingBraceAtEndOfFileMissingComment()" comment.
9+
10+
function closingBraceAtEndOfFileMissingComment() {
11+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
//end closingBraceAtEndOfFileMissingComment()
2+
3+
<?php
4+
5+
// This should be the only test in this file.
6+
// Testing that the sniff is triggered and the fixer works when the closing bracket is
7+
// the last character in the file (no newline after it) and the content of the first token
8+
// is a "//end closingBraceAtEndOfFileMissingComment()" comment.
9+
10+
function closingBraceAtEndOfFileMissingComment() {
11+
}//end closingBraceAtEndOfFileMissingComment()

src/Standards/Squiz/Tests/Commenting/ClosingDeclarationCommentUnitTest.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,15 @@ public function getErrorList($testFile='')
5252
110 => 1,
5353
];
5454

55+
case 'ClosingDeclarationCommentUnitTest.4.inc':
56+
return [8 => 1];
57+
58+
case 'ClosingDeclarationCommentUnitTest.5.inc':
59+
return [11 => 1];
60+
5561
default:
5662
return [];
57-
}
63+
}//end switch
5864

5965
}//end getErrorList()
6066

0 commit comments

Comments
 (0)