From f95dc8faff01792d1059246615a5597fff25e118 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Sat, 11 Apr 2015 13:57:55 +0100 Subject: [PATCH] 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 --- includes/specialpage/RedirectSpecialPage.php | 19 ++++--- includes/specials/SpecialDiff.php | 8 ++- includes/specials/SpecialFilepath.php | 11 +++-- includes/specials/SpecialMyLanguage.php | 27 +++++----- includes/specials/SpecialMyRedirectPages.php | 52 ++++++++++++++------ includes/specials/SpecialPermanentLink.php | 8 ++- 6 files changed, 84 insertions(+), 41 deletions(-) diff --git a/includes/specialpage/RedirectSpecialPage.php b/includes/specialpage/RedirectSpecialPage.php index 2e6e55a779..a866ba7749 100644 --- 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 799e526bfb..9f91a10c9c 100644 --- 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 93232117e1..542589f333 100644 --- 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 fce1cdcb00..3d8ff97bb2 100644 --- 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 9b8d52bb32..c8db1d87cd 100644 --- 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 17115e8863..53789c0da6 100644 --- 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 -- 2.20.1