From 5b597d8fc00d05b9a1b953023e94f9f651c5c8aa Mon Sep 17 00:00:00 2001
From: Rodrigo Primo <rodrigosprimo@gmail.com>
Date: Wed, 6 Mar 2024 11:11:06 -0300
Subject: [PATCH] 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.
---
 .../Commenting/ClosingDeclarationCommentSniff.php     |  2 +-
 .../ClosingDeclarationCommentUnitTest.4.inc           |  8 ++++++++
 .../ClosingDeclarationCommentUnitTest.4.inc.fixed     |  8 ++++++++
 .../ClosingDeclarationCommentUnitTest.5.inc           | 11 +++++++++++
 .../ClosingDeclarationCommentUnitTest.5.inc.fixed     | 11 +++++++++++
 .../Commenting/ClosingDeclarationCommentUnitTest.php  |  8 +++++++-
 6 files changed, 46 insertions(+), 2 deletions(-)
 create mode 100644 src/Standards/Squiz/Tests/Commenting/ClosingDeclarationCommentUnitTest.4.inc
 create mode 100644 src/Standards/Squiz/Tests/Commenting/ClosingDeclarationCommentUnitTest.4.inc.fixed
 create mode 100644 src/Standards/Squiz/Tests/Commenting/ClosingDeclarationCommentUnitTest.5.inc
 create mode 100644 src/Standards/Squiz/Tests/Commenting/ClosingDeclarationCommentUnitTest.5.inc.fixed

diff --git a/src/Standards/Squiz/Sniffs/Commenting/ClosingDeclarationCommentSniff.php b/src/Standards/Squiz/Sniffs/Commenting/ClosingDeclarationCommentSniff.php
index 85c39b3ce3..f059d05f93 100644
--- a/src/Standards/Squiz/Sniffs/Commenting/ClosingDeclarationCommentSniff.php
+++ b/src/Standards/Squiz/Sniffs/Commenting/ClosingDeclarationCommentSniff.php
@@ -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) {
diff --git a/src/Standards/Squiz/Tests/Commenting/ClosingDeclarationCommentUnitTest.4.inc b/src/Standards/Squiz/Tests/Commenting/ClosingDeclarationCommentUnitTest.4.inc
new file mode 100644
index 0000000000..194fd0b2bc
--- /dev/null
+++ b/src/Standards/Squiz/Tests/Commenting/ClosingDeclarationCommentUnitTest.4.inc
@@ -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() {
+}
\ No newline at end of file
diff --git a/src/Standards/Squiz/Tests/Commenting/ClosingDeclarationCommentUnitTest.4.inc.fixed b/src/Standards/Squiz/Tests/Commenting/ClosingDeclarationCommentUnitTest.4.inc.fixed
new file mode 100644
index 0000000000..58cfc77280
--- /dev/null
+++ b/src/Standards/Squiz/Tests/Commenting/ClosingDeclarationCommentUnitTest.4.inc.fixed
@@ -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()
\ No newline at end of file
diff --git a/src/Standards/Squiz/Tests/Commenting/ClosingDeclarationCommentUnitTest.5.inc b/src/Standards/Squiz/Tests/Commenting/ClosingDeclarationCommentUnitTest.5.inc
new file mode 100644
index 0000000000..15abec370c
--- /dev/null
+++ b/src/Standards/Squiz/Tests/Commenting/ClosingDeclarationCommentUnitTest.5.inc
@@ -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() {
+}
\ No newline at end of file
diff --git a/src/Standards/Squiz/Tests/Commenting/ClosingDeclarationCommentUnitTest.5.inc.fixed b/src/Standards/Squiz/Tests/Commenting/ClosingDeclarationCommentUnitTest.5.inc.fixed
new file mode 100644
index 0000000000..e588634e4d
--- /dev/null
+++ b/src/Standards/Squiz/Tests/Commenting/ClosingDeclarationCommentUnitTest.5.inc.fixed
@@ -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()
\ No newline at end of file
diff --git a/src/Standards/Squiz/Tests/Commenting/ClosingDeclarationCommentUnitTest.php b/src/Standards/Squiz/Tests/Commenting/ClosingDeclarationCommentUnitTest.php
index b2aa99db34..e1752039b1 100644
--- a/src/Standards/Squiz/Tests/Commenting/ClosingDeclarationCommentUnitTest.php
+++ b/src/Standards/Squiz/Tests/Commenting/ClosingDeclarationCommentUnitTest.php
@@ -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()