From 4a053666c63fdb5b4be170016ddeec94ec61cd54 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 28 Oct 2022 09:39:25 +0200 Subject: [PATCH 1/2] Core: move rules related to object instantiations from `Extra` to `Core` > 1. When instantiating a new instance of an object, parenthesis must always be used, even when not strictly necessary. > 2. There should be no space between the name of the class being instantiated and the opening parenthesis. Note: the spacing inside the parentheses when instantiating anonymous classes will not (yet) be checked by the `PEAR.Functions.FunctionCallSignature` sniff. A PR is open upstream to add support for this: squizlabs/PHP_CodeSniffer 3636 Once the upstream PR has been merged, WPCS will automatically inherit the support as this sniff is already included in WPCS. The following will no longer be checked by WPCS: > 3. Assigning the return value of an object instantiation by reference is not allowed. This is a parse error since PHP 7.0, so should not be the concern of WPCS (and hasn't been added to the handbook for the same reason). See: https://3v4l.org/W2Qj6 --- As discussed in 1884, this means that JS support for checking object instantiations will be dropped, but as WPCS is generally not used for checking JS code, this should not be a problem. --- Refs and related issues: * https://make.wordpress.org/core/2020/03/20/updating-the-coding-standards-for-modern-php/ - Object instantiation section * https://developer.wordpress.org/coding-standards/wordpress-coding-standards/php/#object-instantiation * WordPress/wpcs-docs 109 * WordPress/WordPress-Coding-Standards 919 * WordPress/WordPress-Coding-Standards 1033 * WordPress/WordPress-Coding-Standards 1721 * squizlabs/PHP_CodeSniffer 3200 * squizlabs/PHP_CodeSniffer 3636 (not yet merged) * PHPCSStandards/PHPCSExtra 76 * PHPCSStandards/PHPCSExtra 120 Fixes 1884 Fixes 2033 --- WordPress-Core/ruleset.xml | 19 +++++++++++++++++++ WordPress-Extra/ruleset.xml | 11 ----------- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/WordPress-Core/ruleset.xml b/WordPress-Core/ruleset.xml index 32f1bd0e11..73d712ed0d 100644 --- a/WordPress-Core/ruleset.xml +++ b/WordPress-Core/ruleset.xml @@ -425,6 +425,25 @@ + + + + + + + + + + + + + + - - - From 47d93c01bade641c3590b4605bdbd117682640d6 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 23 Jul 2022 16:17:28 +0200 Subject: [PATCH 2/2] Remove the Classes/ClassInstantiation sniff ... as it is now no longer used. --- .../Classes/ClassInstantiationSniff.php | 205 ------------------ .../Classes/ClassInstantiationUnitTest.inc | 106 --------- .../ClassInstantiationUnitTest.inc.fixed | 106 --------- .../Classes/ClassInstantiationUnitTest.js | 4 - .../ClassInstantiationUnitTest.js.fixed | 4 - .../Classes/ClassInstantiationUnitTest.php | 87 -------- 6 files changed, 512 deletions(-) delete mode 100644 WordPress/Sniffs/Classes/ClassInstantiationSniff.php delete mode 100644 WordPress/Tests/Classes/ClassInstantiationUnitTest.inc delete mode 100644 WordPress/Tests/Classes/ClassInstantiationUnitTest.inc.fixed delete mode 100644 WordPress/Tests/Classes/ClassInstantiationUnitTest.js delete mode 100644 WordPress/Tests/Classes/ClassInstantiationUnitTest.js.fixed delete mode 100644 WordPress/Tests/Classes/ClassInstantiationUnitTest.php diff --git a/WordPress/Sniffs/Classes/ClassInstantiationSniff.php b/WordPress/Sniffs/Classes/ClassInstantiationSniff.php deleted file mode 100644 index d426685c1e..0000000000 --- a/WordPress/Sniffs/Classes/ClassInstantiationSniff.php +++ /dev/null @@ -1,205 +0,0 @@ -classname_tokens = Tokens::$emptyTokens; - $this->classname_tokens[ \T_NS_SEPARATOR ] = \T_NS_SEPARATOR; - $this->classname_tokens[ \T_STRING ] = \T_STRING; - $this->classname_tokens[ \T_SELF ] = \T_SELF; - $this->classname_tokens[ \T_STATIC ] = \T_STATIC; - $this->classname_tokens[ \T_PARENT ] = \T_PARENT; - $this->classname_tokens[ \T_ANON_CLASS ] = \T_ANON_CLASS; - - // Classname in a variable. - $this->classname_tokens[ \T_VARIABLE ] = \T_VARIABLE; - $this->classname_tokens[ \T_DOUBLE_COLON ] = \T_DOUBLE_COLON; - $this->classname_tokens[ \T_OBJECT_OPERATOR ] = \T_OBJECT_OPERATOR; - $this->classname_tokens[ \T_OPEN_SQUARE_BRACKET ] = \T_OPEN_SQUARE_BRACKET; - $this->classname_tokens[ \T_CLOSE_SQUARE_BRACKET ] = \T_CLOSE_SQUARE_BRACKET; - $this->classname_tokens[ \T_CONSTANT_ENCAPSED_STRING ] = \T_CONSTANT_ENCAPSED_STRING; - $this->classname_tokens[ \T_LNUMBER ] = \T_LNUMBER; - - return array( - \T_NEW, - \T_STRING, // JS. - ); - } - - /** - * Processes this test, when one of its tokens is encountered. - * - * @param int $stackPtr The position of the current token in the stack. - * - * @return void - */ - public function process_token( $stackPtr ) { - // Make sure we have the right token, JS vs PHP. - if ( ( ( isset( $this->phpcsFile->tokenizerType ) === false || 'PHP' === $this->phpcsFile->tokenizerType ) - && \T_NEW !== $this->tokens[ $stackPtr ]['code'] ) - || ( ( isset( $this->phpcsFile->tokenizerType ) && 'JS' === $this->phpcsFile->tokenizerType ) - && ( \T_STRING !== $this->tokens[ $stackPtr ]['code'] - || 'new' !== strtolower( $this->tokens[ $stackPtr ]['content'] ) ) ) - ) { - return; - } - - /* - * Check for new by reference used in PHP files. - */ - if ( isset( $this->phpcsFile->tokenizerType ) === false || 'PHP' === $this->phpcsFile->tokenizerType ) { - $prev_non_empty = $this->phpcsFile->findPrevious( - Tokens::$emptyTokens, - ( $stackPtr - 1 ), - null, - true - ); - - if ( false !== $prev_non_empty && 'T_BITWISE_AND' === $this->tokens[ $prev_non_empty ]['type'] ) { - $this->phpcsFile->recordMetric( $stackPtr, 'Assigning new by reference', 'yes' ); - - $this->phpcsFile->addError( - 'Assigning the return value of new by reference is no longer supported by PHP.', - $stackPtr, - 'NewByReferenceFound' - ); - } else { - $this->phpcsFile->recordMetric( $stackPtr, 'Assigning new by reference', 'no' ); - } - } - - /* - * Check for parenthesis & correct placement thereof. - */ - $next_non_empty_after_class_name = $this->phpcsFile->findNext( - $this->classname_tokens, - ( $stackPtr + 1 ), - null, - true, - null, - true - ); - - if ( false === $next_non_empty_after_class_name ) { - // Live coding. - return; - } - - // Walk back to the last part of the class name. - $has_comment = false; - for ( $classname_ptr = ( $next_non_empty_after_class_name - 1 ); $classname_ptr >= $stackPtr; $classname_ptr-- ) { - if ( ! isset( Tokens::$emptyTokens[ $this->tokens[ $classname_ptr ]['code'] ] ) ) { - // Prevent a false positive on variable variables, disregard them for now. - if ( $stackPtr === $classname_ptr ) { - return; - } - - break; - } - - if ( \T_WHITESPACE !== $this->tokens[ $classname_ptr ]['code'] ) { - $has_comment = true; - } - } - - if ( \T_OPEN_PARENTHESIS !== $this->tokens[ $next_non_empty_after_class_name ]['code'] ) { - $this->phpcsFile->recordMetric( $stackPtr, 'Object instantiation with parenthesis', 'no' ); - - $fix = $this->phpcsFile->addFixableError( - 'Parenthesis should always be used when instantiating a new object.', - $classname_ptr, - 'MissingParenthesis' - ); - - if ( true === $fix ) { - $this->phpcsFile->fixer->addContent( $classname_ptr, '()' ); - } - } else { - $this->phpcsFile->recordMetric( $stackPtr, 'Object instantiation with parenthesis', 'yes' ); - - if ( ( $next_non_empty_after_class_name - 1 ) !== $classname_ptr ) { - $this->phpcsFile->recordMetric( - $stackPtr, - 'Space between classname and parenthesis', - ( $next_non_empty_after_class_name - $classname_ptr ) - ); - - $error = 'There must be no spaces between the class name and the open parenthesis when instantiating a new object.'; - $error_code = 'SpaceBeforeParenthesis'; - - if ( false === $has_comment ) { - $fix = $this->phpcsFile->addFixableError( $error, $next_non_empty_after_class_name, $error_code ); - - if ( true === $fix ) { - $this->phpcsFile->fixer->beginChangeset(); - for ( $i = ( $next_non_empty_after_class_name - 1 ); $i > $classname_ptr; $i-- ) { - $this->phpcsFile->fixer->replaceToken( $i, '' ); - } - $this->phpcsFile->fixer->endChangeset(); - } - } else { - $this->phpcsFile->addError( $error, $next_non_empty_after_class_name, $error_code ); - } - } else { - $this->phpcsFile->recordMetric( $stackPtr, 'Space between classname and parenthesis', 0 ); - } - } - } - -} diff --git a/WordPress/Tests/Classes/ClassInstantiationUnitTest.inc b/WordPress/Tests/Classes/ClassInstantiationUnitTest.inc deleted file mode 100644 index d0a9ae7afa..0000000000 --- a/WordPress/Tests/Classes/ClassInstantiationUnitTest.inc +++ /dev/null @@ -1,106 +0,0 @@ -inline_diff_renderer(); -$b = ( new MyClass() )->my_function(); -$b = ( new MyClass() )::$property; - -class ClassA { - public static function get_instance() { - return new self(); - } - - public static function get_other_instance() { - return new static(); - } -} - -class ClassB extends ClassA { - public function get_parent_instance() { - return new parent(); - } -} - -// PHP 7: Anonymous classes. -$util->setLogger( new class() {} ); -$b = new class( 10 ) extends SomeClass implements SomeInterface {}; - - -/* - * Bad. - */ -$a = new MyClass; -$a = new $varHoldingClassName; -$a = new self::$transport[$cap_string]; -$renderer = new $this->inline_diff_renderer; -$b = ( new MyClass )->my_function(); -$b = ( new MyClass )::$property; - -class ClassAA { - public static function get_instance() { - return new self; - } - - public static function get_other_instance() { - return new static; - } -} - -class ClassBB extends ClassA { - public function get_parent_instance() { - return new parent; - } -} - -// PHP 7: Anonymous classes. -$util->setLogger( new class {} ); -$b = new class extends SomeClass implements SomeInterface {}; - - -// Test some non-typical spacing. -$renderer = new $this-> - inline_diff_renderer (); -$renderer = new $this-> // There can be a comment here. - inline_diff_renderer (); -$renderer = new $this-> - inline_diff_renderer /* or here */ (); -$a = new self :: $transport [ $cap_string ] (); - -$renderer = new $this-> - inline_diff_renderer; -$renderer = new $this-> // There can be a comment here. - inline_diff_renderer; -$renderer = new $this-> - inline_diff_renderer /* or here */ ; -$a = new self :: $transport [ $cap_string ]; - - -// Assigning new by reference. -$b = &new Foobar(); -$b = & new Foobar(); - - -// Currently not accounted for by the sniff, i.e. false negatives. -$a = new $$varHoldingClassName; -$a = new ${$varHoldingClassName}; - -// Namespaced classes: OK. -$a = new \MyClass(); -$a = new \MyNamespace\MyClass(); - -// Namespaced classes: Bad. -$a = new \MyClass; -$a = new \MyNamespace\MyClass; - -// Non-variable keys: OK. -$a = new $array['key'](); -$a = new $array[0](); - -// Non-variable keys: Bad. -$a = new $array['key']; -$a = new $array[0]; diff --git a/WordPress/Tests/Classes/ClassInstantiationUnitTest.inc.fixed b/WordPress/Tests/Classes/ClassInstantiationUnitTest.inc.fixed deleted file mode 100644 index d906b2d4eb..0000000000 --- a/WordPress/Tests/Classes/ClassInstantiationUnitTest.inc.fixed +++ /dev/null @@ -1,106 +0,0 @@ -inline_diff_renderer(); -$b = ( new MyClass() )->my_function(); -$b = ( new MyClass() )::$property; - -class ClassA { - public static function get_instance() { - return new self(); - } - - public static function get_other_instance() { - return new static(); - } -} - -class ClassB extends ClassA { - public function get_parent_instance() { - return new parent(); - } -} - -// PHP 7: Anonymous classes. -$util->setLogger( new class() {} ); -$b = new class( 10 ) extends SomeClass implements SomeInterface {}; - - -/* - * Bad. - */ -$a = new MyClass(); -$a = new $varHoldingClassName(); -$a = new self::$transport[$cap_string](); -$renderer = new $this->inline_diff_renderer(); -$b = ( new MyClass() )->my_function(); -$b = ( new MyClass() )::$property; - -class ClassAA { - public static function get_instance() { - return new self(); - } - - public static function get_other_instance() { - return new static(); - } -} - -class ClassBB extends ClassA { - public function get_parent_instance() { - return new parent(); - } -} - -// PHP 7: Anonymous classes. -$util->setLogger( new class() {} ); -$b = new class() extends SomeClass implements SomeInterface {}; - - -// Test some non-typical spacing. -$renderer = new $this-> - inline_diff_renderer(); -$renderer = new $this-> // There can be a comment here. - inline_diff_renderer(); -$renderer = new $this-> - inline_diff_renderer /* or here */ (); -$a = new self :: $transport [ $cap_string ](); - -$renderer = new $this-> - inline_diff_renderer(); -$renderer = new $this-> // There can be a comment here. - inline_diff_renderer(); -$renderer = new $this-> - inline_diff_renderer() /* or here */ ; -$a = new self :: $transport [ $cap_string ](); - - -// Assigning new by reference. -$b = &new Foobar(); -$b = & new Foobar(); - - -// Currently not accounted for by the sniff, i.e. false negatives. -$a = new $$varHoldingClassName; -$a = new ${$varHoldingClassName}; - -// Namespaced classes: OK. -$a = new \MyClass(); -$a = new \MyNamespace\MyClass(); - -// Namespaced classes: Bad. -$a = new \MyClass(); -$a = new \MyNamespace\MyClass(); - -// Non-variable keys: OK. -$a = new $array['key'](); -$a = new $array[0](); - -// Non-variable keys: Bad. -$a = new $array['key'](); -$a = new $array[0](); diff --git a/WordPress/Tests/Classes/ClassInstantiationUnitTest.js b/WordPress/Tests/Classes/ClassInstantiationUnitTest.js deleted file mode 100644 index 4800633f98..0000000000 --- a/WordPress/Tests/Classes/ClassInstantiationUnitTest.js +++ /dev/null @@ -1,4 +0,0 @@ -var firstBook = new Book(); // OK. -var secondBook = new Book; // Bad. -var thirdBook = new Book (); // Bad. -var fourthBook = new Book ( 'title' ); // Bad. diff --git a/WordPress/Tests/Classes/ClassInstantiationUnitTest.js.fixed b/WordPress/Tests/Classes/ClassInstantiationUnitTest.js.fixed deleted file mode 100644 index 73116a47a9..0000000000 --- a/WordPress/Tests/Classes/ClassInstantiationUnitTest.js.fixed +++ /dev/null @@ -1,4 +0,0 @@ -var firstBook = new Book(); // OK. -var secondBook = new Book(); // Bad. -var thirdBook = new Book(); // Bad. -var fourthBook = new Book( 'title' ); // Bad. diff --git a/WordPress/Tests/Classes/ClassInstantiationUnitTest.php b/WordPress/Tests/Classes/ClassInstantiationUnitTest.php deleted file mode 100644 index 42253fdbfb..0000000000 --- a/WordPress/Tests/Classes/ClassInstantiationUnitTest.php +++ /dev/null @@ -1,87 +0,0 @@ - => - */ - public function getErrorList( $testFile = '' ) { - - $phpcs_version = Helper::getVersion(); - $is_phpcs_4 = version_compare( $phpcs_version, '3.99.99', '>' ); - - switch ( $testFile ) { - case 'ClassInstantiationUnitTest.inc': - return array( - 37 => 1, - 38 => 1, - 39 => 1, - 40 => 1, - 41 => 1, - 42 => 1, - 46 => 1, - 50 => 1, - 56 => 1, - 61 => 1, - 62 => 1, - 67 => 1, - 69 => 1, - 71 => 1, - 72 => 1, - 75 => 1, - 77 => 1, - 79 => 1, - 80 => 1, - 84 => 1, - 85 => 1, - 97 => 1, - 98 => 1, - 105 => 1, - 106 => 1, - ); - - case 'ClassInstantiationUnitTest.js': - return array( - 2 => ( true === $is_phpcs_4 ? 0 : 1 ), - 3 => ( true === $is_phpcs_4 ? 0 : 1 ), - 4 => ( true === $is_phpcs_4 ? 0 : 1 ), - ); - - default: - return array(); - } - } - - /** - * Returns the lines where warnings should occur. - * - * @return array => - */ - public function getWarningList() { - return array(); - } - -}