From 46895e99bcbf88369a9e7705fea60fde94a48d43 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 28 Jul 2020 05:00:32 +0200 Subject: [PATCH 1/2] PreGetPosts: improve the isEarlyMainQueryCheck() method [1] This adds a unit test which results in the error as reported in issue 499 and fixes the error properly. The error was caused by the presumption in the code that if an a check for `is_main_query` has a scope condition which is a `closure`, that the `closure` is the callback in the hook function call and that therefore the outer parenthesis (those of the function call) should be disgarded and the next parenthesis will be the ones for the `if` statement which will always have a scope opener and closer. Now read the above sentence again and count the number of assumptions in that statement ;-) Either way, the fix I've now added should stabilize this part of the code. Fixes 499 --- .../Sniffs/Hooks/PreGetPostsSniff.php | 21 ++++++++++++++++--- .../Tests/Hooks/PreGetPostsUnitTest.inc | 16 ++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/WordPressVIPMinimum/Sniffs/Hooks/PreGetPostsSniff.php b/WordPressVIPMinimum/Sniffs/Hooks/PreGetPostsSniff.php index 8beca64f..a2df7603 100644 --- a/WordPressVIPMinimum/Sniffs/Hooks/PreGetPostsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Hooks/PreGetPostsSniff.php @@ -304,9 +304,24 @@ private function isEarlyMainQueryCheck( $stackPtr ) { return false; } - $nestedParenthesisEnd = array_shift( $this->tokens[ $stackPtr ]['nested_parenthesis'] ); - if ( true === in_array( 'PHPCS_T_CLOSURE', $this->tokens[ $stackPtr ]['conditions'], true ) ) { - $nestedParenthesisEnd = array_shift( $this->tokens[ $stackPtr ]['nested_parenthesis'] ); + $parentheses = $this->tokens[ $stackPtr ]['nested_parenthesis']; + do { + $nestedParenthesisEnd = array_shift( $parentheses ); + if ( null === $nestedParenthesisEnd ) { + // Nothing left in the array. No parenthesis found with a non-closure owner. + return false; + } + + if ( isset( $this->tokens[ $nestedParenthesisEnd ]['parenthesis_owner'] ) + && T_CLOSURE !== $this->tokens[ $this->tokens[ $nestedParenthesisEnd ]['parenthesis_owner'] ]['code'] + ) { + break; + } + } while ( true ); + + $owner = $this->tokens[ $nestedParenthesisEnd ]['parenthesis_owner']; + if ( isset( $this->tokens[ $owner ]['scope_opener'], $this->tokens[ $owner ]['scope_closer'] ) === false ) { + return false; } $next = $this->phpcsFile->findNext( diff --git a/WordPressVIPMinimum/Tests/Hooks/PreGetPostsUnitTest.inc b/WordPressVIPMinimum/Tests/Hooks/PreGetPostsUnitTest.inc index 6ff848b1..f0ebf896 100644 --- a/WordPressVIPMinimum/Tests/Hooks/PreGetPostsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Hooks/PreGetPostsUnitTest.inc @@ -90,3 +90,19 @@ add_action( 'pre_get_posts', function( $wp_query ) { } } ); + +class undefined_index_issue_499 { + + public function __construct() { + add_action( 'pre_get_posts', array( $this, 'pre_get_posts_499' ) ); + } + + public function pre_get_posts_499( $wp_query ) { + + if ( function() { return ( $wp_query->is_main_query() === false ) }() === false ) { + return; + } + + $wp_query->set( 'cat', '-5' ); + } +} From 82d1232802600919e47af8f5a7094e7c1dde154d Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 28 Jul 2020 05:00:32 +0200 Subject: [PATCH 2/2] PreGetPosts: improve the isEarlyMainQueryCheck() method [2] This adds a second unit test which is based on the actual code which originally triggered the error. The error was caused by the code in question using inline control structures (without braces) for the early main query check. After the previous fix, that code would now throw a false positive. I've fixed this now by adding an additional check for a `return` statement straight after the parenthesis closer of the `if()` statement. Fixes 499 --- .../Sniffs/Hooks/PreGetPostsSniff.php | 12 ++++++++++++ .../Tests/Hooks/PreGetPostsUnitTest.inc | 10 ++++++++++ 2 files changed, 22 insertions(+) diff --git a/WordPressVIPMinimum/Sniffs/Hooks/PreGetPostsSniff.php b/WordPressVIPMinimum/Sniffs/Hooks/PreGetPostsSniff.php index a2df7603..655bbdae 100644 --- a/WordPressVIPMinimum/Sniffs/Hooks/PreGetPostsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Hooks/PreGetPostsSniff.php @@ -321,6 +321,18 @@ private function isEarlyMainQueryCheck( $stackPtr ) { $owner = $this->tokens[ $nestedParenthesisEnd ]['parenthesis_owner']; if ( isset( $this->tokens[ $owner ]['scope_opener'], $this->tokens[ $owner ]['scope_closer'] ) === false ) { + // This may be an inline control structure (no braces). + $next = $this->phpcsFile->findNext( + Tokens::$emptyTokens, + ( $nestedParenthesisEnd + 1 ), + null, + true + ); + + if ( false !== $next && T_RETURN === $this->tokens[ $next ]['code'] ) { + return true; + } + return false; } diff --git a/WordPressVIPMinimum/Tests/Hooks/PreGetPostsUnitTest.inc b/WordPressVIPMinimum/Tests/Hooks/PreGetPostsUnitTest.inc index f0ebf896..09c5f06e 100644 --- a/WordPressVIPMinimum/Tests/Hooks/PreGetPostsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Hooks/PreGetPostsUnitTest.inc @@ -106,3 +106,13 @@ class undefined_index_issue_499 { $wp_query->set( 'cat', '-5' ); } } + +add_action('pre_get_posts', 'inline_control_structures', 10, 1); + +function inline_control_structures( $query ) { + if( !$query->is_main_query() && !is_front_page()) return; + if(is_single() || is_search() || is_archive()) return; + + $query->set('meta_query', 'foo'); + return $query; +}