From: Bartosz Dziewoński Date: Fri, 6 Oct 2017 20:39:13 +0000 (+0200) Subject: Use PHP 7 '<=>' operator in 'sort()' callbacks X-Git-Tag: 1.34.0-rc.0~5228^2 X-Git-Url: http://git.cyclocoop.org/data/modifier.php?a=commitdiff_plain;h=b191e5e860f24e1dd05e3d3d782364e4ea75b176;p=lhc%2Fweb%2Fwiklou.git Use PHP 7 '<=>' operator in 'sort()' callbacks `$a <=> $b` returns `-1` if `$a` is lesser, `1` if `$b` is lesser, and `0` if they are equal, which are exactly the values 'sort()' callbacks are supposed to return. It also enables the neat idiom `$a[x] <=> $b[x] ?: $a[y] <=> $b[y]` to sort arrays of objects first by 'x', and by 'y' if they are equal. * Replace a common pattern like `return $a < $b ? -1 : 1` with the new operator (and similar patterns with the variables, the numbers or the comparison inverted). Some of the uses were previously not correctly handling the variables being equal; this is now automatically fixed. * Also replace `return $a - $b`, which is equivalent to `return $a <=> $b` if both variables are integers but less intuitive. * (Do not replace `return strcmp( $a, $b )`. It is also equivalent when both variables are strings, but if any of the variables is not, 'strcmp()' converts it to a string before comparison, which could give different results than '<=>', so changing this would require careful review and isn't worth it.) * Also replace `return $a > $b`, which presumably sort of works most of the time (returns `1` if `$b` is lesser, and `0` if they are equal or `$a` is lesser) but is erroneous. Change-Id: I19a3d2fc8fcdb208c10330bd7a42c4e05d7f5cf3 --- diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php index 6f49a141e1..a9cad25fee 100644 --- a/includes/GlobalFunctions.php +++ b/includes/GlobalFunctions.php @@ -121,7 +121,7 @@ function wfArrayDiff2_cmp( $a, $b ) { if ( is_string( $a ) && is_string( $b ) ) { return strcmp( $a, $b ); } elseif ( count( $a ) !== count( $b ) ) { - return count( $a ) < count( $b ) ? -1 : 1; + return count( $a ) <=> count( $b ); } else { reset( $a ); reset( $b ); diff --git a/includes/MagicWord.php b/includes/MagicWord.php index 93c8a71c9e..17a4a0fff6 100644 --- a/includes/MagicWord.php +++ b/includes/MagicWord.php @@ -395,13 +395,7 @@ class MagicWord { public function compareStringLength( $s1, $s2 ) { $l1 = strlen( $s1 ); $l2 = strlen( $s2 ); - if ( $l1 < $l2 ) { - return 1; - } elseif ( $l1 > $l2 ) { - return -1; - } else { - return 0; - } + return $l2 <=> $l1; // descending } /** diff --git a/includes/OutputPage.php b/includes/OutputPage.php index 564641aa38..d76ed771df 100644 --- a/includes/OutputPage.php +++ b/includes/OutputPage.php @@ -3968,12 +3968,8 @@ class OutputPage extends ContextSource { uksort( $logosPerDppx, function ( $a , $b ) { $a = floatval( $a ); $b = floatval( $b ); - - if ( $a == $b ) { - return 0; - } // Sort from smallest to largest (e.g. 1x, 1.5x, 2x) - return ( $a < $b ) ? -1 : 1; + return $a <=> $b; } ); foreach ( $logosPerDppx as $dppx => $src ) { diff --git a/includes/Title.php b/includes/Title.php index fd7451cbc3..78d9850f07 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -784,11 +784,8 @@ class Title implements LinkTarget { * @return int Result of string comparison, or namespace comparison */ public static function compare( LinkTarget $a, LinkTarget $b ) { - if ( $a->getNamespace() == $b->getNamespace() ) { - return strcmp( $a->getText(), $b->getText() ); - } else { - return $a->getNamespace() - $b->getNamespace(); - } + return $a->getNamespace() <=> $b->getNamespace() + ?: strcmp( $a->getText(), $b->getText() ); } /** diff --git a/includes/api/ApiQueryUserContribs.php b/includes/api/ApiQueryUserContribs.php index 140ff6d8c2..f1c4f9407a 100644 --- a/includes/api/ApiQueryUserContribs.php +++ b/includes/api/ApiQueryUserContribs.php @@ -367,11 +367,11 @@ class ApiQueryUserContribs extends ApiQueryBase { if ( $batchSize === 1 ) { // One user, can't be different $ret = 0; } elseif ( $this->orderBy === 'id' ) { - $ret = $a[0]->rev_user - $b[0]->rev_user; + $ret = $a[0]->rev_user <=> $b[0]->rev_user; } elseif ( $this->orderBy === 'name' ) { $ret = strcmp( $a[0]->rev_user_text, $b[0]->rev_user_text ); } else { - $ret = $a[0]->rev_actor - $b[0]->rev_actor; + $ret = $a[0]->rev_actor <=> $b[0]->rev_actor; } if ( !$ret ) { @@ -382,7 +382,7 @@ class ApiQueryUserContribs extends ApiQueryBase { } if ( !$ret ) { - $ret = $a[0]->rev_id - $b[0]->rev_id; + $ret = $a[0]->rev_id <=> $b[0]->rev_id; } return $neg * $ret; diff --git a/includes/auth/AuthManager.php b/includes/auth/AuthManager.php index 9ed6d13a26..258e5203f1 100644 --- a/includes/auth/AuthManager.php +++ b/includes/auth/AuthManager.php @@ -2286,9 +2286,10 @@ class AuthManager implements LoggerAwareInterface { $spec = [ 'sort2' => $i++ ] + $spec + [ 'sort' => 0 ]; } unset( $spec ); + // Sort according to the 'sort' field, and if they are equal, according to 'sort2' usort( $specs, function ( $a, $b ) { - return ( (int)$a['sort'] ) - ( (int)$b['sort'] ) - ?: $a['sort2'] - $b['sort2']; + return $a['sort'] <=> $b['sort'] + ?: $a['sort2'] <=> $b['sort2']; } ); $ret = []; diff --git a/includes/changes/ChangesListFilterGroup.php b/includes/changes/ChangesListFilterGroup.php index 3e2c464aaf..79e8e33e8a 100644 --- a/includes/changes/ChangesListFilterGroup.php +++ b/includes/changes/ChangesListFilterGroup.php @@ -358,7 +358,7 @@ abstract class ChangesListFilterGroup { } usort( $this->filters, function ( $a, $b ) { - return $b->getPriority() - $a->getPriority(); + return $b->getPriority() <=> $a->getPriority(); } ); foreach ( $this->filters as $filterName => $filter ) { diff --git a/includes/collation/IcuCollation.php b/includes/collation/IcuCollation.php index d6ab0ff201..3fb7d8b579 100644 --- a/includes/collation/IcuCollation.php +++ b/includes/collation/IcuCollation.php @@ -390,8 +390,7 @@ class IcuCollation extends Collation { wfDebug( "Primary collision '$letter' '{$letterMap[$key]}' (comparison: $comp)\n" ); // If that also has a collision, use codepoint as a tiebreaker. if ( $comp === 0 ) { - // TODO Use <=> operator when PHP 7 is allowed. - $comp = UtfNormal\Utils::utf8ToCodepoint( $letter ) - + $comp = UtfNormal\Utils::utf8ToCodepoint( $letter ) <=> UtfNormal\Utils::utf8ToCodepoint( $letterMap[$key] ); } if ( $comp < 0 ) { diff --git a/includes/libs/Timing.php b/includes/libs/Timing.php index 7b1a9140df..73afbb2d96 100644 --- a/includes/libs/Timing.php +++ b/includes/libs/Timing.php @@ -155,7 +155,7 @@ class Timing implements LoggerAwareInterface { */ private function sortEntries() { uasort( $this->entries, function ( $a, $b ) { - return 10000 * ( $a['startTime'] - $b['startTime'] ); + return $a['startTime'] <=> $b['startTime']; } ); } diff --git a/includes/libs/XhprofData.php b/includes/libs/XhprofData.php index 5af22ed5b2..90e52f08dd 100644 --- a/includes/libs/XhprofData.php +++ b/includes/libs/XhprofData.php @@ -370,11 +370,10 @@ class XhprofData { return function ( $a, $b ) use ( $key, $sub ) { if ( isset( $a[$key] ) && isset( $b[$key] ) ) { // Descending sort: larger values will be first in result. - // Assumes all values are numeric. // Values for 'main()' will not have sub keys $valA = is_array( $a[$key] ) ? $a[$key][$sub] : $a[$key]; $valB = is_array( $b[$key] ) ? $b[$key][$sub] : $b[$key]; - return $valB - $valA; + return $valB <=> $valA; } else { // Sort datum with the key before those without return isset( $a[$key] ) ? -1 : 1; diff --git a/includes/libs/virtualrest/VirtualRESTServiceClient.php b/includes/libs/virtualrest/VirtualRESTServiceClient.php index e3b9376f21..96d7929169 100644 --- a/includes/libs/virtualrest/VirtualRESTServiceClient.php +++ b/includes/libs/virtualrest/VirtualRESTServiceClient.php @@ -105,10 +105,7 @@ class VirtualRESTServiceClient { $cmpFunc = function ( $a, $b ) { $al = substr_count( $a, '/' ); $bl = substr_count( $b, '/' ); - if ( $al === $bl ) { - return 0; // should not actually happen - } - return ( $al < $bl ) ? 1 : -1; // largest prefix first + return $bl <=> $al; // largest prefix first }; $matches = []; // matching prefixes (mount points) diff --git a/includes/page/ImagePage.php b/includes/page/ImagePage.php index b5ff80594e..ecd21d3c8e 100644 --- a/includes/page/ImagePage.php +++ b/includes/page/ImagePage.php @@ -1015,11 +1015,8 @@ EOT * @return int Result of string comparison, or namespace comparison */ protected function compare( $a, $b ) { - if ( $a->page_namespace == $b->page_namespace ) { - return strcmp( $a->page_title, $b->page_title ); - } else { - return $a->page_namespace - $b->page_namespace; - } + return $a->page_namespace <=> $b->page_namespace + ?: strcmp( $a->page_title, $b->page_title ); } /** diff --git a/includes/parser/Parser.php b/includes/parser/Parser.php index dfd9602de3..0bc1a068c2 100644 --- a/includes/parser/Parser.php +++ b/includes/parser/Parser.php @@ -593,7 +593,7 @@ class Parser { // Add on template profiling data in human/machine readable way $dataByFunc = $this->mProfiler->getFunctionStats(); uasort( $dataByFunc, function ( $a, $b ) { - return $a['real'] < $b['real']; // descending order + return $b['real'] <=> $a['real']; // descending order } ); $profileReport = []; foreach ( array_slice( $dataByFunc, 0, 10 ) as $item ) { diff --git a/includes/profiler/ProfilerSectionOnly.php b/includes/profiler/ProfilerSectionOnly.php index a7bc1375e9..504859d225 100644 --- a/includes/profiler/ProfilerSectionOnly.php +++ b/includes/profiler/ProfilerSectionOnly.php @@ -73,10 +73,7 @@ class ProfilerSectionOnly extends Profiler { protected function getFunctionReport() { $data = $this->getFunctionStats(); usort( $data, function ( $a, $b ) { - if ( $a['real'] === $b['real'] ) { - return 0; - } - return ( $a['real'] > $b['real'] ) ? -1 : 1; // descending + return $b['real'] <=> $a['real']; // descending } ); $width = 140; diff --git a/includes/profiler/ProfilerXhprof.php b/includes/profiler/ProfilerXhprof.php index ffa441ed87..9c9666168f 100644 --- a/includes/profiler/ProfilerXhprof.php +++ b/includes/profiler/ProfilerXhprof.php @@ -201,10 +201,7 @@ class ProfilerXhprof extends Profiler { protected function getFunctionReport() { $data = $this->getFunctionStats(); usort( $data, function ( $a, $b ) { - if ( $a['real'] === $b['real'] ) { - return 0; - } - return ( $a['real'] > $b['real'] ) ? -1 : 1; // descending + return $b['real'] <=> $a['real']; // descending } ); $width = 140; diff --git a/includes/profiler/output/ProfilerOutputText.php b/includes/profiler/output/ProfilerOutputText.php index 100304ffb2..db3da3e9eb 100644 --- a/includes/profiler/output/ProfilerOutputText.php +++ b/includes/profiler/output/ProfilerOutputText.php @@ -48,7 +48,7 @@ class ProfilerOutputText extends ProfilerOutput { } ); // Sort descending by time elapsed usort( $stats, function ( $a, $b ) { - return $a['real'] < $b['real']; + return $b['real'] <=> $a['real']; } ); array_walk( $stats, diff --git a/includes/session/SessionInfo.php b/includes/session/SessionInfo.php index 287da9dde3..577e03a264 100644 --- a/includes/session/SessionInfo.php +++ b/includes/session/SessionInfo.php @@ -282,7 +282,7 @@ class SessionInfo { * @return int Negative if $a < $b, positive if $a > $b, zero if equal */ public static function compare( $a, $b ) { - return $a->getPriority() - $b->getPriority(); + return $a->getPriority() <=> $b->getPriority(); } } diff --git a/includes/specialpage/AuthManagerSpecialPage.php b/includes/specialpage/AuthManagerSpecialPage.php index b9745de1c6..81e13f00f0 100644 --- a/includes/specialpage/AuthManagerSpecialPage.php +++ b/includes/specialpage/AuthManagerSpecialPage.php @@ -718,8 +718,8 @@ abstract class AuthManagerSpecialPage extends SpecialPage { $field['__index'] = $i++; } uasort( $formDescriptor, function ( $first, $second ) { - return self::getField( $first, 'weight', 0 ) - self::getField( $second, 'weight', 0 ) - ?: $first['__index'] - $second['__index']; + return self::getField( $first, 'weight', 0 ) <=> self::getField( $second, 'weight', 0 ) + ?: $first['__index'] <=> $second['__index']; } ); foreach ( $formDescriptor as &$field ) { unset( $field['__index'] ); diff --git a/includes/specialpage/ChangesListSpecialPage.php b/includes/specialpage/ChangesListSpecialPage.php index 9e61ef738b..1f1d46e9a6 100644 --- a/includes/specialpage/ChangesListSpecialPage.php +++ b/includes/specialpage/ChangesListSpecialPage.php @@ -1220,7 +1220,7 @@ abstract class ChangesListSpecialPage extends SpecialPage { ]; usort( $this->filterGroups, function ( $a, $b ) { - return $b->getPriority() - $a->getPriority(); + return $b->getPriority() <=> $a->getPriority(); } ); foreach ( $this->filterGroups as $groupName => $group ) { diff --git a/includes/specials/SpecialVersion.php b/includes/specials/SpecialVersion.php index d4e5151264..13b6bdf9e7 100644 --- a/includes/specials/SpecialVersion.php +++ b/includes/specials/SpecialVersion.php @@ -656,13 +656,7 @@ class SpecialVersion extends SpecialPage { * @return int */ public function compare( $a, $b ) { - if ( $a['name'] === $b['name'] ) { - return 0; - } else { - return $this->getLanguage()->lc( $a['name'] ) > $this->getLanguage()->lc( $b['name'] ) - ? 1 - : -1; - } + return $this->getLanguage()->lc( $a['name'] ) <=> $this->getLanguage()->lc( $b['name'] ); } /** diff --git a/maintenance/findDeprecated.php b/maintenance/findDeprecated.php index ae2ee421a8..d4f9c2d0c7 100644 --- a/maintenance/findDeprecated.php +++ b/maintenance/findDeprecated.php @@ -59,7 +59,7 @@ class DeprecatedInterfaceFinder extends FileAwareNodeVisitor { // Sort results by version, then by filename, then by name. foreach ( $this->foundNodes as $version => &$nodes ) { uasort( $nodes, function ( $a, $b ) { - return ( $a['filename'] . $a['name'] ) < ( $b['filename'] . $b['name'] ) ? -1 : 1; + return ( $a['filename'] . $a['name'] ) <=> ( $b['filename'] . $b['name'] ); } ); } ksort( $this->foundNodes ); diff --git a/maintenance/namespaceDupes.php b/maintenance/namespaceDupes.php index bbe4469ae6..71241e7033 100644 --- a/maintenance/namespaceDupes.php +++ b/maintenance/namespaceDupes.php @@ -161,17 +161,8 @@ class NamespaceDupes extends Maintenance { // break the tie by sorting by name $origSpaces = $spaces; uksort( $spaces, function ( $a, $b ) use ( $origSpaces ) { - if ( $origSpaces[$a] < $origSpaces[$b] ) { - return -1; - } elseif ( $origSpaces[$a] > $origSpaces[$b] ) { - return 1; - } elseif ( $a < $b ) { - return -1; - } elseif ( $a > $b ) { - return 1; - } else { - return 0; - } + return $origSpaces[$a] <=> $origSpaces[$b] + ?: $a <=> $b; } ); $ok = true; diff --git a/profileinfo.php b/profileinfo.php index ca8c1bbbfa..9ebd57b7a2 100644 --- a/profileinfo.php +++ b/profileinfo.php @@ -291,24 +291,26 @@ function compare_point( profile_point $a, profile_point $b ) { global $sort; switch ( $sort ) { + // Sorted ascending: case 'name': return strcmp( $a->name(), $b->name() ); + // Sorted descending: case 'time': - return $a->time() > $b->time() ? -1 : 1; + return $b->time() <=> $a->time(); case 'memory': - return $a->memory() > $b->memory() ? -1 : 1; + return $b->memory() <=> $a->memory(); case 'count': - return $a->count() > $b->count() ? -1 : 1; + return $b->count() <=> $a->count(); case 'time_per_call': - return $a->timePerCall() > $b->timePerCall() ? -1 : 1; + return $b->timePerCall() <=> $a->timePerCall(); case 'memory_per_call': - return $a->memoryPerCall() > $b->memoryPerCall() ? -1 : 1; + return $b->memoryPerCall() <=> $a->memoryPerCall(); case 'calls_per_req': - return $a->callsPerRequest() > $b->callsPerRequest() ? -1 : 1; + return $b->callsPerRequest() <=> $a->callsPerRequest(); case 'time_per_req': - return $a->timePerRequest() > $b->timePerRequest() ? -1 : 1; + return $b->timePerRequest() <=> $a->timePerRequest(); case 'memory_per_req': - return $a->memoryPerRequest() > $b->memoryPerRequest() ? -1 : 1; + return $b->memoryPerRequest() <=> $a->memoryPerRequest(); } } diff --git a/tests/phpunit/MediaWikiTestCase.php b/tests/phpunit/MediaWikiTestCase.php index a27e9f9cda..2e6c011584 100644 --- a/tests/phpunit/MediaWikiTestCase.php +++ b/tests/phpunit/MediaWikiTestCase.php @@ -1821,7 +1821,7 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase { uasort( $array, function ( $a, $b ) { - return serialize( $a ) > serialize( $b ) ? 1 : -1; + return serialize( $a ) <=> serialize( $b ); } ); }