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

PHP 8.1 compatibility fixes #400

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

PHP 8.1 compatibility fixes #400

wants to merge 11 commits into from

Conversation

zozlak
Copy link
Collaborator

@zozlak zozlak commented Sep 9, 2022

This pull request is a systematic approach to the EasyRdf PHP 8.1 compatibility issues. Instead of depending solely on tests, a static code analyzer phpstan has been used to detect all incompatibilities.

As a side effect fixes for other problems reported by the code analyzer have been provided like making docstring type hints in line with the code, resolving broken inheritance of ARC/JSON and RdfPhp serialisers/parsers, declaring few undeclared class properties, etc.

Last but not least examples and example tests have been fixed both for PHP 8.0/8.1 incompatibilities and for changes in the data they rely upon so that make test reports no errors nor warnings.

It's worth noting the adjustments required for the PHP 8.1 compatibility introduce backward-incompatible API changes. ArrayAccess interface offsetSet() and offsetUnset() methods must return void while in EasyRdf they sometimes returned a value. I don't expect it to affect many users and this behavior hasn't been covered by tests but it means an EasyRdf release incorporating this pull request will require EasyRdf major version bump.

* change all PHPUnit `assertContains($expected, $tested)` calls to
  `assertStringContainsString($expected, $tested)` for PHPUnit 8 and 9
  compatibility
* test for the `get_magic_quotes_gpc()` existence before calling it
  as it has been dropped in PHP 8.0
* change the `dbc:Member_states_of_the_United_Nations` to the
  dbc:Current_member_states_of_the_United_Nations in dbpedia
  examples and their tests to follow the current dbpedia content
* explicitly set flags in `htmlspecialchars()` calls to the `ENT_COMPAT`
  to assure same behavior accross various PHP versions (default value
  has been changed in PHP 8.1)
Set phpstan's anlysis level to 3 as it already catches the PHP 8.1
no-goes and going above it generates too many warnings to fix them.
…running under PHP 8.1

Backward-incompatible API changes:

* Assure `offsetSet()` and `offsetUnset()` methods implementing the
  `ArrayAccess` interface don't return any value as those methods should
  return `void`.
  See https://www.php.net/manual/en/arrayaccess.offsetset.php and
  https://www.php.net/manual/en/arrayaccess.offsetunset.php
  Affected classes: `EasyRdf\Collection`, `EasyRdf\Container`,
  `EasyRdf\Graph`

Other changes:

* Make the `EasyRdf\Parser\Arc` and the `EasyRdf\Parser\Json)` inherit
  directly from the `EasyRdf\Parser` instead of `EasyRdf\Parser\RdfPhp`.
  Similarly make the `EasyRdf\Serialiser\Arc` and
  `EasyRdf\Serialiser\Json` inherit directly from the
  `EasyRdf\Serialiser` instead of the `EasyRdf\Serialiser\RdfPhp`.
  These changes were required to fix the variadic return type of the
  `parse()`/`serialize()` methods. In the proper object model the child
  class can make the return type only narrower and `string` return type
  of the ARC/JSON parser/serialiser is not a subtype of the `array` type
  returned by the `RdfPhp` parser/serialiser classes. As the ARC/JSON
  parser/serialiser don't access any private/protected RdfPhp
  parser/serialiser properties, they can simply instantiate when needed
  instead of extening its class.
* Add missing `void`, `int` and `bool` return types for methods
  implementing the `ArrayAccess` interface.
* Mark methods implementing the ArrayAccess interface which should have
  the `mixed` return type with the `#[\ReturnTypeWillChange]` attribute
  and assure they have a `@return mixed` docstring.
  The `mixed` type hint can't be used as we want to keep PHP 7
  compatibility.
* Add explicit `return null` statements for methods implicitly returning
  null as implicit null return is not compatible with PHP 8.1
* Assure all methods which may return null indicate it in the
  docstring's `@return` attribute
* Add undeclared class property definitions:
  * `Http\Response::$version`
  * `Parser\RdfXml::$xmlParser`
* Fix the `Http\Client::$rawPostData` initialization and handling
  (`string` type doesn't cover `null` and a property with value `null`
  tested with `isset()` returns `true` so there should be no initial
  value, cleaning should be done with the `unset()` and testing for a
  value with `isset()`; alternatively the property value should be
  `string|null`, cleaning should be done with assigning `null` and
  testing for a value with `empty()`)
* Add explicit cast to `string` in all places where PHP methods
  requiring a string value (like the `strlen()`) are called and there is
  a risk a value can be `null`.
* Specify `htmlspecialchars()` flags explicitly to assure same behavior
  across all PHP versions (the default value has changed in PHP 8.1)
* Set the `EasyRdf\Literal::getValue()` `@return` docstring attribute to
  `mixed` as its subclasses do return various value types
* Fix the `EasyRdf\Serialiser::serialise` `@return` docstring attribute to
  `string|array` as one of its subclasses (`EasyRdf\Serialiser\RdfPhp`)
  returns an array.
* Fix `@return` docstring for the `EasyRdf\Literal\Date::getValue()`
* Fix the `@var` typehint in `lib/Serialiser/Turtle.php:313`
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.

None yet

1 participant