Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Nov 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 {}
5 changes: 4 additions & 1 deletion Yoast/Tests/Commenting/FileCommentUnitTest.14.inc
Original file line number Diff line number Diff line change
@@ -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
*/
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
2 changes: 2 additions & 0 deletions Yoast/Tests/Commenting/FileCommentUnitTest.21.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<!-- Edge case test: empty file, fall through to parent sniff and flag for missing file comment.
<?php
8 changes: 8 additions & 0 deletions Yoast/Tests/Commenting/FileCommentUnitTest.22.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?php

namespace\functionCall();

/**
* Class docblock. No namespace (above is operator, not declaration), file comment is needed, but missing.
*/
class Testing {}
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() {}
Loading