filter empty statements when checking block emptiness #3878
Conversation
calebcartwright
force-pushed
the
block-empty-statements
branch
from
dcad861
to
9c84d17
Compare
| @@ -2,33 +2,33 @@ | |||
|
|
|||
| fn main() { | |||
| loop { | |||
| (); | |||
| (); | |||
| let foo = (); | |||
There was a problem hiding this comment.
@calebcartwright Why do we need to update all the tests?
There was a problem hiding this comment.
Sorry i meant to leave a note on that. Apparently both ; and (); are parsed as empty tuple exprs in the block statements, so with the updated logic filtering out empty statements, these blocks were now being detected as empty.
loop {
();
();
}would be flattened to
loop {}which was making these tests fail since they're trying to validate brace placement. the updates were just to convert to non-empty statements in those blocks to prevent the flattening.
Is this the correct behavior? Should we treat a block that only has a (); statement the same as one that only has a ; (both as empty blocks)?
If not I can do some minor refactoring to get the SnippetProvider available when checking the block statements, and in the event of the empty tuple grab the associated snippet and check for ; vs ();
There was a problem hiding this comment.
Thank you for your explanation!
Is this the correct behavior? Should we treat a block that only has a (); statement the same as one that only has a ; (both as empty blocks)?
Removing (); seems fine to me, though I am afraid that we may need to version-gate this as we didn't use to remove it. As we will be preparing for 2.0 release in the near future, I would like to merge this PR after we released 1.4.10 so that we don't need to add an extra version-gate which will be removed soon.
|
@calebcartwright Sorry, would you mind resolving the conflict? I should have merged this before #3880. |
Sure, will do! |
|
Thank you for the update! |
Fixes #3868