From 49edc86a78cb83a85a9b1fe2002e197b3acc2c24 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mat=C4=9Bj=20Such=C3=A1nek?= <matejsuchanek97@gmail.com>
Date: Wed, 5 Jul 2023 11:59:52 +0200
Subject: [PATCH] Split VariableGenerator::addEditVars

This method actually consists of two: add derived vars, and initialize
content vars. The former part depends on no parameters of this method.
On the other hand, the latter part combines multiple implementations
for some of the content variables using branching.

The branching is a dirty workaround and inferior to the GRASP principle:
"When related alternatives or behaviors vary by type, assign
responsibility for the behavior to the types for which the behavior
varies."
In other words, the callers (extensions) should be responsible for
choosing the initialization strategy themselves, instead of letting
VariableGenerator figure it out.

As the first step, split the former part to a separate method.
For now, it will be implicitly called by ::addEditVars.

Change-Id: I5ff00dbdbf29ec54eabfd95c44a4fd7f713969f5
---
 .../VariableGenerator/VariableGenerator.php   | 36 +++++++++++--------
 tests/phpunit/RCVariableGeneratorTest.php     |  1 +
 .../VariableGeneratorTest.php                 |  1 +
 3 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/includes/VariableGenerator/VariableGenerator.php b/includes/VariableGenerator/VariableGenerator.php
index 3cb767caf..9d244c7ad 100644
--- a/includes/VariableGenerator/VariableGenerator.php
+++ b/includes/VariableGenerator/VariableGenerator.php
@@ -172,20 +172,7 @@ class VariableGenerator {
 		return $this;
 	}
 
-	/**
-	 * @param WikiPage $page
-	 * @param UserIdentity $userIdentity The current user
-	 * @param bool $forFilter Whether the variables should be computed for an ongoing action
-	 *   being filtered
-	 * @param PreparedUpdate|null $update
-	 * @return $this For chaining
-	 */
-	public function addEditVars(
-		WikiPage $page,
-		UserIdentity $userIdentity,
-		bool $forFilter = true,
-		PreparedUpdate $update = null
-	): self {
+	public function addDerivedEditVars(): self {
 		$this->vars->setLazyLoadVar( 'edit_diff', 'diff',
 			[ 'oldtext-var' => 'old_wikitext', 'newtext-var' => 'new_wikitext' ] );
 		$this->vars->setLazyLoadVar( 'edit_diff_pst', 'diff',
@@ -208,9 +195,30 @@ class VariableGenerator {
 			[ 'oldlink-var' => 'old_links', 'newlink-var' => 'all_links' ] );
 		$this->vars->setLazyLoadVar( 'removed_links', 'link-diff-removed',
 			[ 'oldlink-var' => 'old_links', 'newlink-var' => 'all_links' ] );
+
+		// Text
 		$this->vars->setLazyLoadVar( 'new_text', 'strip-html',
 			[ 'html-var' => 'new_html' ] );
 
+		return $this;
+	}
+
+	/**
+	 * @param WikiPage $page
+	 * @param UserIdentity $userIdentity The current user
+	 * @param bool $forFilter Whether the variables should be computed for an ongoing action
+	 *   being filtered
+	 * @param PreparedUpdate|null $update
+	 * @return $this For chaining
+	 */
+	public function addEditVars(
+		WikiPage $page,
+		UserIdentity $userIdentity,
+		bool $forFilter = true,
+		PreparedUpdate $update = null
+	): self {
+		$this->addDerivedEditVars();
+
 		if ( $forFilter && $update ) {
 			$this->vars->setLazyLoadVar( 'all_links', 'links-from-update',
 				[ 'update' => $update ] );
diff --git a/tests/phpunit/RCVariableGeneratorTest.php b/tests/phpunit/RCVariableGeneratorTest.php
index 9553023aa..c17ecc5cb 100644
--- a/tests/phpunit/RCVariableGeneratorTest.php
+++ b/tests/phpunit/RCVariableGeneratorTest.php
@@ -200,6 +200,7 @@ class RCVariableGeneratorTest extends MediaWikiIntegrationTestCase {
 
 	/**
 	 * @covers ::addEditVars
+	 * @covers ::addDerivedEditVars
 	 * @covers ::addEditVarsForRow
 	 * @covers ::addGenericVars
 	 * @covers \MediaWiki\Extension\AbuseFilter\Variables\LazyVariableComputer
diff --git a/tests/phpunit/unit/VariableGenerator/VariableGeneratorTest.php b/tests/phpunit/unit/VariableGenerator/VariableGeneratorTest.php
index a59cd88ca..e74606cd9 100644
--- a/tests/phpunit/unit/VariableGenerator/VariableGeneratorTest.php
+++ b/tests/phpunit/unit/VariableGenerator/VariableGeneratorTest.php
@@ -159,6 +159,7 @@ class VariableGeneratorTest extends MediaWikiUnitTestCase {
 
 	/**
 	 * @covers ::addEditVars
+	 * @covers ::addDerivedEditVars
 	 * @dataProvider provideForFilter
 	 */
 	public function testAddEditVars( bool $forFilter ) {