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

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 ] );

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

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 ) {