Problem/Motivation

The PHP Manual seems to imply that when the list() function skips an array value the corresponding position should be occupied by a single space and a comma. It gives this example:

$info = array('coffee', 'brown', 'caffeine');
// Let's skip to only the third one
list( , , $power) = $info;
echo "I need $power!\n";

This might be seen as conflicting with our policy that opening parentheses should NOT be followed by any spaces. Can we define an explicit policy and add it to the documentation? Thanks!

Benefits

If we adopted this change, the Drupal Project would benefit by ...

Three supporters required

  1. https://www.drupal.org/u/drunken-monkey (2024-06-22)
  2. https://www.drupal.org/u/kingdutch (2024-07-03)
  3. https://www.drupal.org/u/{userid} (yyyy-mm-dd they added support)

Proposed changes

Provide all proposed changes to the Drupal Coding standards. Give a link to each section that will be changed, and show the current text and proposed text as in the following layout:

1. New subsection “Array unpacking” under PHP coding standards » Arrays

Current text

(None)

Proposed text

To assign variables to an array, use the […] = syntax. When skipping the first entry, there should be no space between the opening bracket [ and the comma ,.

Examples:

$stars = ['blue', 'yellow', 'red'];

[$hottest, $hotter, $coldest] = $stars;
[$hottest] = $stars;
[, , $coldest] = $stars;
[1 => $hotter] = $stars;

Remaining tasks

  1. Create this issue in the Coding Standards queue, using the defined template
  2. Add supporters
  3. Create a Change Record
  4. Review by the Coding Standards Committee
  5. Coding Standards Committee takes action as required
  6. Discussed by the Core Committer Committee, if it impacts Drupal Core
  7. Final review by Coding Standards Committee
  8. Documentation updates
    1. Edit all pages
    2. Publish change record
    3. Remove 'Needs documentation edits' tag
  9. If applicable, create follow-up issues for PHPCS rules/sniffs changes

For a full explanation of these steps see the Coding Standards project page

Comments

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Issue tags: +Coding standards

tagging

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

Here are a couple examples from core:

  1. modules/field/field.attach.inc:191: list(, , $bundle) = entity_extract_ids($entity_type, $entity);
  2. modules/field/field.form.inc:15: list($id, , ) = entity_extract_ids($entity_type, $entity);
  3. modules/field/field.module:911: list($id) = entity_extract_ids($entity_type, $entity);

I think #3 is definitely wrong as well.

chx’s picture

chx commented

Nope, it's not wrong. Compare 1 to 3. But, I think 2 needs those commas removed for consistency. As for the issue itself, I disagree. There is nothing in PHP manual that would talk about spaces. There is no need for list parenthesis space comma -- list parenthesis comma is just fine.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

Then that can be our documented standard. This issue title only says we have to have one, not that we have to follow what is suggested in the OP.

And if list($id) is preferred over list($id, , ), then we should document that too.

traviscarden’s picture

traviscarden commented
Issue summary: View changes

Softened assertion. :-P

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Project: Drupal core » Drupal Technical Working Group
Version: 8.0.x-dev »
Component: documentation » Documentation
Issue summary: View changes

Coding standards decisions are now supposed to be made by the TWG

tizzo’s picture

tizzo commented
Project: Drupal Technical Working Group » Coding Standards

Moving this issue to the Coding Standards queue per the new workflow defined in #2428153: Create and document a process for updating coding standards.

quietone’s picture

quietone commented
Issue summary: View changes

Discussed at #3456119: Coding Standards Meeting Tuesday 2024-06-18 2100 UTC and there was agreement that this is something to pursue. I have updated the IS with the new template.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Issue summary: View changes

I propose using the following syntax, and explicitly encouraging (or even requiring) [] over list():

[$country, $state, $city] = $this->getPlaceInfo();
[$country] = $this->getPlaceInfo();
[, , $city] = $this->getPlaceInfo();
['city' => $city] = $this->getPlaceInfoAssociative();
quietone’s picture

quietone commented

That looks like what needs to be on the left side of the assignment. But I think we should have a simpler example, the current suggestion expects the reader to know what $this->getPlaceInfo() and $this->getPlaceInfoAssociative(). And I would like to have an example that is inclusive regardless of where people live or work.

quietone’s picture

quietone commented

What about using stars?

$stars = ['blue', 'yellow', 'red'];

[$hottest, $hotter, $coldest] = $stars;
[$hottest] = $stars;
[, , $coldest] = $stars;
kingdutch’s picture

kingdutch
he/him
Primary language English
commented

I'm happy to second quietone's example since it's understandable for people who may have never seen this syntax (you can see $stars is an array).

I initially wasn't sure about not have a space before the first command (proposed [, , $foo] vs my brain's [ , , $foo]) but after writing them out and trying to change them, the proposed spaceless first comma is closest to how it would look if there was a variable, since in that case there would be a variable there and not space. So you can go from [, , $foo] to [$bar, , $foo] without having to remove any characters. That makes sense to me.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented

I fully agree with #11, I think that example makes sense.

quietone’s picture

quietone commented
Issue summary: View changes

Thanks. I updated the Issue summary with the 'stars' example. I also modified the text a) so it only shows what to do b) uses similar language to the php manual page for lists and c) remove parentheses around characters to be consistent with the coding standards page.

quietone’s picture

quietone commented

I did some testing.

No errors on these lines:

    list(, , $coldest) = $stars;
    [$hottest, $hotter, $coldest] = $stars;
    [, , $coldest] = $stars;

Errors on these lines

[ $hottest] = $stars; ERROR | [x] There should be no white space after an opening "["
[ , , $coldest ] = $stars; ERROR | [x] There should be no white space after an opening "[" and ERROR | [x] There should be no white space before a closing "]"

So, perhaps the sniff just needs to detect 'list(.*)'?

kingdutch’s picture

kingdutch
he/him
Primary language English
commented
Issue summary: View changes

Dated as per #1539738-11: list() syntax

catch’s picture

catch
he/him
Primary language English
commented

Does this now need to cover the shorthand array destructuring syntax?

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented

Thanks, @quietone, I agree that doing this without an invented method makes more sense.
The new example just has the slight downside that it doesn’t include an example for destructuring an associative array.
Do you think just adding [1 => $hotter] = $stars; would help, maybe preceded by a comment explaining that this is how you’d destructure an associative array? Or do we add a second, associative array? (Or is the majority against adding an example for associative destructuring?)

Does this now need to cover the shorthand array destructuring syntax?

Could you please elaborate, what syntax do you mean?

quietone’s picture

quietone commented
Title: Define and document a policy on list() syntax » list() syntax
Component: Documentation » Coding Standards
Issue summary: View changes

Extra examples are usually beneficial so I added that. I don't think it necessary to do more because the style and spacing doesn't change.

catch’s picture

catch
he/him
Primary language English
commented

@drunken_monkey:

I mean:

$array = ['foo', 'bar'];

[$foo, $bar] = $array;

See https://stitcher.io/blog/array-destructuring-with-list-in-php