diff --git a/WordPressVIPMinimum/Sniffs/Classes/DeclarationCompatibilitySniff.php b/WordPressVIPMinimum/Sniffs/Classes/DeclarationCompatibilitySniff.php index 9a930319..10381feb 100644 --- a/WordPressVIPMinimum/Sniffs/Classes/DeclarationCompatibilitySniff.php +++ b/WordPressVIPMinimum/Sniffs/Classes/DeclarationCompatibilitySniff.php @@ -10,35 +10,54 @@ namespace WordPressVIPMinimum\Sniffs\Classes; use PHP_CodeSniffer\Files\File; -use PHP_CodeSniffer\Sniffs\AbstractScopeSniff; +use PHP_CodeSniffer\Sniffs\Sniff; use PHPCSUtils\Utils\FunctionDeclarations; use PHPCSUtils\Utils\ObjectDeclarations; /** * Class WordPressVIPMinimum_Sniffs_Classes_DeclarationCompatibilitySniff */ -class DeclarationCompatibilitySniff extends AbstractScopeSniff { +class DeclarationCompatibilitySniff implements Sniff { /** - * The name of the class we are currently checking. + * A list of classes and methods to check. + * + * @deprecated 3.1.0 This should never have been a public property. * - * @var string + * @var array>>> */ - private $currentClass = ''; + public $checkClasses = []; /** - * A list of classes and methods to check. + * List of grouped classes with same methods (as they extend the same parent class). * - * @var array>>> + * @deprecated 3.1.0 This should never have been a public property. + * + * @var array + */ + public $checkClassesGroups = []; + + /** + * A list of classes and information on the methods to check for those classes. + * + * @var array>>> */ - public $checkClasses = [ + private $methodSignatures = [ 'WP_Widget' => [ - 'widget' => [ 'args', 'instance' ], - 'update' => [ 'new_instance', 'old_instance' ], - 'form' => [ 'instance' ], + 'widget' => [ + 'args' => [], + 'instance' => [], + ], + 'update' => [ + 'new_instance' => [], + 'old_instance' => [], + ], + 'form' => [ + 'instance' => [], + ], 'WP_Widget' => [ - 'id_base', - 'name', + 'id_base' => [], + 'name' => [], 'widget_options' => [ 'default' => 'array()', ], @@ -46,16 +65,22 @@ class DeclarationCompatibilitySniff extends AbstractScopeSniff { 'default' => 'array()', ], ], - 'get_field_name' => [ 'field_name' ], - 'get_field_id' => [ 'field_name' ], + 'get_field_name' => [ + 'field_name' => [], + ], + 'get_field_id' => [ + 'field_name' => [], + ], '_register' => [], - '_set' => [ 'number' ], + '_set' => [ + 'number' => [], + ], '_get_display_callback' => [], '_get_update_callback' => [], '_get_form_callback' => [], 'is_preview' => [], 'display_callback' => [ - 'args', + 'args' => [], 'widget_args' => [ 'default' => '1', ], @@ -70,14 +95,17 @@ class DeclarationCompatibilitySniff extends AbstractScopeSniff { 'default' => '1', ], ], - 'register_one' => [ + '_register_one' => [ 'number' => [ 'default' => '-1', ], ], - 'save_settings' => [ 'settings' ], + 'save_settings' => [ + 'settings' => [], + ], 'get_settings' => [], ], + 'Walker' => [ 'start_lvl' => [ 'output' => [ @@ -105,7 +133,7 @@ class DeclarationCompatibilitySniff extends AbstractScopeSniff { 'output' => [ 'pass_by_reference' => true, ], - 'data_object', + 'data_object' => [], 'depth' => [ 'default' => '0', ], @@ -117,50 +145,50 @@ class DeclarationCompatibilitySniff extends AbstractScopeSniff { ], ], 'end_el' => [ - 'output' => [ + 'output' => [ 'pass_by_reference' => true, ], - 'data_object', - 'depth' => [ + 'data_object' => [], + 'depth' => [ 'default' => '0', ], - 'args' => [ + 'args' => [ 'default' => 'array()', ], ], 'display_element' => [ - 'element', + 'element' => [], 'children_elements' => [ 'pass_by_reference' => true, ], - 'max_depth', - 'depth', - 'args', + 'max_depth' => [], + 'depth' => [], + 'args' => [], 'output' => [ 'pass_by_reference' => true, ], ], 'walk' => [ - 'elements', - 'max_depth', - 'args' => [ + 'elements' => [], + 'max_depth' => [], + 'args' => [ 'variable_length' => true, ], ], 'paged_walk' => [ - 'elements', - 'max_depth', - 'page_num', - 'per_page', - 'args' => [ + 'elements' => [], + 'max_depth' => [], + 'page_num' => [], + 'per_page' => [], + 'args' => [ 'variable_length' => true, ], ], 'get_number_of_root_elements' => [ - 'elements', + 'elements' => [], ], 'unset_children' => [ - 'element', + 'element' => [], 'children_elements' => [ 'pass_by_reference' => true, ], @@ -169,143 +197,213 @@ class DeclarationCompatibilitySniff extends AbstractScopeSniff { ]; /** - * List of grouped classes with same methods (as they extend the same parent class) + * Classes this sniff checks for being extended. * - * @var array + * @var array Key is the name of a potentially extended class, + * value the canonical name for the method signatures definition. */ - public $checkClassesGroups = [ - 'Walker' => [ - 'Walker_Category_Checklist', - 'Walker_Category', - 'Walker_CategoryDropdown', - 'Walker_PageDropdown', - 'Walker_Nav_Menu', - 'Walker_Page', - 'Walker_Comment', - ], + private $extendedClassToSignatures = [ + 'WP_Widget' => 'WP_Widget', + 'Walker' => 'Walker', + 'Walker_Category_Checklist' => 'Walker', + 'Walker_Category' => 'Walker', + 'Walker_CategoryDropdown' => 'Walker', + 'Walker_PageDropdown' => 'Walker', + 'Walker_Nav_Menu' => 'Walker', + 'Walker_Page' => 'Walker', + 'Walker_Comment' => 'Walker', ]; /** - * Constructs the test with the tokens it wishes to listen for. + * Translate from case-insensitive names to proper case method names. + * + * @var array> Primary key is the class name in proper case. + * Value is an array with method names in lowercase as keys + * and these same method names in proper case as values. */ - public function __construct() { - parent::__construct( [ T_CLASS ], [ T_FUNCTION ], true ); - } + private $methodToProperCase = []; /** - * Processes this test when one of its tokens is encountered. - * - * @param File $phpcsFile The PHP_CodeSniffer file where the token was found. - * @param int $stackPtr The position of the current token in the stack passed in $tokens. - * @param int $currScope A pointer to the start of the scope. + * Translate from case-insensitive names to proper case class names. * - * @return void + * @var array Key is the lowercase name of a class, value the proper case. */ - protected function processTokenWithinScope( File $phpcsFile, $stackPtr, $currScope ) { - - $className = ObjectDeclarations::getName( $phpcsFile, $currScope ); + private $classToProperCase = []; - if ( $className !== $this->currentClass ) { - $this->currentClass = $className; + /** + * Returns the token types that this sniff is interested in. + * + * @return array + */ + public function register() { + // Lowercase all names to allow for correct comparisons, as PHP treats class/function names case-insensitively. + // But also store translation tables to be able to get the proper case. + foreach ( $this->methodSignatures as $key => $value ) { + $methodNames = array_keys( $value ); + $this->methodToProperCase[ $key ] = array_change_key_case( array_combine( $methodNames, $methodNames ), CASE_LOWER ); + + $this->methodSignatures[ $key ] = array_change_key_case( $value, CASE_LOWER ); } - $methodName = FunctionDeclarations::getName( $phpcsFile, $stackPtr ); + $classNames = array_keys( $this->extendedClassToSignatures ); + $this->classToProperCase = array_change_key_case( array_combine( $classNames, $classNames ), CASE_LOWER ); + $this->extendedClassToSignatures = array_change_key_case( $this->extendedClassToSignatures, CASE_LOWER ); + + return [ + T_CLASS, + T_ANON_CLASS, + ]; + } - $parentClassName = ObjectDeclarations::findExtendedClassName( $phpcsFile, $currScope ); + /** + * Processes the tokens that this sniff is interested in. + * + * @param \PHP_CodeSniffer\Files\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( File $phpcsFile, $stackPtr ) { + $parentClassName = ObjectDeclarations::findExtendedClassName( $phpcsFile, $stackPtr ); if ( $parentClassName === false ) { // This class does not extend any other class. return; } - // Need to define the originalParentClassName since we might override the parentClassName due to signature notations grouping. - $originalParentClassName = $parentClassName; - - if ( array_key_exists( $parentClassName, $this->checkClasses ) === false ) { + $parentClassNameLC = ltrim( strtolower( $parentClassName ), '\\' ); // Trim off potential FQN indicator. + if ( isset( $this->extendedClassToSignatures[ $parentClassNameLC ] ) === false ) { // This class does not extend a class we are interested in. - foreach ( $this->checkClassesGroups as $parent => $children ) { - // But it might be one of the grouped classes. - foreach ( $children as $child ) { - if ( $child === $parentClassName ) { - $parentClassName = $parent; - break 2; - } - } - } - if ( array_key_exists( $parentClassName, $this->checkClasses ) === false ) { - // This class really does not extend a class we are interested in. - return; - } + return; } - if ( array_key_exists( $methodName, $this->checkClasses[ $parentClassName ] ) === false && - in_array( $methodName, $this->checkClasses[ $parentClassName ], true ) === false - ) { - // This method is not a one we are interested in. + // Store the originalParentClassName since we might override the parentClassName due to signature notations grouping. + $originalParentClassNamePC = $this->classToProperCase[ $parentClassNameLC ]; + $parentClassName = $this->extendedClassToSignatures[ $parentClassNameLC ]; + + $methods = ObjectDeclarations::getDeclaredMethods( $phpcsFile, $stackPtr ); + if ( empty( $methods ) ) { return; } - $signatureParams = FunctionDeclarations::getParameters( $phpcsFile, $stackPtr ); + foreach ( $methods as $methodName => $functionPtr ) { + $methodNameLC = strtolower( $methodName ); + if ( isset( $this->methodSignatures[ $parentClassName ][ $methodNameLC ] ) === false ) { + // This method is not one we are interested in. + continue; + } + + $methodNamePC = $this->methodToProperCase[ $parentClassName ][ $methodNameLC ]; + + $childParams = FunctionDeclarations::getParameters( $phpcsFile, $functionPtr ); + $childParamCount = count( $childParams ); - $parentSignature = $this->checkClasses[ $parentClassName ][ $methodName ]; + $parentParams = $this->methodSignatures[ $parentClassName ][ $methodNameLC ]; + $parentParamCount = count( $parentParams ); - if ( count( $signatureParams ) > count( $parentSignature ) ) { - $extra_params = array_slice( $signatureParams, count( $parentSignature ) - count( $signatureParams ) ); - $all_extra_params_have_default = true; - foreach ( $extra_params as $extra_param ) { - if ( array_key_exists( 'default', $extra_param ) === false || $extra_param['default'] !== 'true' ) { - $all_extra_params_have_default = false; + /* + * If there are parameters, verify if the last parameter of both the parent and the child are variadic. + * Only the last parameter can be variadic and if the parent has this, the child must also, + * independently of potential extra optional parameters having been inserted before that last parameter. + * + * Also note that a child can make the last parameter variadic, even if the parent parameter was not. + * This will no longer trigger a warning since PHP 8.0. + */ + if ( $childParamCount > 0 && $parentParamCount > 0 ) { + $childLastParam = $childParams[ $childParamCount - 1 ]; + $parentLastParam = $parentParams[ array_keys( $parentParams )[ $parentParamCount - 1 ] ]; + + if ( ( isset( $parentLastParam['variable_length'] ) === true && $parentLastParam['variable_length'] === true ) + && $childLastParam['variable_length'] !== true + ) { + $this->addError( $phpcsFile, $functionPtr, $stackPtr, $originalParentClassNamePC, $methodNamePC, $childParams, $parentParams ); + continue; } } - if ( $all_extra_params_have_default === true ) { - return; // We're good. + + if ( $childParamCount > 0 ) { + // Check that no other parameters in the child signature are declared as variadic. + for ( $i = 0; $i < ( $childParamCount - 1 ); $i++ ) { + if ( $childParams[ $i ]['variable_length'] === true ) { + $this->addError( $phpcsFile, $functionPtr, $stackPtr, $originalParentClassNamePC, $methodNamePC, $childParams, $parentParams ); + continue 2; + } + } } - } - if ( count( $signatureParams ) !== count( $parentSignature ) ) { - $this->addError( $originalParentClassName, $methodName, $signatureParams, $parentSignature, $phpcsFile, $stackPtr ); - return; - } + if ( $childParamCount > $parentParamCount ) { + $extra_params = array_slice( $childParams, $parentParamCount - $childParamCount ); + $all_extra_params_have_default = true; + foreach ( $extra_params as $extra_param ) { + if ( isset( $extra_param['default'] ) === false + && $extra_param['variable_length'] === false + ) { + $all_extra_params_have_default = false; + break; + } + } - $i = 0; - foreach ( $parentSignature as $key => $param ) { - if ( is_array( $param ) === true ) { + if ( $all_extra_params_have_default === false ) { + $this->addError( $phpcsFile, $functionPtr, $stackPtr, $originalParentClassNamePC, $methodNamePC, $childParams, $parentParams ); + continue; + } + } elseif ( $childParamCount !== $parentParamCount ) { + $this->addError( $phpcsFile, $functionPtr, $stackPtr, $originalParentClassNamePC, $methodNamePC, $childParams, $parentParams ); + continue; + } + + $i = 0; + foreach ( $parentParams as $param ) { if ( ( - array_key_exists( 'default', $param ) === true && - array_key_exists( 'default', $signatureParams[ $i ] ) === false + isset( $param['default'] ) === true + && isset( $childParams[ $i ]['default'] ) === false + && $childParams[ $i ]['variable_length'] === false ) || ( - array_key_exists( 'pass_by_reference', $param ) === true && - $param['pass_by_reference'] !== $signatureParams[ $i ]['pass_by_reference'] + // Parameter in parent class has reference, child does not. + isset( $param['pass_by_reference'] ) === true + && $param['pass_by_reference'] !== $childParams[ $i ]['pass_by_reference'] ) || ( - array_key_exists( 'variable_length', $param ) === true && - $param['variable_length'] !== $signatureParams[ $i ]['variable_length'] + // Parameter in parent class does *not* have reference, child does. + ( isset( $param['pass_by_reference'] ) === false + || $param['pass_by_reference'] === false ) + && $childParams[ $i ]['pass_by_reference'] === true ) ) { - $this->addError( $originalParentClassName, $methodName, $signatureParams, $parentSignature, $phpcsFile, $stackPtr ); - return; + $this->addError( $phpcsFile, $functionPtr, $stackPtr, $originalParentClassNamePC, $methodNamePC, $childParams, $parentParams ); + continue 2; } + ++$i; } - ++$i; } } /** - * Generates an error with nice current and parent class method notations + * Generates an error with nice current and parent class method notations. * - * @param string $parentClassName The name of the extended (parent) class. - * @param string $methodName The name of the method currently being examined. - * @param array $currentMethodSignature The list of params and their options of the method which is being examined. - * @param array $parentMethodSignature The list of params and their options of the parent class method. - * @param File $phpcsFile The PHP_CodeSniffer file where the token was found. - * @param int $stackPtr The position of the current token in the stack. + * @param File $phpcsFile The PHP_CodeSniffer file where the token was found. + * @param int $stackPtr The position of the current T_FUNCTION token in the stack. + * @param int $currScope A pointer to the start of the OO scope. + * @param string $parentClassName The name of the extended (parent) class. + * @param string $methodName The name of the method currently being examined. + * @param array> $currentMethodSignature The list of params and their options of the method + * which is being examined. + * @param array> $parentMethodSignature The list of params and their options of the parent class method. * * @return void */ - private function addError( $parentClassName, $methodName, $currentMethodSignature, $parentMethodSignature, $phpcsFile, $stackPtr ) { + private function addError( File $phpcsFile, $stackPtr, $currScope, $parentClassName, $methodName, $currentMethodSignature, $parentMethodSignature ) { + $tokens = $phpcsFile->getTokens(); + $currentClassName = '[AnonymousClass]'; + if ( $tokens[ $currScope ]['code'] !== T_ANON_CLASS ) { + $currentClassName = ObjectDeclarations::getName( $phpcsFile, $currScope ); + } - $currentSignature = sprintf( '%s::%s(%s)', $this->currentClass, $methodName, implode( ', ', $this->generateParamList( $currentMethodSignature ) ) ); + $currentSignature = implode( ', ', $this->generateParamList( $currentMethodSignature ) ); + $currentSignature = sprintf( '%s::%s(%s)', $currentClassName, $methodName, $currentSignature ); - $parentSignature = sprintf( '%s::%s(%s)', $parentClassName, $methodName, implode( ', ', $this->generateParamList( $parentMethodSignature ) ) ); + $parentSignature = implode( ', ', $this->generateParamList( $parentMethodSignature ) ); + $parentSignature = sprintf( '%s::%s(%s)', $parentClassName, $methodName, $parentSignature ); $message = 'Declaration of `%s` should be compatible with `%s`.'; $data = [ $currentSignature, $parentSignature ]; @@ -323,26 +421,26 @@ private function generateParamList( $methodSignature ) { $paramList = []; foreach ( $methodSignature as $param => $options ) { $paramName = '$'; - if ( is_array( $options ) === false ) { - $paramList[] = '$' . $options; + if ( empty( $options ) === true ) { + $paramList[] = '$' . $param; continue; } - if ( array_key_exists( 'name', $options ) === true ) { + if ( isset( $options['name'] ) === true ) { $paramName = $options['name']; } else { $paramName .= $param; } - if ( array_key_exists( 'variable_length', $options ) === true && $options['variable_length'] === true ) { + if ( isset( $options['variable_length'] ) === true && $options['variable_length'] === true ) { $paramName = '...' . $paramName; } - if ( array_key_exists( 'pass_by_reference', $options ) === true && $options['pass_by_reference'] === true ) { + if ( isset( $options['pass_by_reference'] ) === true && $options['pass_by_reference'] === true ) { $paramName = '&' . $paramName; } - if ( array_key_exists( 'default', $options ) === true && empty( $options['default'] ) === false ) { + if ( isset( $options['default'] ) === true && empty( $options['default'] ) === false ) { $paramName .= ' = ' . trim( $options['default'] ); } @@ -351,12 +449,4 @@ private function generateParamList( $methodSignature ) { return $paramList; } - - /** - * Do nothing outside the scope. Has to be implemented accordingly to parent abstract class. - * - * @param File $phpcsFile PHPCS File. - * @param int $stackPtr Stack position. - */ - public function processTokenOutsideScope( File $phpcsFile, $stackPtr ) {} } diff --git a/WordPressVIPMinimum/Tests/Classes/DeclarationCompatibilityUnitTest.1.inc b/WordPressVIPMinimum/Tests/Classes/DeclarationCompatibilityUnitTest.1.inc new file mode 100644 index 00000000..8ae13ced --- /dev/null +++ b/WordPressVIPMinimum/Tests/Classes/DeclarationCompatibilityUnitTest.1.inc @@ -0,0 +1,189 @@ + Key is the line number, value is the number of expected errors. */ - public function getErrorList() { - return [ - 4 => 1, - 7 => 1, - 10 => 1, - 13 => 1, - 16 => 1, - 19 => 1, - 25 => 1, - 40 => 1, - 43 => 1, - 46 => 1, - 49 => 1, - 52 => 1, - 61 => 1, - 67 => 1, - 70 => 1, - 76 => 1, - 79 => 1, - 88 => 1, - 106 => 1, - 112 => 1, - 119 => 1, - 128 => 1, - 134 => 1, - 137 => 1, - 140 => 1, - ]; + public function getErrorList( $testFile = '' ) { + switch ( $testFile ) { + case 'DeclarationCompatibilityUnitTest.1.inc': + return [ + 53 => 1, + 54 => 1, + 55 => 1, + 56 => 1, + 57 => 1, + 58 => 1, + 59 => 1, + 60 => 1, + 61 => 1, + 62 => 1, + 63 => 1, + 64 => 1, + 68 => 1, + 69 => 1, + 70 => 1, + 71 => 1, + 75 => 1, + 76 => 1, + 77 => 1, + 78 => 1, + 79 => 1, + 80 => 1, + 81 => 1, + 82 => 1, + 83 => 1, + 84 => 1, + 85 => 1, + 86 => 1, + 87 => 1, + 88 => 1, + 89 => 1, + 90 => 1, + 91 => 1, + 92 => 1, + 96 => 1, + 97 => 1, + 98 => 1, + 99 => 1, + 100 => 1, + 104 => 1, + 105 => 1, + 106 => 1, + 107 => 1, + 108 => 1, + 109 => 1, + 110 => 1, + 111 => 1, + 112 => 1, + 113 => 1, + 114 => 1, + 115 => 1, + 156 => 1, + 157 => 1, + 158 => 1, + 159 => 1, + 160 => 1, + 161 => 1, + 162 => 1, + 163 => 1, + 164 => 1, + 165 => 1, + 166 => 1, + 171 => 1, + 172 => 1, + 178 => 1, + 186 => 1, + ]; + + case 'DeclarationCompatibilityUnitTest.2.inc': + return [ + 43 => 1, + 44 => 1, + 45 => 1, + 46 => 1, + 47 => 1, + 48 => 1, + 49 => 1, + 50 => 1, + 51 => 1, + 55 => 1, + 56 => 1, + 57 => 1, + 58 => 1, + 59 => 1, + 60 => 1, + 61 => 1, + 62 => 1, + 66 => 1, + 67 => 1, + 68 => 1, + 72 => 1, + 73 => 1, + 74 => 1, + 75 => 1, + 79 => 1, + 80 => 1, + 81 => 1, + 82 => 1, + 83 => 1, + 84 => 1, + 88 => 1, + 92 => 1, + 93 => 1, + 97 => 1, + 98 => 1, + 99 => 1, + 100 => 1, + 101 => 1, + 102 => 1, + 103 => 1, + 104 => 1, + 105 => 1, + 132 => 1, + 133 => 1, + 134 => 1, + 135 => 1, + 136 => 1, + 137 => 1, + 138 => 1, + 143 => 1, + 144 => 1, + ]; + + default: + return []; + } } /**