php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Request #64730 preg_replace_callback vs. preg_replace eval related
Submitted: 2013-04-28 17:35 UTC Modified: 2016-03-27 20:44 UTC
Votes:1
Avg. Score:3.0 ± 0.0
Reproduced:0 of 0 (0.0%)
From: imbolk at gmail dot com Assigned: laruence (profile)
Status: Closed Package: Regexps related
PHP Version: 5.5.0beta4 OS: Mac OS X 10.8.3
Private report: No CVE-ID: None
View Add Comment Developer Edit
Anyone can comment on a bug. Have a simpler test case? Does it work for you on a different platform? Let us know!
Just going to say 'Me too!'? Don't clutter the database with that please !
Your email address:
MUST BE VALID
Solve the problem:
25 + 11 = ?
Subscribe to this entry?

 
 [2013-04-28 17:35 UTC] imbolk at gmail dot com
Description:
------------
In PHP 5.5 'e' key preg_replace is deprecated: 
https://wiki.php.net/rfc/remove_preg_replace_eval_modifier

But I don't know how to replace evaled preg_replace with preg_replace_callback in 
some case.

For example:
$repl = [
            '/(\d{2}|(?<!\d))([pm])(\d{2}|)([PMc])/e' => '$this->_op("$3", "$4", 
rtrim($this->_op("$1", "$2"), ";"))',

            '/(\d{2}|)([MPmplrc])/e' => '$this->_op("$1", "$2")',
        ];

$str = preg_replace(array_keys($repl), array_values($repl), $str);

Test script:
---------------
$repl = [
            '/(\d{2}|(?<!\d))([pm])(\d{2}|)([PMc])/e' => function($m) { return $this->_op($m[3], $m[4], rtrim($this->_op($m[1], $m[2]), ";"))'; },

            '/(\d{2}|)([MPmplrc])/e' => function ($m) { return $this->_op($m[1], $m[2]); },
        ];

$str = preg_replace(array_keys($repl), array_values($repl), $str);

Expected result:
----------------
It works.

Actual result:
--------------
Warning: preg_replace_callback(): Requires argument 2, 'Array', to be a valid 
callback in my.php on line 359

Patches

second_arg_rege_key.patch (last revision 2013-05-04 12:35 UTC by laruence@php.net)
sencode_argument.patch (last revision 2013-04-29 16:30 UTC by laruence@php.net)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2013-04-29 15:52 UTC] laruence@php.net
or, add a third argument to callback, which is the "regex self" or the regex 
index?
 [2013-04-29 16:30 UTC] laruence@php.net
The following patch has been added/updated:

Patch Name: sencode_argument.patch
Revision:   1367253036
URL:        https://bugs.php.net/patch-display.php?bug=64730&patch=sencode_argument.patch&revision=1367253036
 [2013-04-29 16:31 UTC] imbolk at gmail dot com
Oops… sorry. My mistake. Test script is:

$repl = [
            '/(\d{2}|(?<!\d))([pm])(\d{2}|)([PMc])/e' => function($m) { return 
$this->_op($m[3], $m[4], rtrim($this->_op($m[1], $m[2]), ";"))'; },

            '/(\d{2}|)([MPmplrc])/e' => function ($m) { return $this->_op($m[1], 
$m[2]); },
        ];

$str = preg_replace_callback(array_keys($repl), array_values($repl), $str);
 [2013-04-29 16:49 UTC] laruence@php.net
a simple patch attached, please also see my proposal: 
http://news.php.net/php.internals/67199
 [2013-04-29 16:52 UTC] laruence@php.net
-Assigned To: +Assigned To: laruence
 [2013-04-29 18:03 UTC] imbolk at gmail dot com
I think it would be better if prey_replace_callback function will accept array of 
callbacks as a 2nd argument.
 [2013-04-30 21:09 UTC] ww dot galen at gmail dot com
Accepting an array of callbacks can lead to unreconcilable ambiguities. For example:

    class A {
        function __toString() { ... }
        function __invoke($a) { ... }
        function foo($a) { ... }
    }
    function foo($a) { ... }
    
    $a = new A;
    preg_replace_callback([..., ...], [$a, 'foo'], $subject);

There are three different ways of interpreting the callback argument, all equally valid:

1. `(string)$a` and `foo(...)`
2. `$a(...)` and `foo(...)`
3. `$a->foo(...)`
 [2013-05-01 02:08 UTC] imbolk at gmail dot com
Yes, you are quite right.
 [2013-05-04 12:35 UTC] laruence@php.net
The following patch has been added/updated:

Patch Name: second_arg_rege_key.patch
Revision:   1367670928
URL:        https://bugs.php.net/patch-display.php?bug=64730&patch=second_arg_rege_key.patch&revision=1367670928
 [2013-08-09 13:55 UTC] joel at umbrellasource dot com
I don't like the idea of passing the regex as a second callback argument. The main argument against it is having to test for the regex itself means you have to either globalize an array of regex or duplicate code to add a test inside the callback. Neither should be considered a good coding solution.

The idea behind preg_replace is that you have two arrays and the index of the matched regex is the index we need to use for the replace (i.e. we matched $search[2] so we're going to use $replace[2]). So the second argument of the callback should be the index of the regex in the array that was matched. Here's what I would like to see, expressed in code form

$str = preg_replace_callback(['/\d/', '/\s/'], function(Array $match, $index) { 
    $replace = ['digit', 'space'];
    return $replace[$index];
}, $str);

This is more intuitive, it's cleaner code-wise, it avoids any complicated solutions(no global array of callbacks or regex), it keeps the same basic functionality of preg_replace (array in, array out), and it's still simple enough where you could use an anonymous function inline (which the documentation currently recommends).
 [2016-03-27 20:44 UTC] nikic@php.net
-Status: Assigned +Status: Closed
 [2016-03-27 20:44 UTC] nikic@php.net
PHP 7 adds preg_replace_callback_array() to cover this use case: http://php.net/manual/en/function.preg-replace-callback-array.php
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Fri Mar 29 01:01:28 2024 UTC