Correct behaviour to match PHP 8.4 PDO returned types for connect and __construct for the internal pdo object

[srjlewis]

I have done some changes to keep the behaviour the same as the PDO calls with the correct returned types for the pdo object.

Thanks to @frederikbosch as I took some of the ideas from #228 (comment), hope you don't mind

[harikt]

I thought we don't need the connect method itself as this is implemented internally in the PDO.

[srjlewis]

I was looking at the final release of the new PDO

PDO::__construct() always returns PDO and PDO::connect() returns Pdo\{driver} or PDO depending on available drivers

So if you use ExtendedPdo::getPdo() you would get the correct typed object.

[harikt]

That behaviour of PDO is expected. But in our case we are not calling the parent construct.

[aik099]

I'm getting this error when trying to use this library on PHP 8.4 ( https://github.com/console-helpers/db-migration/actions/runs/12323306770/job/34398667606 ):

PHP Fatal error:  Cannot make static method PDO::connect() non static in class Aura\Sql\AbstractExtendedPdo in .../vendor/aura/sql/src/AbstractExtendedPdo.php on line 161

I'm hoping this PR will help with that.

[srjlewis]

@aik099 what version of PHP are you using and what does your composer file look like?

[aik099]

@aik099 what version of PHP are you using and what does your composer file look like?

@srjlewis , I'm using PHP 8.4.

The composer.json file content is ( from https://github.com/console-helpers/db-migration/blob/master/composer.json ):

{
	"name": "console-helpers/db-migration",
	"description": "Database migrations made simple",
	"keywords": ["database", "migration", "sqlite"],
	"license": "BSD-3-Clause",
	"authors": [
		{
			"name": "Alexander Obuhovich",
			"email": "[email protected]"
		}
	],
	"require": {
		"php": ">=5.6",
		"aura/sql": "^2.5 || ^3.0 || ^4.0 || ^5.0"
	},
	"require-dev": {
		"aik099/coding-standard": "dev-master",
		"yoast/phpunit-polyfills": "^2.0",
		"phpspec/prophecy": "^1.10",
		"console-helpers/prophecy-phpunit": "^3.0"
	},
	"autoload": {
		"psr-4": {
			"ConsoleHelpers\\DatabaseMigration\\": "src/DatabaseMigration/"
		}
	},
	"autoload-dev": {
		"psr-4": {
			"Tests\\ConsoleHelpers\\DatabaseMigration\\": "tests/DatabaseMigration/"
		}
	},
	"extra": {
		"branch-alias": {
			"dev-master": "0.1.x-dev"
		}
	}
}

[srjlewis]

@aik099 you would need to change your composer to

"require": {
	"php": ">=5.6",
	"aura/sql": "^2.5 || ^3.0 || ^4.0 || ^5.0||dev-6.x"
}

this will use 6.x branch that is compatable with PHP 8.4.

I awaitig #230 being merged which would allow swapping between aura/sql versions, this is only targeting 5.x branch but can be backported to previous version.

but yes this PR would help aswell to some degree in your case.

All this is due to PHP 8.4 introducing PDO::connect() this is the issue regarding the changes #228

I have both of these PR's in my personal repo and can easily switch between versions.

@harikt please could you review these PR's and tag the 6.x branch for release, it would be helpful for PHP 8.4 users.

[aik099]

@srjlewis , thank you for an explanation. I now understand why it's failing. I'll wait for Aura/Sql 6.x release. We're not switching to PHP 8.4 this year.

[harikt]

@srjlewis yes, I have been checking this. But I am not sure if we need this change or not. @koriym , @frederikbosch can you guys look into this and give some suggestion if we need this specific change ?

1. I think connect method can be removed from ExtendedPDO

   Aura.Sql/src/ExtendedPdo.php

   Line 88 in 0d02dd3

   public static function connect(

2. In this we are passing to options driverSpecific, immediate etc which is not PDO specific and can cause more issues in the future with PDO specific changes.

[srjlewis]

@harikt,

1. We need the ExtendedPdo::connect() method, if we remove it you would be able to call ExtendedPdo::connect() and get a PDO object instead of a ExtendedPdo, so we wrap the connect method so we return a ExtendedPdo object consistently

2. Yes I agree there are other ways to pass the flags, but I did like the idea of how @frederikbosch was added to the options array in Support for PHP 8.4 #228 (comment), also as PHP internals only use INT's as there values, and I am using strings there would be no confilict, and when passed to a PDO object they are just ignored.

[harikt]

can't we actually use [] instead of null, this will also remove the usage of $options ??= []; right ?