Merge pull request #581 from kevinfodness/hotfix/abstract-filter-call…

…back Ignore return values when a filter callback is abstract
Automattic · Sep 14, 2020 · 200dec1 · 200dec1
2 parents 05789f8 + 6c190d2
commit 200dec1
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 3 deletions.
diff --git a/WordPressVIPMinimum/Sniffs/Hooks/AlwaysReturnInFilterSniff.php b/WordPressVIPMinimum/Sniffs/Hooks/AlwaysReturnInFilterSniff.php
@@ -175,6 +175,21 @@ private function processFunction( $stackPtr, $start = 0, $end = null ) {
 	 */
 	private function processFunctionBody( $stackPtr ) {
 
+		$filterName = $this->tokens[ $this->filterNamePtr ]['content'];
+
+		$methodProps = $this->phpcsFile->getMethodProperties( $stackPtr );
+		if ( $methodProps['is_abstract'] === true ) {
+			$message = 'The callback for the `%s` filter hook-in points to an abstract method. Please ensure that child class implementations of this method always return a value.';
+			$data    = [ $filterName ];
+			$this->phpcsFile->addWarning( $message, $stackPtr, 'AbstractMethod', $data );
+			return;
+		}
+
+		if ( isset( $this->tokens[ $stackPtr ]['scope_opener'], $this->tokens[ $stackPtr ]['scope_closer'] ) === false ) {
+			// Live coding, parse or tokenizer error.
+			return;
+		}
+
 		$argPtr = $this->phpcsFile->findNext(
 			array_merge( Tokens::$emptyTokens, [ T_STRING, T_OPEN_PARENTHESIS ] ),
 			$stackPtr + 1,
@@ -189,8 +204,6 @@ private function processFunctionBody( $stackPtr ) {
 			return;
 		}
 
-		$filterName = $this->tokens[ $this->filterNamePtr ]['content'];
-
 		$functionBodyScopeStart = $this->tokens[ $stackPtr ]['scope_opener'];
 		$functionBodyScopeEnd   = $this->tokens[ $stackPtr ]['scope_closer'];
 

diff --git a/WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.inc b/WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.inc
@@ -172,3 +172,26 @@ class bad_example_class_short_array { // Error.
 	}
 }
 
+abstract class warning_filter_points_to_abstract_method {
+	public function __construct() {
+		add_filter( 'good_example_abstract_class', [ $this, 'abstract_method' ] );
+	}
+
+	abstract public function abstract_method( $param ); // Warning.
+}
+
+class tokenizer_bug_test {
+	public function __construct() {
+		add_filter( 'tokenizer_bug', [ $this, 'tokenizer_bug' ] );
+	}
+
+	public function tokenizer_bug( $param ): namespace\Foo {} // Ok (tokenizer bug in PHPCS < 3.5.7/3.6.0).
+}
+
+// Intentional parse error. This has to be the last test in the file!
+class parse_error_test {
+	public function __construct() {
+		add_filter( 'parse_error', [ $this, 'parse_error' ] );
+	}
+
+	public function parse_error( $param ) // Ok, parse error ignored.
diff --git a/WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.php b/WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.php
@@ -40,7 +40,9 @@ public function getErrorList() {
 	 * @return array <int line number> => <int number of warnings>
 	 */
 	public function getWarningList() {
-		return [];
+		return [
+			180 => 1,
+		];
 	}
 
 }