Formatter overhaul: never remove comments, fmt directives, line wrapping, K&R style - #7346
Formatter overhaul: never remove comments, fmt directives, line wrapping, K&R style#7346ewels wants to merge 1 commit into
Conversation
✅ Deploy Preview for nextflow-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
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 |
a60cfe8 to
2cb9593
Compare
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
2cb9593 to
860dc81
Compare
|
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. |
The formatter (
nextflow lint -formatand 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 innf-langso 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: ondirectives, line wrapping at a configurable maximum length, K&R-styleif/elseandtry/catch, blank-line normalization, multi-line string re-indentation, and include sorting.Closes:
nextflow lint -formatis removes comments #6365elseformatting language-server#153Every 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 -formatnow 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_COMMENTSmetadata) 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:NLtokens on the default channel), so string literals, GStrings and slashy strings are handled exactly — no regex heuristics.)/], 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.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/onErrorhandlers (themain:label was previously omitted, producing invalid output).Features
fmt: skipandfmt: 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-lengthCLI option (0 disables).} else {and} catch (e: Exception) {; a commentedelse/catchfalls back to its own line so the comment can be emitted.-sort-declarationsoption): case-insensitive by module path within blank-line-separated groups; group header comments stay pinned to the group top.Safety check in
nextflow lintCommentReattacher.commentTextscollects the comments of a source text as a sorted, normalized list.nextflow lint -formatcompares 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
FormattingOptionsgained amaxLineLengthcomponent; the previous canonical constructor is kept, so existing positional callers compile unchanged.SourceUnit, so existing 2-arg callers keep working.CommentReattacher.countComments/commentTextsare public so that other formatter front-ends (e.g. the language server) can implement the same refuse-to-format guard.Testing
nf-langformatter suites (about 100 new), including a pinned reproduction for every issue closed by this PR and for the previously-closed formatter issues.FormatterCorpusHarnessformats 255 fixture files under 48 option combinations (tabs/spaces × harshilAlignment × maheshForm × sortDeclarations × maxLineLength 0/40/120), assertingcommentTexts(before) == commentTexts(after)andformat(format(x)) == format(x)throughout: 12,240 checks, 0 failures.🤖 Generated with Claude Code
https://claude.ai/code/session_01WFMWUB1sQEXpHgLa74xsRQ
Generated by Claude Code