Replace confusing count() on associative arrays with known keys

Calling count() on an array with string keys technically works. But
it's confusing and ultimately meaningless. An array might contain
2 elements, but we have no idea how they are named. The code
continues to access some very specific elements, but really just
assumes they are there. We are lucky this never crashed.

We have very powerful language features like isset() or ?? to
express much more precisely what this code is supposed to do.

Change-Id: I7a37371b577c1e44a3c8514316198cf0f4288250
This commit is contained in:
thiemowmde 2024-08-16 16:01:20 +02:00
parent 52d708f9b3
commit 7b5e9c7856

View file

@ -38,13 +38,9 @@ class TitleBlacklist {
/**
* Get an instance of this class
*
* @return TitleBlacklist
*/
public static function singleton() {
if ( self::$instance === null ) {
self::$instance = new self;
}
public static function singleton(): self {
self::$instance ??= new self();
return self::$instance;
}
@ -119,23 +115,31 @@ class TitleBlacklist {
/**
* Get the text of a blacklist from a specified source
*
* @param array $source A blacklist source from $wgTitleBlacklistSources
* @param array{type: string, src: ?string} $source A blacklist source from $wgTitleBlacklistSources
* @return string The content of the blacklist source as a string
*/
private static function getBlacklistText( $source ) {
if ( !is_array( $source ) || count( $source ) <= 0 ) {
if ( !is_array( $source ) || !isset( $source['type'] ) ) {
// Return empty string in error case
return '';
}
if ( $source['type'] == 'message' ) {
if ( $source['type'] === 'message' ) {
return wfMessage( 'titleblacklist' )->inContentLanguage()->text();
} elseif ( $source['type'] == 'localpage' && count( $source ) >= 2 ) {
$title = Title::newFromText( $source['src'] );
if ( $title === null ) {
}
$src = $source['src'] ?? null;
// All following types require the "src" element in the array
if ( !$src ) {
return '';
}
if ( $title->getNamespace() == NS_MEDIAWIKI ) {
if ( $source['type'] === 'localpage' ) {
$title = Title::newFromText( $src );
if ( !$title ) {
return '';
}
if ( $title->inNamespace( NS_MEDIAWIKI ) ) {
$msg = wfMessage( $title->getText() )->inContentLanguage();
return $msg->isDisabled() ? '' : $msg->text();
} else {
@ -145,13 +149,10 @@ class TitleBlacklist {
return ( $content instanceof TextContent ) ? $content->getText() : "";
}
}
} elseif ( $source['type'] == 'url' && count( $source ) >= 2 ) {
return self::getHttp( $source['src'] );
} elseif ( $source['type'] == 'file' && count( $source ) >= 2 ) {
if ( !file_exists( $source['src'] ) ) {
return '';
}
return file_get_contents( $source['src'] );
} elseif ( $source['type'] === 'url' ) {
return self::getHttp( $src );
} elseif ( $source['type'] === 'file' ) {
return file_exists( $src ) ? file_get_contents( $src ) : '';
}
return '';