WordPress / WordPress-Coding-Standards

PHP_CodeSniffer rules (sniffs) to enforce WordPress coding conventions
MIT License
2.56k stars 485 forks source link

WPCS itself: Disallow long array syntax #1607

Closed GaryJones closed 5 years ago

GaryJones commented 5 years ago

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 https://github.com/Automattic/VIP-Coding-Standards/pull/320 for a similar change in VIPCS.

jrfnl commented 5 years ago

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 commented 5 years ago

@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 commented 3 years ago

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

dingo-d commented 3 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

owaisahmed5300 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

Updating the closing as Not Planned rather than Completed.