twilio / twilio-php

A PHP library for communicating with the Twilio REST API and generating TwiML.
MIT License
1.56k stars 559 forks source link

Function create_function() is deprecated in php 7.2.0 #452

Closed myultimabot closed 6 years ago

myultimabot commented 7 years ago

twilio/sdk/Services/Twilio/Resource.php

/**
 * Return camelized version of a word
 * Examples: sms_messages => SMSMessages, calls => Calls,
 * incoming_phone_numbers => IncomingPhoneNumbers
 *
 * @param string $word The word to camelize
 * @return string
 */
public static function camelize($word) {
    $callback = create_function('$matches', 'return strtoupper("$matches[2]");');

    return preg_replace_callback('/(^|_)([a-z])/',
        $callback,
        $word);
}
jmctwilio commented 7 years ago

@iamtheoleo Thank you for bringing this to our attention. The library is expected to be compatible with PHP version 7.1 and very likely works with 7.2. There are a few third-party dependencies in the library which also use create_function. We will keep an eye on the changes coming in PHP 7.3 and will prepare for this deprecation. I believe our oldest compatible version of PHP is 5.5 which should support proper closures, so it should be a fairly simple change.

https://wiki.php.net/rfc/deprecations_php_7_2#create_function

mnwalker commented 6 years ago

Would be great if this can be addressed soon rather than leaving it for php7.3 to get close.

Many PHP frameworks (eg. Laravel) encourage enforce good code by throwing a full exception for any kind of error even if it could be ignored and code continue. As such my app using Twilio breaks in php7.2 which is officially launching very soon.

I could suppress deprecated errors in config but that would defeat the purpose of getting them and would much prefer to run good compatible code.

Is this package controlled purely by the Twilio team or should I help with a pull request for a fix to this?

reedmaniac commented 6 years ago

Yes, we also ran into this problem with an app whose production instance is using PHP 7.2. Going to need to find a solution.

Gigamick commented 6 years ago

I just deployed my app to Heroku and it has bombed out due to this function. Rather irritating. Is there a fix?

Macuacua1 commented 6 years ago

I had the same error today on heroku, but 1 moth ago worked perfectly

florinfratica commented 6 years ago

I could suppress deprecated errors in config but that would defeat the purpose of getting them and would much prefer to run good compatible code.

How can you do so in Laravel?

mnwalker commented 6 years ago

How can you do so in Laravel? @florinfratica https://stackoverflow.com/questions/18497788/laravel-breaks-entire-app-on-php-notices

But php7.2 is released now and the biggest PHP framework crashes on site of this deprecation notice unless config is changed from default. So now is definitely a good time to work on making things future compatible, especially if as reported backwards compatibility is not an issue.

@jmctwilio do you accept pull requests on this repo if I was to prepare this update?

petermeester commented 6 years ago

You can fix this issue by changing two functions in Resource.php.

public static function camelize($word) {
        return preg_replace_callback('/(^|_)([a-z])/',
    function ($matches) {
            return ucfirst(strtolower($matches[2]));
        }, $word);
}
public static function decamelize($word) {
        return preg_replace_callback(
            '/(^|[a-z])([A-Z])/',
        function ($matches) {
                    return strtolower(strlen($matches[1]) ? $matches[1].$matches[2] : $matches[2]);
            }, $word
        );
}
codejudas commented 6 years ago

Hi all, apologies for not getting to this earlier, we've been slow during the holiday period. I'll schedule this for the team to take a look once we're all back in the office next week.

@mnwalker To answer your question this and the 5 other languages of helper libraries twilio offers are maintained by the api and developer education teams, while we do our best to stay on top of issues and bugs most of our time is spent keeping the libraries up to date with new features in the rest api, we always appreciate help in the form of PRs if you know how to fix the problem and want a quicker turn around time :)

codejudas commented 6 years ago

I looked in to this and realized the examples above are using our legacy library (ie 4.x and lower). Unfortunately we don't have the bandwidth on the team to update the older versions and so 5.x is the only currently maintained version.

I've checked and 5.x does not use create_function so it should be fully compatible with php 7.3. If you'd like to use twilio with php 7.3 in an officially supported manner then you'll have to update your library to the 5.x line, this will also keep you up to date with all the new resources, properties, and parameters we've added in the past year or so.

I understand it can be a large amount of work to port your existing code to use a new version with breaking changes, if its not worth it to you then I suggest using one of the workarounds mentioned above.

evictor commented 5 years ago

I created this branch with fixes on 4.12.1 for PHP 7.2: https://github.com/evictor/twilio-php/tree/4.12.1-create-function-fix

You can source this in your composer.json like this:

1. Add a repositories key to your composer.json

  "repositories": [
    {
      "type": "git",
      "url": "https://github.com/evictor/twilio-php"
    }
  ],

2. Specify dependency

"twilio/sdk": "dev-4.12.1-create-function-fix as 4.12.1",

Note "4.12.1-create-function-fix" is the branch name on my fork.

hopeseekr commented 5 years ago

Thanks to @evictor, I have also needed to port a client to PHP 7.2, so here is the same thing for v3.13.1 of the Twilio SDK:

1. Add the repo

  "repositories": [
    {
      "type": "git",
      "url": "https://github.com/hopeseekr/twilio-php"
    }
  ],

2. Specify dependency

"twilio/sdk": "dev-3.13.1-create-function-fix",
TomasVotruba commented 5 years ago

If you need to upgrade more than one create_function to anonymous function (official recommended upgrade), you can use a CLI tool for that.

It's tested on 30 various (and really weird :)) cases.

-$callback = create_function('$a', 'return "<cas:proxy>$a</cas:proxy>";');    
+$callback = function ($a) {
+    return "<cas:proxy>{$a}</cas:proxy>";
+};

Includes concat (.), string quotes and inclined function calls:

-$func = create_function('$atts, $content = null','return "<div class=\"' . $class_list . '\">" . do_shortcode($content) . "</div>";' );
+$func = function ($atts, $content = null) use ($class_list) {
+    return "<div class=\"{$class_list}\">" . do_shortcode($content) . "</div>";
+};

Do you want to automate the hard work?

1. Instal Rector

composer require rector/rector --dev

2. Create config

# rector.yml
services:
    Rector\Php\Rector\FuncCall\CreateFunctionToAnonymousFunctionRector: ~

3. Upgrade your Code

vendor/bin/rector process src --config rector.yml --dry-run
vendor/bin/rector process src --config rector.yml

Enjoy!

whitleyhorn commented 5 years ago

You can fix this issue by changing two functions in Resource.php.

public static function camelize($word) {
        return preg_replace_callback('/(^|_)([a-z])/',
  function ($matches) {
          return ucfirst(strtolower($matches[2]));
        }, $word);
}
public static function decamelize($word) {
        return preg_replace_callback(
            '/(^|[a-z])([A-Z])/',
      function ($matches) {
                  return strtolower(strlen($matches[1]) ? $matches[1].$matches[2] : $matches[2]);
          }, $word
        );
}

@petermeester why did you change camelize() to use strtolower()? the original callback was: $callback = create_function('$matches', 'return strtoupper("$matches[2]");');

whitleyhorn commented 5 years ago

Also the comment for camelize() is wrong in the source code, sms_messages becomes SmsMessages not SMSMessages