specials: Clean up redirect special pages ($subpage can be null)

$subpage being null is clearly documented in SpecialPage::run,
SpecialPage::execute, and most special page subclasses.

But all the redirect subclasses only copied part of the typehint,
making it look like it's always a string.

For SpecialMyLanguage, follows-up b1853bba0. Don't cast null to
empty string, and don't bother giving Title::newFromText an empty
string only to bail out with null again.

Also:
* Add visibility 'public' where missing.
* Add or correct relevant documentation comments.
* In SpecialMyRedirectPages, handle error first and avoid having
  final return inside a conditional; Remove redunant 'else'.

Change-Id: Ie3543f44011832b198bb3d3e32528b6a2868dee1

diff --git a/includes/specialpage/RedirectSpecialPage.php b/includes/specialpage/RedirectSpecialPage.php
index 2e6e55a..a866ba7 100644 (file)
--- a/includes/specialpage/RedirectSpecialPage.php
+++ b/includes/specialpage/RedirectSpecialPage.php
@@ -33,8 +33,11 @@ abstract class RedirectSpecialPage extends UnlistedSpecialPage {
        // Query parameters added by redirects
        protected $mAddedRedirectParams = array();
 
-       public function execute( $par ) {
-               $redirect = $this->getRedirect( $par );
+       /**
+        * @param string|null $subpage
+        */
+       public function execute( $subpage ) {
+               $redirect = $this->getRedirect( $subpage );
                $query = $this->getRedirectQuery();
                // Redirect to a page title with possible query parameters
                if ( $redirect instanceof Title ) {
@@ -58,10 +61,10 @@ abstract class RedirectSpecialPage extends UnlistedSpecialPage {
         * If the special page is a redirect, then get the Title object it redirects to.
         * False otherwise.
         *
-        * @param string $par Subpage string
+        * @param string|null $subpage
         * @return Title|bool
         */
-       abstract public function getRedirect( $par );
+       abstract public function getRedirect( $subpage );
 
        /**
         * Return part of the request string for a special redirect page
@@ -112,12 +115,16 @@ abstract class SpecialRedirectToSpecial extends RedirectSpecialPage {
                $this->mAddedRedirectParams = $addedRedirectParams;
        }
 
+       /**
+        * @param string|null $subpage
+        * @return Title|bool
+        */
        public function getRedirect( $subpage ) {
                if ( $this->redirSubpage === false ) {
                        return SpecialPage::getTitleFor( $this->redirName, $subpage );
-               } else {
-                       return SpecialPage::getTitleFor( $this->redirName, $this->redirSubpage );
                }
+
+               return SpecialPage::getTitleFor( $this->redirName, $this->redirSubpage );
        }
 }
 

diff --git a/includes/specials/SpecialDiff.php b/includes/specials/SpecialDiff.php
index 799e526..9f91a10 100644 (file)
--- a/includes/specials/SpecialDiff.php
+++ b/includes/specials/SpecialDiff.php
@@ -37,12 +37,16 @@
  * @since 1.23
  */
 class SpecialDiff extends RedirectSpecialPage {
-       function __construct() {
+       public function __construct() {
                parent::__construct( 'Diff' );
                $this->mAllowedRedirectParams = array();
        }
 
-       function getRedirect( $subpage ) {
+       /**
+        * @param string|null $subpage
+        * @return Title|bool
+        */
+       public function getRedirect( $subpage ) {
                $parts = explode( '/', $subpage );
 
                // Try to parse the values given, generating somewhat pretty URLs if possible

diff --git a/includes/specials/SpecialFilepath.php b/includes/specials/SpecialFilepath.php
index 9323211..542589f 100644 (file)
--- a/includes/specials/SpecialFilepath.php
+++ b/includes/specials/SpecialFilepath.php
@@ -27,13 +27,18 @@
  * @ingroup SpecialPage
  */
 class SpecialFilepath extends RedirectSpecialPage {
-       function __construct() {
+       public function __construct() {
                parent::__construct( 'Filepath' );
                $this->mAllowedRedirectParams = array( 'width', 'height' );
        }
 
-       // implement by redirecting through Special:Redirect/file
-       function getRedirect( $par ) {
+       /**
+        * Implement by redirecting through Special:Redirect/file.
+        *
+        * @param string|null $subpage
+        * @return Title
+        */
+       public function getRedirect( $par ) {
                $file = $par ?: $this->getRequest()->getText( 'file' );
 
                if ( $file ) {

diff --git a/includes/specials/SpecialMyLanguage.php b/includes/specials/SpecialMyLanguage.php
index fce1cdc..3d8ff97 100644 (file)
--- a/includes/specials/SpecialMyLanguage.php
+++ b/includes/specials/SpecialMyLanguage.php
@@ -41,11 +41,11 @@ class SpecialMyLanguage extends RedirectSpecialArticle {
         * If the special page is a redirect, then get the Title object it redirects to.
         * False otherwise.
         *
-        * @param string $par Subpage string
-        * @return Title|bool
+        * @param string|null $subpage
+        * @return Title
         */
-       public function getRedirect( $par ) {
-               $title = $this->findTitle( $par );
+       public function getRedirect( $subpage ) {
+               $title = $this->findTitle( $subpage );
                // Go to the main page if given invalid title.
                if ( !$title ) {
                        $title = Title::newMainPage();
@@ -59,19 +59,22 @@ class SpecialMyLanguage extends RedirectSpecialArticle {
         * it returns Page/fi if it exists, otherwise Page/de if it exists,
         * otherwise Page.
         *
-        * @param string $par
+        * @param string|null $subpage
         * @return Title|null
         */
-       public function findTitle( $par ) {
-               $par = (string)$par;
+       public function findTitle( $subpage ) {
                // base = title without language code suffix
                // provided = the title as it was given
-               $base = $provided = Title::newFromText( $par );
+               $base = $provided = null;
+               if ( $subpage !== null ) {
+                       $provided = Title::newFromText( $subpage );
+                       $base = $provided;
+               }
 
-               if ( $base && strpos( $par, '/' ) !== false ) {
-                       $pos = strrpos( $par, '/' );
-                       $basepage = substr( $par, 0, $pos );
-                       $code = substr( $par, $pos + 1 );
+               if ( $provided && strpos( $subpage, '/' ) !== false ) {
+                       $pos = strrpos( $subpage, '/' );
+                       $basepage = substr( $subpage, 0, $pos );
+                       $code = substr( $subpage, $pos + 1 );
                        if ( strlen( $code ) && Language::isKnownLanguageTag( $code ) ) {
                                $base = Title::newFromText( $basepage );
                        }

diff --git a/includes/specials/SpecialMyRedirectPages.php b/includes/specials/SpecialMyRedirectPages.php
index 9b8d52b..c8db1d8 100644 (file)
--- a/includes/specials/SpecialMyRedirectPages.php
+++ b/includes/specials/SpecialMyRedirectPages.php
@@ -30,16 +30,20 @@
  * @ingroup SpecialPage
  */
 class SpecialMypage extends RedirectSpecialArticle {
-       function __construct() {
+       public function __construct() {
                parent::__construct( 'Mypage' );
        }
 
-       function getRedirect( $subpage ) {
-               if ( strval( $subpage ) !== '' ) {
-                       return Title::makeTitle( NS_USER, $this->getUser()->getName() . '/' . $subpage );
-               } else {
+       /**
+        * @param string|null $subpage
+        * @return Title
+        */
+       public function getRedirect( $subpage ) {
+               if ( $subpage === null || $subpage === '' ) {
                        return Title::makeTitle( NS_USER, $this->getUser()->getName() );
                }
+
+               return Title::makeTitle( NS_USER, $this->getUser()->getName() . '/' . $subpage );
        }
 }
 
@@ -49,16 +53,20 @@ class SpecialMypage extends RedirectSpecialArticle {
  * @ingroup SpecialPage
  */
 class SpecialMytalk extends RedirectSpecialArticle {
-       function __construct() {
+       public function __construct() {
                parent::__construct( 'Mytalk' );
        }
 
-       function getRedirect( $subpage ) {
-               if ( strval( $subpage ) !== '' ) {
-                       return Title::makeTitle( NS_USER_TALK, $this->getUser()->getName() . '/' . $subpage );
-               } else {
+       /**
+        * @param string|null $subpage
+        * @return Title
+        */
+       public function getRedirect( $subpage ) {
+               if ( $subpage === null || $subpage === '' ) {
                        return Title::makeTitle( NS_USER_TALK, $this->getUser()->getName() );
                }
+
+               return Title::makeTitle( NS_USER_TALK, $this->getUser()->getName() . '/' . $subpage );
        }
 }
 
@@ -68,13 +76,17 @@ class SpecialMytalk extends RedirectSpecialArticle {
  * @ingroup SpecialPage
  */
 class SpecialMycontributions extends RedirectSpecialPage {
-       function __construct() {
+       public function __construct() {
                parent::__construct( 'Mycontributions' );
                $this->mAllowedRedirectParams = array( 'limit', 'namespace', 'tagfilter',
                        'offset', 'dir', 'year', 'month', 'feed' );
        }
 
-       function getRedirect( $subpage ) {
+       /**
+        * @param string|null $subpage
+        * @return Title
+        */
+       public function getRedirect( $subpage ) {
                return SpecialPage::getTitleFor( 'Contributions', $this->getUser()->getName() );
        }
 }
@@ -85,12 +97,16 @@ class SpecialMycontributions extends RedirectSpecialPage {
  * @ingroup SpecialPage
  */
 class SpecialMyuploads extends RedirectSpecialPage {
-       function __construct() {
+       public function __construct() {
                parent::__construct( 'Myuploads' );
                $this->mAllowedRedirectParams = array( 'limit', 'ilshowall', 'ilsearch' );
        }
 
-       function getRedirect( $subpage ) {
+       /**
+        * @param string|null $subpage
+        * @return Title
+        */
+       public function getRedirect( $subpage ) {
                return SpecialPage::getTitleFor( 'Listfiles', $this->getUser()->getName() );
        }
 }
@@ -101,12 +117,16 @@ class SpecialMyuploads extends RedirectSpecialPage {
  * @ingroup SpecialPage
  */
 class SpecialAllMyUploads extends RedirectSpecialPage {
-       function __construct() {
+       public function __construct() {
                parent::__construct( 'AllMyUploads' );
                $this->mAllowedRedirectParams = array( 'limit', 'ilsearch' );
        }
 
-       function getRedirect( $subpage ) {
+       /**
+        * @param string|null $subpage
+        * @return Title
+        */
+       public function getRedirect( $subpage ) {
                $this->mAddedRedirectParams['ilshowall'] = 1;
 
                return SpecialPage::getTitleFor( 'Listfiles', $this->getUser()->getName() );

diff --git a/includes/specials/SpecialPermanentLink.php b/includes/specials/SpecialPermanentLink.php
index 17115e8..53789c0 100644 (file)
--- a/includes/specials/SpecialPermanentLink.php
+++ b/includes/specials/SpecialPermanentLink.php
@@ -27,12 +27,16 @@
  * @ingroup SpecialPage
  */
 class SpecialPermanentLink extends RedirectSpecialPage {
-       function __construct() {
+       public function __construct() {
                parent::__construct( 'PermanentLink' );
                $this->mAllowedRedirectParams = array();
        }
 
-       function getRedirect( $subpage ) {
+       /**
+        * @param string|null $subpage
+        * @return Title|bool
+        */
+       public function getRedirect( $subpage ) {
                $subpage = intval( $subpage );
                if ( $subpage === 0 ) {
                        # throw an error page when no subpage was given