feat: introduce PhpdocListTypeFixer

[kubawerlos]

Extracted, as per #7781 (comment)

[coveralls]

coverage: 94.752% (+0.002%) from 94.75%
when pulling ea0a447 on 6b7562617765726c6f73:add_list_fixer
into d12282c on PHP-CS-Fixer:master.

[keradus]

❤️

[bendavies]

This change is a touch annoying.

array<T> is a well defined, well understood type, by static analysis tooling.

array<T> is the same as <array<array-key, T>>
array<T> is not the same as list<int>

https://phpstan.org/r/0f491ccd-f33a-448f-8824-69f198b24477

Converting array<T> to list<int> is just plain wrong.
There is nothing "risky" about it. just wrong.

If you want to have an opinionated fixer that disallows array<T>, then you should convert it to the equivalent type <array<array-key, T>>

[kubawerlos]

array<T> is not the same as list<int>

Has anyone, anywhere claimed that they are the same? Where have you been when rule php_unit_strict was introduced to prevent it, as clearly, assertions assertEquals and assertSame are not the same 😉

Converting array<T> to list<int> is just plain wrong.

Have you ever considered that there are people in the world that can disagree with this statement? To name some - that would be @keradus (he explained it very well here) and myself.

This change is a touch annoying.

The most important, and quite concerning question: is anyone forcing you to use this rule?

[theofidry]

FWIW I do think that forcing a legitimate T[], which is understood everywhere, into <array<array-key, T>>, which is a convention of some static analyser, is not a good idea. Moreover it does feel this would be something to enforce by a static analyser rather than a CS fixer.

But in the end you are the ones maintaining it so ¯_(ツ)_/¯

[bendavies]

Converting array<T> to list<int> is just plain wrong.

Have you ever considered that there are people in the world that can disagree with this statement? To name some - that would be @keradus (he explained it very well here) and myself.

Um, clearly I don't have to consider it, because this PR shows that someone disagrees with me.
That is fine, are we not allowed to discuss things?
Am I also not allowed to have a voice?

Converting array<T> to list<int> is just plain wrong, in my opinion.
This is like writing a fixer that converts all int type hints to float.

This change is a touch annoying.

The most important, and quite concerning question: is anyone forcing you to use this rule?

I'll ignore this straw-man argument.

Like I said, if you want to have an opinionated fixer that disallows the short form array<T>, which is reasonable, then you should convert it to the equivalent type <array<array-key, T>>, rather than list<T>

[Wirone]

I am not a fan of this rule, but honestly I don't understand the reception it gets. It is a rule marked explicitly as risky, not included in any rule set, meant to be used as an opt-in when someone needs to standardise such a particular thing in the project. It may or may not be used, just like many other rules 🤷‍♂️😅.

[bendavies]

meant to be used as an opt-in when someone needs to standardise such a particular thing in the project

That's exactly my argument - there is a better standardisation that can be made: <array<array-key, T>>

[kubawerlos]

Converting array<T> to list<int> is just plain wrong, in my opinion, and that of PHPStan and Psalm.

Can you point me to a place where PHPStan and Psalm state that?

Like I said, if you want to have an opinionated fixer that disallows the short form array<T>, which is reasonable, then you should convert it to the equivalent type <array<array-key, T>>, rather than list<T>

Are you an eternal authority, to tell people what they "should" do? Because this is how it sounds. The issue was opened over 4 years ago and no one proposed anything, and then, when I did, the hell broke loose.

I understand that you want a different preference, and I have nothing against it (nothing really stops you from raising PR with a configuration option to this rule).

Can't you understand that other people have different preferences, and those are not wrong because they are different from your preferences?

[bendavies]

Converting array<T> to list<int> is just plain wrong, in my opinion, and that of PHPStan and Psalm.

Can you point me to a place where PHPStan and Psalm state that?

I edited that part of my message to remove PHPStan and Psalm a while ago.
What I meant was that phpstan/psalm don't think that array<T> and list<int> are equivalent.
I removed it because, that is not your position either.

Like I said, if you want to have an opinionated fixer that disallows the short form array<T>, which is reasonable, then you should convert it to the equivalent type <array<array-key, T>>, rather than list<T>

Are you an eternal authority, to tell people what they "should" do? Because this is how it sounds. The issue was opened over 4 years ago and no one proposed anything, and then, when I did, the hell broke loose.

Not sure what you are trying to say, sorry.
I am not trying to position my self as someone with authority, just someone with an opinion.
Again, you are arguing with straw mans.

I understand that you want a different preference, and I have nothing against it (nothing really stops you from raising PR with a configuration option to this rule).

👍

Can't you understand that other people have different preferences, and those are not wrong because they are different from your preferences?

I can. I don't know why you keep asking me this.

Thanks.

[Wirone]

That's exactly my argument - there is a better standardisation that can be made: <array<array-key, T>>

Not when your goal is enforcing list where applicable 😉. With this rule you can do it automatically, check with SA, and manually fix problems if needed. The background for this rule comes from @keradus himself, who wanted list<> being standard in this project (where applicable, obviously). For expanding array<T> to array<array-key, T> may be another rule 🙂.

[bendavies]

Not when your goal is enforcing list where applicable

that's fair, thanks

[kubawerlos]

What I meant was that phpstan/psalm don't think that array<T> and list<int> are equivalent.

I also know that they are NOT equivalent, I wrote that, like 5 times this week, in issues around here 😉. I never claimed they were the same.

Can't you understand that other people have different preferences, and those are not wrong because they are different from your preferences?

I can. I don't know why you keep asking me this.

Because you wrote:

If you want to have an opinionated fixer that disallows array<T>, then you should convert it to the equivalent type <array<array-key, T>>

This sends a message that the only current solution is the one suggested. If you change "should" to "could" that would be a different story.

[kubawerlos]

For expanding array<T> to array<array-key, T> may be another rule 🙂.

@Wirone I'd rather see it as an option in this fixer (probably by deprecating this one and having a new name for it).

[Wirone]

For expanding array<T> to array<array-key, T> may be another rule 🙂.

@Wirone I'd rather see it as an option in this fixer (probably by deprecating this one and having a new name for it).

@kubawerlos I don't think it's a good idea, because that kind of change is not risky and it would be better to use it in regular flow.

[kubawerlos]

Yeah, right, I forgot this one is risky.

[keradus]

for safe convention we have #7781 , that's remain the "same same type" (and non-risky).

We all know the rule in this PR is risky as it changes meaning of code. That's the definition of risky fixer and we explicitly claimed so in it's description. It's not supposed to be blindly use by anyone.
Big thanks for bringing this rule to core repo @kubawerlos , it's my favourite rule this week.