Skip to content

Conversation

@jrfnl
Copy link
Member

@jrfnl jrfnl commented Oct 17, 2020

WordPress should not throw any notices.

array_map() passes by value, while array_walk() passes by reference.

The use of array_map() on line 52 within the export_entries() method will cause a "Warning: PO::export_entry(): Argument #1 ($entry) must be passed by reference, value given" warning on PHP 8.0 because the export_entry() method is declared to expect the $entry object to be passed by reference.

Since PHP 5.0, objects are passed by reference by default and this does not have to be declared in the function signature of a method expecting this anymore.

Also, the code within the method doesn't even try to change $entry and return a value.

So, all in all, the reference in the function declaration is redundant, old-school and should be removed.

While this could be considered a BC-break, I have done a search for use of this method in plugins and reviewed a significant number of the returned results. None of these would run into trouble with the change now made. Rather this change fixes the same issue for a number of plugins also using array_map().

All the same, this change should be mentioned in a dev-note as it is a very small and insignificant BC-break.

Search results: https://wpdirectory.net/search/01EMWBEKAM2B6ECSTCTEDAZGQW

Trac ticket: https://core.trac.wordpress.org/ticket/50913


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

WordPress should not throw any notices.

`array_map()` passes by value, while `array_walk()` passes by reference.

The use of `array_map()` on line 52 within the `export_entries()` method will cause a "Warning: PO::export_entry(): Argument #1 ($entry) must be passed by reference, value given" warning on PHP 8.0 because the `export_entry()` method is declared to expect the `$entry` object to be passed by reference.

Since PHP 5.0, objects are passed by reference by default and this does not have to be declared in the function signature of a method expecting this anymore.

Also, the code within the method doesn't even try to change `$entry` and return a value.

So, all in all, the reference in the function declaration is redundant, old-school and should be removed.

While this could be considered a BC-break, I have done a search for use of this method in plugins and reviewed a significant number of the returned results. None of these would run into trouble with the change now made. Rather this change fixes the same issue for a number of plugins also using `array_map()`.

All the same, this change should be mentioned in a dev-note as it is a very small and insignificant BC-break.

Search results: https://wpdirectory.net/search/01EMWBEKAM2B6ECSTCTEDAZGQW
@jrfnl
Copy link
Member Author

jrfnl commented Oct 18, 2020

Closing as committed.

@jrfnl jrfnl closed this Oct 18, 2020
@jrfnl jrfnl deleted the trac-50913/fix-another-warning branch October 18, 2020 16:31
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.

1 participant