Skip to content

chore: enforce list via array_values()#9054

Merged
gharlan merged 6 commits intoPHP-CS-Fixer:masterfrom
gharlan:enforce-list
Sep 22, 2025
Merged

chore: enforce list via array_values()#9054
gharlan merged 6 commits intoPHP-CS-Fixer:masterfrom
gharlan:enforce-list

Conversation

@gharlan
Copy link
Copy Markdown
Member

@gharlan gharlan commented Sep 13, 2025

No description provided.

@coveralls
Copy link
Copy Markdown

coveralls commented Sep 13, 2025

Coverage Status

coverage: 94.406% (+0.01%) from 94.396%
when pulling 58e1ca3 on gharlan:enforce-list
into d4123ea on PHP-CS-Fixer:master.

$riskyFixers = array_map(
static fn (FixerInterface $fixer): string => $fixer->getName(),
array_filter(
array_values(array_filter(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just curious, as I was always told, would this cause another array copy operation (next to the one done by array_filter)?

Copy link
Copy Markdown
Member Author

@gharlan gharlan Sep 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would this cause another array copy operation (next to the one done by array_filter)?

I think so. array_values uses zend_array_to_list. It creates a new array and adds all values in a loop.

But without array_values we don't have a list. And we pass the array to methods requiring a list. The methods also work with non-list array, but phpdoc says they want a list.

So I think we should either actually passing lists or relaxing the param type to array. I think it is ok to use array_values in the given cases.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so. array_values uses zend_array_to_list. And it creates a new array and adds all values in a loop.

But note that zend_array_to_list is only called if it is not already a list.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the detailed explanation and reasoning.

# Conflicts:
#	dev-tools/phpstan/baseline/argument.type.php
# Conflicts:
#	dev-tools/phpstan/baseline/argument.type.php
@gharlan gharlan enabled auto-merge (squash) September 22, 2025 09:44
@gharlan gharlan merged commit 2fe0fcc into PHP-CS-Fixer:master Sep 22, 2025
30 of 32 checks passed
@gharlan gharlan deleted the enforce-list branch September 22, 2025 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants