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

Update FunctionCallSignatureSniff to allow non-isolated parentheses on multiline calls #330

Conversation

westonruter
Copy link
Contributor

This allows the following code block to be valid if the new property requireIsolatedMultilineCallParentheses is set to false on the PEAR.Functions.FunctionCallSignature sniff:

foreach ( $core_options as $option_name => $real_value ) {
    add_filter( "pre_option_{$option_name}", function( $value ) use ( $real_current_blog_id, $real_value ) {
        if ( get_current_blog_id() == $real_current_blog_id ) {
            return $real_value + 1;
        } else {
            return $value;
        }
    } );
}

See WordPress/WordPress-Coding-Standards#272

@gsherwood
Copy link
Member

I think this option is actually too specific, especially given it sounds generic (closing brace doesnt need to be isolated) but actually it is checking a whitelist of tokens.

But what I can do is stop this indent error from being printed for the closing brace if the closing brace is already throwing an error about not being on a line by itself. Then you can mute the closing brace issue but the indent checks will still work correctly.

What you don't get is the guarantee that the token before the closing brace is a curly brace of parenthesis, but you could write another sniff if you want to specifically check for that.

Does that sound like it would work?

@westonruter
Copy link
Contributor Author

@gsherwood thanks a lot for the reply.

To recap, the problem is with phpcs-fixer checked out, in addition to the WordPress standard's WordPress/WordPress-Coding-Standards#272 being checked out, the following code:

<?php
foreach ( $core_options as $option_name => $real_value ) {
    add_filter( "pre_option_{$option_name}", function( $value ) use ( $real_current_blog_id, $real_value ) {
        if ( get_current_blog_id() == $real_current_blog_id ) {
            return $real_value + 1;
        } else {
            return $value;
        }
    } );
}

Yields the following:

phpcs -s --standard=WordPress-Core test.php
--------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------------
 3 | ERROR | [x] Opening parenthesis of a multi-line function call must be the
   |       |     last content on the line
   |       |     (PEAR.Functions.FunctionCallSignature.ContentAfterOpenBracket)
 9 | ERROR | [x] Multi-line function call not indented correctly; expected 8
   |       |     spaces but found 4
   |       |     (PEAR.Functions.FunctionCallSignature.Indent)
 9 | ERROR | [x] Closing parenthesis of a multi-line function call must be on a
   |       |     line by itself
   |       |     (PEAR.Functions.FunctionCallSignature.CloseBracketLine)
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

Time: 14ms; Memory: 2.5Mb

So you're saying that you could make it so that the PEAR.Functions.FunctionCallSignature.Indent error is suppressed when PEAR.Functions.FunctionCallSignature.CloseBracketLine is present? And then we could explicitly suppress the PEAR.Functions.FunctionCallSignature.ContentAfterOpenBracket and PEAR.Functions.FunctionCallSignature.CloseBracketLine codes via:

    <rule ref="PEAR.Functions.FunctionCallSignature.ContentAfterOpenBracket">
        <severity>0</severity>
    </rule>
    <rule ref="PEAR.Functions.FunctionCallSignature.CloseBracketLine">
        <severity>0</severity>
    </rule>

Is that right?

@westonruter
Copy link
Contributor Author

If so, it sounds perfect.

@gsherwood
Copy link
Member

@westonruter yes, this is what I was suggesting. I'll commit this change so you can test that out.

@gsherwood
Copy link
Member

I've committed this change here: efd8cd9

Please let me know if it doesn't work out for you.

@gsherwood gsherwood closed this Dec 1, 2014
@westonruter
Copy link
Contributor Author

Looking good! Thanks!

@westonruter westonruter deleted the feature/FunctionCallSignatureSniff-requireIsolatedMultilineCallParentheses branch December 1, 2014 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants