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

:sparkles: New sniff to verify correct array indentation #941

Merged
merged 3 commits into from May 6, 2017

Conversation

Projects
None yet
3 participants
@jrfnl
Contributor

jrfnl commented May 6, 2017

  • Multi-line arrays should be indented by one tab from the beginning of the line containing the array opener.
  • The array closer should be level with the indentation of the line containing the array opener.

While the ScopeIndent sniff checks this type of alignment for most structures, it turns out it doesn't do so for arrays (I suspect this is because PHPCS itself prefers a different array style).

This PR fixes that by adding a new sniff. Both the errors this sniff throws have been made fixable.

The new sniff has been added to the WordPress-Core ruleset.

Violations against this sniff in the WPCS codebase have been fixed.

N.B.: I consider this sniff a candidate for pulling upstream to Generic in due time.

@jrfnl jrfnl added this to the 0.12.0 milestone May 6, 2017

@jrfnl

This comment has been minimized.

Show comment
Hide comment
@jrfnl

jrfnl May 6, 2017

Contributor

Hmm.. something weird going on with tabs vs spaces. Trying to fix it now, but can't reproduce the travis results locally. Never mind. Fixed.

Contributor

jrfnl commented May 6, 2017

Hmm.. something weird going on with tabs vs spaces. Trying to fix it now, but can't reproduce the travis results locally. Never mind. Fixed.

Show outdated Hide outdated WordPress/Sniffs/Arrays/ArrayIndentationSniff.php
* Determine the indentation of the line containing the array opener.
*/
$indentation = '';
$column = 1;

This comment has been minimized.

@grappler

grappler May 6, 2017

Contributor

The alignment is a bit off here.

@grappler

grappler May 6, 2017

Contributor

The alignment is a bit off here.

This comment has been minimized.

@JDGrimes

JDGrimes May 6, 2017

Contributor

Looks to be an issue of tabs instead of spaces.

@JDGrimes

JDGrimes May 6, 2017

Contributor

Looks to be an issue of tabs instead of spaces.

This comment has been minimized.

@JDGrimes

JDGrimes May 6, 2017

Contributor

Note that some assignments below are also aligned via tabs, but should just use spaces, IMO.

@JDGrimes

JDGrimes May 6, 2017

Contributor

Note that some assignments below are also aligned via tabs, but should just use spaces, IMO.

This comment has been minimized.

@jrfnl

jrfnl May 6, 2017

Contributor

It was & is now fixed.

Yet another thing for which I'm wondering why there is no sniff to auto detect & fix this ;-)

@jrfnl

jrfnl May 6, 2017

Contributor

It was & is now fixed.

Yet another thing for which I'm wondering why there is no sniff to auto detect & fix this ;-)

This comment has been minimized.

@jrfnl

jrfnl May 6, 2017

Contributor

Right, sorted: #942

@jrfnl

jrfnl May 6, 2017

Contributor

Right, sorted: #942

jrfnl added some commits May 6, 2017

New sniff to verify correct array indentation
* Multi-line arrays should be indented by one tab from the beginning of the line containing the array opener.
* The array closer should be level with the indentation of the line containing the array opener.

While the `ScopeIndent` sniff checks this type of alignment for most structures, it turns out it doesn't do so for arrays (I suspect this is because PHPCS itself prefers a different array style).

This PR fixes that by adding a new sniff. Both the errors this sniff throws have been made fixable.

The new sniff has been added to the `WordPress-Core` ruleset.

@JDGrimes JDGrimes merged commit fea1faa into develop May 6, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@JDGrimes JDGrimes deleted the feature/new-array-indent-sniff branch May 6, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment