Skip to content

[Repo Assist] Fix trailing indented comment after last match clause / DU case de-indented to column 0#3306

Closed
github-actions[bot] wants to merge 2 commits intomainfrom
repo-assist/fix-issue-2653-trailing-comment-indentation-841e1499442ca5c1
Closed

[Repo Assist] Fix trailing indented comment after last match clause / DU case de-indented to column 0#3306
github-actions[bot] wants to merge 2 commits intomainfrom
repo-assist/fix-issue-2653-trailing-comment-indentation-841e1499442ca5c1

Conversation

@github-actions
Copy link
Contributor

🤖 This PR was created by Repo Assist, an automated AI assistant.

Closes #2653
Closes #2606

Root Cause

When a trailing comment appears after the last match clause or discriminated union case, its range falls outside the range of ExprMatchNode / TypeDefnUnionNode. However, includeTrivia in ASTTransformer.fs extends Oak.Range to cover all code comments, so findNodeWhereRangeFitsIn returns Some(Oak).

simpleTriviaToTriviaInstruction then falls through to the "last child AddAfter" path and attaches the comment as ContentAfter on ModuleOrNamespaceNode (indent = 0), causing it to be printed at column 0 instead of its original column.

Before:

let x =
    match 1 with
    | 1 -> 1
    | 2 -> 2
// | Value3 -> 3   ← de-indented to column 0

After:

let x =
    match 1 with
    | 1 -> 1
    | 2 -> 2
    // | Value3 -> 3   ← correct column 4
```

## Fix

**`Trivia.fs`**: Added `findNodeEndingOnLine` (shallowest-first traversal). Before falling back to the `ModuleOrNamespaceNode` attachment, `simpleTriviaToTriviaInstruction` now checks for a node that ends on the line immediately above the comment and whose start column matches. Shallowest-first ensures we find `ExprMatchNode` (or `TypeDefnUnionNode`) rather than a deeply nested child token that happens to share the same end position.

**`CodePrinter.fs`**: `addFinalNewline` previously checked only the head `WriterEvent`. Because `WriteLineBecauseOfTrivia` is now emitted inside `atCurrentColumn`, structural `RestoreAtColumn`/`RestoreIndent` events appear after it. Updated `addFinalNewline` to skip those non-content events (matching the approach already used by `lastWriteEventIsNewline`) so no extra blank line is appended.

## Tests Added

- `PatternMatchingTests.fs`: "trailing indented comment after last match clause preserves indentation, 2653"  
- `UnionTests.fs`: "trailing indented comment after last union case preserves indentation, 2606"

## Test Status

```
dotnet test artifacts/bin/Fantomas.Core.Tests/debug/Fantomas.Core.Tests.dll

Passed! - Failed: 0, Passed: 2742, Skipped: 7, Total: 2749, Duration: 11s

All 2742 tests pass including the two new regression tests.

Generated by Repo Assist ·

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@d1d884596e62351dd652ae78465885dd32f0dd7d

github-actions bot and others added 2 commits March 23, 2026 01:56
…s in mutually recursive type (fixes #3174)

Root cause: genOnelinerAttributes merged all AttributeListNodes into a single
one-liner, dropping ContentBefore trivia (including #if/#endif directives) from
all but the MultipleAttributeListNode itself. When the define was active, extra
attribute lists existed and their per-list directives were silently dropped,
producing a different hash-fragment count than the no-define variant and causing
mergeMultipleFormatResults to throw a FormatException.

Fix (CodePrinter.fs):
- genOnelinerAttributes detects when any AttributeListNode at index ≥ 1 has a
  compiler directive in its ContentBefore, and falls back to rendering each list
  individually via genNode (preserving ContentBefore trivia) with sepNln between
  lists and sepSpace after the last, keeping the type name on the same line as
  the final attribute.
- genTypeDefn detects directive-containing attribute lists for 'and' type
  definitions and forces indent/unindent so the formatted output is valid F# in
  both define variants (column-0 attributes after 'and' are a parse error).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… de-indented to column 0

Closes #2653
Closes #2606

Root cause: when a trailing comment appears after the last match clause or
discriminated union case, its range falls outside the range of
ExprMatchNode / TypeDefnUnionNode. However, because includeTrivia extends
Oak.Range to cover all code comments, findNodeWhereRangeFitsIn returns
Some(Oak). simpleTriviaToTriviaInstruction then attaches the comment as
ContentAfter on ModuleOrNamespaceNode (which has indent 0), causing the
comment to be printed at column 0 instead of its original column.

Fix (Trivia.fs): Before falling back to the last-child AddAfter attachment,
check whether there is a shallower node that ends on the line immediately
before the comment and whose start column matches the comment's start column.
findNodeEndingOnLine uses a shallowest-first traversal so that it finds
ExprMatchNode (or TypeDefnUnionNode) rather than a deeply nested child token
that happens to share the same end position. The comment is then attached as
ContentAfter on that node, which is rendered at the correct indentation level.

Fix (CodePrinter.fs): addFinalNewline previously checked only the head
WriterEvent. Because the trivia-triggered WriteLineBecauseOfTrivia is now
emitted inside atCurrentColumn, structural RestoreAtColumn/RestoreIndent
events appear after it. Updated addFinalNewline to skip those non-content
events (matching the approach already used by lastWriteEventIsNewline) so
that a trailing trivia newline is still recognised and no extra blank line
is appended.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@nojaf
Copy link
Contributor

nojaf commented Mar 23, 2026

Insufficient

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Comment on last match case has wrong indentation Comment unindented after DU cases

1 participant