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

Add sniff and tests for concatenation operator spacing #536

Merged
merged 1 commit into from
Mar 9, 2016

Conversation

samwilson
Copy link
Contributor

This change adds the ConcatenationSpacingSniff, which checks that the concatenation operator is correctly surrounded by spaces. Tests are also included.

The relevant WordPress coding standard reads: "Always put spaces after commas, and on both sides of logical, comparison, string and assignment operators."

Both the sniff and test classes are simply extensions of the Squiz.Strings.ConcatenationSpacing code (which kindly already supported the functionality we require here).

Refs #535.

@westonruter
Copy link
Member

Humm. Looks like the tests fail only when the PHPCS_BRANCH is 2.2.0. Wonder why that is.

@samwilson
Copy link
Contributor Author

Yes, I was just looking into that. I'm a bit confused so far though. Any ideas?

@samwilson
Copy link
Contributor Author

Actually, scratch this whole thing. The right way to do this is perhaps to add the following to WordPress-Core/ruleset.xml

<rule ref="Squiz.Strings.ConcatenationSpacing">
    <properties>
        <property name="spacing" value="1"/>
        <property name="ignoreNewlines" value="true"/>
    </properties>
</rule>

@samwilson
Copy link
Contributor Author

Ok, I've updated the PR. I don't think there's any need to have tests for this, because the spacing behaviour is already tested in PHP_CodeSniffer.

westonruter added a commit that referenced this pull request Mar 9, 2016
Add sniff and tests for concatenation operator spacing
@westonruter westonruter merged commit 35d4359 into WordPress:develop Mar 9, 2016
@westonruter
Copy link
Member

Perfect! 😍

@samwilson
Copy link
Contributor Author

Thanks :)

@jrfnl jrfnl added this to the 0.10.0 milestone Aug 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants