Krinkle has uploaded a new change for review.
https://gerrit.wikimedia.org/r/203533
Change subject: specials: Clean up redirect special pages ($subpage can be null)
......................................................................
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
---
M includes/specialpage/RedirectSpecialPage.php
M includes/specials/SpecialDiff.php
M includes/specials/SpecialFilepath.php
M includes/specials/SpecialMyLanguage.php
M includes/specials/SpecialMyRedirectPages.php
M includes/specials/SpecialPermanentLink.php
6 files changed, 74 insertions(+), 39 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/33/203533/1
diff --git a/includes/specialpage/RedirectSpecialPage.php
b/includes/specialpage/RedirectSpecialPage.php
index 2e6e55a..668488b 100644
--- a/includes/specialpage/RedirectSpecialPage.php
+++ b/includes/specialpage/RedirectSpecialPage.php
@@ -33,8 +33,11 @@
// 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 @@
* 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,6 +115,9 @@
$this->mAddedRedirectParams = $addedRedirectParams;
}
+ /**
+ * @param string|null $subpage
+ */
public function getRedirect( $subpage ) {
if ( $this->redirSubpage === false ) {
return SpecialPage::getTitleFor( $this->redirName,
$subpage );
diff --git a/includes/specials/SpecialDiff.php
b/includes/specials/SpecialDiff.php
index 799e526..5e5064c 100644
--- a/includes/specials/SpecialDiff.php
+++ b/includes/specials/SpecialDiff.php
@@ -37,12 +37,15 @@
* @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
+ */
+ 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
--- 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
--- a/includes/specials/SpecialMyLanguage.php
+++ b/includes/specials/SpecialMyLanguage.php
@@ -41,11 +41,11 @@
* 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 @@
* 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..cf1fb22 100644
--- a/includes/specials/SpecialMyRedirectPages.php
+++ b/includes/specials/SpecialMyRedirectPages.php
@@ -30,16 +30,19 @@
* @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
+ */
+ 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 +52,19 @@
* @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
+ */
+ 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 +74,16 @@
* @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
+ */
+ public function getRedirect( $subpage ) {
return SpecialPage::getTitleFor( 'Contributions',
$this->getUser()->getName() );
}
}
@@ -85,12 +94,15 @@
* @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
+ */
+ public function getRedirect( $subpage ) {
return SpecialPage::getTitleFor( 'Listfiles',
$this->getUser()->getName() );
}
}
@@ -101,12 +113,15 @@
* @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
+ */
+ 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..a02ac59 100644
--- a/includes/specials/SpecialPermanentLink.php
+++ b/includes/specials/SpecialPermanentLink.php
@@ -27,12 +27,15 @@
* @ingroup SpecialPage
*/
class SpecialPermanentLink extends RedirectSpecialPage {
- function __construct() {
+ public function __construct() {
parent::__construct( 'PermanentLink' );
$this->mAllowedRedirectParams = array();
}
- function getRedirect( $subpage ) {
+ /**
+ * @param string|null $subpage
+ */
+ public function getRedirect( $subpage ) {
$subpage = intval( $subpage );
if ( $subpage === 0 ) {
# throw an error page when no subpage was given
--
To view, visit https://gerrit.wikimedia.org/r/203533
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie3543f44011832b198bb3d3e32528b6a2868dee1
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Krinkle <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
