Make Language::hasVariant() more strict

In d59f27aeab we made
LanguageConverter::validateVariant() try harder to convert a variant
into an acceptable MediaWiki-internal form, looking at deprecated
codes and BCP 47 aliases.  However, this misled Language::hasVariant()
into thinking that bogus names (like all-uppercase strings) were
acceptable variant names, which then led exceptions when they were
passed to the various conversion methods.

This is a belt-and-suspenders patch for T207433 -- in that case we
shouldn't have created a Language object with code 'sr-cyrl' in the
first place, but once one was created we shouldn't have tried to
ask LanguageSr to convert texts to 'sr-cyrl'.  The latter problem
is fixed by this patch.

Bug: T207433
Change-Id: Id993bc7989144b5031a551662e8e492bd23f698a

languages/Language.php

@@ -4263,14 +4263,17 @@ class Language {
 	}
 
 	/**
-	 * Check if the language has the specific variant
+	 * Strict check if the language has the specific variant.
+	 *
+	 * Compare to LanguageConverter::validateVariant() which does a more
+	 * lenient check and attempts to coerce the given code to a valid one.
 	 *
 	 * @since 1.19
 	 * @param string $variant
 	 * @return bool
 	 */
 	public function hasVariant( $variant ) {
-		return (bool)$this->mConverter->validateVariant( $variant );
+		return $variant && ( $variant === $this->mConverter->validateVariant( $variant ) );
 	}
 
 	/**

languages/LanguageConverter.php

@@ -212,9 +212,13 @@ class LanguageConverter {
 	}
 
 	/**
-	 * Validate the variant
+	 * Validate the variant and return an appropriate strict internal
+	 * variant code if one exists.  Compare to Language::hasVariant()
+	 * which does a strict test.
+	 *
 	 * @param string|null $variant The variant to validate
-	 * @return mixed Returns the variant if it is valid, null otherwise
+	 * @return mixed Returns an equivalent valid variant code if possible,
+	 *   null otherwise
 	 */
 	public function validateVariant( $variant = null ) {
 		if ( $variant === null ) {

tests/phpunit/languages/LanguageTest.php

@@ -1878,6 +1878,19 @@ class LanguageTest extends LanguageClassesTestCase {
 		];
 	}
 
+	/**
+	 * @covers Language::hasVariant
+	 */
+	public function testHasVariant() {
+		// See LanguageSrTest::testHasVariant() for additional tests
+		$en = Language::factory( 'en' );
+		$this->assertTrue( $en->hasVariant( 'en' ), 'base is always a variant' );
+		$this->assertFalse( $en->hasVariant( 'en-bogus' ), 'bogus en variant' );
+
+		$bogus = Language::factory( 'bogus' );
+		$this->assertTrue( $bogus->hasVariant( 'bogus' ), 'base is always a variant' );
+	}
+
 	/**
 	 * @covers Language::equals
 	 */

tests/phpunit/languages/classes/LanguageSrTest.php

@@ -21,6 +21,63 @@
  * @covers SrConverter
  */
 class LanguageSrTest extends LanguageClassesTestCase {
+	/**
+	 * @covers Language::hasVariants
+	 */
+	public function testHasVariants() {
+		$this->assertTrue( $this->getLang()->hasVariants(), 'sr has variants' );
+	}
+
+	/**
+	 * @covers Language::hasVariant
+	 */
+	public function testHasVariant() {
+		$langs = [
+			'sr' => $this->getLang(),
+			'sr-ec' => Language::factory( 'sr-ec' ),
+			'sr-cyrl' => Language::factory( 'sr-cyrl' ),
+		];
+		foreach ( $langs as $code => $l ) {
+			$p = $l->getParentLanguage();
+			$this->assertTrue( $p !== null, 'parent language exists' );
+			$this->assertEquals( 'sr', $p->getCode(), 'sr is parent language' );
+			$this->assertTrue( $p instanceof LanguageSr, 'parent is LanguageSr' );
+			// This is a valid variant of the base
+			$this->assertTrue( $p->hasVariant( $l->getCode() ) );
+			// This test should be tweaked if/when sr-ec is renamed (T117845)
+			// to swap the roles of sr-ec and sr-Cyrl
+			$this->assertTrue( $l->hasVariant( 'sr-ec' ), 'sr-ec exists' );
+			// note that sr-cyrl is an alias, not a (strict) variant name
+			foreach ( [ 'sr-EC', 'sr-Cyrl', 'sr-cyrl', 'sr-bogus' ] as $v ) {
+				$this->assertFalse( $l->hasVariant( $v ), "$v is not a variant of $code" );
+			}
+		}
+	}
+
+	/**
+	 * @covers Language::hasVariant
+	 */
+	public function testHasVariantBogus() {
+		$langs = [
+			// Note that case matters when calling Language::factory();
+			// these are all bogus language codes
+			'sr-EC' => Language::factory( 'sr-EC' ),
+			'sr-Cyrl' => Language::factory( 'sr-Cyrl' ),
+			'sr-bogus' => Language::factory( 'sr-bogus' ),
+		];
+		foreach ( $langs as $code => $l ) {
+			$p = $l->getParentLanguage();
+			$this->assertTrue( $p === null, 'no parent for bogus language' );
+			$this->assertFalse( $l instanceof LanguageSr, "$code is not sr" );
+			$this->assertFalse( $this->getLang()->hasVariant( $code ), "$code is not a sr variant" );
+			foreach ( [ 'sr', 'sr-ec', 'sr-EC', 'sr-Cyrl', 'sr-cyrl', 'sr-bogus' ] as $v ) {
+				if ( $v !== $code ) {
+					$this->assertFalse( $l->hasVariant( $v ), "no variant $v" );
+				}
+			}
+		}
+	}
+
 	/**
 	 * @covers LanguageConverter::convertTo
 	 */