-
Notifications
You must be signed in to change notification settings - Fork 1
[MultilineFunctionDeclaration] Make multiline in the middle #23
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
base: 2.x
Are you sure you want to change the base?
Conversation
| { | ||
| public function setFoo( | ||
| #[Beep] | ||
| Foo $new |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this have a trailing comma
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, and should also be fixed in the readme example and in the other fixed file
| elseif ( | ||
| !$tokensCollection->sameLine( | ||
| $tokensCollection->parenthesisOpener($stackPtr), | ||
| $tokensCollection->parenthesisCloser($stackPtr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trailing comma missing?
| public function setFoo( | ||
| #[Beep] | ||
| Foo $new | ||
| ): void { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this wrong?
Perhaps the fixed version should only fix the single problem the test is testing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it is wrong.
The tests use all sniffs available, and even for the formatting, but since I don't have #35 merged yet I apparently missed this. I apparently just fixed it in the other test, not this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or wait, you weren't referring to the missing ,?
@bix0r what part is wrong with this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was actually referring to the empty method body. Isn't supposed to be: ): void {}. But yeah, I did miss the trailing comma :o
If a function is having multiple rows in the parameters, then nothing can be on the first row.
If a function is having multiple rows in the parameters, then nothing can be on the first row.
POC for #18