-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Generic opening brace placement sniffs do not correctly support function return types #1931
Comments
I think the solution is to make sure this code doesn't produce an error: function myFunction($a, $lot, $of, $params)
: array {
return null;
} Because the opening brace is technically on the same line as the declaration, but the declaration is split over multiple lines, which is the job of another sniff to check. That will also make it consistent with this code, which also produces no error: function myFunction(
$a,
$lot,
$of,
$params
) {
return null;
} |
…ctly support function return types
Should be fixed now. Thanks for reporting it. |
@gsherwood Thanks for working on this. Unfortunately, the fix has issues with trailing comments: function myFunction($a, $lot, $of, $params)
: array // comment
{
return null;
}
function myFunction($a, $lot, $of, $params)
: array // phpcs:ignore Standard.Category.Sniff -- for reasons.
{
return null;
} When run against function myFunction($a, $lot, $of, $params)
: array { // comment
return null;
}
function myFunction($a, $lot, $of, $params)
: array { // phpcs:ignore Standard.Category.Sniff -- for reasons.
return null;
} In reality, this will now be fixed (broken) as: function myFunction($a, $lot, $of, $params)
: array {
// comment
return null;
}
function myFunction($a, $lot, $of, $params)
: array {
// phpcs:ignore Standard.Category.Sniff -- for reasons.
return null;
} |
@jrfnl That's not a broken fix - that's how this sniff fixes the brace position when there is a comment on the end. There's a unit test for it already (added here: 5c2a8a1) although it's been fixing code like that for a lot longer. Edit: I will add more tests to confirm this behaviour across multi-line declarations as well. |
@gsherwood For the "normal" comment I can understand this to be a valid fix, however for the new PHPCS annotations, it changes the meaning of the annotation completely, so I honestly do think something needs to be done about that. |
The auto-fixer in the
Generic.Functions.OpeningFunctionBraceKernighanRitchie
sniff currently will break when a function has a return type declaration which is not on the same line as the closing parenthesis of the function declaration.Example code:
The text was updated successfully, but these errors were encountered: