Skip to content

fix(fmt): preserve braces to prevent dangling-else rebinding#14913

Merged
grandizzy merged 4 commits into
masterfrom
steven/fmt-dangling-else
May 26, 2026
Merged

fix(fmt): preserve braces to prevent dangling-else rebinding#14913
grandizzy merged 4 commits into
masterfrom
steven/fmt-dangling-else

Conversation

@stevencartavia
Copy link
Copy Markdown
Collaborator

@stevencartavia stevencartavia commented May 26, 2026

forge fmt drops braces around a single-statement nested if, causing the else to rebind to the inner if and silently changing semantics.

Input:

if (a) { if (b) return 1; } else return 2;

Output:

if (a) if (b) return 1;
else return 2;

With a=true, b=false: original returns 0, formatted version returns 2.

Fix: in is_single_line_block, refuse to elide braces when the then block wraps a single if (or while, whose body has the same risk) and the surrounding if has an else.

Copy link
Copy Markdown
Collaborator

@grandizzy grandizzy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, pls check comment re including for loops

Comment thread crates/fmt/src/state/sol.rs Outdated
Copy link
Copy Markdown
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review note from local pass; leaving the concrete issue inline.

Comment thread crates/fmt/src/state/sol.rs Outdated
@grandizzy grandizzy requested review from grandizzy and mattsse May 26, 2026 06:17
Copy link
Copy Markdown
Collaborator

@grandizzy grandizzy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thank you!

@grandizzy grandizzy enabled auto-merge (squash) May 26, 2026 07:13
@grandizzy grandizzy merged commit 676598e into master May 26, 2026
19 checks passed
@grandizzy grandizzy deleted the steven/fmt-dangling-else branch May 26, 2026 07:44
@github-project-automation github-project-automation Bot moved this to Done in Foundry May 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants