Feature No Code Attached Yet
avatar Hackwar
Hackwar
19 Oct 2022

I've run the static code analyser phan over most of our current 4.3 codebase. The result are roughly 6000 reported issues, even though most of them are duplicates of for example the toolbar buttons. I'm attaching a file with those results here so that people can have a look at this and maybe fix some of them.
phan2.txt

avatar Hackwar Hackwar - open - 19 Oct 2022
avatar joomla-cms-bot joomla-cms-bot - change - 19 Oct 2022
Labels Added: No Code Attached Yet
avatar joomla-cms-bot joomla-cms-bot - labeled - 19 Oct 2022
avatar nikosdion
nikosdion - comment - 25 Oct 2022

A lot of the static code analysis results are misfires because (among other things):

  • there is no type hinting of the actual model or view type when getting an object using the controller's getView / getModel or the MVCFactory's createView / createModel methods
  • HTMLHelper::_() is meant to take a variable number of arguments with a variadic operator in the second argument which throws static analysis off
  • View templates do not have type-hinting for which object $this refers to
  • Whenever an application object is used the variable's inferred type is almost always ApplicationInterface, not even CMSApplication, making static code analysis complain about missing methods, properties and so on
  • There is not a single core or core component table having concrete properties for the database fields or even @property PHPdoc annotations. This is something we should REALLY fix to get rid of CMSObject usage in there a.s.a.p., see the PRs on the subject by me and George.

There are many variations of the same all over the place.

Most of these issues can be solved with type-hinting comments, some of them by introducing concrete properties. Some issues are core CMS, some issues are Platform.

Here are my questions:

  • Can we file PRs for core issues if they are only adding PHPdoc comments even in patch releases and if so, who should we tag to review them? I am really annoyed by these issues and I'd love to fix (some of) them but I've been reluctant because they are touching high churn code and I don't want to be stuck merging changes in a PR's branch for six months :)
  • Are we going to introduce concrete properties in tables or should we just go gang-ho and add @property declarations? Both methods are valid, both work now and with the proposed PRs for removing CMSObject
  • If something is in the Platform do we make a PR to the platform and, if so, should we only expect the merged PR to be used in the next minor or the next major Joomla version? (if so, which one, major or minor?)

Thank you in advance.

avatar Hackwar Hackwar - change - 22 Feb 2023
Labels Added: ?
avatar Hackwar Hackwar - labeled - 22 Feb 2023
avatar Hackwar Hackwar - change - 25 Sep 2024
Status New Closed
Closed_Date 0000-00-00 00:00:00 2024-09-25 16:52:03
Closed_By Hackwar
Labels Added: Feature
Removed: ?
avatar Hackwar Hackwar - close - 25 Sep 2024
avatar Hackwar
Hackwar - comment - 25 Sep 2024

In the last 2 years a lot of improvements have been made in this regard and this issue is not current anymore. To answer the questions of @nikosdion:
Yes, we can create PRs which only add PHPdoc comments. You don't have to tag anyone, they are normal PRs and will be handled by the maintainers among others. I've created several PRs to fix this stuff and it might need some time, but the stuff gets done eventually.
I don't know about the properties in the table objects. Lets see what 5.3/6.0 will bring.
Issues in the platform should be fixed in the platform and normally will be updated in the CMS with the next minor release. In some cases it might even be updated in a patch version.

Add a Comment

Login with GitHub to post a comment