Use PHP 7 '<=>' operator in 'sort()' callbacks
authorBartosz Dziewoński <matma.rex@gmail.com>
Fri, 6 Oct 2017 20:39:13 +0000 (22:39 +0200)
committerJames D. Forrester <jforrester@wikimedia.org>
Thu, 31 May 2018 01:05:20 +0000 (18:05 -0700)
`$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

24 files changed:
includes/GlobalFunctions.php
includes/MagicWord.php
includes/OutputPage.php
includes/Title.php
includes/api/ApiQueryUserContribs.php
includes/auth/AuthManager.php
includes/changes/ChangesListFilterGroup.php
includes/collation/IcuCollation.php
includes/libs/Timing.php
includes/libs/XhprofData.php
includes/libs/virtualrest/VirtualRESTServiceClient.php
includes/page/ImagePage.php
includes/parser/Parser.php
includes/profiler/ProfilerSectionOnly.php
includes/profiler/ProfilerXhprof.php
includes/profiler/output/ProfilerOutputText.php
includes/session/SessionInfo.php
includes/specialpage/AuthManagerSpecialPage.php
includes/specialpage/ChangesListSpecialPage.php
includes/specials/SpecialVersion.php
maintenance/findDeprecated.php
maintenance/namespaceDupes.php
profileinfo.php
tests/phpunit/MediaWikiTestCase.php

index 6f49a14..a9cad25 100644 (file)
@@ -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 );
index 93c8a71..17a4a0f 100644 (file)
@@ -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
        }
 
        /**
index 564641a..d76ed77 100644 (file)
@@ -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 ) {
index fd7451c..78d9850 100644 (file)
@@ -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() );
        }
 
        /**
index 140ff6d..f1c4f94 100644 (file)
@@ -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;
index 9ed6d13..258e520 100644 (file)
@@ -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 = [];
index 3e2c464..79e8e33 100644 (file)
@@ -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 ) {
index d6ab0ff..3fb7d8b 100644 (file)
@@ -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 ) {
index 7b1a914..73afbb2 100644 (file)
@@ -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'];
                } );
        }
 
index 5af22ed..90e52f0 100644 (file)
@@ -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;
index e3b9376..96d7929 100644 (file)
@@ -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)
index b5ff805..ecd21d3 100644 (file)
@@ -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 );
        }
 
        /**
index dfd9602..0bc1a06 100644 (file)
@@ -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 ) {
index a7bc137..504859d 100644 (file)
@@ -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;
index ffa441e..9c96661 100644 (file)
@@ -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;
index 100304f..db3da3e 100644 (file)
@@ -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,
index 287da9d..577e03a 100644 (file)
@@ -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();
        }
 
 }
index b9745de..81e13f0 100644 (file)
@@ -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'] );
index 9e61ef7..1f1d46e 100644 (file)
@@ -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 ) {
index d4e5151..13b6bdf 100644 (file)
@@ -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'] );
        }
 
        /**
index ae2ee42..d4f9c2d 100644 (file)
@@ -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 );
index bbe4469..71241e7 100644 (file)
@@ -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;
index ca8c1bb..9ebd57b 100644 (file)
@@ -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();
        }
 }
 
index a27e9f9..2e6c011 100644 (file)
@@ -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 );
                        }
                );
        }