diff --git a/Linchpin/Sniffs/Performance/SlowOrderBySniff.php b/Linchpin/Sniffs/Performance/SlowOrderBySniff.php index 22dc66c..3923191 100644 --- a/Linchpin/Sniffs/Performance/SlowOrderBySniff.php +++ b/Linchpin/Sniffs/Performance/SlowOrderBySniff.php @@ -7,6 +7,7 @@ namespace Linchpin\Sniffs\Performance; +use PHPCSUtils\Utils\TextStrings; use WordPressCS\WordPress\AbstractArrayAssignmentRestrictionsSniff; /** @@ -14,16 +15,14 @@ */ class SlowOrderBySniff extends AbstractArrayAssignmentRestrictionsSniff { - /** - * Current stack pointer. - * - * @var int - */ - protected $stackPtr; - /** * Groups of variables to restrict. * + * The message uses %2$s rather than %s on purpose. The parent emits with + * `array( $key, $value )` as the replacements, so %s resolves to the array key + * — always the literal "orderby" — and %2$s is the assigned value, which is the + * part worth naming. + * * @return array */ public function getGroups() @@ -31,7 +30,7 @@ public function getGroups() return [ 'slow_order' => [ 'type' => 'warning', - 'message' => 'Ordering query results by %s is not performant.', + 'message' => 'Ordering query results by %2$s is not performant.', 'keys' => [ 'orderby', ], @@ -40,22 +39,31 @@ public function getGroups() } /** - * Process a token. + * Callback to process each confirmed key, to check value. * - * Overrides the parent to store the stackPtr for later use. + * Returning true lets the parent emit the group message above. This sniff used + * to call `$this->addMessage()` here and return false to suppress the built-in + * message — the shape WPCS 2 offered on WordPressCS\WordPress\Sniff. WPCS 3 + * removed that method, so the call resolved to nothing on the class or its + * parent and the sniff fatalled the moment it found something to report: * - * @param int $stackPtr The position of the current token in the stack. - */ - public function process_token( $stackPtr ) - { - $this->stackPtr = $stackPtr; - parent::process_token($stackPtr); - unset($this->stackPtr); - } - - /** - * Callback to process each confirmed key, to check value. - * This must be extended to add the logic to check assignment value. + * Uncaught Error: Call to undefined method + * Linchpin\Sniffs\Performance\SlowOrderBySniff::addMessage() + * + * Because it only ever ran on a query that actually ordered by one of these + * values, it looked healthy on any codebase that never hit the pattern, and + * consuming projects worked around it by excluding the sniff outright. + * + * Deferring to the parent rather than re-adding a shim also drops the + * `$stackPtr` property and the `process_token()` override that existed only to + * carry a token pointer to the manual call. The parent already reports against + * the key's own token, which is the more accurate position. + * + * The quotes have to come off first. The parent builds the value from the raw + * tokens, so a literal arrives here as `'rand'` — quotes included — and the + * comparison against `rand` could never match. Combined with the fatal above, + * the sniff had two independent reasons never to work: it matched nothing, and + * anything it did match would have crashed. * * @param string $key Array index / key. * @param mixed $val Assigned value. @@ -66,20 +74,11 @@ public function process_token( $stackPtr ) */ public function callback( $key, $val, $line, $group ) { - switch ( $val ) { + switch ( TextStrings::stripQuotes( (string) $val ) ) { case 'rand': case 'meta_value': case 'meta_value_num': - $this->addMessage( - 'Ordering query results by %s is not performant.', - $this->stackPtr, - 'warning', - 'slow_order', - [ $val ] - ); - - // Skip built-in message. - return false; + return true; default: // No match. diff --git a/Linchpin/ruleset.xml b/Linchpin/ruleset.xml index 7e8e785..fc99e8d 100644 --- a/Linchpin/ruleset.xml +++ b/Linchpin/ruleset.xml @@ -14,8 +14,21 @@ - - + + node_modules/* vendor/*