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

Rule to force syntax of array notation in PHPDocs #4698

Closed
julienfalque opened this issue Dec 13, 2019 · 11 comments · Fixed by #7812
Closed

Rule to force syntax of array notation in PHPDocs #4698

julienfalque opened this issue Dec 13, 2019 · 11 comments · Fixed by #7812
Labels
contribution welcome Core team is looking for a contribution easy pick has userland alternative What's requested is already possible with userland packages kind/feature request topic/phpdoc PHPDoc tags, Doctrine Annotations etc.

Comments

@julienfalque
Copy link
Member

In PHPDocs, there are two possible syntaxes for array values:

/**
 * @param Foo[] $arrayOfFoos
 */
/**
 * @param array<Foo> $arrayOfFoos
 */

For consistence I think it would be nice to have a rule that forces a single syntax.

@Wirone
Copy link
Member

Wirone commented Apr 4, 2023

@Wirone Wirone added topic/phpdoc PHPDoc tags, Doctrine Annotations etc. easy pick contribution welcome Core team is looking for a contribution labels May 16, 2023
@ruudk
Copy link
Contributor

ruudk commented May 26, 2023

For what it's worth: Since 8.1 we have the concept of list in PHP.

That makes Foo[] ambiguous because is it could be:

  • a list
  • an array where the key is important

So to me, that only makes the following flavors possible:

/**
 * @param array<string, Foo> Key is important, and it's a string
 * @param array<int, Foo>    Key is important, and it's an integer
 * @param list<Foo>          It's a list, only values are important, everything is indexed from 0
 */

I hope to find a rule that automatically converts Foo[] to list<Foo>. Then, PHPstan will propably fail when it's not what you want. It will move you towards changing it to array<type-of-the-key, Foo>, where you have to think about what the type of the key is.

@Wirone
Copy link
Member

Wirone commented May 26, 2023

@ruudk thank you for the input! Personally I am not sure if it matters that much, list<Foo> is almost the same as array<Foo> (implicit int keys) or array<int, Foo> with a slight difference you can be sure that integers in keys consecutive numbers from 0 up. Mentioned fixer from @kubawerlos does minimal job and keeps exact same intent, because in Foo[] you also assume int keys in an array and you don't know if it's a list - it's just an iterable collection of Foo. I believe it could be done as strategy within that fixer - being able to use array or list for initial conversion 🙂.

@github-actions

This comment was marked as outdated.

This comment has been minimized.

@Wirone Wirone added has userland alternative What's requested is already possible with userland packages and removed status/stale labels Nov 25, 2023
@keradus
Copy link
Member

keradus commented Jan 2, 2024

myself, I would love to forbid usage of Foo[] or array<Foo> and instead use list<Foo>, iterable<Foo>, array<keyType, Foo> or array{Foo, Foo, Foo} (tuple).

array is very overpacked and one cannot be sure what they will deal with, when receiving it..


above is different than normalization of Foo[] <> array<Foo>. I see possibility for that unification, but with my input on top, I would skip step of that unification and start migrating away from overpacked array metatype

@Wirone
Copy link
Member

Wirone commented Jan 3, 2024

@keradus I believe that your preference is out of the scope of this particular issue / feature request. Even though it may be valid, it's something more strict that not everyone in userland is interested in. This issue is about standardising the format of array types in phpDoc, not about making it more strict, which is IMHO a separate thing, more for tools like PHPStan than Fixer.

@keradus
Copy link
Member

keradus commented Jan 3, 2024

you can treat my comment as:

  • eagerness of having even stronger rule instead of one proposed
  • not preventing originally proposed rule to be going in
    • yet... if, for example, I were implementing sth myself, i would only implement the stronger rule (Because of my preference)

yes, we can keep original idea of unification open, no harm in it. just no commitment into implementing it by community since 2019 🤷🏻

@Wirone
Copy link
Member

Wirone commented Jan 3, 2024

just no commitment into implementing it by community since 2019 🤷🏻

Actually it was implemented 2 years ago, but not in the core 😉. Why it was done like this is a separate story.

@keradus
Copy link
Member

keradus commented Jan 3, 2024

if it matters in context of main repo, we can either have own implementation (not sure if makes sense), incorporate custom rule to main repo (suggested for rules in general few times, some owners raised some of rules to be incorporated to main repo ;) ), or say it's not worth to implement due to userland solution.

I doubt it makes sense to keep this idea open, if we are not progressing on it for so many years. let's be realistic :)

@Wirone
Copy link
Member

Wirone commented Jan 4, 2024

I doubt it makes sense to keep this idea open, if we are not progressing on it for so many years. let's be realistic :)

I agree, I kept this issue open only because I was hoping that @kubawerlos is open to migrate this fixer to the core (himself, or by letting others to do it), that's why it has "easy pick" label. If it's not the case, then I believe we should close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution welcome Core team is looking for a contribution easy pick has userland alternative What's requested is already possible with userland packages kind/feature request topic/phpdoc PHPDoc tags, Doctrine Annotations etc.
Projects
None yet
4 participants