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: FunctionCallSignatureNoParams #972

Merged
merged 1 commit into from Jun 17, 2017

Conversation

Projects
None yet
2 participants
@jrfnl
Contributor

jrfnl commented Jun 17, 2017

Report on and remove whitespace between function call parenthesis when the function call has no parameters.

This sniff will result in contradictory messages when run with PHPCS, but that can't really be helped.

 5 | ERROR | [x] Expected 1 spaces after opening bracket; 6 found
   |       |     (PEAR.Functions.FunctionCallSignature.SpaceAfterOpenBracket)
 5 | ERROR | [x] Expected 1 spaces before closing bracket; 6 found
   |       |     (PEAR.Functions.FunctionCallSignature.SpaceBeforeCloseBracket)
 5 | ERROR | [x] Function calls without parameters should have no spaces
   |       |     between the parenthesis.
   |       |     (WordPress.Functions.FunctionCallSignatureNoParams.WhitespaceFound)

I've chosen to put this sniff in the Functions category to be consistent with the related sniff in the PEAR standard.

Includes an auto-fixer.

This auto-fixer has been tested to not conflict with the PEAR.Functions.FunctionCallSignature sniff (at this time).

Fixes #970

N.B.: I've also opened upstream issue squizlabs/PHP_CodeSniffer#1512 about this, but there are two reasons to pull this PR anyway:

  1. Chances are that a fix for this won't make it into PHPCS 2.x which is where we currently need it.
  2. The inconsistency I reported upstream can be fixed in two different manners. The most consistent way to fix it would be to end up with 1 space between the parenthesis, which is not what we want.

As it is currently unclear how, when and in which branch this will be fixed upstream, pulling this as an interim solution seems appropriate. Depending on if & when this will be addressed upstream and in which branch, this sniff may be removed at a later date or may need adjusting to not conflict with the updated version of the PEAR sniff.

New sniff: FunctionCallSignatureNoParams
Report on and remove whitespace between function call parenthesis when the function call has no parameters.

This sniff *will* result in contradictory messages when run with PHPCS, but that can't really be helped.

```
 5 | ERROR | [x] Expected 1 spaces after opening bracket; 6 found
   |       |     (PEAR.Functions.FunctionCallSignature.SpaceAfterOpenBracket)
 5 | ERROR | [x] Expected 1 spaces before closing bracket; 6 found
   |       |     (PEAR.Functions.FunctionCallSignature.SpaceBeforeCloseBracket)
 5 | ERROR | [x] Function calls without parameters should have no spaces
   |       |     between the parenthesis.
   |       |     (WordPress.Functions.FunctionCallSignatureNoParams.WhitespaceFound)
```

Includes an auto-fixer.

This auto-fixer has been tested to not conflict with the `PEAR.Functions.FunctionCallSignature` sniff.

Fixes issue 970

@jrfnl jrfnl added this to the 0.12.0 milestone Jun 17, 2017

$search[] = T_BITWISE_AND;
$previous = $this->phpcsFile->findPrevious( $search, ( $stackPtr - 1 ), null, true );
if ( T_FUNCTION === $this->tokens[ $previous ]['code'] ) {
// It's a function definition, not a function call.

This comment has been minimized.

@JDGrimes

JDGrimes Jun 17, 2017

Contributor

Why skip if it is a function definition? Shouldn't they also follow this rule?

I'd think that we'd want to catch this as well:

function some_func(   ) {}

Or am I missing a technical consideration?

@JDGrimes

JDGrimes Jun 17, 2017

Contributor

Why skip if it is a function definition? Shouldn't they also follow this rule?

I'd think that we'd want to catch this as well:

function some_func(   ) {}

Or am I missing a technical consideration?

This comment has been minimized.

@jrfnl

jrfnl Jun 17, 2017

Contributor

For a function declaration, this is already reported on via the Squiz.Functions.FunctionDeclarationArgumentSpacing.SpacingBetween sniff/error code.

Excluding function declarations from the sniff in this pull is intended to prevent duplicate messages from being thrown.

@jrfnl

jrfnl Jun 17, 2017

Contributor

For a function declaration, this is already reported on via the Squiz.Functions.FunctionDeclarationArgumentSpacing.SpacingBetween sniff/error code.

Excluding function declarations from the sniff in this pull is intended to prevent duplicate messages from being thrown.

@JDGrimes JDGrimes merged commit 786dce5 into develop Jun 17, 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/issue-970-function-call-no-params branch Jun 17, 2017

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