Skip to content

Commit

Permalink
Commenting/FileComment: allow for namespaced procedural files
Browse files Browse the repository at this point in the history
The `Yoast.Commenting.FileComment` sniff would previously demand a file docblock for non-namespaced files and recommend a file docblock be removed for namespaced files.

This is all well and good for namespaced files containing an OO declaration, where the OO structure will generally have a docblock, but for namespaced procedural files, this might mean the file will end up having no documentation whatsoever, which could be counter-productive.

While namespaced procedural files are not that common in the Yoast codebases, typically a PHPUnit bootstrap file may be a namespaced procedural file.

This commit updates the sniff to only recommend removing the file docblock if the file _also_ contains a named OO declaration.

For namespaced files which don't contain an OO declaration, the file docblock will still be allowed (but not required) and will no longer be flagged as unnecessary.
If a docblock is found in such a file, it will be checked for compliance with file docblock rules like any other file docblock.

Includes ample tests.
Includes updated XML documentation.
  • Loading branch information
jrfnl committed Nov 4, 2023
1 parent 347caf6 commit 7eb259e
Show file tree
Hide file tree
Showing 27 changed files with 324 additions and 34 deletions.
2 changes: 1 addition & 1 deletion Yoast/Docs/Commenting/FileCommentStandard.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
81 changes: 54 additions & 27 deletions Yoast/Sniffs/Commenting/FileCommentSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {

Expand All @@ -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 );
Expand Down Expand Up @@ -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 );
}
}
2 changes: 1 addition & 1 deletion Yoast/Tests/Commenting/FileCommentUnitTest.13.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
2 changes: 1 addition & 1 deletion Yoast/Tests/Commenting/FileCommentUnitTest.14.inc
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?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.
Expand Down
3 changes: 2 additions & 1 deletion Yoast/Tests/Commenting/FileCommentUnitTest.20.inc
Original file line number Diff line number Diff line change
@@ -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
Expand Down
20 changes: 20 additions & 0 deletions Yoast/Tests/Commenting/FileCommentUnitTest.23.inc
Original file line number Diff line number Diff line change
@@ -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();
}
9 changes: 9 additions & 0 deletions Yoast/Tests/Commenting/FileCommentUnitTest.24.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?php

/**
* Interface docblock. No namespace, file comment is needed, but missing.
*/
interface Testing {
public function test();
}

11 changes: 11 additions & 0 deletions Yoast/Tests/Commenting/FileCommentUnitTest.25.inc
Original file line number Diff line number Diff line change
@@ -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();
}

15 changes: 15 additions & 0 deletions Yoast/Tests/Commenting/FileCommentUnitTest.26.inc
Original file line number Diff line number Diff line change
@@ -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();
}
22 changes: 22 additions & 0 deletions Yoast/Tests/Commenting/FileCommentUnitTest.27.inc
Original file line number Diff line number Diff line change
@@ -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.
}
}
6 changes: 6 additions & 0 deletions Yoast/Tests/Commenting/FileCommentUnitTest.28.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<?php

/**
* Trait docblock. No namespace, file comment is needed, but missing.
*/
trait Testing {}
8 changes: 8 additions & 0 deletions Yoast/Tests/Commenting/FileCommentUnitTest.29.inc
Original file line number Diff line number Diff line change
@@ -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 {}
2 changes: 1 addition & 1 deletion Yoast/Tests/Commenting/FileCommentUnitTest.3.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
13 changes: 13 additions & 0 deletions Yoast/Tests/Commenting/FileCommentUnitTest.30.inc
Original file line number Diff line number Diff line change
@@ -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 {}
22 changes: 22 additions & 0 deletions Yoast/Tests/Commenting/FileCommentUnitTest.31.inc
Original file line number Diff line number Diff line change
@@ -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.
}
}
6 changes: 6 additions & 0 deletions Yoast/Tests/Commenting/FileCommentUnitTest.32.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<?php

/**
* Enum docblock. No namespace, file comment is needed, but missing.
*/
enum Testing {}
8 changes: 8 additions & 0 deletions Yoast/Tests/Commenting/FileCommentUnitTest.33.inc
Original file line number Diff line number Diff line change
@@ -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 {}
13 changes: 13 additions & 0 deletions Yoast/Tests/Commenting/FileCommentUnitTest.34.inc
Original file line number Diff line number Diff line change
@@ -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 {}
22 changes: 22 additions & 0 deletions Yoast/Tests/Commenting/FileCommentUnitTest.35.inc
Original file line number Diff line number Diff line change
@@ -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.
}
8 changes: 8 additions & 0 deletions Yoast/Tests/Commenting/FileCommentUnitTest.36.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?php

/**
* Function docblock. No namespace, file comment is needed, but missing.
*/
function test() {}

do_something();
10 changes: 10 additions & 0 deletions Yoast/Tests/Commenting/FileCommentUnitTest.37.inc
Original file line number Diff line number Diff line change
@@ -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() {}
22 changes: 22 additions & 0 deletions Yoast/Tests/Commenting/FileCommentUnitTest.38.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php
/**
* File Comment for a file WITH a namespace, but NOT containing an OO structure. This should NOT be flagged as unnecessary.
*
* 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)
*/

namespace Yoast\Plugin\Sub;

use Ab\C;

/**
* Function docblock.
*/
function test() {}
Loading

0 comments on commit 7eb259e

Please sign in to comment.