Skip to content

fix: stable formatting for conditional expression as if/else body#797

Draft
bartlomieju wants to merge 1 commit into
dprint:mainfrom
bartlomieju:fix/conditional-expr-statement-body-idempotency
Draft

fix: stable formatting for conditional expression as if/else body#797
bartlomieju wants to merge 1 commit into
dprint:mainfrom
bartlomieju:fix/conditional-expr-statement-body-idempotency

Conversation

@bartlomieju
Copy link
Copy Markdown
Collaborator

Summary

Fixes a "Formatting not stable" non-idempotency: a single-statement if/else if body that is a conditional (ternary) expression could format differently on each run.

Reproduction

With the deno config preset:

function w(n) {
  if ("play" === n) {
    var w = 1;
    play({ ids: [b] });
  } else if ("queue" === n) playing() ? queue({ ids: [b] }) : queue({ ids: [b] });
}

Formatting once produces a wrapped ternary inside added braces; formatting again collapses it to one line:

   } else if ("queue" === n) {
-    playing()
-      ? queue({ ids: [b] })
-      : queue({ ids: [b] });
+    playing() ? queue({ ids: [b] }) : queue({ ids: [b] });
   }

So the output is not a fixed point. In a consumer that re-formats to verify stability (e.g. deno fmt, which retries up to 5 times), this surfaces as Formatting not stable. Bailed after 5 tries.

Root cause

The inline form } else if ("queue" === n) playing() ? ... : ...; is 82 columns, over the 80 limit, so:

  1. the ternary is measured at the inline (post-header) column ~26 and wraps;
  2. the wrapped body is multiple lines, so WhenNotSingleLine adds braces;
  3. once braced, the body moves to its own line at indent 4 (58 columns), where it would fit on one line — but it has already been wrapped against the pre-brace column.

The brace decision is re-evaluated via the printer's look-ahead/reevaluation, but update_state_to_save_point in dprint-core restores writer position without clearing already-resolved conditions, so the conditional expression's break decision (resolved at the inline column) sticks. Binary, call, and member-expression bodies don't hit this because they lay out inside a single new_line_group that re-fits cleanly on reprint; the conditional expression emits its SpaceOrNewLine breaks at the top level, outside such a group.

Fix

Wrap the single non-block statement body in a new_line_group so its line-break decisions are re-measured at the body's final indentation, matching the behavior of the constructs that were already stable. The change is one line in gen_conditional_brace_body.

Tests

  • New spec tests/specs/deno/Deno_ConditionalExpressionStatementBody.txt. The spec runner formats twice (format_twice: true), so this asserts idempotency; it fails on main and passes with the fix.
  • Full suite passes (649 tests).

Scope / caveats

This was found while investigating denoland/deno#26840 (large minified bundle bailing with "Formatting not stable"). This patch fixes the ternary-body class of instability. That particular file has additional, independent non-idempotency (e.g. single-statement bodies under multi-line-wrapped if headers whose comma-sequence body sits at the width boundary), driven by the maintain-source-line-breaks behavior when preferSingleLine is false. Those are not addressed here; this PR is one self-contained increment.

A single-statement if/else body that is a conditional (ternary) expression
could format differently on each run: when the inline form exceeds the line
width, braces are added and the body moves to its own indented line, where the
ternary then fits on one line, but it was being wrapped based on the pre-brace
column. This left the formatter producing different output on the next run
("Formatting not stable").

Wrap the single non-block statement body in a new_line_group so its line-break
decisions are re-measured at the body's final indentation, matching how binary
and call expressions already behave (which were already stable).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant