Create \Drupal\Component\Utility\ArrayObject

Created on 11 May 2018, over 6 years ago
Updated 23 June 2023, over 1 year ago

Problem/Motivation

Drupal has arrays.

Drupal has LOTS of arrays.

Arrays are, sometimes, quite painful to manipulate and test.

We need a base object that we can start creating other strongly typed objects on that do nothing but manipulate and extract information from arrays (at their core, no pun intended):

Additional use-cases for existing issues:

Unfortunately, the SPL \ArrayObject isn't really all that "feature rich" with all the various tasks that we commonly do with arrays.

Proposed resolution

Implement our own custom base ArrayObject class, similar to something that resembles:

https://cgit.drupalcode.org/plus/tree/src/Utility/ArrayObject.php?h=8.x-4.x

Remaining tasks

  • Create patch
  • Create tests

User interface changes

None

API changes

Utility/API addition

Data model changes

None

πŸ“Œ Task
Status

Needs work

Version

11.0 πŸ”₯

Component
BaseΒ  β†’

Last updated about 4 hours ago

Created by

πŸ‡ΊπŸ‡ΈUnited States markhalliwell

Live updates comments and jobs are added and updated live.
  • PHP 8.2

    The issue particularly affects sites running on PHP version 8.2.0 or later.

  • Needs framework manager review

    It is used to alert the framework manager core committer(s) that an issue significantly impacts (or has the potential to impact) multiple subsystems or represents a significant change or addition in architecture or public APIs, and their signoff is needed (see the governance policy draft for more information). If an issue significantly impacts only one subsystem, use Needs subsystem maintainer review instead, and make sure the issue component is set to the correct subsystem.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Seems to have 8.2 failures. Should that be hold up or no?

  • Status changed to Needs review almost 2 years ago
  • πŸ‡ΊπŸ‡¦Ukraine voleger Ukraine, Rivne

    No, these were unrelated issues during the test run. The patch contains new additions only.

    1. +++ b/core/lib/Drupal/Component/Utility/ArrayObject.php
      @@ -0,0 +1,100 @@
      +  public function set(mixed $key, mixed $value): void {
      ...
      +  public function offsetSet(mixed $offset, mixed $value): void {
      

      I suggest using union type argument declarations:
      `?string|?int` for `$key` argument

    2. +++ b/core/lib/Drupal/Component/Utility/ArrayObject.php
      @@ -0,0 +1,100 @@
      +  public function offsetExists(mixed $offset): bool {
      ...
      +  public function offsetGet(mixed $offset): mixed {
      ...
      +  public function offsetUnset(mixed $offset): void {
      

      I suggest using a union type argument declaration:
      `string|int`

    Also, why not introduce the method chaining support for setter methods?

  • πŸ‡¦πŸ‡ΊAustralia kim.pepper πŸ„β€β™‚οΈπŸ‡¦πŸ‡ΊSydney, Australia

    Thanks for kicking this issue along!

    #25

    1. `?string|?int` is not valid as you can't mix union types with nullable types. Added string|int $key for get and set methods instead.
    2. We cannot change the method signatures for methods implementing the \ArrayAccess interface.

    Added a fluid setter and bumped to 10.1.x

  • Status changed to RTBC almost 2 years ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Change looks like a good consistentcy addition

  • Status changed to Needs work almost 2 years ago
  • πŸ‡¬πŸ‡§United Kingdom catch

    Are any of the issues that reference this one planning to use it? There's no indication as such in the issue summary. Without a concrete use-case, it won't help with conversions. For example the builder pattern issue provides a class that creates a render array, it doesn't object-ify the render array itself, so it won't help there.

    Additionally, if we added something like this, I don't think we should call it ArrayObject because that already exists in SPL.

  • πŸ‡¦πŸ‡ΊAustralia kim.pepper πŸ„β€β™‚οΈπŸ‡¦πŸ‡ΊSydney, Australia

    I came here from #2024043-53: Add Module, Theme, Profile, and Extension value objects β†’ where @larowlan suggested this. I think we need a base object that can support the existing arrays that are currently a dumping ground.

    Maybe we can call this BaseArrayObject to make that more clear?

  • Issue was unassigned.
  • πŸ‡¦πŸ‡ΊAustralia kim.pepper πŸ„β€β™‚οΈπŸ‡¦πŸ‡ΊSydney, Australia
  • last update over 1 year ago
    29,407 pass
  • πŸ‡©πŸ‡ͺGermany donquixote

    I am skeptical about the use of ArrayAccess objects.
    I also don't like these grand base classes, which tend to give their children more capabilities than they need.
    E.g. would the extension info classes really need getIterator()?

    I would want to see examples how this can improve DX elsewhere - see also my comment in πŸ“Œ Add Module, Theme, Profile, and Extension value objects Needs work .
    We can create proof-of-concept branches that show how this would be used.
    (test cases alone are not sufficient for this)
    So +1 to @catch in #28.

    I did recently post a proposal involving ArrayAccess in slack, but this was for a very niche and controversial case to allow $cache[$cid] ??= ..., very different from what we do here.

    Btw, plain php arrays _do_ have some benefits over general-purpose ArrayAccess objects:
    A php array is passed by value, and is therefore in a way "immutable": If you pass it around to other functions, your own copy of the array won't be manipulated. The same cannot be said about mutable ArrayAccess objects.
    Also there are operations and functions for arrays that don't work with ArrayAccess objects.

    The DX with arrays can be improved with phpstan/psalm advanced types.

    I do like value objects, but these work best with a known and limited list of properties.
    Or, if we do want to support custom properties in addition to known properties, for the custom ones we can simply have ->get($key) instead of [$key].
    This way, when reading the code, we know it is an object.

    The only other reason for ArrayAccess might be for BC support, as a smooth replacement for arrays.
    But this is already limited if the code we want to support uses array type hints, or things like preg_grep() etc.

Production build 0.71.5 2024