Respond to some messages from Phan on PHP 8.1

* ForkController, OrderedStreamingForkController: indeed pcntl_fork()
  can't return false.
* RL\Image: Specify type instead of using suppression, since the issue
  name changes.
* VueComponentParser: Accept complaint about nullable nodeValue.
* Disable PHP 8.0 polyfill stubs when running on PHP 8.0+ to avoid
  duplicate interface errors.
* Add Socket stub and use it in LegacyHandler instead of multiple
  existing suppressions.
* MemcachedPeclBagOStuff: accept complaint recommending !$result over
  $result === false when the type is boolean.
* MemcachedPeclBagOStuff: fix probable bug, ignoring errors from
  Memcached::getMulti(). Phan noticed that $res=false was unreachable,
  but it should probably be reachable.
* DatabaseMysqli: accept complaint that $this->conn->errno is already
  known to be an int. It was probably a hack for some previous version
  of Phan.
* BcryptPassword, MWOldPassword, MWSaltedPassword: accept complaint that
  the !is_string() checks are unnecessary, after code review of PHP.
* Pbkdf2PasswordUsingHashExtension: note that contrary to Phan's
  suggestion, this check is necessary.
* DefaultPreferencesFactory: remove an existing hack for
  array_diff_key(), no longer necessary on 7.4 and causes an error on
  8.1. Use coalesce instead of cast for the remaining
  array_intersect_key() hack since it better shows that we are casting
  away null.
* FullSearchResultWidget: fix likely bug involving strict comparison
  between a float and an int.
* SpecialWatchlist: accept complaint that $selectedHours is
  unconditionally a float, being the return value of round(), and thus
  the cast is unnecessary.
* Add stub for AllowDynamicProperties, resolving an error in User.php.
* Xml: accept complaint that $encMonth is already known to be an int.

Six errors remain. These need suppressions or otherwise conflict with
PHP 7.4 support.

Bug: T322278
Change-Id: Ie375bbc8ccf22330b9a169e8da98f2bbe26ec8b9
This commit is contained in:
Tim Starling 2022-11-03 12:55:46 +11:00
parent 5e521ed278
commit 7b3e7c017a
18 changed files with 52 additions and 23 deletions

View file

@ -30,6 +30,8 @@ $cfg['file_list'] = array_merge(
class_exists( PEAR::class ) ? [] : [ '.phan/stubs/mail.php' ],
defined( 'PASSWORD_ARGON2ID' ) ? [] : [ '.phan/stubs/password.php' ],
class_exists( ValueError::class ) ? [] : [ '.phan/stubs/ValueError.php' ],
class_exists( Socket::class ) ? [] : [ '.phan/stubs/Socket.php' ],
class_exists( AllowDynamicProperties::class ) ? [] : [ '.phan/stubs/AllowDynamicProperties.php' ],
[
// This makes constants and globals known to Phan before processing all other files.
// You can check the parser order with --dump-parsed-file-list
@ -51,6 +53,20 @@ $cfg['exclude_file_list'] = array_merge(
]
);
if ( PHP_VERSION_ID >= 80000 ) {
// Exclude PHP 8.0 polyfills if PHP 8.0+ is running
$cfg['exclude_file_list'] = array_merge(
$cfg['exclude_file_list'],
[
'vendor/symfony/polyfill-php80/Resources/stubs/Attribute.php',
'vendor/symfony/polyfill-php80/Resources/stubs/PhpToken.php',
'vendor/symfony/polyfill-php80/Resources/stubs/Stringable.php',
'vendor/symfony/polyfill-php80/Resources/stubs/UnhandledMatchError.php',
'vendor/symfony/polyfill-php80/Resources/stubs/ValueError.php',
]
);
}
$cfg['analyzed_file_extensions'] = array_merge(
$cfg['analyzed_file_extensions'] ?? [ 'php' ],
[ 'inc' ]

View file

@ -0,0 +1,6 @@
<?php
// Stub for PHP 8.2's AllowDynamicProperties class
#[Attribute]
class AllowDynamicProperties {
}

5
.phan/stubs/Socket.php Normal file
View file

@ -0,0 +1,5 @@
<?php
// Stub for PHP 8.1's Socket class
final class Socket {
}

View file

@ -189,7 +189,7 @@ class ForkController {
for ( $i = 0; $i < $numProcs; $i++ ) {
// Do the fork
$pid = pcntl_fork();
if ( $pid === -1 || $pid === false ) {
if ( $pid === -1 ) {
echo "Error creating child processes\n";
exit( 1 );
}

View file

@ -94,7 +94,7 @@ class OrderedStreamingForkController extends ForkController {
$sockets = stream_socket_pair( STREAM_PF_UNIX, STREAM_SOCK_STREAM, STREAM_IPPROTO_IP );
// Do the fork
$pid = pcntl_fork();
if ( $pid === -1 || $pid === false ) {
if ( $pid === -1 ) {
echo "Error creating child processes\n";
exit( 1 );
}

View file

@ -357,7 +357,7 @@ class Image {
// Reattach all direct children of the `<svg>` root node to the `<g>` wrapper
while ( $root->firstChild ) {
$node = $root->firstChild;
// @phan-suppress-next-line PhanUndeclaredProperty False positive
'@phan-var \DOMElement $node'; /** @var \DOMElement $node */
if ( !$titleNode && $node->nodeType === XML_ELEMENT_NODE && $node->tagName === 'title' ) {
// Remember the first encountered `<title>` node
$titleNode = $node;

View file

@ -91,7 +91,7 @@ class VueComponentParser {
$template = $this->getTemplateHtml( $html, $options['minifyTemplate'] ?? false );
return [
'script' => trim( $nodes['script']->nodeValue ),
'script' => trim( $nodes['script']->nodeValue ?? '' ),
'template' => $template,
'style' => $styleData ? $styleData['style'] : null,
'styleLang' => $styleData ? $styleData['lang'] : null
@ -160,7 +160,7 @@ class VueComponentParser {
* @throws Exception If an invalid language is used, or if the 'scoped' attribute is set.
*/
private function getStyleAndLang( DOMElement $styleNode ): array {
$style = trim( $styleNode->nodeValue );
$style = trim( $styleNode->nodeValue ?? '' );
$styleLang = $styleNode->hasAttribute( 'lang' ) ?
$styleNode->getAttribute( 'lang' ) : 'css';
if ( $styleLang !== 'css' && $styleLang !== 'less' ) {

View file

@ -24,6 +24,7 @@ use LogicException;
use MediaWiki\Logger\LegacyLogger;
use Monolog\Handler\AbstractProcessingHandler;
use Monolog\Logger;
use Socket;
use UnexpectedValueException;
/**
@ -62,7 +63,7 @@ class LegacyHandler extends AbstractProcessingHandler {
/**
* Log sink
* @var resource|null
* @var Socket|resource|null
*/
protected $sink;
@ -142,7 +143,6 @@ class LegacyHandler extends AbstractProcessingHandler {
$domain = AF_INET;
}
// @phan-suppress-next-line PhanTypeMismatchProperty False positive caused by PHP 8.0 resource transition
$this->sink = socket_create( $domain, SOCK_DGRAM, SOL_UDP );
} else {
@ -216,7 +216,6 @@ class LegacyHandler extends AbstractProcessingHandler {
}
socket_sendto(
// @phan-suppress-next-line PhanTypeMismatchArgumentInternal False positive caused by PHP 8.0 transition
$this->sink,
$text,
strlen( $text ),
@ -233,9 +232,7 @@ class LegacyHandler extends AbstractProcessingHandler {
public function close(): void {
if ( $this->sink ) {
if ( $this->useUdp() ) {
// @phan-suppress-next-line PhanTypeMismatchArgumentInternal
socket_close( $this->sink );
} else {
fclose( $this->sink );
}

View file

@ -208,7 +208,7 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff {
$result = $this->client->set( $routeKey, $value, $this->fixExpiry( $exptime ) );
ScopedCallback::consume( $noReplyScope );
return ( $result === false && $this->client->getResultCode() === Memcached::RES_NOTSTORED )
return ( !$result && $this->client->getResultCode() === Memcached::RES_NOTSTORED )
// "Not stored" is always used as the mcrouter response with AllAsyncRoute
? true
: $this->checkResult( $key, $result );
@ -235,7 +235,7 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff {
$result = $this->client->delete( $routeKey );
ScopedCallback::consume( $noReplyScope );
return ( $result === false && $this->client->getResultCode() === Memcached::RES_NOTFOUND )
return ( !$result && $this->client->getResultCode() === Memcached::RES_NOTFOUND )
// "Not found" is counted as success in our interface
? true
: $this->checkResult( $key, $result );
@ -379,7 +379,7 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff {
// The PECL implementation uses multi-key "get"/"gets"; no need to pipeline.
// T257003: avoid Memcached::GET_EXTENDED; no tokens are needed and that requires "gets"
// https://github.com/libmemcached/libmemcached/blob/eda2becbec24363f56115fa5d16d38a2d1f54775/libmemcached/get.cc#L272
$resByRouteKey = $this->client->getMulti( $routeKeys ) ?: [];
$resByRouteKey = $this->client->getMulti( $routeKeys );
if ( is_array( $resByRouteKey ) ) {
$res = [];
@ -390,7 +390,8 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff {
$res = false;
}
return $this->checkResult( false, $res );
$res = $this->checkResult( false, $res );
return $res !== false ? $res : [];
}
protected function doSetMulti( array $data, $exptime = 0, $flags = 0 ) {

View file

@ -196,7 +196,7 @@ class DatabaseMysqli extends DatabaseMysqlBase {
*/
public function lastErrno() {
if ( $this->conn instanceof mysqli ) {
return (int)$this->conn->errno;
return $this->conn->errno;
} else {
return mysqli_connect_errno();
}

View file

@ -77,7 +77,7 @@ class BcryptPassword extends ParameterizedPassword {
$hash = crypt( $password,
sprintf( '$2y$%02d$%s', (int)$this->params['rounds'], $this->args[0] ) );
if ( !is_string( $hash ) || strlen( $hash ) <= 13 ) {
if ( strlen( $hash ) <= 13 ) {
throw new PasswordError( 'Error when hashing password.' );
}

View file

@ -49,7 +49,7 @@ class MWOldPassword extends ParameterizedPassword {
$this->hash = md5( $plaintext );
}
if ( !is_string( $this->hash ) || strlen( $this->hash ) < 32 ) {
if ( strlen( $this->hash ) < 32 ) {
throw new PasswordError( 'Error when hashing password.' );
}
}

View file

@ -45,7 +45,7 @@ class MWSaltedPassword extends ParameterizedPassword {
$this->hash = md5( $this->args[0] . '-' . md5( $plaintext ) );
if ( !is_string( $this->hash ) || strlen( $this->hash ) < 32 ) {
if ( strlen( $this->hash ) < 32 ) {
throw new PasswordError( 'Error when hashing password.' );
}
}

View file

@ -44,6 +44,8 @@ class Pbkdf2PasswordUsingHashExtension extends AbstractPbkdf2Password {
int $length
): string {
$hash = hash_pbkdf2( $digestAlgo, $password, $salt, $rounds, $length, true );
// Note: this check is unnecessary in PHP 8.0+, since the warning cases
// were all converted to exceptions
if ( !is_string( $hash ) ) {
throw new PasswordError( 'Error when hashing password.' );
}

View file

@ -1505,8 +1505,10 @@ class DefaultPreferencesFactory implements PreferencesFactory {
$thumbNamespaces
)
);
$defaultThumbNamespacesFormatted = (array)array_intersect_key( $thumbNamespacesFormatted, [ NS_FILE => 1 ] );
$extraThumbNamespacesFormatted = (array)array_diff_key( $thumbNamespacesFormatted, [ NS_FILE => 1 ] );
$defaultThumbNamespacesFormatted =
array_intersect_key( $thumbNamespacesFormatted, [ NS_FILE => 1 ] ) ?? [];
$extraThumbNamespacesFormatted =
array_diff_key( $thumbNamespacesFormatted, [ NS_FILE => 1 ] );
if ( $extraThumbNamespacesFormatted ) {
$defaultPreferences['search-thumbnail-extra-namespaces'] = [
'type' => 'toggle',

View file

@ -399,7 +399,7 @@ class FullSearchResultWidget implements SearchResultWidget {
// we'll only deal with width from now on since conventions for
// standard sizes have formed around width; height will simply
// follow according to aspect ratio
$rescaledWidth = round( $rescaleCoefficient * $thumbnail->getWidth() );
$rescaledWidth = (int)round( $rescaleCoefficient * $thumbnail->getWidth() );
// we'll also be looking at $wgThumbLimits to ensure that we pick
// from within the predefined list of sizes

View file

@ -824,7 +824,7 @@ class SpecialWatchlist extends ChangesListSpecialPage {
] ) );
asort( $hours );
$select = new XmlSelect( 'days', 'days', (float)( $selectedHours / 24 ) );
$select = new XmlSelect( 'days', 'days', $selectedHours / 24 );
foreach ( $hours as $value ) {
if ( $value < 24 ) {

View file

@ -183,7 +183,7 @@ class Xml {
$timestamp = MWTimestamp::getInstance();
$thisMonth = intval( $timestamp->format( 'n' ) );
$thisYear = intval( $timestamp->format( 'Y' ) );
if ( intval( $encMonth ) > $thisMonth ) {
if ( $encMonth > $thisMonth ) {
$thisYear--;
}
$encYear = $thisYear;