Deprecate ParserOutput::addWarning() in favor of ::addWarningMsg()

Encourage localization and factor out common code by taking a message
key as the first argument to ::addWarningMsg() instead of a wikitext
string.  This also plays nicer with Parsoid by separating out the
localization code from the parse.

Bug: T293515
Change-Id: I6a7c04c67ac586ab00d4edcbb3d09485a7794e23

RELEASE-NOTES-1.38

@@ -230,6 +230,7 @@ because of Phabricator reports.
    - ::getFlag() - use ::getOutputFlag()
    - ::setFlag() - use ::setOutputFlag()
    - ::getAllFlags() - this method is now marked @internal
+   - ::addWarning() - use ::addWarningMsg()
 * The following methods were deprecated; use ::setPreventClickjacking(..)
   instead:
    - OutputPage::preventClickjacking()

includes/parser/CoreParserFunctions.php

@@ -501,11 +501,10 @@ class CoreParserFunctions {
 				return '';
 			}
 		} else {
-			$parser->getOutput()->addWarning(
-				wfMessage( 'restricted-displaytitle',
+			$parser->getOutput()->addWarningMsg(
+				'restricted-displaytitle',
 				// Message should be parsed, but this param should only be escaped.
-					wfEscapeWikiText( $text )
-				)->text()
+				Message::plaintextParam( $text )
 			);
 			$parser->addTrackingCategory( 'restricted-displaytitle-ignored' );
 		}

includes/parser/PPFrame_Hash.php

@@ -121,10 +121,12 @@ class PPFrame_Hash implements PPFrame {
 					// Numbered parameter
 					$index = $bits['index'] - $indexOffset;
 					if ( isset( $namedArgs[$index] ) || isset( $numberedArgs[$index] ) ) {
-						$this->parser->getOutput()->addWarning( wfMessage( 'duplicate-args-warning',
-							wfEscapeWikiText( $this->title ),
-							wfEscapeWikiText( $title ),
-							wfEscapeWikiText( $index ) )->text() );
+						$this->parser->getOutput()->addWarningMsg(
+							'duplicate-args-warning',
+							Message::plaintextParam( (string)$this->title ),
+							Message::plaintextParam( (string)$title ),
+							Message::numParam( $index )
+						);
 						$this->parser->addTrackingCategory( 'duplicate-args-category' );
 					}
 					$numberedArgs[$index] = $bits['value'];
@@ -133,11 +135,12 @@ class PPFrame_Hash implements PPFrame {
 					// Named parameter
 					$name = trim( $this->expand( $bits['name'], PPFrame::STRIP_COMMENTS ) );
 					if ( isset( $namedArgs[$name] ) || isset( $numberedArgs[$name] ) ) {
-						$this->parser->getOutput()->addWarning( wfMessage( 'duplicate-args-warning',
-							wfEscapeWikiText( $this->title ),
-							wfEscapeWikiText( $title ),
-							// @phan-suppress-next-line SecurityCheck-DoubleEscaped taint track for named args
-							wfEscapeWikiText( $name ) )->text() );
+						$this->parser->getOutput()->addWarningMsg(
+							'duplicate-args-warning',
+							Message::plaintextParam( (string)$this->title ),
+							Message::plaintextParam( (string)$title ),
+							Message::plaintextParam( (string)$name )
+						);
 						$this->parser->addTrackingCategory( 'duplicate-args-category' );
 					}
 					$namedArgs[$name] = $bits['value'];

includes/parser/Parser.php

@@ -2950,9 +2950,11 @@ class Parser {
 		# does no harm if $current and $max are present but are unnecessary for the message
 		# Not doing ->inLanguage( $this->mOptions->getUserLangObj() ), since this is shown
 		# only during preview, and that would split the parser cache unnecessarily.
-		$warning = wfMessage( "$limitationType-warning" )->numParams( $current, $max )
-			->text();
-		$this->mOutput->addWarning( $warning );
+		$this->mOutput->addWarningMsg(
+			"$limitationType-warning",
+			Message::numParam( $current ),
+			Message::numParam( $max )
+		);
 		$this->addTrackingCategory( "$limitationType-category" );
 	}
 
@@ -3237,8 +3239,10 @@ class Parser {
 					. wfMessage( 'parser-template-loop-warning', $titleText )->inContentLanguage()->text()
 					. '</span>';
 				$this->addTrackingCategory( 'template-loop-category' );
-				$this->mOutput->addWarning( wfMessage( 'template-loop-warning',
-					wfEscapeWikiText( $titleText ) )->text() );
+				$this->mOutput->addWarningMsg(
+					'template-loop-warning',
+					Message::plaintextParam( $titleText )
+				);
 				$this->logger->debug( __METHOD__ . ": template loop broken at '$titleText'" );
 			}
 		}
@@ -3283,7 +3287,7 @@ class Parser {
 			// T91154: {{=}} is deprecated when it doesn't expand to `=`;
 			// use {{Template:=}} if you must.
 			$this->addTrackingCategory( 'template-equals-category' );
-			$this->mOutput->addWarning( wfMessage( 'template-equals-warning' )->text() );
+			$this->mOutput->addWarningMsg( 'template-equals-warning' );
 		}
 
 		# Replace raw HTML by a placeholder

includes/parser/ParserOutput.php

@@ -838,10 +838,27 @@ class ParserOutput extends CacheTime {
 		$this->mLanguageLinks[] = $t;
 	}
 
+	/**
+	 * @deprecated since 1.38; use ::addWarningMsg() instead
+	 */
 	public function addWarning( $s ) {
 		$this->mWarnings[$s] = 1;
 	}
 
+	/**
+	 * Add a warning to the output for this page.
+	 * @param string $msg The localization message key for the warning
+	 * @param mixed ...$args Optional arguments for the message
+	 * @since 1.38
+	 */
+	public function addWarningMsg( string $msg, ...$args ): void {
+		$s = wfMessage( $msg, ...$args )
+		   // some callers set the title here?
+		   ->inContentLanguage() // because this ends up in cache
+		   ->text();
+		$this->mWarnings[$s] = 1;
+	}
+
 	public function addOutputHook( $hook, $data = false ) {
 		$this->mOutputHooks[] = [ $hook, $data ];
 	}

tests/phpunit/includes/parser/ParserOutputTest.php

@@ -816,8 +816,8 @@ EOF
 		// flags & co
 		$a = new ParserOutput();
 
-		$a->addWarning( 'Oops' );
-		$a->addWarning( 'Whoops' );
+		$a->addWarningMsg( 'duplicate-args-warning', 'A', 'B', 'C' );
+		$a->addWarningMsg( 'template-loop-warning', 'D' );
 
 		$a->setFlag( 'foo' );
 		$a->setFlag( 'bar' );
@@ -827,8 +827,9 @@ EOF
 
 		$b = new ParserOutput();
 
-		$b->addWarning( 'Yikes' );
-		$b->addWarning( 'Whoops' );
+		$b->addWarningMsg( 'template-equals-warning' );
+		$b->addWarningMsg( 'template-loop-warning', 'D' );
+		$b->addWarning( 'Old School' ); // test the deprecated ::addWarning()
 
 		$b->setFlag( 'zoo' );
 		$b->setFlag( 'bar' );
@@ -837,7 +838,12 @@ EOF
 		$b->recordOption( 'Bar' );
 
 		yield 'flags' => [ $a, $b, [
-			'getWarnings' => [ 'Oops', 'Whoops', 'Yikes' ],
+			'getWarnings' => [
+				wfMessage( 'duplicate-args-warning', 'A', 'B', 'C' )->text(),
+				wfMessage( 'template-loop-warning', 'D' )->text(),
+				wfMessage( 'template-equals-warning' )->text(),
+				'Old School',
+			],
 			'$mFlags' => [ 'foo' => true, 'bar' => true, 'zoo' => true ],
 			'getUsedOptions' => [ 'Foo', 'Bar', 'Zoo' ],
 		] ];