Deprecate MagicWordFactory::getSubstIDs

The main motivation is to further reduce the complexity of the class:
* There is no code that ever writes to $this->mSubstIDs. It's
  effectively a constant.
* According to CodeSearch the getSubstIDs() method is not used
  anywhere. It's @internal to the parser.
* I find it weird that the parser needs to call 2 factory methods to
  do 1 thing.
* I still find it a good idea to keep the knowledge encapsulated in
  the factory and not have the [ 'subst', 'safesubst' ] array in the
  parser. That's why I propose the new method.

Change-Id: I5c147c75200c3c34a410d93a0328b56ea00a050f

RELEASE-NOTES-1.42

@@ -109,6 +109,7 @@ because of Phabricator reports.
   Instead use DatabaseBlockStore::newFromID().
 * DatabaseBlock::getAutoblockExpiry() was deprecated without replacement.
 * MagicWord::getId() has been deprecated.
+* MagicWordFactory::getSubstIDs() has been deprecated.
 * Status::setMessageLocalizer is deprecated. Callers that want to control the
   localization of the Status object should obtain a StatusFormatter from
   the StatusFormatterFactory.

includes/parser/MagicWordFactory.php

@@ -141,12 +141,6 @@ class MagicWordFactory {
 		'nocontentconvert',
 	];
 
-	/** @var string[] */
-	private array $mSubstIDs = [
-		'subst',
-		'safesubst',
-	];
-
 	/** @var array<string,MagicWord> */
 	private array $mObjects = [];
 	private ?MagicWordArray $mDoubleUnderscoreArray = null;
@@ -200,9 +194,18 @@ class MagicWordFactory {
 	 * Get an array of parser substitution modifier IDs
 	 *
 	 * @return string[]
+	 * @deprecated since 1.42, use {@see getSubstArray} instead
 	 */
 	public function getSubstIDs(): array {
-		return $this->mSubstIDs;
+		wfDeprecated( __METHOD__, '1.42' );
+		return [ 'subst', 'safesubst' ];
+	}
+
+	/**
+	 * @internal for use in {@see Parser::braceSubstitution} only
+	 */
+	public function getSubstArray(): MagicWordArray {
+		return $this->newArray( [ 'subst', 'safesubst' ] );
 	}
 
 	/**

includes/parser/Parser.php

@@ -2815,10 +2815,9 @@ class Parser {
 	 */
 	private function initializeVariables() {
 		$variableIDs = $this->magicWordFactory->getVariableIDs();
-		$substIDs = $this->magicWordFactory->getSubstIDs();
 
 		$this->mVariables = $this->magicWordFactory->newArray( $variableIDs );
-		$this->mSubstWords = $this->magicWordFactory->newArray( $substIDs );
+		$this->mSubstWords = $this->magicWordFactory->getSubstArray();
 	}
 
 	/**

tests/phpunit/includes/parser/MagicWordFactoryTest.php

@@ -54,13 +54,13 @@ class MagicWordFactoryTest extends MediaWikiIntegrationTestCase {
 		$this->assertContainsOnly( 'string', $varIds );
 	}
 
-	public function testGetSubstIDs() {
+	public function testGetSubstArray() {
 		$magicWordFactory = $this->makeMagicWordFactory();
-		$substIds = $magicWordFactory->getSubstIDs();
+		$substArray = $magicWordFactory->getSubstArray();
 
-		$this->assertIsArray( $substIds );
-		$this->assertNotEmpty( $substIds );
-		$this->assertContainsOnly( 'string', $substIds );
+		$text = 'SafeSubst:x';
+		$this->assertSame( 'safesubst', $substArray->matchStartAndRemove( $text ) );
+		$this->assertSame( 'x', $text );
 	}
 
 	public function testGetDoubleUnderscoreArray() {

tests/phpunit/includes/parser/ParserTest.php

@@ -37,10 +37,9 @@ class ParserTest extends MediaWikiIntegrationTestCase {
 		// Stub out a MagicWordFactory so the Parser can initialize its
 		// function hooks when it is created.
 		$mwFactory = $this->createNoOpMock( MagicWordFactory::class,
-			[ 'get', 'getVariableIDs', 'getSubstIDs', 'newArray' ] );
+			[ 'get', 'getVariableIDs', 'getSubstArray', 'newArray' ] );
 		$mwFactory->method( 'get' )->willReturn( $mw );
 		$mwFactory->method( 'getVariableIDs' )->willReturn( [] );
-		$mwFactory->method( 'getSubstIDs' )->willReturn( [] );
 
 		$urlUtils = $this->createNoOpMock( UrlUtils::class, [ 'validProtocols' ] );
 		$urlUtils->method( 'validProtocols' )->willReturn( '' );

tests/phpunit/unit/includes/parser/ParserFactoryTest.php

@@ -36,10 +36,9 @@ class ParserFactoryTest extends MediaWikiUnitTestCase {
 		// Stub out a MagicWordFactory so the Parser can initialize its
 		// function hooks when it is created.
 		$mwFactory = $this->createNoOpMock( MagicWordFactory::class,
-			[ 'get', 'getVariableIDs', 'getSubstIDs', 'newArray' ] );
+			[ 'get', 'getVariableIDs', 'getSubstArray', 'newArray' ] );
 		$mwFactory->method( 'get' )->willReturn( $mw );
 		$mwFactory->method( 'getVariableIDs' )->willReturn( [] );
-		$mwFactory->method( 'getSubstIDs' )->willReturn( [] );
 
 		$urlUtils = $this->createNoOpMock( UrlUtils::class, [ 'validProtocols' ] );
 		$urlUtils->method( 'validProtocols' )->willReturn( '' );