fix(fmt): dropped statements for single-line **nested** if statements#13560
Open
quangloc99 wants to merge 4 commits intofoundry-rs:masterfrom
Open
fix(fmt): dropped statements for single-line **nested** if statements#13560quangloc99 wants to merge 4 commits intofoundry-rs:masterfrom
quangloc99 wants to merge 4 commits intofoundry-rs:masterfrom
Conversation
262f86d to
e690477
Compare
print_state_as_block multi-statements inline checkinge690477 to
36eb7c7
Compare
2 tasks
Member
|
cc @DaniPopes / @0xrusowsky when time permits |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Originally I attempted to fix #13555 (now fix #13675 ). After it is closes with #13566, to be frank I was still a bit skeptical with the solution. After some testing, the following case fails:
A more realistic case for this is
while (cond) if (cond) { ... }.More tests can be seen in the commit daf10d2
Solution
The solution presented in this PR is to change
is_multiline_block_stmtto check the deeper structure of the statement. But besides the missing checks, I think the root cause is the caching ofsingle_line_stmt: this variable is updated only at the top level of when looking at theifstatement, and only the topifstatement is examined carefully, while the nestedifis only consider a normal statement. So imo my fix might not be enough.I understand that caching here is necessary to make sure the work is done once. But to fix this root cause, a more reliable way should be calculating this variable for all nodes of the tree from bottom-up instead of top-down like this.
PR Checklist