I'm not sure it belongs anywhere. It's only used in two places: by update.module, to look for .info.yml files, and install.inc, to detect... a database driver, of all things.

So I'd suggest removing this one from the patch and filing a separate issue to deprecate and get rid of it entirely. It has two usages that are not even really conceptually related. Seems like a legacy thing.

Follow-up

#3115143: Remove deprecated DRUPAL_PHP_FUNCTION_PATTERN

Notes for developers after this change

It should be mentioned that ExtensionDiscovery::PHP_FUNCTION_PATTERN is NOT identical with DRUPAL_PHP_FUNCTION_PATTERN.

ExtensionDiscovery::PHP_FUNCTION_PATTERN contains regex delimiters and start/end symbols.
DRUPAL_PHP_FUNCTION_PATTERN does/did not have delimiters and start/end symbols.

DRUPAL_PHP_FUNCTION_PATTERN could be used as a fragment of a bigger regular expression.
ExtensionDiscovery::PHP_FUNCTION_PATTERN cannot be used as a fragment of a bigger regular expression.

class ExtensionDiscovery {
  [..]
  const PHP_FUNCTION_PATTERN = '/^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*$/';

const DRUPAL_PHP_FUNCTION_PATTERN = '[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*';
CommentFileSizeAuthor
#33 2936105-33.patch2.09 KBsershevchyk
#32 2936105-32.patch2.05 KBsershevchyk
#30 2936105-30.patch2.01 KBsershevchyk
#29 interdiff_15-29.txt2.15 KBsershevchyk
#29 2936105-29.patch1.51 KBsershevchyk
#27 interdiff_15-27.txt2.13 KBsershevchyk
#27 2936105-27.patch2.02 KBsershevchyk
#25 interdiff_15-25.txt2.45 KBsershevchyk
#25 2936105-25.patch2.24 KBsershevchyk
#23 interdiff_15-23.txt3.21 KBsershevchyk
#23 2936105-23.patch2.27 KBsershevchyk
#22 2936105-22.patch2.23 KBsershevchyk
#22 interdiff_15-22.txt3.17 KBsershevchyk
#20 interdiff_15-20.txt3.12 KBsershevchyk
#20 2936105-20.patch2.24 KBsershevchyk
#19 interdiff_15-19.txt2.42 KBsershevchyk
#19 2936105-19.patch2.25 KBsershevchyk
#15 interdiff-2936105-14-15.txt1.67 KBvoleger
#15 2936105-15.patch3.33 KBvoleger
#14 2936105-14.patch2.01 KBsershevchyk
#13 2936105-13.patch2.01 KBsershevchyk
#9 2936105-9.patch1.99 KBsershevchyk
#2 2936105-2.patch2.84 KBrosk0

Comments

RoSk0 created an issue. See original summary.

rosk0’s picture

rosk0
he/him
Primary language Russian
Location Wellington
commented
Assigned: rosk0 » Unassigned
Status: Active » Needs review
StatusFileSize
new 2936105-2.patch2.84 KB

Original implementation.

  1. Added deprecation
  2. Added CR placeholder
  3. Replaced DRUPAL_PHP_FUNCTION_PATTERN usage in core/includes/install.inc with introduced Drupal\Core\Database\Database::DRIVER_NAME_PATTERN
  4. Removed pattern from update.module completely because it doesn't feel like we have a restriction here, also there is no paatern matching in here \Drupal\Core\Updater\Updater::findInfoFile()
rosk0’s picture

rosk0
he/him
Primary language Russian
Location Wellington
commented
Issue summary: View changes

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
commented
Status: Needs review » Needs work 
+++ b/core/includes/bootstrap.inc
@@ -109,6 +109,10 @@
+ * @deprecated in Drupal 8.5.0, will be removed before Drupal 9.0.0.

Deprecation message need to be updated

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
commented
Related issues: +#2999721: [META] Deprecate the legacy include files before Drupal 9
sershevchyk’s picture

sershevchyk
Primary language Ukrainian
Location Lutsk
commented
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new 2936105-9.patch1.99 KB
sershevchyk’s picture

sershevchyk
Primary language Ukrainian
Location Lutsk
commented
Issue summary: View changes
voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
commented
Status: Needs review » Needs work
Issue tags: +Needs reroll 
+++ b/core/includes/bootstrap.inc
@@ -140,6 +140,10 @@
+ * @deprecated in Drupal 8.8.0, will be removed before Drupal 9.0.0.

This is not correct deprecation message.
Refer to the deprecation policy to find correct deprecation message structure https://www.drupal.org/core/deprecation

ghost of drupal past’s picture

ghost of drupal past
he/him
commented
Related issues: +#944198: Functions that call drupal_system_listing() act on potentially invalid system items

Added the issue which added this regular expression. git hash is 203b6a88. I do not know whether it's relevant still, I am just doing the git archeology.

sershevchyk’s picture

sershevchyk
Primary language Ukrainian
Location Lutsk
commented
Status: Needs work » Needs review
StatusFileSize
new 2936105-13.patch2.01 KB

Updated patch for Drupal:8.8.0

sershevchyk’s picture

sershevchyk
Primary language Ukrainian
Location Lutsk
commented
StatusFileSize
new 2936105-14.patch2.01 KB
voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
commented
StatusFileSize
new 2936105-15.patch3.33 KB
new interdiff-2936105-14-15.txt1.67 KB
sershevchyk’s picture

sershevchyk
Primary language Ukrainian
Location Lutsk
commented
Status: Needs review » Reviewed & tested by the community

Looks good for me

ghost of drupal past’s picture

ghost of drupal past
he/him
commented

http://grep.xnddx.ru/search?text=DRUPAL_PHP_FUNCTION_PATTERN&filename= seems it is not used much in contrib either, only API uses it.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work

@larowlan pointed out we already have https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Extension...

Also given we have this, update module should probably be using it.

sershevchyk’s picture

sershevchyk
Primary language Ukrainian
Location Lutsk
commented
Status: Needs work » Needs review
StatusFileSize
new 2936105-19.patch2.25 KB
new interdiff_15-19.txt2.42 KB
sershevchyk’s picture

sershevchyk
Primary language Ukrainian
Location Lutsk
commented
StatusFileSize
new 2936105-20.patch2.24 KB
new interdiff_15-20.txt3.12 KB

Status: Needs review » Needs work

The last submitted patch, 20: 2936105-20.patch, failed testing. View results

sershevchyk’s picture

sershevchyk
Primary language Ukrainian
Location Lutsk
commented
StatusFileSize
new interdiff_15-22.txt3.17 KB
new 2936105-22.patch2.23 KB
sershevchyk’s picture

sershevchyk
Primary language Ukrainian
Location Lutsk
commented
StatusFileSize
new 2936105-23.patch2.27 KB
new interdiff_15-23.txt3.21 KB
sershevchyk’s picture

sershevchyk
Primary language Ukrainian
Location Lutsk
commented
Status: Needs work » Needs review
sershevchyk’s picture

sershevchyk
Primary language Ukrainian
Location Lutsk
commented
StatusFileSize
new 2936105-25.patch2.24 KB
new interdiff_15-25.txt2.45 KB
ghost of drupal past’s picture

ghost of drupal past
he/him
commented

const DRUPAL_PHP_FUNCTION_PATTERN = ExtensionDiscovery::PHP_FUNCTION_PATTERN; that's valid PHP code?? o_O I always said a day when I learn nothing new is a day wasted, thanks for making this day not wasted.

sershevchyk’s picture

sershevchyk
Primary language Ukrainian
Location Lutsk
commented
StatusFileSize
new 2936105-27.patch2.02 KB
new interdiff_15-27.txt2.13 KB
ghost of drupal past’s picture

ghost of drupal past
he/him
commented

Now I lost you, I am not sure what the goal is here because there are no comments as to what is happening in all these patches but I know from git archeology this pattern was introduced to catch false positive results -- not sure whether that applies here but I feel by dropping it from update.inc we change the meaning of the code, is this what we want? Instead of DRUPAL_PHP_FUNCTION_PATTERN .info.yml we now parse *.info.yml. Is this the desired new workings of the code? And, are we just dropping the constant, wasn't the plan to deprecate it...?

sershevchyk’s picture

sershevchyk
Primary language Ukrainian
Location Lutsk
commented
StatusFileSize
new 2936105-29.patch1.51 KB
new interdiff_15-29.txt2.15 KB

You wrote the only API uses this constant. I checked core and saw only files from the patch used this constant in 2 places.
But DRUPAL_PHP_FUNCTION_PATTERN different instid ExtensionDiscovery::PHP_FUNCTION_PATTERN.
We can't use almost ExtensionDiscovery::PHP_FUNCTION_PATTERN for mask.

In the patch I returned back const DRUPAL_PHP_FUNCTION_PATTERN

sershevchyk’s picture

sershevchyk
Primary language Ukrainian
Location Lutsk
commented
StatusFileSize
new 2936105-30.patch2.01 KB
voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
commented
Status: Needs review » Needs work

The deprecation message needs to be updated.

sershevchyk’s picture

sershevchyk
Primary language Ukrainian
Location Lutsk
commented
Status: Needs work » Needs review
StatusFileSize
new 2936105-32.patch2.05 KB

Updated message for const DRUPAL_PHP_FUNCTION_PATTERN

sershevchyk’s picture

sershevchyk
Primary language Ukrainian
Location Lutsk
commented
StatusFileSize
new 2936105-33.patch2.09 KB

Updated the deprecation message for const DRUPAL_PHP_FUNCTION_PATTERN

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
commented
Status: Needs review » Reviewed & tested by the community

Yes, that finally looks good.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

sershevchyk’s picture

sershevchyk
Primary language Ukrainian
Location Lutsk
commented

Tested for 8.9.x-dev patch works as expected

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 33: 2936105-33.patch, failed testing. View results

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs reroll

Some CI hiccups, back to RTBC.

  • catch committed 618f0bc on 9.0.x 
    Issue #2936105 by SerShevchyk, voleger, RoSk0, Charlie ChX Negyesi:...

  • catch committed 7508818 on 8.9.x 
    Issue #2936105 by SerShevchyk, voleger, RoSk0, Charlie ChX Negyesi:...

  • catch committed 47f71d2 on 8.8.x 
    Issue #2936105 by SerShevchyk, voleger, RoSk0, Charlie ChX Negyesi:...
catch’s picture

catch
he/him
Primary language English
commented
Version: 8.9.x-dev » 8.8.x-dev
Status: Reviewed & tested by the community » Fixed

Since this was deprecated originally in 8.5.x and we're just fixing the deprecation docs and removing usages here I think this is still fine for 8.8.x. Committed 618f0bc and pushed to 9.0.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

donquixote’s picture

donquixote commented
Issue summary: View changes

It should be mentioned that ExtensionDiscovery::PHP_FUNCTION_PATTERN is NOT identical with DRUPAL_PHP_FUNCTION_PATTERN.

ExtensionDiscovery::PHP_FUNCTION_PATTERN contains regex delimiters and start/end symbols.
DRUPAL_PHP_FUNCTION_PATTERN does/did not have delimiters and start/end symbols.

DRUPAL_PHP_FUNCTION_PATTERN could be used as a fragment of a bigger regular expression.
ExtensionDiscovery::PHP_FUNCTION_PATTERN cannot be used as a fragment of a bigger regular expression.