Skip to content

Commit

Permalink
Squiz/ClosingDeclarationCommentSniff: prevent non-problematic bug
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
rodrigoprimo authored and jrfnl committed Apr 9, 2024
1 parent 579f081 commit 5b597d8
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public function process(File $phpcsFile, $stackPtr)
$data = [$comment];
if (isset($tokens[($closingBracket + 1)]) === false || $tokens[($closingBracket + 1)]['code'] !== T_COMMENT) {
$next = $phpcsFile->findNext(T_WHITESPACE, ($closingBracket + 1), null, true);
if (rtrim($tokens[$next]['content']) === $comment) {
if ($next !== false && rtrim($tokens[$next]['content']) === $comment) {
// The comment isn't really missing; it is just in the wrong place.
$fix = $phpcsFile->addFixableError('Expected %s directly after closing brace', $closingBracket, 'Misplaced', $data);
if ($fix === true) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?php

// This should be the only test in this file.
// Testing that the sniff is triggered when the closing bracket is
// the last character in the file (no newline after it).

function closingBraceAtEndOfFile() {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?php

// This should be the only test in this file.
// Testing that the sniff is triggered when the closing bracket is
// the last character in the file (no newline after it).

function closingBraceAtEndOfFile() {
}//end closingBraceAtEndOfFile()
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
//end closingBraceAtEndOfFileMissingComment()

<?php

// This should be the only test in this file.
// Testing that the sniff is triggered and the fixer works when the closing bracket is
// the last character in the file (no newline after it) and the content of the first token
// is a "//end closingBraceAtEndOfFileMissingComment()" comment.

function closingBraceAtEndOfFileMissingComment() {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
//end closingBraceAtEndOfFileMissingComment()

<?php

// This should be the only test in this file.
// Testing that the sniff is triggered and the fixer works when the closing bracket is
// the last character in the file (no newline after it) and the content of the first token
// is a "//end closingBraceAtEndOfFileMissingComment()" comment.

function closingBraceAtEndOfFileMissingComment() {
}//end closingBraceAtEndOfFileMissingComment()
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,15 @@ public function getErrorList($testFile='')
110 => 1,
];

case 'ClosingDeclarationCommentUnitTest.4.inc':
return [8 => 1];

case 'ClosingDeclarationCommentUnitTest.5.inc':
return [11 => 1];

default:
return [];
}
}//end switch

}//end getErrorList()

Expand Down

0 comments on commit 5b597d8

Please sign in to comment.