Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix Site::getPath() + MediaWikiSite::getFileUrl() confusion

Browse source 
Site::getPath() has returned null since change I08ffa6a970 (commit
a00337c3f8), almost ten years ago; I think on the whole it’s safer to
fix the prose documentation and keep the @return and actual returned
value as they are, rather than the other way around.

MediaWikiSite::getPageUrl() compared the $filePath to false instead of
null, so the comparison would always be true; but on closer inspection,
I think this code was supposed to check $path, not $filePath, in the
first place. The only reason this didn’t come up so far is that all
known callers pass in a custom path (specifically, all non-test callers
pass in 'api.php'), so the $path = false default was never used.

To avoid a deprecation warning in PHP 8.1 when passing a null $filePath
into str_replace(), throw in that case instead – the function isn’t
documented to return string|null, and none of the callers seem prepared
to handle a null return as far as I can tell, so throwing seems better
than returning null. Tests that produce a null $filePath need to be
fixed to complete the Site object (compare Wikibase change If20688411e).

Bug: T319219
Change-Id: I5ed7e169c9753486c765fd816d1b21016c5c1def
Depends-On: If20688411e3f0108b0c0ba3b91e5cc264b70a208
This commit is contained in:
Lucas Werkmeister 2022-11-02 13:33:42 +01:00
parent ff80fd3ee9
commit 779df10df7
2 changed files with 6 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
includes/site/MediaWikiSite.php
View file

@ -208,11 +208,15 @@ class MediaWikiSite extends Site {
* @param string|bool $path
*
* @return string
* @throws MWException If the file path cannot be determined.
*/
public function getFileUrl( $path = false ) {
$filePath = $this->getPath( self::PATH_FILE );
if ( $filePath === null ) {
throw new MWException( "PATH_FILE for site {$this->getGlobalId()} not known" );
}
if ( $filePath !== false ) {
if ( $path !== false ) {
$filePath = str_replace( '$1', $path, $filePath );
}

2
includes/site/Site.php
View file

@ -615,7 +615,7 @@ class Site implements Serializable {
}
/**
* Returns the path of the provided type or false if there is no such path.
* Returns the path of the provided type or null if there is no such path.
*
* @since 1.21
*