yiisoft / yii

Yii PHP Framework 1.1.x
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
4.84k stars 2.28k forks source link

PHP 8.3 Compatibility #4552

Closed sandippingle closed 2 months ago

sandippingle commented 7 months ago

What steps will reproduce the problem?

PHP Deprecated the following features/functions in their release of 8.3:

https://www.php.net/manual/en/migration83.deprecated.php

Saner Increment/Decrement operators

Reflection

Calling ReflectionProperty::setValue() with only one parameter is deprecated. To set static properties, pass null as the first parameter.

Possibly impacted code:

public function setLocaleDataPath($value)
    {
        $property=new ReflectionProperty($this->localeClass,'dataPath');
        $property->setValue($value);
    }

SQLite3

Using exceptions is now preferred, warnings will be removed in the future. Calling SQLite3::enableExceptions(false) will trigger a deprecation warning in this version.

How can we plan PHP 8.3 compatibility?

Additional info

Q A
Yii version 1.1.29
PHP version 8.3
Operating system Linux
marcovtwout commented 7 months ago

Regarding the reflection deprecation, that point seems valid. Those lines could probably be replaced with:

$class=new ReflectionClass($this->localeClass);
$class->setStaticPropertyValue('dataPath',$value);

Regarding SQLite3 enableExceptions(), I don't see that being used anywhere in framework code?

TheGithubDev commented 5 months ago

PHP 8.3 introduces several deprecations that impact the current codebase. Specifically:

Proposed Changes

  1. Saner Increment/Decrement Operators

    • Action: Conduct a thorough review of the codebase to identify any usages of increment/decrement operators that rely on deprecated behaviors. Update the code to align with PHP 8.3's expectations.
    • Impact: Ensure no deprecated warnings are triggered by these operators.
  2. ReflectionProperty::setValue() Method

    • Action: Modify the setLocaleDataPath method to comply with the new requirement of passing null as the first parameter when setting static properties.
    • Implementation:
      public function setLocaleDataPath($value)
      {
       $property = new ReflectionProperty($this->localeClass, 'dataPath');
       if ($property->isStatic()) {
           $property->setValue(null, $value);
       } else {
           $property->setValue($this, $value);
       }
      }
    • Impact: Ensure the method works correctly with both static and non-static properties under PHP 8.3.
  3. SQLite3 Exception Handling

    • Action: Refactor the SQLite3 usage to utilize exceptions properly, removing any calls to SQLite3::enableExceptions(false).
    • Implementation:
      try {
       $db = new SQLite3('database.db');
       $db->enableExceptions(true);
      } catch (Exception $e) {
       echo 'Caught exception: ', $e->getMessage(), "\n";
      }
    • Impact: Properly handle exceptions and avoid deprecated warnings related to SQLite3.

Implementation Plan

  1. Code Audit and Refactoring

    • Perform a comprehensive audit of the codebase to identify any usage of deprecated features/functions.
    • Refactor the identified areas according to the proposed changes.
  2. Testing

    • Set up a testing environment with PHP 8.3.
    • Run unit and integration tests to ensure all functionality works as expected.
    • Address any issues that arise during testing.
  3. Update Dependencies

    • Check all dependencies for PHP 8.3 compatibility.
    • Update any third-party libraries and frameworks to their latest versions that support PHP 8.3.
  4. Documentation

    • Update project documentation to reflect changes made for PHP 8.3 compatibility.
    • Include information on any new exception handling logic and refactored methods.

      4554

Hope this helps out!

Regards.

marcovtwout commented 5 months ago

@TheGithubDev was the post above generated with AI? Is SQLite3 enableExceptions() used anywhere in framework code?

christophwariszlovich commented 3 months ago

Hi. When will PHP 8.3 support be released for Yii 1.1? Thank you.

marcovtwout commented 3 months ago

@christophwariszlovich You can use https://github.com/yiisoft/yii/pull/4553 for now and please report any new issues you can find :)

christophwariszlovich commented 3 months ago

@christophwariszlovich You can use #4553 for now and please report any new issues you can find :)

Issue that I found:

$criteria = new CDbCriteria();
$criteria->limit = 10;
$bookings = Booking::model()->findAll($criteria);

Code causes this error:

PHP Error[2]: foreach() argument must be of type array|object, null given
    in file /var/www/html/appname/vendor/yiisoft/yii/framework/db/schema/CDbCriteria.php at line 159
#0 /var/www/html/appname/vendor/yiisoft/yii/framework/base/CApplication.php(832): CErrorHandler->handle()
#1 /var/www/html/appname/vendor/yiisoft/yii/framework/db/schema/CDbCriteria.php(159): CConsoleApplication->handleError()
#2 /var/www/html/appname/vendor/yiisoft/yii/framework/db/ar/CActiveRecord.php(330): CDbCriteria->__construct()
#3 /var/www/html/appname/vendor/yiisoft/yii/framework/db/ar/CActiveRecord.php(1410): Booking->getDbCriteria()
#4 /var/www/html/appname/vendor/yiisoft/yii/framework/db/ar/CActiveRecord.php(1352): Booking->applyScopes()
#5 /var/www/html/appname/vendor/yiisoft/yii/framework/db/ar/CActiveRecord.php(1479): Booking->query()
#6 /var/www/html/appname/api/protected/commands/ToolsCommand.php(4305): Booking->findAll()
#7 unknown(0): ToolsCommand->actionTest()
#8 /var/www/html/appname/vendor/yiisoft/yii/framework/console/CConsoleCommand.php(177): ReflectionMethod->invokeArgs()
#9 /var/www/html/appname/vendor/yiisoft/yii/framework/console/CConsoleCommandRunner.php(71): ToolsCommand->run()
#10 /var/www/html/appname/vendor/yiisoft/yii/framework/console/CConsoleApplication.php(92): CConsoleCommandRunner->run()
#11 /var/www/html/appname/vendor/yiisoft/yii/framework/base/CApplication.php(185): CConsoleApplication->processRequest()
#12 /var/www/html/appname/api/protected/yiic.php(23): CConsoleApplication->run()
#13 /var/www/html/appname/api/protected/yiic(4): require_once()
marcovtwout commented 3 months ago

@christophwariszlovich That doesn't look like a PHP 8.3 error at first sight, looks like an application-specific error with your Booking class default scopes or dbcriteria. You probably get the same error message on earlier PHP versions.

christophwariszlovich commented 3 months ago

@marcovtwout We have the app running on PHP 5.6 and PHP 8.0.18 running where I dont get the error, we were looking to upgrade to 8.3 and encountered this error. Will check if its a possible problem on our side. Thank you.

christophwariszlovich commented 3 months ago

@marcovtwout Thank you for pointing me in the right direction. There was a defaultScope function in the model but it didnt return anything!

christophwariszlovich commented 3 months ago

@marcovtwout I found an issue with the function initConnection in CDbConnection class. As of PHP 8.1 the MySQL driver will convert int and float to the PHP data types (see here: https://www.php.net/manual/en/migration81.incompatible.php#migration81.incompatible.pdo.mysql).

In Yii 1.1 there is already code for this but it only applies to sqlite driver:

if(PHP_VERSION_ID >= 80100 && strncasecmp($this->getDriverName(),'sqlite',6)===0)
            $pdo->setAttribute(PDO::ATTR_STRINGIFY_FETCHES, true);

For our application we need the values to be strings, so I overwrote the function in an own class. But I think this should be fixed, as this might legacy code to break when using PHP >= 8.1.

Thank you.

marcovtwout commented 3 months ago

@christophwariszlovich We are using the same behavior here as Yii2 is doing. I'm not entirely sure why, but Yii2 only applies ATTR_STRINGIFY_FETCHES for sqlite. One could argue it's not really a bug/regression but more of a typing improvement in the underlying PDO library.

If your application wants the old behavior, you can implement it like this: Yii::app()->db->setAttribute(PDO::ATTR_STRINGIFY_FETCHES, true); (or in the application config under 'db' => [..., 'attributes' => [PDO::ATTR_STRINGIFY_FETCHES => true]

marcovtwout commented 2 months ago

Fixed with https://github.com/yiisoft/yii/pull/4553