mirror of
https://github.com/doctrine/orm.git
synced 2026-03-23 22:42:18 +01:00
Consistent use of (psalm) phpdoc array annotation syntax #6832
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @simonberger on GitHub (Sep 20, 2021).
My last state of information was that only a notation like
string[]should be used for annotations likeparamandreturn.I was about to request a change in #9024 but then noticed that those few lines of changed lines revealed three different types of array annotations in the old code:
array<string>array<string, string>string[]It would be great to have a rule which array syntax is allowed for standard phpdoc annotation and which needs to be put into the psalm variants for more detailed description.
Personally I would prefer to just allow
array<string, string>- kind syntax everywhere but then we should move them all out of the psalm variants. Otherwise some sort of CI check would be helpful to get some consistency.As the first and third syntax of the example are the same it would be great to allow just one but that's maybe too pedantic.
@phansys commented on GitHub (Sep 20, 2021):
IMO, the syntax
array<type, type>is useful when we must tell the type used in the keys. For the rest of the cases, I think thetype[]syntax is simpler.@simonberger commented on GitHub (Sep 21, 2021):
@phansys
array<type, type>has more detail this is not really questionable. The issue is about what is allowed to be used with standard PHPdoc annotations and when psalm-annotations have to be used.@greg0ire commented on GitHub (Sep 21, 2021):
IMO it should be
array<type, type>orlist<type>if the type inside the list doesn't matter. It's kinda unfriendly to consumers that pass the result ofarray_filter, but usinglistshows that we already looked into the type of the keys, and that it shouldn't matter. IMO, any time we would be using a list in another language that supports them (such as Python), we should uselisthere. So I'd say, let's avoidtype[]entirely.@stof commented on GitHub (Oct 21, 2021):
list<type>does not mean that you don't care about the type of keys. It means you actually want a list, i.e. sequencial integer keys from0to$count - 1(seearray_is_listin PHP 8.1). Psalm actually enforces that meaning forlist(phpstan just treats it as an alias ofarray<int, type>for now)@greg0ire commented on GitHub (Oct 21, 2021):
@stof yeah but conceptually, it's called a list because that's exactly the data type that is meant (not required, meant) to be passed here. If you use the PHP syntax to create a list, you will end up with an array that happens to keys in a sequence.
So while on the one hand, using lists add an unwanted requirement, on the other hand, it lets the user know that they should not expect anything special to happen if they pass a map instead, and might even let them know they have a bug in their code because of that expectation.
@stof commented on GitHub (Oct 22, 2021):
@greg0ire if we really don't care about the keys (some places do care), I'd rather use
type[]thanlist<type>