Joomla! Issue Tracker - CMS

Download
Launch
Feeling Lucky

[#37823] - Use of PHP reserved word in function name

No Code Attached Yet
avatar ProfDavidMora
ProfDavidMora
17 May 2022

Problem identified

This was initially discussed in the Inteliphense project in this issue.
The tool is detecting an instance of a function using the name match in the CMS core code. This is a PHP keyword since version 8.0 according to the manual

Proposed solution

Change the name of this functions both at definition:

joomla-cms/libraries/src/Environment/Browser.php

Line 514 in a8adadf

public function match($userAgent = null, $accept = null)

And the calling location (there seems to be only one place):

joomla-cms/libraries/src/Environment/Browser.php

Line 477 in a8adadf

$this->match($userAgent, $accept);

Some candidates for the new name:

  • _match
  • match_useragent
  • match_features

Open questions

I am not fully aware of all applicable coding rules (naming conventions), so I prefer to have someone with more expertise address the final code and include in the project if relevant. Does this need to be changed and what is the preferred final code for it?

avatar ProfDavidMora ProfDavidMora - open - 17 May 2022
avatar joomla-cms-bot joomla-cms-bot - change - 17 May 2022
Labels Added: No Code Attached Yet
avatar joomla-cms-bot joomla-cms-bot - labeled - 17 May 2022
avatar wilsonge
wilsonge - comment - 17 May 2022

As far as I can tell from the PHP RFC https://wiki.php.net/rfc/match_expression_v2

match was added as a keyword (reserved_non_modifiers). This means it can’t be used in the following contexts anymore:

- namespaces
- class names
- function names
- global constants

Note that it will continue to work in method names and class constants.

We're using it in a method name here which means everything works fine. Given that this is a public method available for extension developers to use I'm not sure that we really want to change the function name - certainly we would have to keep the original in J4 as an alias even if we did set a new name

avatar eeshaanSA
eeshaanSA - comment - 18 May 2022

Hi, i would like to work on this issue, if allowed, as @wilsonge pointed out. I assume this is a good issue for beginners. Please point me out if i am wrong.

avatar ProfDavidMora
ProfDavidMora - comment - 18 May 2022

@wilsonge, thanks for the reference to the RFC. It indeed works on the web server, most definitely, otherwise the webapp would have been broken during testing. I will contact the guys at Intelephense again so that they tweak the code sensing and allow this particular one in the stated contexts.

Given that fact, I understand there is no need for code changes, but maybe it should follow the path of deprecation so that extension developers have time to move away from it and eventually it can be taken out altogether and align with the evolution of PHP. Possibly not a top priority, understandably, but also something that can be done specially having a volunteer already jumping in. Thoughts?

avatar laoneo laoneo - change - 30 May 2022
Status New ⇒ Closed
Closed_Date 0000-00-00 00:00:00 ⇒ 2022-05-30 12:03:35
Closed_By ⇒ laoneo
avatar laoneo laoneo - close - 30 May 2022
avatar laoneo
laoneo - comment - 30 May 2022

I'm closing this as it is an issue on the parser side.

Add a Comment

Login with GitHub to post a comment