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

Universal.Arrays.MixedArrayKeyTypes should report on explicit keys, if implicit keys are the majority #279

Closed
1 task
kkmuffme opened this issue Oct 6, 2023 · 2 comments

Comments

@kkmuffme
Copy link

kkmuffme commented Oct 6, 2023

Is your feature request related to a problem?

array(
'hello' => 'world',
'foo',
'bar',
)

phpdoc for code above could be array<string> or string[]

The error reports for all lines that have a numeric key (implicit or explicit).
However in the above case - if there are more implicit keys, than there are explicit - often the string key can be removed completely, instead of adding an explicit key for the majority of array elements.
This has the advantage, that the type can then also be specified more narrowly as list<string> which is shorter and has more useful properties compared to array<string, string> which this error wants to get.

Describe the solution you'd like

If the majority of keys is implicit, report an error for the explicit keys only.

Additional context (optional)

This also makes sense for Universal.Arrays.MixedKeyedUnkeyedArray

  • I intend to create a pull request to implement this feature.
@jrfnl
Copy link
Member

jrfnl commented Oct 8, 2023

@kkmuffme That is not what this sniff is about. This sniff flags arrays with items which result in a mix of numeric and string keys. The sniff has no opinion on what type of keys the array should have. It only demands that the key type of the array is consistent: either all numeric keys, whether implicitly/explicitly set, or all string keys.

An array like the below will, for instance, not be flagged by this sniff as all items will end up having an integer key:

$array = array(
    'a',
    2 => 'b',
    '3' => 'c',
    4 => 'd',
);

often the string key can be removed completely, instead of adding an explicit key for the majority of array elements.

That is a perfectly valid way to solve the error this sniff throws, but it is not for this sniff to have an opinion on whether that is the preferred way to solve the error.

This feature request seems to be more closely related to the MixedKeyedUnkeyedArray sniff which demands that all array items have either an explicit key or an implicit key, but flags arrays which have a mix of both.

@jrfnl
Copy link
Member

jrfnl commented Dec 28, 2023

Closing for lack of response.

@jrfnl jrfnl closed this as not planned Won't fix, can't repro, duplicate, stale Dec 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants