-
-
Notifications
You must be signed in to change notification settings - Fork 480
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #596 from jrfnl/WPCS/feature/class-brace-same-line
Add (new) sniff for class opening brace on same line to `core` ruleset.
- Loading branch information
Showing
5 changed files
with
360 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
134 changes: 134 additions & 0 deletions
134
WordPress/Sniffs/Classes/ClassOpeningStatementSniff.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,134 @@ | ||
<?php | ||
/** | ||
* WordPress Coding Standard. | ||
* | ||
* @category PHP | ||
* @package PHP_CodeSniffer | ||
* @link https://make.wordpress.org/core/handbook/best-practices/coding-standards/ | ||
*/ | ||
|
||
/** | ||
* WordPress_Classes_ClassOpeningStatementSniff. | ||
* | ||
* Checks that the opening brace of a class or interface is on the same line | ||
* as the class declaration. | ||
* | ||
* Also checks that the brace is the last thing on that line and has precisely one space before it. | ||
* | ||
* Loosely based on Generic_Sniffs_Functions_OpeningFunctionBraceKernighanRitchieSniff. | ||
* | ||
* @category PHP | ||
* @package PHP_CodeSniffer | ||
* @author Juliette Reinders Folmer <wpplugins_nospam@adviesenzo.nl> | ||
*/ | ||
class WordPress_Sniffs_Classes_ClassOpeningStatementSniff implements PHP_CodeSniffer_Sniff { | ||
|
||
/** | ||
* Returns an array of tokens this test wants to listen for. | ||
* | ||
* @return array | ||
*/ | ||
public function register() { | ||
return array( | ||
T_CLASS, | ||
T_INTERFACE, | ||
T_TRAIT, | ||
); | ||
|
||
} // end register() | ||
|
||
/** | ||
* Processes this test, when one of its tokens is encountered. | ||
* | ||
* @param PHP_CodeSniffer_File $phpcsFile The file being scanned. | ||
* @param int $stackPtr The position of the current token in the | ||
* stack passed in $tokens. | ||
* | ||
* @return void | ||
*/ | ||
public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr ) { | ||
$tokens = $phpcsFile->getTokens(); | ||
$scope_identifier = $phpcsFile->findNext( T_STRING, ( $stackPtr + 1 ) ); | ||
$errorData = array( | ||
strtolower( $tokens[ $stackPtr ]['content'] ) . ' ' . $tokens[ $scope_identifier ]['content'] | ||
); | ||
|
||
if ( ! isset( $tokens[ $stackPtr ]['scope_opener'] ) ) { | ||
$error = 'Possible parse error: %s missing opening or closing brace'; | ||
$phpcsFile->addWarning( $error, $stackPtr, 'MissingBrace', $errorData ); | ||
return; | ||
} | ||
|
||
$openingBrace = $tokens[ $stackPtr ]['scope_opener']; | ||
|
||
/* | ||
* Is the brace on the same line as the class/interface/trait declaration ? | ||
*/ | ||
$lastClassLineToken = $phpcsFile->findPrevious( T_STRING, ( $openingBrace - 1 ), $stackPtr ); | ||
$lastClassLine = $tokens[ $lastClassLineToken ]['line']; | ||
$braceLine = $tokens[ $openingBrace ]['line']; | ||
$lineDifference = ( $braceLine - $lastClassLine ); | ||
|
||
if ( $lineDifference > 0 ) { | ||
$phpcsFile->recordMetric( $stackPtr, 'Class opening brace placement', 'new line' ); | ||
$error = 'Opening brace should be on the same line as the declaration for %s'; | ||
$fix = $phpcsFile->addFixableError( $error, $openingBrace, 'BraceOnNewLine', $errorData ); | ||
if ( true === $fix ) { | ||
$phpcsFile->fixer->beginChangeset(); | ||
$phpcsFile->fixer->addContent( $lastClassLineToken, ' {' ); | ||
$phpcsFile->fixer->replaceToken( $openingBrace, '' ); | ||
$phpcsFile->fixer->endChangeset(); | ||
} | ||
} else { | ||
$phpcsFile->recordMetric( $stackPtr, 'Class opening brace placement', 'same line' ); | ||
} | ||
|
||
/* | ||
* Is the opening brace the last thing on the line ? | ||
*/ | ||
$next = $phpcsFile->findNext( T_WHITESPACE, ( $openingBrace + 1 ), null, true ); | ||
if ( $tokens[ $next ]['line'] === $tokens[ $openingBrace ]['line'] ) { | ||
if ( $next === $tokens[ $stackPtr ]['scope_closer'] ) { | ||
// Ignore empty classes. | ||
return; | ||
} | ||
|
||
$error = 'Opening brace must be the last content on the line'; | ||
$fix = $phpcsFile->addFixableError( $error, $openingBrace, 'ContentAfterBrace' ); | ||
if ( true === $fix ) { | ||
$phpcsFile->fixer->addNewline( $openingBrace ); | ||
} | ||
} | ||
|
||
// Only continue checking if the opening brace looks good. | ||
if ( $lineDifference > 0 ) { | ||
return; | ||
} | ||
|
||
/* | ||
* Is there precisely one space before the opening brace ? | ||
*/ | ||
if ( T_WHITESPACE !== $tokens[ ( $openingBrace - 1 ) ]['code'] ) { | ||
$length = 0; | ||
} elseif ( "\t" === $tokens[ ( $openingBrace - 1 ) ]['content'] ) { | ||
$length = '\t'; | ||
} else { | ||
$length = strlen( $tokens[ ( $openingBrace - 1 ) ]['content'] ); | ||
} | ||
|
||
if ( 1 !== $length ) { | ||
$error = 'Expected 1 space before opening brace; found %s'; | ||
$data = array( $length ); | ||
$fix = $phpcsFile->addFixableError( $error, $openingBrace, 'SpaceBeforeBrace', $data ); | ||
if ( true === $fix ) { | ||
if ( 0 === $length || '\t' === $length ) { | ||
$phpcsFile->fixer->addContentBefore( $openingBrace, ' ' ); | ||
} else { | ||
$phpcsFile->fixer->replaceToken( ( $openingBrace - 1 ), ' ' ); | ||
} | ||
} | ||
} | ||
|
||
} // end process() | ||
|
||
} // end class |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
<?php | ||
|
||
// The following are all ok. | ||
interface Test_Interface_Ok_A { | ||
} | ||
|
||
class Test_Class_Ok_A { | ||
} | ||
|
||
class Test_Class_Ok_B extends Test_Class_Ok_A { | ||
} | ||
|
||
class Test_Class_Ok_C implements Test_Interface_Ok_A { | ||
} | ||
|
||
|
||
// These are all incorrect. | ||
interface Test_Interface_Bad_A | ||
{ // There should be no content after the brace. | ||
} | ||
|
||
class Test_Class_Bad_A | ||
{ | ||
} | ||
|
||
class Test_Class_Bad_B extends Test_Class_Bad_A | ||
|
||
{ // There should be no content after the brace. | ||
} | ||
|
||
class Test_Class_Bad_C implements Test_Interface_Bad_A | ||
|
||
|
||
{ | ||
} | ||
|
||
// These should all be flagged for wrong whitespace before opening brace. | ||
interface Test_Interface_Bad_C { | ||
} | ||
|
||
class Test_Class_Bad_G { | ||
} | ||
|
||
class Test_Class_Bad_H extends Test_Class_Bad_G { | ||
} | ||
|
||
class Test_Class_Bad_I implements Test_Interface_Bad_C{ | ||
} | ||
|
||
// This one should be flagged as a potential parse error. | ||
class Test_Class_Bad_H | ||
|
||
// This is OK. | ||
class A_Class_With_Really_Long_Name | ||
extends Another_Class_With_A_Really_Long_Name { | ||
|
||
} | ||
|
||
// This is OK too. | ||
class A_Class_With_Really_Long_Name_2 | ||
extends Another_Class_With_A_Really_Long_Name | ||
implements Some_Interface_With_A_Long_Name, | ||
Another_Interface_With_A_Long_Name { | ||
|
||
} | ||
|
||
// But this is not. | ||
class A_Class_With_Really_Long_Name_3 | ||
extends Another_Class_With_A_Really_Long_Name | ||
{ | ||
|
||
} | ||
|
||
// Nor is this. | ||
class A_Class_With_Really_Long_Name_4 | ||
extends Another_Class_With_A_Really_Long_Name | ||
implements Some_Interface_With_A_Long_Name, | ||
Another_Interface_With_A_Long_Name | ||
{ | ||
|
||
} |
81 changes: 81 additions & 0 deletions
81
WordPress/Tests/Classes/ClassOpeningStatementUnitTest.inc.fixed
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
<?php | ||
|
||
// The following are all ok. | ||
interface Test_Interface_Ok_A { | ||
} | ||
|
||
class Test_Class_Ok_A { | ||
} | ||
|
||
class Test_Class_Ok_B extends Test_Class_Ok_A { | ||
} | ||
|
||
class Test_Class_Ok_C implements Test_Interface_Ok_A { | ||
} | ||
|
||
|
||
// These are all incorrect. | ||
interface Test_Interface_Bad_A { | ||
// There should be no content after the brace. | ||
} | ||
|
||
class Test_Class_Bad_A { | ||
|
||
} | ||
|
||
class Test_Class_Bad_B extends Test_Class_Bad_A { | ||
|
||
// There should be no content after the brace. | ||
} | ||
|
||
class Test_Class_Bad_C implements Test_Interface_Bad_A { | ||
|
||
|
||
|
||
} | ||
|
||
// These should all be flagged for wrong whitespace before opening brace. | ||
interface Test_Interface_Bad_C { | ||
} | ||
|
||
class Test_Class_Bad_G { | ||
} | ||
|
||
class Test_Class_Bad_H extends Test_Class_Bad_G { | ||
} | ||
|
||
class Test_Class_Bad_I implements Test_Interface_Bad_C { | ||
} | ||
|
||
// This one should be flagged as a potential parse error. | ||
class Test_Class_Bad_H | ||
|
||
// This is OK. | ||
class A_Class_With_Really_Long_Name | ||
extends Another_Class_With_A_Really_Long_Name { | ||
|
||
} | ||
|
||
// This is OK too. | ||
class A_Class_With_Really_Long_Name_2 | ||
extends Another_Class_With_A_Really_Long_Name | ||
implements Some_Interface_With_A_Long_Name, | ||
Another_Interface_With_A_Long_Name { | ||
|
||
} | ||
|
||
// But this is not. | ||
class A_Class_With_Really_Long_Name_3 | ||
extends Another_Class_With_A_Really_Long_Name { | ||
|
||
|
||
} | ||
|
||
// Nor is this. | ||
class A_Class_With_Really_Long_Name_4 | ||
extends Another_Class_With_A_Really_Long_Name | ||
implements Some_Interface_With_A_Long_Name, | ||
Another_Interface_With_A_Long_Name { | ||
|
||
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
<?php | ||
/** | ||
* Unit test class for WordPress Coding Standard. | ||
* | ||
* @category PHP | ||
* @package PHP_CodeSniffer | ||
* @link https://make.wordpress.org/core/handbook/best-practices/coding-standards/ | ||
*/ | ||
|
||
/** | ||
* Unit test class for the ClassOpeningStatement sniff. | ||
* | ||
* A sniff unit test checks a .inc file for expected violations of a single | ||
* coding standard. Expected errors and warnings are stored in this class. | ||
* | ||
* @category PHP | ||
* @package PHP_CodeSniffer | ||
* @author Juliette Reinders Folmer <wpplugins_nospam@adviesenzo.nl> | ||
*/ | ||
class WordPress_Tests_Classes_ClassOpeningStatementUnitTest extends AbstractSniffUnitTest { | ||
|
||
/** | ||
* Returns the lines where errors should occur. | ||
* | ||
* 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. | ||
* | ||
* @return array<int, int> | ||
*/ | ||
public function getErrorList() { | ||
|
||
return array( | ||
19 => 2, | ||
23 => 1, | ||
28 => 2, | ||
34 => 1, | ||
34 => 1, | ||
38 => 1, | ||
41 => 1, | ||
44 => 1, | ||
47 => 1, | ||
70 => 1, | ||
79 => 1, | ||
); | ||
|
||
} // end getErrorList() | ||
|
||
/** | ||
* Returns the lines where warnings should occur. | ||
* | ||
* 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. | ||
* | ||
* @return array<int, int> | ||
*/ | ||
public function getWarningList() { | ||
return array( | ||
51 => 1, | ||
); | ||
|
||
} // end getWarningList() | ||
|
||
} // end class |