RTC PR-4.1-dev

Pending

User tests: Successful: Unsuccessful:

avatar beat
beat
24 Jan 2022

Pull Request for Issue # none

Summary of Changes

Fixes Deprecated: Automatic conversion of false to array is deprecated in /home/beat/www/4.0/libraries/src/Form/Field/RulesField.php on line 182

In cases where an access.xml file isn't present for an extension, in Joomla 3.10 (any PHP version) as well as in Joomla 4.0 up to PHP 8.0.x, the permissions rules field can be used without issues. With PHP 8.1, when the file is not present, Access::getActionsFromFile() on line 171 returns false instead of an empty array. When adding elements, the boolean is converted to array at previous line 181, which raises that deprecation notice.

This PR fixes this, converting to array that $this->actions property which should anyway always be of type array.

I have discovered this issue testing permissions of a CB add-on, that uses com_comprofiler.plugin.pluginname as asset name. And obviously, there is no access.xml file for that field.

Testing Instructions

Source-code review should be enough here.

But to test, just go to permissions of Joomla to check everything works as before.

Actual result BEFORE applying this Pull Request

Works.

Expected result AFTER applying this Pull Request

Still works.

Documentation Changes Required

None.

avatar beat beat - open - 24 Jan 2022
avatar beat beat - change - 24 Jan 2022
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 24 Jan 2022
Category Libraries
avatar beat beat - change - 24 Jan 2022
Title
[4.0] [PHP 8.1] Fix PHP 8.1 deprecation notice for permissions field
[4.0] [PHP 8.1] Fix PHP 8.1 deprecation notice for Rules Field
avatar beat beat - edited - 24 Jan 2022
avatar richard67
richard67 - comment - 24 Jan 2022

@bembelimen Shouldn't this be better made for the 4.1-dev branch meanwhile?

avatar beat beat - change - 25 Jan 2022
Labels Added: PR-4.0-dev
avatar bembelimen
bembelimen - comment - 25 Jan 2022

Yes, please every new PR against 4.1+

avatar beat
beat - comment - 25 Jan 2022

Yes, please every new PR against 4.1+

Done! Thank you @bembelimen and @richard67 👍

avatar beat beat - change - 25 Jan 2022
Title
[4.0] [PHP 8.1] Fix PHP 8.1 deprecation notice for Rules Field
[4.1] [PHP 8.1] Fix PHP 8.1 deprecation notice for Rules Field
avatar beat beat - edited - 25 Jan 2022
avatar RickR2H
RickR2H - comment - 25 Jan 2022

I have tested this item ✅ successfully on c661001


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36834.

avatar RickR2H RickR2H - test_item - 25 Jan 2022 - Tested successfully
avatar ghazal ghazal - test_item - 26 Jan 2022 - Tested successfully
avatar ghazal
ghazal - comment - 26 Jan 2022

I have tested this item ✅ successfully on c661001

Thanks for your clear explanation.
I encountered this Warning and had no idea where it came from, ergo, correct it.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36834.

avatar richard67 richard67 - change - 26 Jan 2022
Status Pending Ready to Commit
avatar richard67
richard67 - comment - 26 Jan 2022

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36834.

avatar bembelimen
bembelimen - comment - 31 Jan 2022

Tbh, I see what you did there, but for me are more the questions:

  • why is it false at all? if we ignore it anyways, probably the initial value should be an empty array?
  • could it have more values than false? For example a string? probably a !is_array check would be better?
avatar beat
beat - comment - 31 Jan 2022

Tbh, I see what you did there, but for me are more the questions:

* why is it `false` at all? if we ignore it anyways, probably the initial value should be an empty array?

The function Access::getActionsFromFile() returns false when the file doesn't exist, and probably empty array [] if it exists but has no records.

I didn't want to change the API, so didn't touch that function.

* could it have more values than `false`? For example a string? probably a `!is_array` check would be better?

The function Access::getActionsFromFile() is documented to return either an array or boolean false, nothing else.

My best guess is that Joomla 5 will want to go into the direction of strict typing, like PHP 8.1 is setting this good design direction, and not add more mixed return types. And without adding a different functionality to that function, I don't see it adding strings in the future without breaking the single-concern principle. So the probability that we want to return suddenly a string from that function would look crazy to me. Now if it would be a third-party library in which we have no confidence on design (and be crazy enough to use it) or no control, I would be more prudent and would have done an !is_array().

If you think that Joomla 5 might go the opposite route and extend types (breaking B/C on that function btw) while whole PHP world is going strict-types, and you prefer !is_array() instead, it's your decision and I will adapt the PR.

Please let me know, my goal is to fix this PHP 8.1 bug :-)

avatar beat beat - change - 31 Jan 2022
Labels Added: RTC PR-4.1-dev
Removed: PR-4.0-dev
avatar bembelimen
bembelimen - comment - 31 Jan 2022

Good enough for now, thanks.

Add a Comment

Login with GitHub to post a comment