Skip to content

[Repo Assist] Fix FormatException when #if directive appears in mutually recursive type definition#3315

Draft
github-actions[bot] wants to merge 1 commit intomainfrom
repo-assist/fix-issue-3174-update-on-main-1774488314-d215a4b9295ac171
Draft

[Repo Assist] Fix FormatException when #if directive appears in mutually recursive type definition#3315
github-actions[bot] wants to merge 1 commit intomainfrom
repo-assist/fix-issue-3174-update-on-main-1774488314-d215a4b9295ac171

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

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

Closes #3174
Supersedes #3290 (rebased on current main after the lambda refactor in #3303)

Root Cause

When an and type definition has compiler directives (#if/#endif) between its attribute lists, two separate bugs interact to cause a FormatException:

  1. genOnelinerAttributes was flattening all AttributeListNode lists into a single combined [(Attr1)][(Attr2)] render, silently dropping the ContentBefore trivia (the #if/#endif directives) of all but the first list. mergeMultipleFormatResults then saw different hash-fragment counts across define variants and threw:

    System.FormatException: Fantomas is trying to format the input multiple times...
    [] has 5 fragments
    [NET5_0_OR_GREATER] has 3 fragments
    
  2. genTypeDefn did not indent the and header when attributes contain directives, which could produce invalid F# in some define paths (e.g. and\n[(Attr)] Y = int at column 0 is a parse error).

Fix

genOnelinerAttributes: detect when any attribute list after the first has a TriviaContent.Directive in its ContentBefore. In that case, render each AttributeListNode individually via genNode (preserving directives) separated by newlines, rather than flattening into a single node. The common (no-directive) case is unchanged.

genTypeDefn: add hasAttributeDirectivesForAndDefinition check. When an and definition has directive trivia in its attribute lists, set shouldIndentAfterKeyword = true to indent the header — the same treatment already applied when there are directives after the leading keyword identifier (hasTriviaAfterLeadingKeyword).

Example

Before (throws FormatException):

type X = int
and
#if NET5_0_OR_GREATER
    [(NotAValue)]
#endif
    [(Attribute)] Y = int

After:

type X = int

and
#if NET5_0_OR_GREATER
    [(NotAValue)]
#endif
    [(Attribute)] Y = int

Trade-offs

  • Minimal change: only genOnelinerAttributes and genTypeDefn are modified
  • The fast path (no directives between attribute lists) is unchanged — no performance impact
  • Existing behaviour for and definitions without directives is unchanged

Test Status

  • ✅ Build: succeeded (0 warnings, 0 errors)
  • ✅ Tests: 2770 passed, 7 skipped, 0 failed
  • ✅ New regression test added: #if in mutually recursive type definition does not throw FormatException, 3174
  • ✅ Reproduction from issue no longer throws FormatException

Generated by Repo Assist

Generated by Repo Assist ·

To install this agentic workflow, run

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

When an `and` type definition has compiler directives (`#if`/`#endif`)
between its attribute lists, genOnelinerAttributes was silently dropping
ContentBefore directives for all but the first AttributeListNode. This
caused mergeMultipleFormatResults to see different hash-fragment counts
across define variants and throw a FormatException.

Also, the type name header was not being indented when emitting the
attributes, which could produce invalid F# (parse error) in the
non-DEBUG define path.

Fix: in genOnelinerAttributes, detect when directives exist between
attribute lists and render each AttributeListNode individually via genNode.
In genTypeDefn, detect when an `and` definition has directive-bearing
attributes and indent the header to ensure validity in all define variants.

Closes #3174

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

#if in mutually recursive class definition throws

0 participants