bug PR-4.4-dev Pending

User tests: Successful: Unsuccessful:

avatar Hackwar
Hackwar
20 Oct 2022

Summary of Changes

This is the result of the static code analyser phan. This fixes several issues which phan marks as PhanTypeArraySuspicious.

Testing Instructions

Testable only through codereview.

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

avatar Hackwar Hackwar - open - 20 Oct 2022
avatar Hackwar Hackwar - change - 20 Oct 2022
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 20 Oct 2022
Category Administration com_admin com_finder Front End com_contact Libraries
avatar richard67
richard67 - comment - 20 Oct 2022

Habe you checked that we safely can replace the initial null values with empty arrays? What if the null values were intentional and are checked somewhere else? In my opinion code review is not a sufficient test method, especially not if people only review the changes but not other places in the code where the changed code is used. I think it always needs real testing if something is broken or not, and not just code review without any further instructions.

avatar Hackwar Hackwar - change - 22 Oct 2022
Labels Added: PR-4.3-dev
avatar Hackwar
Hackwar - comment - 22 Oct 2022

Yes, I did check that the null value isn't used anywhere else.

avatar Hackwar Hackwar - change - 21 Feb 2023
Labels Added: Conflicting Files
avatar laoneo
laoneo - comment - 23 Feb 2023

Hello @Hackwar, thank you for your pull request. It is a very good one.

This changes can break backwards compatibility, so to be on the safe side I wold earliest merge it into Joomla 5. Can you rebase it so people can start testing?

avatar Hackwar
Hackwar - comment - 23 Feb 2023

Since this PR does not break any backwards compatibility and instead fixes PHP notices, I'm not going to rebase this.

avatar laoneo
laoneo - comment - 23 Feb 2023

Thank you very much for your answer, really appreciated. As you change some default values on variables in the Factory class, I fear this might have some side effects into the wild.

avatar Hackwar Hackwar - change - 25 Feb 2023
Labels Removed: Conflicting Files
avatar Hackwar
Hackwar - comment - 25 Feb 2023

Yes, the default is changed, however the factory itself only accesses that as an array and without initialising it first, it would throw a notice. Any code currently accessing this would have to check on array AND if the array-entry is set anyway. There is no situation where this could create issues. The only thing I could think of would be people resetting the cache again by setting this back to null instead of an array and that would throw a notice. But it would do that in the current code too.

avatar Hackwar Hackwar - change - 2 Mar 2023
Title
[4.3] Fix some instances of PhanTypeArraySuspicious
[4.3] Phan: Fix some instances of PhanTypeArraySuspicious
avatar Hackwar Hackwar - edited - 2 Mar 2023
avatar joomla-cms-bot joomla-cms-bot - change - 4 Apr 2023
Category Administration com_admin com_finder Front End com_contact Libraries Unit Tests Repository Administration com_admin com_banners com_config com_content com_contenthistory com_finder com_installer com_joomlaupdate com_languages com_menus com_messages com_postinstall com_templates Language & Strings
avatar joomla-cms-bot joomla-cms-bot - change - 4 Apr 2023
Category Administration com_admin com_finder Unit Tests Repository com_banners com_config com_content com_contenthistory com_installer com_joomlaupdate com_languages com_menus com_messages com_postinstall com_templates Language & Strings Administration com_admin com_finder Front End com_contact Libraries
avatar Hackwar Hackwar - change - 4 Apr 2023
Title
[4.3] Phan: Fix some instances of PhanTypeArraySuspicious
[4.4] Phan: Fix some instances of PhanTypeArraySuspicious
avatar Hackwar Hackwar - edited - 4 Apr 2023
avatar Hackwar Hackwar - change - 4 Apr 2023
Labels Added: ? PR-4.4-dev
Removed: PR-4.3-dev
avatar Hackwar
Hackwar - comment - 4 Apr 2023

I changed the requested value.

avatar laoneo laoneo - change - 4 Apr 2023
Labels Removed: ?
avatar Hackwar Hackwar - change - 11 Apr 2023
Labels Added: ? bug
avatar laoneo laoneo - change - 11 Apr 2023
Labels Removed: ?
avatar laoneo laoneo - close - 11 Apr 2023
avatar laoneo laoneo - merge - 11 Apr 2023
avatar laoneo laoneo - change - 11 Apr 2023
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2023-04-11 10:01:49
Closed_By laoneo

Add a Comment

Login with GitHub to post a comment