Formatter: discarded `checked_sub` never reserves separator width in one-per-line `SeparatedExprs`

Aperta
  • Qualcosa non va?
#6,085 1 commento 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 3 giorni

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
4/5
Tempo stimato
3-5 giorni
Idoneità per principianti
45/100
Tipo di issue
Bug
Chiarezza
Abbastanza chiara
Stato di attività
Tranquilla
Stack tecnologico
rust
Ambito
tooling

Direzione di ricerca

Read prqlc/prqlc/src/codegen/mod.rs around SeparatedExprs::write and compare it with write_inline's consume_width behavior; prqlc/prqlc/src/codegen/ast.rs defines the separator. Run the formatter snapshot tests and inspect cases near the width boundary. The issue explicitly leaves the intended formatter behavior to a maintainer decision; confirm that decision before changing snapshots.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

The nightly code-quality survey flagged a discarded computation in the code formatter that looks like it was meant to reserve width for a trailing separator but doesn't.

In SeparatedExprs::write, the one-per-line branch does:

opt.reset_line()?;
opt.rem_width.checked_sub(self.line_end.len() as u16)?;   // result discarded

r += &expr.write(opt.clone())?;
r += self.line_end;

codegen/mod.rs:168-172

checked_sub is non-mutating, so its result is thrown away — the line only functions as an underflow guard (via ?), it never actually reserves the line_end width before expr.write. Compare the sibling write_inline path, which uses the mutating consume_width to charge width against rem_width.

line_end is "," for tuples and arrays (ast.rs:103-114). Because the separator width is never reserved, an element that exactly fills the remaining width leaves no room for the trailing ,, so that formatted line can exceed max_width by one character.

This is a minor cosmetic edge case, not a correctness bug in the emitted PRQL. But the current line is either a latent off-by-line_end.len() bug (should be opt.consume_width(self.line_end.len() as u16)?; to actually reserve the separator) or the reservation is unwanted and the line is redundant. Which of the two is the intended behavior is a formatter-design call, and changing it would shift fmt snapshot output for elements near the width boundary — so flagging for a maintainer decision rather than opening a speculative PR.

Found during the nightly survey of codegen/mod.rs.

Lingua principale
Rust
Stelle
10.9k
Fork
261
Merge medio
3g 21h
PR unite (30g)
84

Preparare l'ambiente

Apri in Codespaces

Avvia il container di sviluppo del progetto nel browser, con il tuo account GitHub.

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di PRQL/prql

Tutte le issue di PRQL/prql

Issue simili

Altre issue su Rust

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.