From ffafba0695d7b33b041a82c2ef620f8be7d8992f Mon Sep 17 00:00:00 2001
From: thiemowmde <thiemo.kreuz@wikimedia.de>
Date: Mon, 26 Aug 2024 17:27:06 +0200
Subject: [PATCH] Remove meaningless return true from hook handler functions

Returning true is the same as returning nothing. It's only meaningful
when a hook handler can also return false. Some actually do this.
I'm not touching these.

See Icccf60b for the reasoning why the added `@return void` are
beneficial.

Change-Id: I6de7addee853ff183058e6c84e87a5b275c785e8
---
 includes/Hooks.php          | 30 ++++++++++--------------------
 tests/phpunit/HooksTest.php | 17 ++++++++---------
 2 files changed, 18 insertions(+), 29 deletions(-)

diff --git a/includes/Hooks.php b/includes/Hooks.php
index 3d762042..4d6ae6ee 100644
--- a/includes/Hooks.php
+++ b/includes/Hooks.php
@@ -104,33 +104,30 @@ class Hooks implements
 	 * Register parser hooks.
 	 *
 	 * @param Parser $parser
-	 * @return bool
+	 * @return void
 	 */
 	public function onParserFirstCallInit( $parser ) {
 		$parser->setFunctionHook( 'invoke', [ $this, 'invokeHook' ], Parser::SFH_OBJECT_ARGS );
-		return true;
 	}
 
 	/**
 	 * Called when the interpreter is to be reset.
 	 *
 	 * @param Parser $parser
-	 * @return bool
+	 * @return void
 	 */
 	public function onParserClearState( $parser ) {
 		Scribunto::resetParserEngine( $parser );
-		return true;
 	}
 
 	/**
 	 * Called when the parser is cloned
 	 *
 	 * @param Parser $parser
-	 * @return bool
+	 * @return void
 	 */
 	public function onParserCloned( $parser ) {
 		$parser->scribunto_engine = null;
-		return true;
 	}
 
 	/**
@@ -293,12 +290,12 @@ class Hooks implements
 	 *
 	 * @param Title $title
 	 * @param string &$model
-	 * @return bool
+	 * @return void
 	 */
 	public function onContentHandlerDefaultModelFor( $title, &$model ) {
 		if ( $model === 'sanitized-css' ) {
 			// Let TemplateStyles override Scribunto
-			return true;
+			return;
 		}
 		if ( $title->getNamespace() === NS_MODULE ) {
 			if ( str_ends_with( $title->getText(), '.json' ) ) {
@@ -306,9 +303,7 @@ class Hooks implements
 			} elseif ( !Scribunto::isDocPage( $title ) ) {
 				$model = CONTENT_MODEL_SCRIBUNTO;
 			}
-			return true;
 		}
-		return true;
 	}
 
 	/**
@@ -316,14 +311,13 @@ class Hooks implements
 	 *
 	 * @param Parser $parser
 	 * @param ParserOutput $parserOutput
-	 * @return bool
+	 * @return void
 	 */
 	public function onParserLimitReportPrepare( $parser, $parserOutput ) {
 		if ( Scribunto::isParserEnginePresent( $parser ) ) {
 			$engine = Scribunto::getParserEngine( $parser );
 			$engine->reportLimitData( $parserOutput );
 		}
-		return true;
 	}
 
 	/**
@@ -347,14 +341,13 @@ class Hooks implements
 	 * @param EditPage $editor
 	 * @param OutputPage $output
 	 * @param int &$tab Current tabindex
-	 * @return bool
+	 * @return void
 	 */
 	public function onEditPage__showStandardInputs_options( $editor, $output, &$tab ) {
 		if ( $editor->getTitle()->hasContentModel( CONTENT_MODEL_SCRIBUNTO ) ) {
 			$output->addModules( 'ext.scribunto.edit' );
 			$editor->editFormTextAfterTools .= '<div id="mw-scribunto-console"></div>';
 		}
-		return true;
 	}
 
 	/**
@@ -362,14 +355,13 @@ class Hooks implements
 	 *
 	 * @param EditPage $editor
 	 * @param OutputPage $output
-	 * @return bool
+	 * @return void
 	 */
 	public function onEditPage__showReadOnlyForm_initial( $editor, $output ) {
 		if ( $editor->getTitle()->hasContentModel( CONTENT_MODEL_SCRIBUNTO ) ) {
 			$output->addModules( 'ext.scribunto.edit' );
 			$editor->editFormTextAfterContent .= '<div id="mw-scribunto-console"></div>';
 		}
-		return true;
 	}
 
 	/**
@@ -378,13 +370,12 @@ class Hooks implements
 	 * @param EditPage $editor
 	 * @param array &$buttons Button array
 	 * @param int &$tabindex Current tabindex
-	 * @return bool
+	 * @return void
 	 */
 	public function onEditPageBeforeEditButtons( $editor, &$buttons, &$tabindex ) {
 		if ( $editor->getTitle()->hasContentModel( CONTENT_MODEL_SCRIBUNTO ) ) {
 			unset( $buttons['preview'] );
 		}
-		return true;
 	}
 
 	/**
@@ -436,7 +427,7 @@ class Hooks implements
 	 * @param Article $article
 	 * @param bool|ParserOutput|null &$outputDone
 	 * @param bool &$pcache
-	 * @return bool
+	 * @return void
 	 */
 	public function onArticleViewHeader( $article, &$outputDone, &$pcache ) {
 		$title = $article->getTitle();
@@ -445,6 +436,5 @@ class Hooks implements
 				wfMessage( 'scribunto-doc-page-header', $forModule->getPrefixedText() )->parseAsBlock()
 			);
 		}
-		return true;
 	}
 }
diff --git a/tests/phpunit/HooksTest.php b/tests/phpunit/HooksTest.php
index e1e3ec94..32af728d 100644
--- a/tests/phpunit/HooksTest.php
+++ b/tests/phpunit/HooksTest.php
@@ -16,12 +16,12 @@ class HooksTest extends TestCase {
 
 	public static function provideContentHandlerDefaultModelFor() {
 		return [
-			[ 'Module:Foo', CONTENT_MODEL_SCRIBUNTO, true ],
-			[ 'Module:Foo/doc', null, true ],
-			[ 'Module:Foo/styles.css', 'sanitized-css', true, 'sanitized-css' ],
-			[ 'Module:Foo.json', CONTENT_MODEL_JSON, true ],
-			[ 'Module:Foo/subpage.json', CONTENT_MODEL_JSON, true ],
-			[ 'Main Page', null, true ],
+			[ 'Module:Foo', CONTENT_MODEL_SCRIBUNTO ],
+			[ 'Module:Foo/doc', null ],
+			[ 'Module:Foo/styles.css', 'sanitized-css', 'sanitized-css' ],
+			[ 'Module:Foo.json', CONTENT_MODEL_JSON ],
+			[ 'Module:Foo/subpage.json', CONTENT_MODEL_JSON ],
+			[ 'Main Page', null ],
 		];
 	}
 
@@ -29,14 +29,13 @@ class HooksTest extends TestCase {
 	 * @dataProvider provideContentHandlerDefaultModelFor
 	 */
 	public function testContentHandlerDefaultModelFor( $name, $expected,
-		$retVal, $before = null
+		$before = null
 	) {
 		$title = Title::newFromText( $name );
 		$model = $before;
-		$ret = ( new Hooks(
+		( new Hooks(
 			MediaWikiServices::getInstance()->getMainConfig()
 		) )->onContentHandlerDefaultModelFor( $title, $model );
-		$this->assertSame( $retVal, $ret );
 		$this->assertSame( $expected, $model );
 	}
 }