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

Squiz.Functions.FunctionDeclarationSpacing error for multi-line declarations with required spaces greater than zero #658

Merged
merged 1 commit into from Aug 20, 2015

Conversation

JDGrimes
Copy link
Contributor

This fixes a bug in the Squiz.Functions.FunctionDeclarationSpacing
sniff when the requiredSpacesAfterOpen option was set to anything
other than 0.

When the first argument in a multiline function declaration had a
type-hint, this error would be given:

Expected 1 spaces between opening bracket and type hint "array"; 0
found

I don’t think this check was ever intended to affect multiline
functions. The logic was such that it never does when
requiredSpacesAfterOpen is 0, and because this is the default, the
bug was never noticed.

This fixes a bug in the `Squiz.Functions.FunctionDeclarationSpacing`
sniff when the `requiredSpacesAfterOpen` option was set to anything
other than 0.

When the first argument in a multiline function declaration had a
type-hint, this error would be given:

> Expected 1 spaces between opening bracket and type hint "array"; 0
found

I don’t think this check was ever intended to affect multiline
functions. The logic was such that it never does when
`requiredSpacesAfterOpen` is 0, and because this is the default, the
bug was never noticed.
@gsherwood gsherwood changed the title Fix incorrect function declaration spacing error Squiz.Functions.FunctionDeclarationSpacing error for multi-line declarations with required spaces greater than zero Aug 20, 2015
@gsherwood gsherwood merged commit 2eaf28a into squizlabs:master Aug 20, 2015
gsherwood added a commit that referenced this pull request Aug 20, 2015
@gsherwood
Copy link
Member

Thanks for the test and the fix.

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