Make use of new functions from PHP 8.4

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

So not sure the process to evaluate this but looking at https://github.com/symfony/polyfill-php84

These are functions it claims to introduce

mb_ucfirst and mb_lcfirst
array_find, array_find_key, array_any and array_all
Deprecated

I don't see any open issues
Last release was June 19th, 2024 so still seems to be active

I personally don't see any downside,

Is there a way to identify other places this could be used? Looking at the file in the MR core/lib/Drupal/Component/Assertion/Inspector.php definitely seems cleaner

I'm a +1

No sure who can make the call though.

Adding a backend dependency requires release and framework manager signoff.

Since this is a symfony package and we've used their polyfills before, it does not require a full dependency evaluation.

I'm personally in favor of the addition.

Yeah makes sense to me too - we can drop the polyfill once we require PHP 8.4, but we get the benefit of the new functions faster.

Think this can be RTBC? @mstrelan mind rebasing and I don't mind marking

Thanks. Sorry if I’m jumping the gun but have only gotten +1s for this

I think @catch and I together counts as RM signoff, so untagging.

In terms of which branch we target, 11.1 makes sense because that will likely be the first branch to support PHP 8.4. It's also a potential maintenance minor target (i.e., 10.4 backport) since it has to do with enabling PHP compatibility.

Normally, adding a dependency is not allowed in a patch release. I think we do not need to concern ourselves with backporting it to 10.3/11.0 unless those branches also end up being PHP-8.4-compatible (i.e. there are no disruptive fixes required to get to PHP 8.4 compatibility). We could discuss it at that point if that turns out to be possible.

+1 from me as FM to the concept

But needs work because the composer.json in lib/Drupal/Component/Assertion also needs to declare this dependency

Not 100% sure why if a core committer would know?

I broke this when I committed the pre-11.0.0 dep updates, so it will need to be re-generated.

Regarding the "why do we need to add the dependency" question: The IS of the issue the above change record was for (#3272110: Drupal 9 and 10's Drupal\Component composer.json files are totally out of date) explains the problem a bit better. Basically, before that issue, every component's composer.json was maintained by hand and so we got things like them being left as version 8.8 when the core version was 9.4, declaring requirements of very old PHP versions that did not match core syntax anymore, etc. So the issue added ComponentGenerator, which does things like make the core versions match and reconcile the PHP requirement.

However, ComponentGenerator is not psychic; it can't tell that we're using PHP 8.4 syntax in this component now when core's own PHP requirement (and therefore all the components' PHP requirements) are 8.3. So to use the PHP 8.4 syntax, the component also needs to declare a direct dependency on the polyfill, in case it's installed by itself without core. Hope that helps. :)

I do agree that the CR could maybe use some work to be more specific about what ComponentGenerator does (and doesn't do). Also, it would actually be better for the generated message to link permanent documentation (rather than a change record). Let's file a followup to address that.

Looking at it, there's also a second problem that ComponentGenerator hardcodes PHP 7.3.0, rather than using Drupal's own minimum requirement. So that's a second followup issue needed. (It still would not address the issue of needing the dependency in the component, though.)

Fixed CI as config validation job does not calling composer with new lock file after reverting commit

so now job has

Installing dependencies from lock file (including require-dev)
Package operations: 1 install, 1 update, 0 removals
  - Downloading symfony/polyfill-php84 (v1.31.0)
  - Installing symfony/polyfill-php84 (v1.31.0): Extracting archive
  - Upgrading drupal/core (11.x-dev 71e78e0 => 11.x-dev 4a48295): Source already present

possibly, a rector rule could look for:
loops with a condition that returns the current element - array_find
loops with a condition that returns the current key - array_find_key
loops with a condition that returns FALSE - array_all
loops with a condition that returns TRUE - array_any

Changes still look good. Can the changes to the gitlab file though be reverted as seems out of scope for this issue.

Added polyfill without usage in #3523284: Update remaining Composer dependencies for 11.2.0 but not sure it will be accepted

btw there's 1.32 https://github.com/symfony/polyfill-php85

as new array functions already added

https://wiki.php.net/rfc/array_first_last

polyfill added via #3523284: Update remaining Composer dependencies for 11.2.0

renamed and updated IS

yep, rector rule + php85 sounds a good fit here

Agree that #3523284: Update remaining Composer dependencies for 11.2.0 got the polyfill in and feels like there's little point in searching for usages where we can use e.g. \array_find in old code.

Updating saved credits.