Use a closure instead of PathRouterPatternReplacer
authorTim Starling <tstarling@wikimedia.org>
Fri, 6 Jul 2018 05:53:26 +0000 (15:53 +1000)
committerTim Starling <tstarling@wikimedia.org>
Wed, 11 Jul 2018 00:54:35 +0000 (10:54 +1000)
In PHP 5.3+ it's simpler to use closures rather than a replacer class to
pass parameters through to the preg_replace_callback() callback.

Remove without deprecation internal class PathRouterPatternReplacer,
unused in core and Gerrit-hosted extensions.

The new implementation is very similar, except that I renamed some
variables for clarity.

Also fixed an incorrect doc comment parameter type.

Change-Id: I4cd3c0162acdb02d51ab5b7f03b0a16e0a818d99

autoload.php
includes/PathRouter.php

index c535560..40b8acf 100644 (file)
@@ -1085,7 +1085,6 @@ $wgAutoloadLocalClasses = [
        'PasswordReset' => __DIR__ . '/includes/user/PasswordReset.php',
        'PatchSql' => __DIR__ . '/maintenance/patchSql.php',
        'PathRouter' => __DIR__ . '/includes/PathRouter.php',
-       'PathRouterPatternReplacer' => __DIR__ . '/includes/PathRouter.php',
        'PatrolLog' => __DIR__ . '/includes/logging/PatrolLog.php',
        'PatrolLogFormatter' => __DIR__ . '/includes/logging/PatrolLogFormatter.php',
        'Pbkdf2Password' => __DIR__ . '/includes/password/Pbkdf2Password.php',
index cc6fc4a..f24e298 100644 (file)
@@ -258,7 +258,7 @@ class PathRouter {
 
        /**
         * @param string $path
-        * @param string $pattern
+        * @param object $pattern
         * @return array|null
         */
        protected static function extractTitle( $path, $pattern ) {
@@ -319,13 +319,8 @@ class PathRouter {
                                        $value = $paramData['value'];
                                } elseif ( isset( $paramData['pattern'] ) ) {
                                        // For patterns we have to make value replacements on the string
-                                       $value = $paramData['pattern'];
-                                       $replacer = new PathRouterPatternReplacer;
-                                       $replacer->params = $m;
-                                       if ( isset( $pattern->key ) ) {
-                                               $replacer->key = $pattern->key;
-                                       }
-                                       $value = $replacer->replace( $value );
+                                       $value = self::expandParamValue( $m, $pattern->key ?? null,
+                                               $paramData['pattern'] );
                                        if ( $value === false ) {
                                                // Pattern required data that wasn't available, abort
                                                return null;
@@ -352,48 +347,43 @@ class PathRouter {
                return $matches;
        }
 
-}
-
-class PathRouterPatternReplacer {
-
-       public $key, $params, $error;
-
        /**
-        * Replace keys inside path router patterns with text.
-        * We do this inside of a replacement callback because after replacement we can't tell the
-        * difference between a $1 that was not replaced and a $1 that was part of
-        * the content a $1 was replaced with.
-        * @param string $value
+        * Replace $key etc. in param values with the matched strings from the path.
+        *
+        * @param array $pathMatches The match results from the path
+        * @param string|null $key The key of the matching pattern
+        * @param string $value The param value to be expanded
         * @return string|false
         */
-       public function replace( $value ) {
-               $this->error = false;
-               $value = preg_replace_callback( '/\$(\d+|key)/u', [ $this, 'callback' ], $value );
-               if ( $this->error ) {
-                       return false;
-               }
-               return $value;
-       }
+       protected static function expandParamValue( $pathMatches, $key, $value ) {
+               $error = false;
 
-       /**
-        * @param array $m
-        * @return string
-        */
-       protected function callback( $m ) {
-               if ( $m[1] == "key" ) {
-                       if ( is_null( $this->key ) ) {
-                               $this->error = true;
-                               return '';
-                       }
-                       return $this->key;
-               } else {
-                       $d = $m[1];
-                       if ( !isset( $this->params["par$d"] ) ) {
-                               $this->error = true;
-                               return '';
+               $replacer = function ( $m ) use ( $pathMatches, $key, &$error ) {
+                       if ( $m[1] == "key" ) {
+                               if ( is_null( $key ) ) {
+                                       $error = true;
+
+                                       return '';
+                               }
+
+                               return $key;
+                       } else {
+                               $d = $m[1];
+                               if ( !isset( $pathMatches["par$d"] ) ) {
+                                       $error = true;
+
+                                       return '';
+                               }
+
+                               return rawurldecode( $pathMatches["par$d"] );
                        }
-                       return rawurldecode( $this->params["par$d"] );
+               };
+
+               $value = preg_replace_callback( '/\$(\d+|key)/u', $replacer, $value );
+               if ( $error ) {
+                       return false;
                }
-       }
 
+               return $value;
+       }
 }