Merge pull request #333 from Yoast/JRF/yoastcs-filecomment-various-im…

…provements

Commenting/FileComment: allow for namespaced procedural files and other improvements

Yoast/Docs/Commenting/FileCommentStandard.xml

@@ -5,7 +5,7 @@
     >
     <standard>
     <![CDATA[
-    A file containing a (named) namespace declaration does not need a file docblock.
+    A file containing a (named) namespace declaration and an OO declaration does not need a file docblock.
     ]]>
     </standard>
     <code_comparison>

Yoast/Sniffs/Commenting/FileCommentSniff.php

@@ -5,9 +5,13 @@
 use PHP_CodeSniffer\Files\File;
 use PHP_CodeSniffer\Standards\Squiz\Sniffs\Commenting\FileCommentSniff as Squiz_FileCommentSniff;
 use PHP_CodeSniffer\Util\Tokens;
+use PHPCSUtils\Tokens\Collections;
 
 /**
- * Namespaced files do not need a file docblock in YoastCS.
+ * Namespaced files containing an OO structure do not need a file docblock in YoastCS.
+ *
+ * For namespaced files NOT containing an OO structure, a file docblock is permitted,
+ * though not required.
  *
  * Note: Files without a namespace declaration do still need a file docblock.
  * This includes files which have a non-named namespace declaration which
@@ -23,6 +27,8 @@
  * adjustments.}}
  *
  * @since 1.2.0
+ * @since 3.0.0 Behaviour for files containing an OO structure vs without has been updated
+ *              to allow a file docblock when there is no OO structure.
  */
 final class FileCommentSniff extends Squiz_FileCommentSniff {
 
@@ -38,6 +44,9 @@ public function process( File $phpcsFile, $stackPtr ) {
 
 		$tokens = $phpcsFile->getTokens();
 
+		/*
+		 * Check if the file is namespaced. If not, fall through to the parent sniff.
+		 */
 		$namespace_token = $stackPtr;
 		do {
 			$namespace_token = $phpcsFile->findNext( Tokens::$emptyTokens, ( $namespace_token + 1 ), null, true );
@@ -77,39 +86,57 @@ public function process( File $phpcsFile, $stackPtr ) {
 			return parent::process( $phpcsFile, $stackPtr );
 		}
 
+		/*
+		 * As of here, we know we are in a namespaced file.
+		 */
+
 		$comment_start = $phpcsFile->findNext( \T_WHITESPACE, ( $stackPtr + 1 ), $namespace_token, true );
 
-		if ( $comment_start !== false && $tokens[ $comment_start ]['code'] === \T_DOC_COMMENT_OPEN_TAG ) {
-
-			// Respect phpcs:disable comments in the file docblock.
-			$ignore = false;
-			if ( $phpcsFile->config->annotations === true && isset( $tokens[ $comment_start ]['comment_closer'] ) ) {
-				for ( $i = ( $comment_start + 1 ); $i < $tokens[ $comment_start ]['comment_closer']; $i++ ) {
-					if ( $tokens[ $i ]['code'] !== \T_PHPCS_DISABLE ) {
-						continue;
-					}
-
-					if ( empty( $tokens[ $i ]['sniffCodes'] ) === true
-						|| isset( $tokens[ $i ]['sniffCodes']['Yoast'] ) === true
-						|| isset( $tokens[ $i ]['sniffCodes']['Yoast.Commenting'] ) === true
-						|| isset( $tokens[ $i ]['sniffCodes']['Yoast.Commenting.FileComment'] ) === true
-						|| isset( $tokens[ $i ]['sniffCodes']['Yoast.Commenting.FileComment.Unnecessary'] ) === true
-					) {
-						$ignore = true;
-						break;
-					}
+		if ( $comment_start === false || $tokens[ $comment_start ]['code'] !== \T_DOC_COMMENT_OPEN_TAG ) {
+			// No file comment found, we're good.
+			return ( $phpcsFile->numTokens + 1 );
+		}
+
+		// Respect phpcs:disable comments in the file docblock.
+		if ( $phpcsFile->config->annotations === true && isset( $tokens[ $comment_start ]['comment_closer'] ) ) {
+			for ( $i = ( $comment_start + 1 ); $i < $tokens[ $comment_start ]['comment_closer']; $i++ ) {
+				if ( $tokens[ $i ]['code'] !== \T_PHPCS_DISABLE ) {
+					continue;
 				}
-			}
 
-			if ( $ignore === false ) {
-				$phpcsFile->addWarning(
-					'A file containing a (named) namespace declaration does not need a file docblock',
-					$comment_start,
-					'Unnecessary'
-				);
+				if ( empty( $tokens[ $i ]['sniffCodes'] ) === true
+					|| isset( $tokens[ $i ]['sniffCodes']['Yoast'] ) === true
+					|| isset( $tokens[ $i ]['sniffCodes']['Yoast.Commenting'] ) === true
+					|| isset( $tokens[ $i ]['sniffCodes']['Yoast.Commenting.FileComment'] ) === true
+					|| isset( $tokens[ $i ]['sniffCodes']['Yoast.Commenting.FileComment.Unnecessary'] ) === true
+				) {
+					// Applicable disable annotation found.
+					return ( $phpcsFile->numTokens + 1 );
+				}
 			}
 		}
 
+		/*
+		 * Okay, so we have a file docblock in a namespaced file, now check if there is a named
+		 * OO structure declaration.
+		 */
+		$find = Tokens::$ooScopeTokens;
+		unset( $find[ \T_ANON_CLASS ] );
+		$hasOODeclaration = $phpcsFile->findNext( $find, $next_non_empty );
+		if ( $hasOODeclaration === false ) {
+			/*
+			 * This is a file which doesn't contain (just) an OO declaration.
+			 * If there is a file docblock, allow it and check it like any other file docblock.
+			 */
+			return parent::process( $phpcsFile, $stackPtr );
+		}
+
+		$phpcsFile->addWarning(
+			'A file containing a (named) namespace declaration does not need a file docblock',
+			$comment_start,
+			'Unnecessary'
+		);
+
 		return ( $phpcsFile->numTokens + 1 );
 	}
 }

Yoast/Tests/Commenting/FileCommentUnitTest.13.inc

@@ -5,6 +5,6 @@ declare(strict_types=1);
 namespace Yoast\Plugin\Sub;
 
 /**
- * Class docblock. A file docblock is not needed in a namespaced file.
+ * Class docblock. A file docblock is not needed in a namespaced file containing an OO structure.
  */
 class Testing {}

Yoast/Tests/Commenting/FileCommentUnitTest.14.inc

@@ -1,6 +1,9 @@
 <?php
 /**
- * File Comment for a file WITH a namespace. This should be flagged as unnecessary.
+ * File Comment for a file WITH a namespace and containing an OO structure. This should be flagged as unnecessary.
+ *
+ * Note: a namespace _within_ a scoped `declare` statement is a parse error, but that's not the concern of this sniff.
+ * Scoped strict_types declare statements are also not allowed.
  *
  * @package Some\Package
  */

Yoast/Tests/Commenting/FileCommentUnitTest.20.inc

@@ -1,6 +1,7 @@
 <?php
 /**
- * File Comment for a file WITH a namespace, and containing irrelevant ignore comments. This should be flagged as unnecessary.
+ * File Comment for a file WITH a namespace and containing an OO structure, and containing irrelevant ignore comments.
+ * This should be flagged as unnecessary.
  *
  * @phpcs:disable Some.Other.SniffCode
  * @phpcs:disable Yet.Another.SniffCode

Yoast/Tests/Commenting/FileCommentUnitTest.21.inc

@@ -0,0 +1,2 @@
+<!-- Edge case test: empty file, fall through to parent sniff and flag for missing file comment.
+<?php

Yoast/Tests/Commenting/FileCommentUnitTest.22.inc

@@ -0,0 +1,8 @@
+<?php
+
+namespace\functionCall();
+
+/**
+ * Class docblock. No namespace (above is operator, not declaration), file comment is needed, but missing.
+ */
+class Testing {}

Yoast/Tests/Commenting/FileCommentUnitTest.23.inc

@@ -0,0 +1,20 @@
+<?php
+/**
+ * File Comment for a file without a namespace.
+ *
+ * For the purposes of the unit test, the docblock here needs to comply with the
+ * complete Squiz file comment rules as the ruleset is not taken into account
+ * when unit testing sniffs.
+ *
+ * @package    Some\Package
+ * @subpackage Something\Else
+ * @author     Squiz Pty Ltd <[email protected]>
+ * @copyright  2018 Squiz Pty Ltd (ABN 77 084 670 600)
+ */
+
+/**
+ * Interface docblock.
+ */
+interface Testing {
+	public function test();
+}

Yoast/Tests/Commenting/FileCommentUnitTest.24.inc

@@ -0,0 +1,9 @@
+<?php
+
+/**
+ * Interface docblock. No namespace, file comment is needed, but missing.
+ */
+interface Testing {
+	public function test();
+}
+

Yoast/Tests/Commenting/FileCommentUnitTest.25.inc

@@ -0,0 +1,11 @@
+<?php
+
+namespace Yoast\Plugin\Sub;
+
+/**
+ * Interface docblock. A file docblock is not needed in a namespaced file containing an OO structure.
+ */
+interface Testing {
+	public function test();
+}
+

Yoast/Tests/Commenting/FileCommentUnitTest.26.inc

@@ -0,0 +1,15 @@
+<?php
+/**
+ * File Comment for a file WITH a namespace and containing an OO structure. This should be flagged as unnecessary.
+ *
+ * @package Some\Package
+ */
+
+namespace Yoast\Plugin\Sub;
+
+/**
+ * Interface docblock.
+ */
+interface Testing {
+	public function test();
+}

Yoast/Tests/Commenting/FileCommentUnitTest.27.inc

@@ -0,0 +1,22 @@
+<?php
+/**
+ * File Comment for a file without a namespace.
+ *
+ * For the purposes of the unit test, the docblock here needs to comply with the
+ * complete Squiz file comment rules as the ruleset is not taken into account
+ * when unit testing sniffs.
+ *
+ * @package    Some\Package
+ * @subpackage Something\Else
+ * @author     Squiz Pty Ltd <[email protected]>
+ * @copyright  2018 Squiz Pty Ltd (ABN 77 084 670 600)
+ */
+
+/**
+ * Trait docblock.
+ */
+trait Testing {
+	public function test() {
+		echo namespace\SomeClass::$static_property; // This is not a namespace declaration, but use of the namespace operator.
+	}
+}

Yoast/Tests/Commenting/FileCommentUnitTest.28.inc

@@ -0,0 +1,6 @@
+<?php
+
+/**
+ * Trait docblock. No namespace, file comment is needed, but missing.
+ */
+trait Testing {}

Yoast/Tests/Commenting/FileCommentUnitTest.29.inc

@@ -0,0 +1,8 @@
+<?php
+
+namespace Yoast\Plugin\Sub;
+
+/**
+ * Trait docblock. A file docblock is not needed in a namespaced file containing an OO structure.
+ */
+trait Testing {}

Yoast/Tests/Commenting/FileCommentUnitTest.3.inc

@@ -3,6 +3,6 @@
 namespace Yoast\Plugin\Sub;
 
 /**
- * Class docblock. A file docblock is not needed in a namespaced file.
+ * Class docblock. A file docblock is not needed in a namespaced file containing an OO structure.
  */
 class Testing {}

Yoast/Tests/Commenting/FileCommentUnitTest.30.inc

@@ -0,0 +1,13 @@
+<?php
+/**
+ * File Comment for a file WITH a namespace and containing an OO structure. This should be flagged as unnecessary.
+ *
+ * @package Some\Package
+ */
+
+namespace Yoast\Plugin\Sub;
+
+/**
+ * Trait docblock.
+ */
+trait Testing {}

Yoast/Tests/Commenting/FileCommentUnitTest.31.inc

@@ -0,0 +1,22 @@
+<?php
+/**
+ * File Comment for a file without a namespace.
+ *
+ * For the purposes of the unit test, the docblock here needs to comply with the
+ * complete Squiz file comment rules as the ruleset is not taken into account
+ * when unit testing sniffs.
+ *
+ * @package    Some\Package
+ * @subpackage Something\Else
+ * @author     Squiz Pty Ltd <[email protected]>
+ * @copyright  2018 Squiz Pty Ltd (ABN 77 084 670 600)
+ */
+
+/**
+ * Enum docblock.
+ */
+enum Testing {
+	public function test() {
+		echo namespace\SomeClass::$static_property; // This is not a namespace declaration, but use of the namespace operator.
+	}
+}

Yoast/Tests/Commenting/FileCommentUnitTest.32.inc

@@ -0,0 +1,6 @@
+<?php
+
+/**
+ * Enum docblock. No namespace, file comment is needed, but missing.
+ */
+enum Testing {}

Yoast/Tests/Commenting/FileCommentUnitTest.33.inc

@@ -0,0 +1,8 @@
+<?php
+
+namespace Yoast\Plugin\Sub;
+
+/**
+ * Enum docblock. A file docblock is not needed in a namespaced file containing an OO structure.
+ */
+enum Testing {}

Yoast/Tests/Commenting/FileCommentUnitTest.34.inc

@@ -0,0 +1,13 @@
+<?php
+/**
+ * File Comment for a file WITH a namespace and containing an OO structure. This should be flagged as unnecessary.
+ *
+ * @package Some\Package
+ */
+
+namespace Yoast\Plugin\Sub;
+
+/**
+ * Enum docblock.
+ */
+enum Testing {}

Yoast/Tests/Commenting/FileCommentUnitTest.35.inc

@@ -0,0 +1,22 @@
+<?php
+/**
+ * File Comment for a file without a namespace and without OO structure.
+ *
+ * For the purposes of the unit test, the docblock here needs to comply with the
+ * complete Squiz file comment rules as the ruleset is not taken into account
+ * when unit testing sniffs.
+ *
+ * @package    Some\Package
+ * @subpackage Something\Else
+ * @author     Squiz Pty Ltd <[email protected]>
+ * @copyright  2018 Squiz Pty Ltd (ABN 77 084 670 600)
+ */
+
+do_something();
+
+/**
+ * Function docblock.
+ */
+function test() {
+	echo namespace\SomeClass::$static_property; // This is not a namespace declaration, but use of the namespace operator.
+}

Yoast/Tests/Commenting/FileCommentUnitTest.36.inc

@@ -0,0 +1,8 @@
+<?php
+
+/**
+ * Function docblock. No namespace, file comment is needed, but missing.
+ */
+function test() {}
+
+do_something();

Yoast/Tests/Commenting/FileCommentUnitTest.37.inc

@@ -0,0 +1,10 @@
+<?php
+
+namespace Yoast\Plugin\Sub;
+
+do_something();
+
+/**
+ * Function docblock. A file docblock is not needed in a namespaced file, but also not forbidden if the file doesn't contain an OO structure.
+ */
+function testing() {}