Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Use list over array in more places #7905

Merged
merged 22 commits into from
Mar 27, 2024
Merged

Conversation

keradus
Copy link
Member

@keradus keradus commented Mar 23, 2024

No description provided.

@coveralls
Copy link

coveralls commented Mar 23, 2024

Coverage Status

coverage: 96.035%. remained the same
when pulling e6543f6 on keradus:list_aaa
into 3159e49 on PHP-CS-Fixer:master.

@kubawerlos
Copy link
Contributor

Can we add a rule to .php-cs-fixer.dist.php to enforce this?

@Wirone
Copy link
Member

Wirone commented Mar 23, 2024

Exactly: add the rule, apply, fix PHPStan issues if any occur, forget.

@keradus
Copy link
Member Author

keradus commented Mar 25, 2024

as I was describing somewhere in first of PRs in this style - that's the goal.

yet, we cannot blindly replace foo[] or array<foo> into list<foo>, as sometimes we need to use array<key, foo>. I also do not want to apply array<key-type, foo> (as not giving any more value than array<foo>.

auto-apply here cannot do this, but when we are cleaned-up, enabling the rules can at least prevent new cases

@keradus
Copy link
Member Author

keradus commented Mar 25, 2024

example where blind application would apply wrong type, from very this PR:
https://github.com/PHP-CS-Fixer/PHP-CS-Fixer/pull/7905/files#diff-f0f5829e3bd024ac8f67d5e42dc491600f7bde0a372793a1da755c1549369838R506


or newest commit 993d01e

@keradus
Copy link
Member Author

keradus commented Mar 25, 2024

PHPStan

as mentioned in previous PR, PHPStan is not detecting this type of issue - list<foo> vs array<string, foo> (assoc array, non-list), unfortunately - proof with falsy commit commit / ci

if you know how to do it, please show the PHPStan failing on the mentioned example

@Wirone Wirone added kind/refactoring topic/phpdoc PHPDoc tags, Doctrine Annotations etc. labels Mar 27, 2024
@Wirone Wirone changed the title chore: list over array in more places chore: Use list over array in more places Mar 27, 2024
@Wirone Wirone merged commit 9782155 into PHP-CS-Fixer:master Mar 27, 2024
26 checks passed
@keradus keradus deleted the list_aaa branch March 27, 2024 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/refactoring topic/phpdoc PHPDoc tags, Doctrine Annotations etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants