Skip to content

Formatter overhaul: never remove comments, fmt directives, line wrapping, K&R style - #7346

Open
ewels wants to merge 1 commit into
masterfrom
claude/formatter-overhaul-nf-lang-1om5gt
Open

Formatter overhaul: never remove comments, fmt directives, line wrapping, K&R style#7346
ewels wants to merge 1 commit into
masterfrom
claude/formatter-overhaul-nf-lang-1om5gt

Conversation

@ewels

@ewels ewels commented Jul 16, 2026

Copy link
Copy Markdown
Member

The formatter (nextflow lint -format and the language server / VS Code extension) can silently delete or corrupt comments: inline comments are moved off their line, comments after the last statement of a block are dropped, commented-out processes vanish, and CRLF files are corrupted by merging comments with the following code. For a tool that rewrites users' files in place, silent data loss is the worst possible failure mode, and it has been the most-reported formatter problem across both this repo and the language-server tracker. This PR overhauls the formatter in nf-lang so that formatting can never lose or alter a comment, and closes out the backlog of most-requested formatting features on top of that guarantee: fmt: skip / fmt: off / fmt: on directives, line wrapping at a configurable maximum length, K&R-style if/else and try/catch, blank-line normalization, multi-line string re-indentation, and include sorting.

Closes:

Every closed issue's reproduction case is pinned as a unit test, along with the repro cases from previously-closed formatter issues (#6892, #6971, #7328, and language-server #41, #55, #60, #67, #70, #71, #129, #135). Beyond unit tests, the branch adds a corpus harness that formats this repository's own fixtures (tests/, docs/snippets/, validation/ — 255 files) under all 48 formatting-option combinations and asserts comment-text preservation and idempotence: 12,240 checks, all green. As a final backstop, nextflow lint -format now verifies comment preservation before writing a file back and refuses to format (with a warning) rather than ever losing a comment.

Implementation details

Why comments were being lost

The parser only captures comments that immediately precede a statement or declaration (as LEADING_COMMENTS metadata) and drops everything else — comments after the last statement of a block, at the end of the file, in empty blocks, between process/workflow sections, inside multi-line expressions, and so on. The formatter re-synthesizes output from the AST, so a comment that was never captured cannot be emitted. CRLF corruption occurred because "\r\n" entries fail the "\n" checks in the parser and formatter.

CommentReattacher

The core of the fix is a new class, CommentReattacher, which runs before formatting and re-derives all comment metadata from the original source text:

  • It re-lexes the source with the same ANTLR lexer as the parser (comments and newlines are NL tokens on the default channel), so string literals, GStrings and slashy strings are handled exactly — no regex heuristics.
  • It builds a region tree over the AST mirroring the formatter's emission points (file, workflow/process sections, blocks, closures, wrapped expressions) and assigns every comment token positionally: leading comments, same-line trailing comments, dangling comments (end of a block/file, emitted before the closing brace), and dangling-after comments (end of a section with no closing brace, e.g. the last workflow take).
  • Comments inside wrapped expressions are kept in place: method-chain links, call/list/map elements (leading and trailing), construct-end before )/], ternary branches, and catch clauses all have their own comment slots. Every attachment site has a matching force-wrap condition, so an attached comment always has an emission point. Comments in positions the formatter genuinely cannot emit (e.g. inside operator expressions) are hoisted above their statement instead of being dropped.
  • All entries are normalized to LF, fixing CRLF corruption.

The formatter gained the corresponding emission support: trailing comments at end of line, dangling comments before closing braces and at end of file, closures with comments formatted as multi-line blocks, and a fix for workflows with onComplete/onError handlers (the main: label was previously omitted, producing invalid output).

Features

  • fmt: skip and fmt: off / fmt: on (Black semantics): excluded code is emitted verbatim from the source, including its comments and blank lines. Works in scripts and config files, on statements, declarations, process directives, section labels, record fields and enum constants. A region that straddles declaration boundaries is promoted to cover whole declarations, so the formatter can never emit mismatched braces.
  • Line wrapping: when a formatted line exceeds the maximum length, it is rolled back and re-emitted with call arguments, collections and method chains wrapped — first the outermost construct, then all of them if still too long. Source-wrapped expressions stay wrapped (the magic trailing comma forces multi-line collections). Default 120 columns, configurable via the new -line-length CLI option (0 disables).
  • K&R style: } else { and } catch (e: Exception) {; a commented else/catch falls back to its own line so the comment can be emitted.
  • Blank-line normalization: runs of blank lines collapse to one, blank lines at the start of blocks/sections are removed, block declarations get a blank line above, and a shebang is followed by exactly one blank line.
  • Multi-line string re-indentation: triple-quoted string bodies shift with the statement's indentation, preserving relative indentation, never shifting past content.
  • Include sorting (under the existing -sort-declarations option): case-insensitive by module path within blank-line-separated groups; group header comments stay pinned to the group top.

Safety check in nextflow lint

CommentReattacher.commentTexts collects the comments of a source text as a sorted, normalized list. nextflow lint -format compares this list before and after formatting and refuses to write the file (with a warning) on any mismatch — so no future formatter edge case can silently delete, duplicate or alter a comment.

Bugs found by corpus testing

Formatting this repository's own fixtures under every option combination surfaced and fixed several bugs beyond the reported issues: comment duplication in code-snippet scripts (top-level statements), blank lines eaten after compound statements, a sorted-first declaration emitting blank lines at the top of the file, non-idempotent wrapping of method chains rooted at property accesses (foo.out.collect()), stray blank lines from verbatim-suppressed declarations, and comment duplication when re-deriving metadata twice on the same AST (as the language server does with cached ASTs).

API notes

  • FormattingOptions gained a maxLineLength component; the previous canonical constructor is kept, so existing positional callers compile unchanged.
  • The formatting visitors accept the original source text as an optional constructor argument; without it, the source is re-read from the SourceUnit, so existing 2-arg callers keep working.
  • CommentReattacher.countComments / commentTexts are public so that other formatter front-ends (e.g. the language server) can implement the same refuse-to-format guard.

Testing

  • 134 tests in the nf-lang formatter suites (about 100 new), including a pinned reproduction for every issue closed by this PR and for the previously-closed formatter issues.
  • FormatterCorpusHarness formats 255 fixture files under 48 option combinations (tabs/spaces × harshilAlignment × maheshForm × sortDeclarations × maxLineLength 0/40/120), asserting commentTexts(before) == commentTexts(after) and format(format(x)) == format(x) throughout: 12,240 checks, 0 failures.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WFMWUB1sQEXpHgLa74xsRQ


Generated by Claude Code

@ewels
ewels requested review from a team as code owners July 16, 2026 12:47
@netlify

netlify Bot commented Jul 16, 2026

Copy link
Copy Markdown

Deploy Preview for nextflow-docs ready!

Name Link
🔨 Latest commit 860dc81
🔍 Latest deploy log https://app.netlify.com/projects/nextflow-docs/deploys/6a7667e211621400084d9163
😎 Deploy Preview https://deploy-preview-7346--nextflow-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@bentsherman

Copy link
Copy Markdown
Member

I had my own agent research existing approaches (eslint, prettier, ruff, etc) and implement its own solution. It ended up doing something very similar to you. I took the best of both worlds and it looks pretty clean overall. Much simpler than I had feared 😅

I also had it write an ADR to memorialize everything

I'm going to review the code, push it here, and merge. I focused only on the core problem of preserving comments. I will address the remaining issues later

@bentsherman
bentsherman force-pushed the claude/formatter-overhaul-nf-lang-1om5gt branch from a60cfe8 to 2cb9593 Compare August 7, 2026 21:50
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
@bentsherman
bentsherman force-pushed the claude/formatter-overhaul-nf-lang-1om5gt branch from 2cb9593 to 860dc81 Compare August 7, 2026 23:18
@ewels

ewels commented Aug 8, 2026

Copy link
Copy Markdown
Member Author

Great! Yes I pointed it at Ruff in the initial research stage as well I think. Might explain why both agents ended up with similar results 😅 Definitely not a bad thing to have two stabs at this though, as it's a fairly big change.

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.

2 participants