Skip to content

Commit 79ee409

Browse files
authored
Add Pantheon-WP coding standards for comment usage (#19)
* autoload both classes * Group pantheon rules together * exclude the multiline slash comment rule we're redeclaring in Pantheon-WP with a higher severity * duplicate exclusions in -Minimum We need to redeclare because we're redeclaring the parent rulesets * exclude deprecated VIP CSS/JS checks * add our Pantheon-WP checks * add DisallowMultilineSlashCommentSniff to enforce single-line comment usage * feat(sniffs): add sniff to disallow multiline slash comments Introduces the new `DisallowMultilineSlashCommentSniff` to improve comment consistency and readability. This sniff performs two main checks: - It flags consecutive single-line `//` comments, enforcing the use of `/* ... */` block syntax for multiline comments. - It issues a warning for any comment line that exceeds 80 characters to encourage better formatting. * add test cases for sniffs * add helper scripts so we can test things manually easier * break the reusable single line comment code into a static method * use the static method to call the single line comment sniff * update the tess to support passing a ruleset to test * add line breaks between tests for readability * change echo to printf for portability * use file headers to determine test name * just echo an empty line * maybe a space? * add an error param so the severity level can be adjusted * pass an error for Pantheon-WP * include severity in the tests
1 parent 40c96b5 commit 79ee409

File tree

8 files changed

+263
-20
lines changed

8 files changed

+263
-20
lines changed
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
<?php
2+
/**
3+
* Verifies that multiline comments are not used.
4+
*
5+
* @package Pantheon-WP-Minimum
6+
*/
7+
8+
namespace Pantheon_WP_Minimum\Sniffs\Commenting;
9+
10+
use PHP_CodeSniffer\Files\File;
11+
use PHP_CodeSniffer\Sniffs\Sniff;
12+
13+
/**
14+
* Verifies that multiline comments are not used.
15+
*/
16+
class DisallowMultilineSlashCommentSniff implements Sniff {
17+
18+
/**
19+
* Returns an array of tokens this test wants to listen for.
20+
*
21+
* @return array
22+
*/
23+
public function register() {
24+
return [ T_COMMENT ];
25+
}
26+
27+
/**
28+
* Processes this test, when one of its tokens is encountered.
29+
*
30+
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
31+
* @param int $stackPtr The position of the current token in the
32+
* stack passed in $tokens.
33+
*
34+
* @return void
35+
*/
36+
public function process( File $phpcsFile, $stackPtr ) {
37+
self::getConsecutiveSingleLineComments( $phpcsFile, $stackPtr );
38+
}
39+
40+
/**
41+
* Checks for consecutive single-line comments and reports them.
42+
*
43+
* @param File $phpcsFile The file being scanned.
44+
* @param int $stackPtr The position of the current token in the stack.
45+
* @param string $error The type of error to report ('warning' or 'error').
46+
*
47+
* @return void
48+
*/
49+
public static function getConsecutiveSingleLineComments( File $phpcsFile, $stackPtr, $error = 'warning' ) {
50+
$tokens = $phpcsFile->getTokens();
51+
$token = $tokens[ $stackPtr ];
52+
53+
// We are only interested in single-line comments.
54+
if ( '//' !== substr( $token['content'], 0, 2 ) ) {
55+
return;
56+
}
57+
58+
// Check the previous line to see if we are already in a block.
59+
$prevLine = $token['line'] - 1;
60+
$prevComment = $phpcsFile->findPrevious( T_COMMENT, $stackPtr - 1, null, false, null, true );
61+
if ( false !== $prevComment && $tokens[$prevComment]['line'] === $prevLine ) {
62+
if ( '//' === substr( $tokens[$prevComment]['content'], 0, 2 ) ) {
63+
return; // This is not the start of the block.
64+
}
65+
}
66+
67+
// This is the start of a block. Now, count how many lines it spans.
68+
$lineCount = 1;
69+
$nextComment = $stackPtr;
70+
while ( true ) {
71+
$nextComment = $phpcsFile->findNext( T_COMMENT, $nextComment + 1, null, false, null, true );
72+
if ( false === $nextComment || $tokens[$nextComment]['line'] !== ( $token['line'] + $lineCount ) ) {
73+
break; // The block has ended.
74+
}
75+
$lineCount++;
76+
}
77+
78+
if ( $lineCount > 1 ) {
79+
$message = 'A block of %s consecutive single-line comments was found starting on this line. Please use a /* ... */ block instead.';
80+
$data = [ $lineCount ];
81+
if ( 'warning' === $error ) {
82+
$phpcsFile->addWarning( $message, $stackPtr, 'Found', $data );
83+
} else {
84+
$phpcsFile->addError( $message, $stackPtr, 'Found', $data );
85+
}
86+
}
87+
}
88+
}

Pantheon-WP-Minimum/ruleset.xml

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,6 @@
4141
<rule ref="WordPress.DB.RestrictedFunctions" />
4242
<rule ref="WordPress.DB.RestrictedClasses" />
4343

44-
<!-- Disallow rename() function -->
45-
<rule ref="./Sniffs/Files/DisallowRenameFunctionSniff.php">
46-
<type>error</type>
47-
</rule>
48-
4944
<!-- Disallow eval(). (From WordPress-Core) -->
5045
<rule ref="Squiz.PHP.Eval"/>
5146
<rule ref="Squiz.PHP.Eval.Discouraged">
@@ -94,4 +89,14 @@
9489

9590
<!-- Ignore PHP-related errors. -->
9691
<ini name="error_reporting" value="E_ALL &#38; ~E_DEPRECATED" />
92+
93+
<!-- Pantheon specific rules -->
94+
95+
<!-- Disallow rename() function -->
96+
<rule ref="./Sniffs/Files/DisallowRenameFunctionSniff.php">
97+
<type>error</type>
98+
</rule>
99+
100+
<!-- Disallow multiline // comments -->
101+
<rule ref="./Sniffs/Commenting/DisallowMultilineSlashCommentSniff.php"/>
97102
</ruleset>
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
<?php
2+
/**
3+
* Verifies that multiline comments are not used.
4+
*
5+
* @package Pantheon-WP
6+
*/
7+
8+
namespace Pantheon_WP\Sniffs\Commenting;
9+
10+
use PHP_CodeSniffer\Files\File;
11+
use PHP_CodeSniffer\Sniffs\Sniff;
12+
use Pantheon_WP_Minimum\Sniffs\Commenting\DisallowMultilineSlashCommentSniff as PantheonWPMinimumCommenting;
13+
14+
/**
15+
* Verifies that multiline comments are not used.
16+
*/
17+
class DisallowMultilineSlashCommentSniff implements Sniff {
18+
19+
/**
20+
* Returns an array of tokens this test wants to listen for.
21+
*
22+
* @return array
23+
*/
24+
public function register() {
25+
return [ T_COMMENT ];
26+
}
27+
28+
/**
29+
* Processes this test, when one of its tokens is encountered.
30+
*
31+
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
32+
* @param int $stackPtr The position of the current token in the
33+
* stack passed in $tokens.
34+
*
35+
* @return void
36+
*/
37+
public function process( File $phpcsFile, $stackPtr ) {
38+
$tokens = $phpcsFile->getTokens();
39+
$token = $tokens[ $stackPtr ];
40+
41+
// Pull sniff for single-line comments from Pantheon-WP-Minimum.
42+
PantheonWPMinimumCommenting::getConsecutiveSingleLineComments( $phpcsFile, $stackPtr, 'error' );
43+
44+
// Check for long lines within block comments.
45+
if ( substr( $token['content'], 0, 2 ) === '/*' ) {
46+
// First, check the line of the starting token itself.
47+
if ( $tokens[$stackPtr]['length'] > 80 ) {
48+
$warning = 'For readability, it is recommended to break long comment lines into multiple lines.';
49+
$phpcsFile->addWarning( $warning, $stackPtr, 'LongLine' );
50+
return; // Only one warning per comment block.
51+
}
52+
53+
// Then, iterate through subsequent tokens on following lines.
54+
for ( $i = ( $stackPtr + 1 ); $i < count( $tokens ); $i++ ) {
55+
// Stop if we are no longer in a comment.
56+
if ( $tokens[$i]['code'] !== T_COMMENT ) {
57+
break;
58+
}
59+
60+
// Stop if the line doesn't start with a star, indicating the end of the block.
61+
if ( strpos( trim( $tokens[$i]['content'] ), '*' ) !== 0 ) {
62+
break;
63+
}
64+
65+
if ( $tokens[$i]['length'] > 80 ) {
66+
$warning = 'For readability, it is recommended to break long comment lines into multiple lines.';
67+
$phpcsFile->addWarning( $warning, $i, 'LongLine' );
68+
return; // Only one warning per comment block.
69+
}
70+
}
71+
}
72+
}
73+
}

Pantheon-WP/ruleset.xml

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,14 @@
88
<arg name="extensions" value="php" />
99

1010
<!-- Include everything in the minimum ruleset -->
11-
<rule ref="Pantheon-WP-Minimum" />
11+
<rule ref="Pantheon-WP-Minimum">
12+
<exclude name="Pantheon_WP_Minimum.Commenting.DisallowMultilineSlashComment.Found" />
13+
</rule>
1214

1315
<!-- Rulesets to use -->
1416
<rule ref="PHPCompatibilityWP" />
1517
<rule ref="WordPress-Core">
18+
<exclude name="Generic.Functions.CallTimePassByReference" />
1619
<exclude name="Universal.Arrays.DisallowShortArraySyntax" />
1720
<exclude name="Generic.Formatting.MultipleStatementAlignment.NotSameWarning" />
1821
<exclude name="PEAR.Functions.FunctionCallSignature.ContentAfterOpenBracket" />
@@ -48,6 +51,15 @@
4851
<exclude name="WordPressVIPMinimum.Functions.RestrictedFunctions.wp_remote_get_wp_remote_get" />
4952
<exclude name="WordPressVIPMinimum.Files.IncludingFile.UsingVariable" />
5053
<exclude name="WordPressVIPMinimum.Variables.RestrictedVariables.cache_constraints___SERVER__HTTP_USER_AGENT__" />
54+
<exclude name="WordPressVIPMinimum.Functions.RestrictedFunctions.file_ops_rename" />
55+
56+
<!-- Exclude CSS/JS rules that are deprecated in PHPCS 3.x -->
57+
<exclude name="WordPressVIPMinimum.JS.Window" />
58+
<exclude name="WordPressVIPMinimum.JS.DangerouslySetInnerHTML" />
59+
<exclude name="WordPressVIPMinimum.JS.InnerHTML" />
60+
<exclude name="WordPressVIPMinimum.JS.StrippingTags" />
61+
<exclude name="WordPressVIPMinimum.JS.StringConcat" />
62+
<exclude name="WordPressVIPMinimum.JS.HTMLExecutingFunctions" />
5163
</rule>
5264

5365
<!-- Rule exclusions -->
@@ -72,5 +84,8 @@
7284
<rule ref="PSR2R.ControlStructures.NoInlineAssignment" />
7385

7486
<!-- Ignore PHP-related errors. -->
75-
<ini name="error_reporting" value="E_ALL &#38; ~E_DEPRECATED" />
87+
<ini name="error_reporting" value="E_ALL &#38; ~E_DEPRECATED" />
88+
89+
<!-- Pantheon-WP Sniffs -->
90+
<rule ref="./Sniffs/Commenting/DisallowMultilineSlashCommentSniff.php"/>
7691
</ruleset>

composer.json

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,10 @@
2121
"squizlabs/php_codesniffer": "^3.13",
2222
"yoast/phpunit-polyfills": "4.0.0"
2323
},
24-
"autoload-dev": {
24+
"autoload": {
2525
"psr-4": {
26-
"Pantheon_WP_Minimum\\": "Pantheon-WP-Minimum/"
26+
"Pantheon_WP_Minimum\\": "Pantheon-WP-Minimum/",
27+
"Pantheon_WP\\": "Pantheon-WP/"
2728
}
2829
},
2930
"config": {
@@ -33,8 +34,21 @@
3334
}
3435
},
3536
"scripts": {
37+
"lint:minimum": [
38+
"vendor/bin/phpcs -s ./tests --standard=./Pantheon-WP-Minimum/ruleset.xml"
39+
],
40+
"lint:standard": [
41+
"vendor/bin/phpcs -s ./tests --standard=./Pantheon-WP/ruleset.xml"
42+
],
43+
"test:minimum": [
44+
"for test_file in tests/*.php; do ./tests/run-test.sh Pantheon-WP-Minimum \"$test_file\"; done"
45+
],
46+
"test:standard": [
47+
"for test_file in tests/*.php; do ./tests/run-test.sh Pantheon-WP \"$test_file\"; done"
48+
],
3649
"test": [
37-
"./tests/run-test.sh tests/*.php"
50+
"@test:minimum",
51+
"@test:standard"
3852
]
3953
}
4054
}

tests/run-test.sh

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,41 @@
11
#!/bin/bash
2+
set -e
23

3-
TEST_FILE=$1
4+
RULESET=$1
5+
TEST_FILE=$2
6+
7+
if [ -z "$RULESET" ]; then
8+
echo "Usage: $0 <ruleset> <test_file>"
9+
exit 1
10+
fi
411

512
if [ ! -f "$TEST_FILE" ]; then
613
echo "Test file not found: $TEST_FILE"
714
exit 1
815
fi
916

10-
echo "Running test: $TEST_FILE"
17+
# Get the test name from the file header.
18+
TEST_NAME=$(sed -n '3s/ \* //p' "$TEST_FILE")
19+
20+
echo ""
21+
echo "Running test: ${TEST_NAME} with ruleset: $RULESET"
22+
echo "$TEST_FILE"
1123

1224
# Use --basepath=. to ensure relative paths in the report.
13-
JSON_REPORT=$(phpcs --standard=Pantheon-WP-Minimum/ruleset.xml --report=json --basepath=. --warning-severity=0 "$TEST_FILE" 2>/dev/null | sed -n '/{/,$p')
25+
JSON_REPORT=$(phpcs --standard="./$RULESET/ruleset.xml" --report=json --basepath=. "$TEST_FILE" 2>/dev/null | sed -n '/{/,$p')
26+
27+
# Debug
28+
# echo "JSON Report: ${JSON_REPORT}"
1429

15-
# Extract actual and expected errors.
16-
ACTUAL_OUTPUT=$(echo "$JSON_REPORT" | jq -r '.files."'"$TEST_FILE"'".messages[]?.source' | sort)
17-
EXPECTED_OUTPUT=$(sed -n 's/.*@expectedError //p' "$TEST_FILE" | sort)
30+
# Extract actual and expected errors, including severity.
31+
ACTUAL_OUTPUT=$(echo "$JSON_REPORT" | jq -r '.files."'"$TEST_FILE"'".messages[] | "[\(.type | ascii_upcase)] \(.source)"' | sort)
32+
EXPECTED_OUTPUT=$( (sed -n "s/.*@expectedError\[$RULESET\] /\[ERROR\] /p" "$TEST_FILE"; sed -n "s/.*@expectedWarning\[$RULESET\] /\[WARNING\] /p" "$TEST_FILE") | sort)
1833

1934
echo "--------------------------------------------------"
20-
echo "EXPECTED ERRORS:"
35+
echo "EXPECTED ISSUES:"
2136
echo -e "${EXPECTED_OUTPUT:-<none>}"
2237
echo "--------------------------------------------------"
23-
echo "ACTUAL ERRORS:"
38+
echo "ACTUAL ISSUES:"
2439
echo -e "${ACTUAL_OUTPUT:-<none>}"
2540
echo "--------------------------------------------------"
2641

@@ -33,4 +48,6 @@ if ! diff <(echo "$ACTUAL_OUTPUT") <(echo "$EXPECTED_OUTPUT") > /dev/null; then
3348
fi
3449

3550
echo "RESULT: PASSED ✅"
51+
echo " "
52+
3653
exit 0
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php
2+
/**
3+
* Multiline comments should use /* syntax.
4+
*
5+
* @expectedWarning[Pantheon-WP-Minimum] Pantheon_WP_Minimum.Commenting.DisallowMultilineSlashComment.Found
6+
* @expectedError[Pantheon-WP] Pantheon_WP.Commenting.DisallowMultilineSlashComment.Found
7+
* @expectedWarning[Pantheon-WP] Pantheon_WP.Commenting.DisallowMultilineSlashComment.LongLine
8+
*
9+
* @package Pantheon-WP-Coding-Standards
10+
*/
11+
12+
// This comment should fail with a warning.
13+
// Multiline comments should use /* ... */ syntax, not multiple // comments.
14+
// What's with the life preserver? Doc, she's beautiful. She's crazy about me. Look at this,
15+
// look what she wrote me, Doc. That says it all. Doc, you're my only hope. Oh, uh, hey you,
16+
// get your damn hands off her. Do you really think I oughta swear? Well, now we gotta sneak
17+
// this back into my laboratory, we've gotta get you home. Jennifer.
18+
19+
/*
20+
* This comment should not fail but should trigger an info to break the comment up. What's with the life preserver? Doc, she's beautiful. She's crazy about me. Look at this, look what she wrote me, Doc. That says it all. Doc, you're my only hope. Oh, uh, hey you, get your damn hands off her. Do you really think I oughta swear? Well, now we gotta sneak this back into my laboratory, we've gotta get you home. Jennifer.
21+
*/
22+
23+
/*
24+
* This comment is ideal. It uses the correct syntax and is not too long.
25+
* What's with the life preserver? Doc, she's beautiful. She's crazy about me.
26+
* Look at this, look what she wrote me, Doc. That says it all. Doc, you're my
27+
* only hope. Oh, uh, hey you, get your damn hands off her. Do you really think
28+
* I oughta swear? Well, now we gotta sneak this back into my laboratory, we've
29+
* gotta get you home. Jennifer.
30+
*/

tests/test-disallow-rename-function.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
<?php
22
/**
3-
* DisallowRenameFunctionTest.inc
3+
* Disallow usage of the rename() function.
44
*
5-
* @expectedError Pantheon_WP_Minimum.Files.DisallowRenameFunction.RenameFunctionDisallowed
5+
* @expectedError[Pantheon-WP-Minimum] Pantheon_WP_Minimum.Files.DisallowRenameFunction.RenameFunctionDisallowed
6+
* @expectedError[Pantheon-WP] Pantheon_WP_Minimum.Files.DisallowRenameFunction.RenameFunctionDisallowed
67
*
78
* @package Pantheon-WP-Coding-Standards
89
*/

0 commit comments

Comments
 (0)