Merge pull request #381 from rodrigoprimo/closing-declaration-comment-improvements

Squiz/ClosingDeclarationComment: fix a non-problematic bug plus some other small improvements

Author: jrfnl

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

@@ -85,15 +85,10 @@ public function process(File $phpcsFile, $stackPtr)
 
         $closingBracket = $tokens[$stackPtr]['scope_closer'];
 
-        if ($closingBracket === null) {
-            // Possible inline structure. Other tests will handle it.
-            return;
-        }
-
         $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) {

src/Standards/Squiz/Tests/Commenting/ClosingDeclarationCommentUnitTest.inc renamed to src/Standards/Squiz/Tests/Commenting/ClosingDeclarationCommentUnitTest.1.inc

@@ -84,3 +84,35 @@ enum MissingClosingComment {
 
 enum HasClosingComment {
 }//end enum
+
+function misplacedClosingCommentWhitespace() {
+}       //end misplacedClosingCommentWhitespace()
+
+function misplacedClosingCommentMultipleNewlines() {
+}
+
+
+//end misplacedClosingCommentMultipleNewlines()
+
+function missingClosingComment() {
+}
+
+function commentHasMoreIndentationThanFunction() {
+}
+        //end commentHasMoreIndentationThanFunction()
+
+class Foo {
+    function commentHasLessIndentationThanFunction() {
+    }
+  //end commentHasLessIndentationThanFunction()
+
+    function misplacedClosingCommentWithIndentation() {
+    }
+    //end misplacedClosingCommentWithIndentation()
+}//end class
+
+// Anonymous classes don't need end comments.
+$anon = new class {};
+
+// Arrow functions don't need end comments.
+$arrow = fn($a) => $a;

src/Standards/Squiz/Tests/Commenting/ClosingDeclarationCommentUnitTest.inc.fixed renamed to src/Standards/Squiz/Tests/Commenting/ClosingDeclarationCommentUnitTest.1.inc.fixed

@@ -83,3 +83,29 @@ enum MissingClosingComment {
 
 enum HasClosingComment {
 }//end enum
+
+function misplacedClosingCommentWhitespace() {
+}//end misplacedClosingCommentWhitespace()
+
+function misplacedClosingCommentMultipleNewlines() {
+}//end misplacedClosingCommentMultipleNewlines()
+
+function missingClosingComment() {
+}//end missingClosingComment()
+
+function commentHasMoreIndentationThanFunction() {
+}//end commentHasMoreIndentationThanFunction()
+
+class Foo {
+    function commentHasLessIndentationThanFunction() {
+    }//end commentHasLessIndentationThanFunction()
+
+    function misplacedClosingCommentWithIndentation() {
+    }//end misplacedClosingCommentWithIndentation()
+}//end class
+
+// Anonymous classes don't need end comments.
+$anon = new class {};
+
+// Arrow functions don't need end comments.
+$arrow = fn($a) => $a;

src/Standards/Squiz/Tests/Commenting/ClosingDeclarationCommentUnitTest.2.inc

@@ -0,0 +1,7 @@
+<?php
+
+// Intentional parse error (missing opening bracket).
+// This should be the only test in this file.
+// Testing that the sniff is triggered.
+
+class MissingOpeningBracket

src/Standards/Squiz/Tests/Commenting/ClosingDeclarationCommentUnitTest.3.inc

@@ -0,0 +1,7 @@
+<?php
+
+// Intentional parse error (missing closing bracket).
+// This should be the only test in this file.
+// Testing that the sniff is triggered.
+
+class MissingClosingBracket {

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() {
+}

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()

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() {
+}

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()

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

@@ -26,21 +26,41 @@ final class ClosingDeclarationCommentUnitTest extends AbstractSniffUnitTest
      * The key of the array should represent the line number and the value
      * should represent the number of errors that should occur on that line.
      *
+     * @param string $testFile The name of the test file being tested.
+     *
      * @return array<int, int>
      */
-    public function getErrorList()
+    public function getErrorList($testFile='')
     {
-        return [
-            13 => 1,
-            17 => 1,
-            31 => 1,
-            41 => 1,
-            59 => 1,
-            63 => 1,
-            67 => 1,
-            79 => 1,
-            83 => 1,
-        ];
+        switch ($testFile) {
+        case 'ClosingDeclarationCommentUnitTest.1.inc':
+            return [
+                13  => 1,
+                17  => 1,
+                31  => 1,
+                41  => 1,
+                59  => 1,
+                63  => 1,
+                67  => 1,
+                79  => 1,
+                83  => 1,
+                89  => 1,
+                92  => 1,
+                98  => 1,
+                101 => 1,
+                106 => 1,
+                110 => 1,
+            ];
+
+        case 'ClosingDeclarationCommentUnitTest.4.inc':
+            return [8 => 1];
+
+        case 'ClosingDeclarationCommentUnitTest.5.inc':
+            return [11 => 1];
+
+        default:
+            return [];
+        }//end switch
 
     }//end getErrorList()
 
@@ -51,11 +71,23 @@ public function getErrorList()
      * The key of the array should represent the line number and the value
      * should represent the number of warnings that should occur on that line.
      *
+     * @param string $testFile The name of the test file being tested.
+     *
      * @return array<int, int>
      */
-    public function getWarningList()
+    public function getWarningList($testFile='')
     {
-        return [71 => 1];
+        switch ($testFile) {
+        case 'ClosingDeclarationCommentUnitTest.1.inc':
+            return [71 => 1];
+
+        case 'ClosingDeclarationCommentUnitTest.2.inc':
+        case 'ClosingDeclarationCommentUnitTest.3.inc':
+            return [7 => 1];
+
+        default:
+            return [];
+        }
 
     }//end getWarningList()