Fix ObjectIterator for PHP8 #682
Conversation
Signed-off-by: Marcel Kempf <[email protected]>
|
Thanks but given the min php version is still 5.3.3 on this repo, it'd probably be more appropriate to mark all methods with |
| @@ -49,7 +50,7 @@ public function current() | |||
| /** | |||
| * {@inheritdoc} | |||
| */ | |||
| public function next() | |||
| public function next(): void | |||
There was a problem hiding this comment.
In it's current state the library supports >=5.3.3 as percomposer.json requirements. The void keyword was introduced only in PHP 7.1. I know there are plans to drop support for PHP <7.2 (Even approved by @Seldaek) but in an attempt to revive the library and doing some initial triage I did want to point this out for ourselves.
It also applies to the int and bool return types which where added in PHP 7.0.
In order to make this PR considered for merging into the repo changing all to #[\ReturnTypeWillChange] would be needed I guess.
There was a problem hiding this comment.
@R4c00n in an attempt to cleanup this repo we are trying to filter the pull request and see which ones might be closed and which ones we can help resolve. Do you still feel like pursuing this PR or did time catch up on us and are you working on other priorities? Feel free to close it yourself or comments if helpful.
DannyvdSluijs
added
the
Open discussions
|
See #726 for the topic of raising the minimum PHP level and supporting newer versions PHP. |
|
Any chance of this making it into the next 5.x release? Adding |
We are working on bringing the project back alive, you can see more in the discussions. We are however not planning to do anything with the |
|
This PR was ported in #746 which address the raising on the minimum version PHP as a whole including pipelines. |
* ci: Add PHP 8.0 and greater to build matrix * ci: Remove PHP 5.3 - 7.1 from workflows * build: Require minimum PHP 7.2 * build: Upgrade to PHPUnit 8.5 * refactor: Add now required void return types for setup() methods * build: Include phpspec/prophecy dependency * refactor: Replace setExpectedException with expectException/expectExceptionMessage * refactor: Replace @ExpectedException annotation for expectException method * refactor: Replace assertInternalType for assertIsArray * refactor: Replace getMock for createMock * test: Improve test assertions * fix: Solve return type issues with Objectiterator (port of #682) See #682 * build: Update icecave/parity to ^3.0 as 1.0 uses deprecated each() method * style: Correct code style issues * fix: Fix deprecation notices found from GHA workflow run See https://github.com/jsonrainbow/json-schema/actions/runs/10216569969/job/28268331091 * fix: Add fallback to empty string when null value is passed in UriResolver::parse * fix: Port #717: Fixes for implicit nullability deprecation See #717 * ci: Avoid GHA run on each push and pull request; Include PHP 8.4 in matrix * ci(Drop-PHP-8.4-from-matrix): This PR adds phpspec/prophecy as an explicit dependency which is restrictive and doesnt support upcoming PHP versions * refactor: Replace ternary variable with explicit cast to string
Hi, with PHP8.1 the iterator methods of ObjectIterator must be covariant with the PHP core interfaces.
I didn't know if theres a specific return type for
::current, so I went with the suppress annotation here. Happy to change this to a return type, if there is one?