-
-
Notifications
You must be signed in to change notification settings - Fork 523
Arrays/ArrayDeclarationSpacing: replace "associative" with "explicit keys" #2688
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
Arrays/ArrayDeclarationSpacing: replace "associative" with "explicit keys" #2688
Conversation
…keys" Updates one error message and docblocks to use "arrays with explicit keys" instead of "associative arrays" to align with the PHP handbook terminology. The sniff has always checked for arrays with explicit keys (whether string or numeric), not just associative arrays with string keys. This change aligns the wording with the actual behavior. The public property `allow_single_item_single_line_associative_arrays` and the error code `AssociativeArrayFound` are intentionally left unchanged to avoid breaking changes for users who reference them in their ruleset configurations. Ref: wpcs-docs 156, wpcs-docs 157
jrfnl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for opening this PR @rodrigoprimo ! I left two small suggestions inline.
I opted not to change the public property
allow_single_item_single_line_associative_arraysand the error codeAssociativeArrayFoundto avoid breaking changes for users who reference them in their ruleset configurations. I'm unsure if the pros of changing (better alignment with terminology) outweigh the cons (breaking change).
I believe the property and the error code should be considered separately.
We could add a new property with a more appropriate name and deprecate the old property. This would not be a breaking change.
This would mean handling both properties until the 4.0 release, but that shouldn't be much of a problem, and then removing the allow_single_item_single_line_associative_arrays property in the 4.0 release.
We can't do this for the error code as that would be a breaking change, so I agree that it's better not to change the error code at this time.
What you could consider, would be to open an issue for the 4.0 release to change the error code in that release.
Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>
Thanks, @jrfnl! I added the two suggestions that you made.
That works for me. I created an issue to get feedback on how to deprecate the property (#2691) and another to rename the error code before the WPCS 4.0 release (#2692). |
jrfnl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for opening those issues.
This PR is good to go as far as I'm concerned.
|
Just documenting here that I updated the https://github.com/WordPress/WordPress-Coding-Standards/wiki/Customizable-sniff-properties wiki page to use "explicit keys" instead of "associative" as well in the section related to this sniff. |
Description
Updates one error message and docblocks in the
WordPress.Arrays.ArrayDeclarationSpacingsniff to use "arrays with explicit keys" instead of "associative arrays" to align with the new PHP handbook terminology (see WordPress/wpcs-docs#156, WordPress/wpcs-docs#157, and #2682 (review)).The sniff has always checked for arrays with explicit keys (whether string or numeric), not just associative arrays with string keys. This change aligns the wording with the actual behavior.
I opted not to change the public property
allow_single_item_single_line_associative_arraysand the error codeAssociativeArrayFoundto avoid breaking changes for users who reference them in their ruleset configurations. I'm unsure if the pros of changing (better alignment with terminology) outweigh the cons (breaking change).Suggested changelog entry
Changed:
WordPress.Arrays.ArrayDeclarationSpacing: theAssociativeArrayFounderror message now uses "explicit keys" instead of "associative keys".Related issues/external references