Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecation fixes for PHP 8 #298

Merged
merged 3 commits into from
Oct 6, 2022
Merged

Deprecation fixes for PHP 8 #298

merged 3 commits into from
Oct 6, 2022

Conversation

cweiske
Copy link
Contributor

@cweiske cweiske commented Oct 5, 2022

The library itself works with PHP8, but deprecation notices are thrown.

The commits here get rid of those while keeping compatibility with PHP 7.1+ (compat with PHP 5 is not possible anymore).

Related: #293

PHP 8.1 logs a deprecation error:

> PHP Deprecated:  http_build_query():
> Passing null to parameter vgrem#2 ($numeric_prefix) of type string is deprecated

This can be fixed without breaking PHP 7.

Related: vgrem#293
PHP 8.1 logs a deprecation message:
> PHP Deprecated:  strlen(): Passing null to parameter vgrem#1 ($string)
> of type string is deprecated
> in vendor/vgrem/php-spo/src/Runtime/Http/Requests.php on line 137

We fix that by casting the variable to string.

Related: vgrem#293
PHP 8.1 logs deprecation warnings:
> Return type of Office365\Runtime\ClientObjectCollection::getIterator()
> should either be compatible with IteratorAggregate::getIterator(): Traversable,
> or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress
> the notice in vendor/vgrem/php-spo/src/Runtime/ClientObjectCollection.php on line 287

and

> Return type of Office365\Runtime\ClientObjectCollection::offsetExists($offset)
> should either be compatible with ArrayAccess::offsetExists(mixed $offset): bool

and

> Return type of Office365\Runtime\ClientObjectCollection::offsetGet($offset)
> should either be compatible with ArrayAccess::offsetGet(mixed $offset): mixed,
> or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice

and

> Return type of Office365\Runtime\ClientObjectCollection::offsetSet($offset, $value)
> should either be compatible with ArrayAccess::offsetSet(mixed $offset, mixed $value): void

and

> Return type of Office365\Runtime\ClientObjectCollection::offsetUnset($offset)
> should either be compatible with ArrayAccess::offsetUnset(mixed $offset): void

Those can be fixed by adding return types for getIterator(), offsetExists(),
offsetSet() and offsetUnset().
For those, the minimum required PHP version needs to be raised to PHP 7.1,
since the "void" return type only exists in there.

The return type for offsetGet() is "mixed", which cannot be expressed in PHP 7.x,
only PHP 8.0 and later.
I decided to use the "#[\ReturnTypeWillChange]" annotation so the lib
can stay compatible with PHP 7.

Related: vgrem#293
@vgrem
Copy link
Owner

vgrem commented Oct 6, 2022

Greetings,

Thank you for those improvements! 👍

Regarding

compat with PHP 5 is not possible anymore

not a slightest chance to keep it still compatible with =>5.5?
If the support will be dropped, how many will be affected by this change so far is unclear.

Seems this particular change breaks the backward compatibility with version >=5.6, perhaps another options are available, perhaps suppress deprecation notices?

@cweiske
Copy link
Contributor Author

cweiske commented Oct 6, 2022

The whole commit "Return types in ClientObjectCollection" would need to be adjusted so that no return types are introduced at all.

The "supported PHP versions" list at https://www.php.net/supported-versions.php says that support for PHP 7.1 ended 2019. I don't think that there is any reason to keep support for 5.x, which was end-of-life'd in 2018.

But that's up to you to decide.

@cweiske
Copy link
Contributor Author

cweiske commented Oct 6, 2022

There are stats for PHP versions that install your package: https://packagist.org/packages/vgrem/php-spo/php-stats

PHP 5.5 and earlier are at 0.0%, PHP 5.6 is 0.6%, PHP 7.0 at 0.1%.

@vgrem
Copy link
Owner

vgrem commented Oct 6, 2022

That's a valuable piece of stats, thank you!
Agree and it feels like a good timing to bring those changes with a major version: 3.0.0

A new version is expected to be released later today, once README.md is updated, seems to be last remaining mention about PHP version 5

@vgrem vgrem merged commit d70a396 into vgrem:master Oct 6, 2022
@cweiske cweiske deleted the php8-fixes branch October 7, 2022 14:35
cweiske added a commit to mogic-le/phpSPO that referenced this pull request Oct 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants