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

PSR2.Methods.FunctionCallSignature strips some comments during fixing #1347

Merged
merged 1 commit into from Feb 17, 2017

Conversation

uniquexor
Copy link
Contributor

This is a somewhat hard to come by bug, but still... The problem was, when using PSR2.Methods.FunctionCallSignatureSniff with requiredSpacesBeforeClose set to 1 and you had a situation like this:

foo('Testing
    multiline text'
 );

Notice a space before the closing parenthesis. The sniff would try to fix it but would get stuck, since it was removing a single token before the parenthesis. What I've done is I try to move the closing parenthesis to the end of the function call parameter, while jumping over any inline T_COMMENT and moving it before any /* */ comments or any other token, so for example, if we had a situation like this:

foo('Testing
    multiline text: ' // . $text
 );

The fixed code would look like this:

foo('Testing
    multiline text: ' ) // . $text
;

But if we had a situation like this:

foo('Testing
    multiline text: ' /* . $text */
 );

The fixed code would look like this:

foo('Testing
    multiline text: ' /* . $text */ );

@gsherwood gsherwood changed the title Fixed PSR2.Methods.FunctionCallSignatureSniff: Sniff could not fix a space following a new line PSR2.Methods.FunctionCallSignature strips some comments during fixing Feb 17, 2017
@gsherwood gsherwood merged commit 55386b0 into squizlabs:master Feb 17, 2017
gsherwood added a commit that referenced this pull request Feb 17, 2017
@gsherwood
Copy link
Member

The sniff was able to fix that space correctly because the fixer loops until there are no more errors. So it fixed it, but it took two loops.

The real problem was fixing the placement of the closing parenthesis, which was stripping comments at the end of the argument list. This PR fixes that, as well as allowing the fixer to fix the space in a single loop. So thank you very much for this.

I made some slight modifications to not do all this work in the simple case of incorrect padding before the closing parenthesis, and I also kept the semicolon with the closing parenthesis where possible.

@uniquexor uniquexor deleted the function-call-sign-fix branch February 17, 2017 06:54
@uniquexor
Copy link
Contributor Author

Thank you. In that case there might have been some other sniff making things worse in my code, since leaving a space did hang up the whole beautifier. It would make 50 passes and stop with an error. Maybe that could be the feature of the v3 - detecting which sniffs cause the infinite loop :)
Either way, I'm glad we fixed it.

@gsherwood
Copy link
Member

Maybe that could be the feature of the v3 - detecting which sniffs cause the infinite loop :)

If you use PHPCS with the --report=diff option instead of PHPCBF, you'll get a diff of the changes PHPCBF would make. If you also add -vv you'll see what fixes are being made and what fixes and being rejected due to conflicts. This is the best way to see which sniffs are conflicting during fixing.

If you use -vvv instead of -vv you'll also see the output of the file after each pass in the fixing process. I'd only use this when checking small sample files.

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