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

WPCS itself: Disallow long array syntax #1607

Closed
GaryJones opened this issue Dec 25, 2018 · 10 comments
Closed

WPCS itself: Disallow long array syntax #1607

GaryJones opened this issue Dec 25, 2018 · 10 comments

Comments

@GaryJones
Copy link
Member

WPCS now has a dependency of PHP 5.4+, so we can now use the short array syntax.

I'd propose adding Generic.Arrays.DisallowLongArraySyntax to the WPCS .phpcs.xml.dist file, and running phpcbf, to change instances using the array() format to [].

See Automattic/VIP-Coding-Standards#320 for a similar change in VIPCS.

@jrfnl
Copy link
Member

jrfnl commented Dec 25, 2018

I'm ambivalent about this. Using new features is great, but the short array syntax has no advantage over the long syntax, other than possibly decreasing the readability.

@jrfnl jrfnl changed the title Disallow long array syntax WPCS itself: Disallow long array syntax Jan 14, 2019
@jrfnl
Copy link
Member

jrfnl commented Oct 7, 2019

@GaryJones As the WP Coding Standards have now banned short arrays (for the time being) and WPCS has an example function, I think we should stick with long arrays for now until such a time that the coding standards change again to allow the use of short arrays.

I suggest closing this issue for now.

@Micu22
Copy link

Micu22 commented Jan 4, 2021

this is ugly, short syntax is both easier and more readable, because of the closing parenthesis. ")" is way less readable than "]"

@dingo-d
Copy link
Member

dingo-d commented Jan 4, 2021

You can comment here: https://make.wordpress.org/core/2019/07/12/php-coding-standards-changes/ and in the core chat if this ever comes up, but for now, nothing we can do about it unfortunately 🤷🏼‍♂️

@seothemes
Copy link

Still no short array syntax? This should really be reopened.

Given that JS uses short array syntax it doesn't make any sense not to support it.

@dingo-d
Copy link
Member

dingo-d commented Sep 19, 2022

Yeah, we can reopen this (and several other debates regarding CS) regarding code style once we're done with WPCS 3.0.0 release🙂

@seothemes
Copy link

@dingo-d nice, that is good to hear!

@owaisahmed5300
Copy link

I think short syntax must be allowed as it is 2023 and PHP 8.2+ has been released. There is no reason to keep this sniff.

@jrfnl
Copy link
Member

jrfnl commented Jun 24, 2023

@owaisahmed5300 Those are not reasons to allow something.

As @dingo-d mentioned above, this will need a Make post and consensus from the WP Core team.

Chances of this change in the coding standards being allowed are slim as:

  1. It will cause a lot of code-churn in WP Core.
  2. The difference it will cause in coding style between "current" WP and older versions of WP will make it harder to backport security fixes.
  3. Opinions about which is better for code readability are very split, so chances of reaching consensus are small.

In the mean time, anyone using the WordPress Coding Standards for non-WP Core work can just ignore the rule or even choose to enforce the opposite (short syntax) via their own custom ruleset:

For WPCS 2.x:

<rule ref="WordPress">
    <exclude name="Generic.Arrays.DisallowShortArraySyntax"/>
</rule>

<!-- Optional: enforce short arrays. -->
<rule ref="Generic.Arrays.DisallowLongArraySyntax"/>

For WPCS 3.x:

<rule ref="WordPress">
    <exclude name="Universal.Arrays.DisallowShortArraySyntax"/>
</rule>

<!-- Optional: enforce short arrays. -->
<rule ref="Generic.Arrays.DisallowLongArraySyntax"/>

@GaryJones
Copy link
Member Author

Updating the closing as Not Planned rather than Completed.

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

6 participants