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.

Sign up for GitHub

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

Jump to bottom

Duplicate implementation of spacing detection in control structures #385

Closed
Rarst opened this issue May 13, 2015 · 2 comments
Closed

Duplicate implementation of spacing detection in control structures #385

Rarst opened this issue May 13, 2015 · 2 comments
Labels
Type: Enhancement
Milestone
0.7.0

Comments

@Rarst
Copy link
Contributor

Rarst commented May 13, 2015

WPCS currently seems to have spacing in control structure implemented as forked WordPress.WhiteSpace.ControlStructureSpacing sniff.

PHP CS now got Squiz.ControlStructures sniffs which can do this.

I think it would make sense to look into getting rid of the forked sniff (whose maintenance is on us) in favor of upstream implementation?
@JDGrimes
Copy link
Contributor

Yes, we should look into this, and also any other sniffs that have upstream duplicates.

Previously: #236, #237, #244, #251, #258.

JDGrimes added a commit that referenced this issue Jul 29, 2015
@JDGrimes
Use upstream FunctionDeclarationArgumentSpacing sniff
110f804 
The upstream version is now configurable to meet our needs, and also
contains improvements, like being fixable.

See #412, #385
JDGrimes added a commit that referenced this issue Jul 29, 2015
@JDGrimes
Check spacing of functions
8d006be 
This adds functions and closures to the control structure spacing
sniff. The sniff checks for proper spacing on each side of each of the
parentheses. It also checks that the opening brace is on the same line.

I’ve actually improved the sniff while I was at it, to check for excess
space in addition to insufficient space.

Because of these improvements, and due to the non-configurability of
the upstream version of this sniff, this sniff is really not a
duplicate, and so #385 can be closed.

Fixes #412, #385
@JDGrimes JDGrimes mentioned this issue Jul 29, 2015
Merged
@JDGrimes JDGrimes added this to the 0.7.0 milestone Aug 25, 2015
@JDGrimes
Copy link
Contributor

Fixed in #417.

@JDGrimes JDGrimes mentioned this issue Oct 3, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Type: Enhancement
Projects
None yet
Milestone
0.7.0
Development

No branches or pull requests
2 participants
@Rarst @JDGrimes