From aa8320f6271a058189086fb82cbeb2abdb2cf429 Mon Sep 17 00:00:00 2001 From: nojaf Date: Fri, 21 Aug 2026 13:02:26 +0200 Subject: [PATCH 01/11] Update FCS to 'Fix #17904 and #19020', commit 9487d36e Bump the vendored compiler sources from ab1f6cea to 9487d36e776a5c7315f04a6ddf2887f92ab60d49, spanning three upstream commits that touch src/Compiler/SyntaxTree: 0abb33dc Address signature generation bugs (#19586) cd1d1804 Address additional signature generation roundtrip bugs (#19609) 9487d36e Fix #17904 and #19020 (#19738) The last of those makes mkSynBinding move a `[]` attribute written in front of a binding out of the binding's attribute list and into the arity information. Fantomas reads attributes from SynBinding only, so those attributes were parsed and then never printed, silently deleting them from the formatted output. Partial active patterns marked `[]` are the common case, and the Fantomas code base itself relies on them. The existing tests only covered the return type annotation form, `let f x : [] int = x`, so the whole suite stayed green while source was being dropped. restoreRotatedReturnAttributes puts those attributes back in the attribute list they were written in. Attributes written on a return type annotation end up in the arity information as well, and are left alone there, because the return type node already prints them. --- Directory.Build.props | 2 +- src/Fantomas.Core.Tests/AttributeTests.fs | 50 +++++++++++++++++++ src/Fantomas.Core/ASTTransformer.fs | 59 ++++++++++++++++++++++- 3 files changed, 109 insertions(+), 2 deletions(-) diff --git a/Directory.Build.props b/Directory.Build.props index 2fc126a694..0310ae7faf 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -45,7 +45,7 @@ Some common use cases include: - ab1f6ceaaec997d2854ac1c07a6c0f107675d95c + 9487d36e776a5c7315f04a6ddf2887f92ab60d49 diff --git a/src/Fantomas.Core.Tests/AttributeTests.fs b/src/Fantomas.Core.Tests/AttributeTests.fs index bc13ad5e82..a91bfdeea3 100644 --- a/src/Fantomas.Core.Tests/AttributeTests.fs +++ b/src/Fantomas.Core.Tests/AttributeTests.fs @@ -417,6 +417,56 @@ open System.Runtime.InteropServices do () """ +[] +let ``should preserve return attribute in front of a binding`` () = + formatSourceString + """ +[] +let (|Foo|_|) x = ValueNone +""" + config + |> prepend newline + |> should + equal + """ +[] +let (|Foo|_|) x = ValueNone +""" + +[] +let ``should preserve return attribute next to another attribute in front of a binding`` () = + formatSourceString + """ +[] +let (|Foo|_|) x = ValueNone +""" + config + |> prepend newline + |> should + equal + """ +[] +let (|Foo|_|) x = ValueNone +""" + +[] +let ``should preserve return attribute in its own attribute list in front of a binding`` () = + formatSourceString + """ +[] +[] +let (|Foo|_|) x = ValueNone +""" + config + |> prepend newline + |> should + equal + """ +[] +[] +let (|Foo|_|) x = ValueNone +""" + [] let ``should preserve single return type attribute`` () = formatSourceString """let f x : [] int = x""" config diff --git a/src/Fantomas.Core/ASTTransformer.fs b/src/Fantomas.Core/ASTTransformer.fs index b545621e17..e78e13ae6c 100644 --- a/src/Fantomas.Core/ASTTransformer.fs +++ b/src/Fantomas.Core/ASTTransformer.fs @@ -2069,11 +2069,68 @@ let (|OperatorWithStar|_|) (si: SynIdent) = ValueSome(IdentifierOrDot.Ident(stn $"( %s{text} )" ident.idRange)) | _ -> ValueNone +/// The parser moves a `[]` attribute written in front of a binding out of the +/// binding's attribute list and into the arity information, where nothing prints it. Put those +/// attributes back in the list they were written in, otherwise they are dropped from the output. +let restoreRotatedReturnAttributes + (attributes: SynAttributes) + (SynValData(valInfo = SynValInfo(returnInfo = SynArgInfo(attributes = arityAttributes)))) + (returnInfo: SynBindingReturnInfo option) + : SynAttributes = + let sortAttributes (attributes: SynAttribute list) = + List.sortBy (fun (a: SynAttribute) -> a.Range.StartLine, a.Range.StartColumn) attributes + + let sortAttributeLists (attributes: SynAttributes) = + List.sortBy (fun (al: SynAttributeList) -> al.Range.StartLine, al.Range.StartColumn) attributes + + // A return attribute written on the return type, `let f x : [] int = x`, ends up in + // the arity information as well. That one is printed by the return type node, so leave it there. + let returnTypeAttributes = + match returnInfo with + | None -> [] + | Some(SynBindingReturnInfo(attributes = attributes)) -> + List.collect (fun (al: SynAttributeList) -> al.Attributes) attributes + + let rotated = + arityAttributes + |> List.collect (fun al -> al.Attributes) + |> List.filter (fun a -> + not (List.exists (fun (rta: SynAttribute) -> equals rta.Range a.Range) returnTypeAttributes)) + + match rotated with + | [] -> attributes + | rotated -> + let wasWrittenIn (al: SynAttributeList) (a: SynAttribute) = + RangeHelpers.rangeContainsRange al.Range a.Range + + let restored = + attributes + |> List.map (fun al -> + match List.filter (wasWrittenIn al) rotated with + | [] -> al + | inThisList -> + { al with + Attributes = sortAttributes (al.Attributes @ inThisList) }) + + // An attribute list that held nothing but return attributes was removed altogether, so it + // has to be recreated. Its own range is gone, the attribute range is the closest we have. + let recreated = + rotated + |> List.choose (fun a -> + if List.exists (fun al -> wasWrittenIn al a) attributes then + None + else + Some({ Attributes = [ a ]; Range = a.Range }: SynAttributeList)) + + sortAttributeLists (restored @ recreated) + let mkBinding (creationAide: CreationAide) - (SynBinding(_, _, _, isMutable, attributes, xmlDoc, _, pat, returnInfo, expr, _, _, trivia)) + (SynBinding(_, _, _, isMutable, attributes, xmlDoc, valData, pat, returnInfo, expr, _, _, trivia)) (inKeyword: SingleTextNode option) = + let attributes = restoreRotatedReturnAttributes attributes valData returnInfo + let mkFunctionName (sli: SynLongIdent) : IdentListNode = match sli.IdentsWithTrivia with | [ prefix; OperatorWithStar operatorNode ] -> From 81f671f6817e6d237a7d1524c77fd6ea54f9d3ae Mon Sep 17 00:00:00 2001 From: nojaf Date: Fri, 21 Aug 2026 13:30:55 +0200 Subject: [PATCH 02/11] Add update FCS command --- .claude/commands/update-fcs.md | 338 +++++++++++++++++++++++++++++++++ 1 file changed, 338 insertions(+) create mode 100644 .claude/commands/update-fcs.md diff --git a/.claude/commands/update-fcs.md b/.claude/commands/update-fcs.md new file mode 100644 index 0000000000..9b032e702c --- /dev/null +++ b/.claude/commands/update-fcs.md @@ -0,0 +1,338 @@ +--- +description: Bump the vendored FCS commit hash one upstream SyntaxTree commit at a time +argument-hint: "[check|fast]" +allowed-tools: Bash(gh api:*), Bash(git:*), Bash(dotnet:*), Bash(grep:*), Bash(sed:*), Bash(mkdir:*), Bash(sort:*), Bash(comm:*), Read, Edit +--- + +Update the vendored F# compiler sources in `src/Fantomas.FCS` by moving `FCSCommitHash` +forward to the **next** upstream commit that touched `src/Compiler/SyntaxTree`. + +Advance **one meaningful commit per invocation**. Never jump straight to `main`: a single hash +bump that crosses many upstream changes makes a broken build impossible to attribute. + +A commit is meaningful only when it touches a file Fantomas actually vendors. A SyntaxTree commit +that changes nothing in the vendored file list cannot change the build, so **skip it** and keep +walking to the next one. Skipping is safe precisely because the landing hash still contains the +skipped commits, their content just does not reach `src/Fantomas.FCS`. Never skip a commit you +have not proven to be a no-op this way. + +This command is meant to be run **repeatedly**, once per meaningful upstream commit, until the +vendored copy has caught up. Most steps will need no repo change at all, the hash moves and +everything still builds. A few will need real work. Treat a clean step as the normal case and +say so plainly, do not go looking for something to fix. + +Every run must open and close with the same progress line, so the user can see the catch-up +shrinking across invocations: + +``` +FCS catch-up: SyntaxTree commit(s) behind dotnet/fsharp main +``` + +`$ARGUMENTS`: + +- `check` — report the pending commits, marked as vendored or no-op, and stop, change nothing. +- `fast` — skip the full default build pipeline in step 5, run only the vendored project build + and the Fantomas.Core unit tests. Use this when walking many commits in a row. The full + pipeline is still the default, and is worth running at least on the last step of a walk. + +## 0. The `.deps` cache + +`.deps/` is gitignored, so keep the walk's scratch data in `.deps/.fcs-walk/`. Create it if +missing. Two things live there: + +`vendored-files.txt` — the list of files Fantomas actually vendors, taken from the `Init` pipeline +file list in `build.fsx`, which is the single source of truth for what gets downloaded: + +``` +mkdir -p .deps/.fcs-walk +grep -oE '"src/Compiler/[^"]+"' build.fsx | tr -d '"' | sort -u > .deps/.fcs-walk/vendored-files.txt +``` + +Regenerate it whenever `build.fsx` is newer than the cache file, otherwise reuse it. It is about +85 paths. + +`commits/.tsv` — the changed-file list of one upstream commit. A commit's file list never +changes, so this is cacheable forever and saves an API round trip on every later run. Read from +the cache when the file exists and is non-empty, otherwise fetch and write it. + +## 1. Read the current hash + +``` +grep -n FCSCommitHash Directory.Build.props +``` + +Get its commit date upstream (needed to list commits after it): + +``` +gh api repos/dotnet/fsharp/commits/ --jq '.commit.committer.date, (.commit.message | split("\n")[0])' +``` + +## 2. Find the pending SyntaxTree commits + +``` +gh api "repos/dotnet/fsharp/commits?path=src/Compiler/SyntaxTree&sha=main&since=&per_page=100" \ + --paginate --jq '.[] | [.sha, .commit.committer.date, (.commit.message | split("\n")[0])] | @tsv' | tail -r +``` + +`tail -r` reverses to oldest-first (this is macOS, there is no `tac`). `since` is inclusive, so +the first row is the current hash itself. Drop it. + +If nothing is left, report that Fantomas is up to date with the SyntaxTree folder and stop. + +## 2b. Pick the target, skipping no-op commits + +Walk the pending list oldest-first. For each candidate, get its changed-file list, from +`.deps/.fcs-walk/commits/.tsv` when cached, otherwise: + +``` +gh api repos/dotnet/fsharp/commits/ --jq '.files[] | [.status, .filename, .additions, .deletions] | @tsv' \ + > .deps/.fcs-walk/commits/.tsv +``` + +Intersect the filenames with `.deps/.fcs-walk/vendored-files.txt`. Empty intersection means the +commit cannot affect the build, so it is a **no-op**: skip it and move to the next candidate. +The first candidate with a non-empty intersection is this run's target. + +A no-op is common. Many SyntaxTree commits only touch files Fantomas does not vendor, or only +touch tests and release notes. Skipping them is the point of this step, it saves a full build +cycle that could not have told you anything. + +If every pending commit is a no-op, bump straight to the newest one, say that the whole remaining +range was vendor-neutral, and still build to prove it. + +Open with the progress line and the numbered list, oldest first, marking skipped commits and the +one this run will take: + +``` +FCS catch-up: 17 SyntaxTree commits behind dotnet/fsharp main + + -- 1. 9487d36e 2026-05-20 Fix #17904 and #19020 (#19738) no vendored file + -> 2. f15535b5 2026-06-03 Fix XmlDoc validation for get/set property pairs (#19884) + 3. bc8a51b7 2026-06-04 Fix parser error for anonymous record type aliases ... (#19762) + ... +``` + +Name the skipped commits explicitly, do not silently drop them. The user is walking this range to +understand it, a skip is information. + +The count is recomputed from upstream on every run, so it shrinks by one per successful step, by +more when commits were skipped, and it can also grow when new commits land on `main`. Say which +it did if the number moved unexpectedly. + +If `$ARGUMENTS` is `check`, classify every pending commit this way, print the list with the no-ops +marked, and stop. Answer the question "how much of this backlog is real work", the cache makes +the second such run cheap. + +## 3. Summarise what the target commit changes + +Before touching anything, give the user a real overview, not just a subject line: + +You already have the file list from step 2b, in `.deps/.fcs-walk/commits/.tsv`. + +Report: + +- The commit subject, the PR number and its link (`https://github.com/dotnet/fsharp/pull/`). +- Which files under `src/Compiler/SyntaxTree` changed, and how much. +- Whether any file was **added, removed or renamed** anywhere under `src/Compiler`. Those need a + matching edit in the file list in `build.fsx` (the `Init` pipeline) and in the `Compile` + items of `src/Fantomas.FCS/Fantomas.FCS.fsproj`. Both lists are explicit, nothing is globbed. +- The parts of the diff that matter to Fantomas: changes to `SyntaxTree.fs(i)`, + `SyntaxTrivia.fs(i)`, `SyntaxTreeOps.fs(i)`, `pars.fsy` and `ParseHelpers.fs`. A new or changed + trivia field, a new `Syn*` case, or a changed case shape ripples into `ASTTransformer.fs`, + `SyntaxOak.fs` and `CodePrinter.fs`. +- Whether files outside `SyntaxTree` changed too. The hash controls **every** vendored file, not + only the SyntaxTree folder, so a SyntaxTree-scoped walk can still pull in unrelated compiler + changes and break the build. + +Fetch the actual patch for the interesting files when the file list alone is not enough to +explain the change: + +``` +gh api repos/dotnet/fsharp/commits/ --jq '.files[] | select(.filename == "src/Compiler/SyntaxTree/SyntaxTrivia.fsi") | .patch' +``` + +## 4. Bump the hash + +Replace the value of `` in `Directory.Build.props` with the full target sha. + +## 5. Download the new sources and build + +``` +dotnet fsi build.fsx -- -p Init +``` + +This downloads the compiler files at the new hash into `.deps/` (gitignored) and rewrites +`FSharp.Compiler` to `Fantomas.FCS` in them. It must run before any build, otherwise the +`.deps/` folder does not exist. + +Then clear the vendored project's build state, so nothing generated from the previous hash +survives into this one: + +``` +dotnet clean src/Fantomas.FCS/Fantomas.FCS.fsproj +``` + +`Init` skips any file that already exists, so revisiting a hash leaves the sources with their +original download time. `FSComp.txt` generates the `SR` module, and MSBuild regenerates it only +when the input looks newer than the generated output. Revisit a hash whose sources predate the +last build and the stale `SR` is reused, giving errors like + +``` +error FS0039: The type 'SR' does not define the field, constructor or member 'featureXyz' +``` + +which look like a real breakage in the target commit and are not. Cleaning removes the whole +class of confusion. This bites whenever the walk moves backwards, which it does when bisecting a +regression. The full pipeline is immune because its `Clean` stage deletes `artifacts` outright, +so this only matters for the standalone compile check below. + +Then a fast compile check of the vendored project only, which fails in seconds rather than +minutes when a signature changed: + +``` +dotnet build src/Fantomas.FCS/Fantomas.FCS.fsproj +``` + +Then the default build pipeline: + +``` +dotnet fsi build.fsx +``` + +This runs check-format, release build, unit tests, pack and docs. It is slow. Tell the user it +is running before you start it. + +When `$ARGUMENTS` is `fast`, run this instead of the default pipeline, and say in the report that +the full pipeline was skipped: + +``` +dotnet test src/Fantomas.Core.Tests/Fantomas.Core.Tests.fsproj +``` + +## 6. On failure, hand over + +If any step fails, **stop and hand the problem to the user**. Do not guess at compiler-semantics +fixes, and do not revert the hash. + +A green pipeline is not proof the step is good. When the target commit changes how the parser +*shapes* the tree, rather than only what it computes, probe the affected syntax by hand with +`scripts/format.fsx` before declaring success. The test suite only covers syntax someone already +wrote a test for, and the failure mode of a shape change is silently dropped source, which no +existing assertion notices. Round-tripping the construct through the local build takes seconds: + +``` +dotnet build src/Fantomas/Fantomas.fsproj -v quiet +dotnet fsi scripts/format.fsx +``` + +Treat dropped or duplicated source found this way exactly like a build failure: stop and hand over. + +### Parse the new freedom, do not print it + +Some upstream commits make the parser accept a layout it used to reject. Verify that the newly +accepted syntax round-trips unchanged and stop there. Do not change a layout decision or add a +`CodePrinter` case to take advantage of the new freedom, however much nicer the output would look. + +The vendored compiler is always ahead of the compiler in the published .NET SDK that users run. +Output only the newer compiler accepts does not compile for them. Adopting a new construct is a +deliberate separate change for much later, with its own settings discussion, not a side effect of +a hash bump. + +### Pin every behaviour change with a test + +A behaviour change that no test covers is the dangerous kind, the suite stays green while the +formatter misbehaves. Before handing over, **add the missing test**, in the existing test file +that already owns that syntax (attributes go in `AttributeTests.fs`, and so on). Match the +surrounding idiom, `formatSourceString ... config |> should equal`, and name it after the syntax, +not after the upstream commit. + +Assert the **correct** output, the one that round-trips the user's source. That test fails right +now, and that is the point: it pins the regression so it cannot be forgotten, and it turns green +the moment someone fixes it. Do not weaken the expectation to match the current broken output, +and do not `Ignore` the test. + +Do this for the syntax that is actually broken, and also for any near neighbour the probe showed +still works but no test covered. The one that works costs nothing and stops the next walk from +depending on luck the way this one did. + +Record it so later steps can tell it apart from new breakage, appending one line to +`.deps/.fcs-walk/open-regressions.tsv` as ``, ``, ``, ``. + +### A known failing test is not a failed step + +Once a regression is pinned, the `UnitTests` stage fails on every later step until it is fixed, +and `Pack` never runs. That must not stall the walk. + +When the pipeline fails, read the failing test names and compare them with +`open-regressions.tsv`. If every failure is already recorded there, the step is fine, say so +plainly, name the pinned regression and carry on. If even one failure is not in that file, it is +new breakage from this step, so stop and hand over as above. + +Present: the failing step, the actual error output (trimmed to the relevant compiler errors), +and your reading of which upstream change caused it. + +You may apply the purely mechanical fixes, but say so and show the diff: + +- A file added or removed upstream: add or remove the matching entry in the `Init` file list in + `build.fsx` and the matching `Compile`/`Link` pair in `src/Fantomas.FCS/Fantomas.FCS.fsproj`. + Order matters in both, F# compilation order follows the upstream order. +- A pure rename with no shape change. + +Anything else is the user's call. Common non-mechanical breakage: + +- A trivia record gained or lost a field, so `ASTTransformer.fs` no longer compiles. +- A `Syn*` union case changed shape, so pattern matches in `ASTTransformer.fs` fail `FS0025` + (incomplete matches are errors in this repo). +- `src/Fantomas.FCS/Parse.fs` drifted from the upstream parser entry points it mirrors. +- A test asserts on formatting that the parser now produces differently. That may be a genuine + improvement, discuss it before rewriting the expectation. + +## 7. On success + +Say which of the two outcomes this was, in one line, before any detail: + +- **Clean bump** — only `Directory.Build.props` changed, everything built. This is the common + case and needs no discussion. `git status --short` proves it. +- **Bump with fixes** — list what else had to change and why. +- **Bump with a pinned regression** — it built, but a probe found a behaviour change and there is + now a failing test naming it. Say which test and what it asserts. + +Then report: + +- The old and new hash, and the upstream subject and PR link. +- Which commits were skipped as no-ops on the way, if any. +- Anything the user should keep in mind for the steps ahead, for example an upstream change that + compiles today but will need Oak or `CodePrinter` work once a later commit builds on it. +- Any **open regression carried over from an earlier step**, in one line, until it is resolved. A + green pipeline does not mean the walk is healthy, and a problem found three steps ago is easy + to forget once the tree keeps building. +- The closing progress line and the next target: + +``` +FCS catch-up: 16 SyntaxTree commits behind dotnet/fsharp main +Next: f15535b5 Fix XmlDoc validation for get/set property pairs (#19884) +``` + +Leave the work uncommitted and **do not ask whether to commit a clean bump**. The user walks +several steps in a row and batches the committing themselves, asking every time is noise. Only +raise committing when a step needed real fixes or pinned a regression. + +A pinned test belongs to the step whose upstream commit **caused** the behaviour change, not to +whatever step the walk had reached when the test was written. Those differ whenever a probe runs +late, or a regression is noticed a step or two after it landed. `open-regressions.tsv` records the +causing sha for exactly this reason, use it. If the walk has already moved past that step and the +bumps are still one uncommitted blob, say so and offer to reconstruct the steps as separate +commits, rather than letting the history blame an innocent later commit. + +When the user does ask for a commit, follow the existing convention in this repo: + +``` +Update FCS to '', commit +``` + +One commit per step. Do not fold several hash bumps into one commit, the point of walking is that +a later bisect lands on a single upstream change. + +Finally, ask whether to run this command again for the next pending commit. If the user answers +with something like "again", "next" or "continue", that is this command, not a new conversation. From 47f2b42aa0fe90a2223295c745aaadca19b4abc7 Mon Sep 17 00:00:00 2001 From: nojaf Date: Fri, 21 Aug 2026 13:42:29 +0200 Subject: [PATCH 03/11] Add note to clean up ASTTransformer if possible. --- .claude/commands/update-fcs.md | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/.claude/commands/update-fcs.md b/.claude/commands/update-fcs.md index 9b032e702c..537b1aadbe 100644 --- a/.claude/commands/update-fcs.md +++ b/.claude/commands/update-fcs.md @@ -144,6 +144,31 @@ Report: only the SyntaxTree folder, so a SyntaxTree-scoped walk can still pull in unrelated compiler changes and break the build. +### Look for what we can stop doing + +A bump is usually a risk to survive, but now and then it is a gift. When the parser starts +providing something Fantomas currently reconstructs, the right response is to delete our version, +not just to keep building. + +The signal is upstream giving a construct a **real position** it did not have before: + +- a new or changed field in `SyntaxTrivia.fs(i)`, which is where keyword ranges live +- a keyword range added to an existing `Syn*` case or its trivia record +- a construct promoted from something synthesized to a proper node, the way `and!` became a + `SynBinding` + +When you see one, look in `ASTTransformer.fs` for the matching place where we do the work +ourselves: a range built with `unionRanges` or `mkRange` that the parser now hands us directly, a +keyword position recovered from the source text through `creationAide.TextFromSource`, or a shape +we rebuild because the tree did not describe it. Say so in the report, with the file and the +function. + +Two cautions. Most commits offer nothing here, so say that plainly rather than manufacturing an +improvement, and check the code before claiming one: earlier bumps may already have removed the +reconstruction, which is why a promising-looking commit often turns out to have nothing left to +give. And do not make the change in the same step as the bump. Report it as an opportunity and let +the user take it separately, so the bump stays clean and attributable. + Fetch the actual patch for the interesting files when the file list alone is not enough to explain the change: From c2cd6474f2977b767edc7a7319f422bc825282f2 Mon Sep 17 00:00:00 2001 From: nojaf Date: Fri, 21 Aug 2026 14:14:48 +0200 Subject: [PATCH 04/11] Update FCS to 'Record spreads', commit f4b785f1 Bump the vendored compiler sources to f4b785f189aedc4a0f1ec22182e3653a0b9dd142 and support the record spread syntax it introduces, RFC FS-1151, dotnet/fsharp#18927. A spread reaches the syntax tree in three places, and each one changed shape: SynTypeDefnSimpleRepr.Record now holds SynFieldOrSpread SynExpr.Record now holds SynExprRecordFieldOrSpread SynExpr.AnonRecd now holds SynExprAnonRecordFieldOrSpread Oak mirrors that with two unions, ExprRecordFieldOrSpread for the expression forms and TypeDefnRecordFieldOrSpread for the record representation of a type definition, carrying ExprSpreadNode and TypeSpreadNode. Nominal and anonymous record items keep sharing RecordFieldNode, which upstream splits in two, and the block separator is left behind since the printer decides separators itself. The spread source is an arbitrary expression, the same grammar as the right-hand side of a field, so it can be multiline. Nothing may come between the dots and the source, which means a spread has no break to decide and prints as the dots followed by the expression. That keeps the existing cramped and aligned field printers untouched. ExprRecordBaseNode.HasFields becomes HasItems, since a record holding only a spread has no fields but is not empty, and the value decides whether a separator follows an inherit or copy-from clause. Also use the range the parser now provides on SynExprAnonRecordField instead of reconstructing it from the field name and the value. Add RecordSpreadTests.fs covering the three positions in implementation and signature files, spreads before, between and after fields, multiple spreads, record and anonymous record literals, applications and property gets as sources, struct anonymous records, Stroustrup, and comments attached to a spread. --- CHANGELOG.md | 4 + Directory.Build.props | 2 +- .../Fantomas.Core.Tests.fsproj | 1 + src/Fantomas.Core.Tests/RecordSpreadTests.fs | 877 ++++++++++++++++++ src/Fantomas.Core/ASTTransformer.fs | 69 +- src/Fantomas.Core/CodePrinter.fs | 99 +- src/Fantomas.Core/SyntaxOak.fs | 66 +- 7 files changed, 1055 insertions(+), 63 deletions(-) create mode 100644 src/Fantomas.Core.Tests/RecordSpreadTests.fs diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f0d14b695..00e3128495 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Added + +- Support for the record spread syntax introduced in F# preview, [RFC FS-1151](https://github.com/fsharp/fslang-design/pull/805). A spread can appear in a record expression, `{ ...source; Field = value }`, in an anonymous record expression, `{| ...source; Field = value |}`, and in the record representation of a type definition, `type Target = { ...Source; Field: int }`, in both implementation and signature files. [#3400](https://github.com/fsprojects/fantomas/pull/3400) + ### Changed - Breaking: warnings and errors are written to standard error instead of standard out. Informational output stays on standard out, including `--version` and the files `--check` reports as needing formatting, so a caller can tell the tool's output apart from its diagnostics by stream. Scripts that capture standard out to detect failures need to capture standard error as well. [#3399](https://github.com/fsprojects/fantomas/pull/3399) diff --git a/Directory.Build.props b/Directory.Build.props index 0310ae7faf..ab614557be 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -45,7 +45,7 @@ Some common use cases include: - 9487d36e776a5c7315f04a6ddf2887f92ab60d49 + f4b785f189aedc4a0f1ec22182e3653a0b9dd142 diff --git a/src/Fantomas.Core.Tests/Fantomas.Core.Tests.fsproj b/src/Fantomas.Core.Tests/Fantomas.Core.Tests.fsproj index b490310d89..2cdef6acea 100644 --- a/src/Fantomas.Core.Tests/Fantomas.Core.Tests.fsproj +++ b/src/Fantomas.Core.Tests/Fantomas.Core.Tests.fsproj @@ -70,6 +70,7 @@ + diff --git a/src/Fantomas.Core.Tests/RecordSpreadTests.fs b/src/Fantomas.Core.Tests/RecordSpreadTests.fs new file mode 100644 index 0000000000..e72c7d34d9 --- /dev/null +++ b/src/Fantomas.Core.Tests/RecordSpreadTests.fs @@ -0,0 +1,877 @@ +module Fantomas.Core.Tests.RecordSpreadTests + +open NUnit.Framework +open FsUnit +open Fantomas.Core.Tests.TestHelpers +open Fantomas.Core + +// Record spreads, RFC FS-1151, dotnet/fsharp#18927. +// The spread reaches the syntax tree in three distinct places: +// +// SynTypeDefnSimpleRepr.Record holds SynFieldOrSpread, carrying SynTypeSpread +// SynExpr.Record holds SynExprRecordFieldOrSpread, carrying SynExprSpread +// SynExpr.AnonRecd holds SynExprAnonRecordFieldOrSpread, carrying SynExprSpread +// +// Each of those is exercised below, in implementation and signature files where the construct +// can appear in both. + +// ------------------------------------------------------------------------------------------- +// Record type definition, SynFieldOrSpread.Spread +// ------------------------------------------------------------------------------------------- + +[] +let ``type definition with only a spread`` () = + formatSourceString """type T = { ...Src }""" config + |> should + equal + """type T = { ...Src } +""" + +[] +let ``type definition with a spread before a field`` () = + formatSourceString """type T = { ...Src; A: int }""" config + |> should + equal + """type T = { ...Src; A: int } +""" + +[] +let ``type definition with a spread after a field`` () = + formatSourceString """type T = { A: int; ...Src }""" config + |> should + equal + """type T = { A: int; ...Src } +""" + +[] +let ``type definition with a spread between fields`` () = + formatSourceString """type T = { A: int; ...Src; B: string }""" config + |> should + equal + """type T = { A: int; ...Src; B: string } +""" + +[] +let ``type definition with multiple spreads`` () = + formatSourceString """type T = { ...First; ...Second }""" config + |> should + equal + """type T = { ...First; ...Second } +""" + +[] +let ``type definition with a generic spread source`` () = + formatSourceString """type T = { ...Src; A: int }""" config + |> should + equal + """type T = { ...Src; A: int } +""" + +[] +let ``type definition with a long identifier spread source`` () = + formatSourceString """type T = { ...Some.Nested.Module.Src; A: int }""" config + |> should + equal + """type T = { ...Some.Nested.Module.Src; A: int } +""" + +[] +let ``multiline type definition with a spread`` () = + formatSourceString + """ +type LongerRecordName = + { + ...SomeSourceRecordType + FirstAdditionalField: int + SecondAdditionalField: string + } +""" + config + |> prepend newline + |> should + equal + """ +type LongerRecordName = + { + ...SomeSourceRecordType + FirstAdditionalField: int + SecondAdditionalField: string + } +""" + +[] +let ``multiline type definition with a trailing spread`` () = + formatSourceString + """ +type LongerRecordName = + { + FirstAdditionalField: int + SecondAdditionalField: string + ...SomeSourceRecordType + } +""" + config + |> prepend newline + |> should + equal + """ +type LongerRecordName = + { + FirstAdditionalField: int + SecondAdditionalField: string + ...SomeSourceRecordType + } +""" + +[] +let ``type definition with a spread and an attribute`` () = + formatSourceString + """ +[] +type T = { ...Src; C: int } +""" + config + |> prepend newline + |> should + equal + """ +[] +type T = { ...Src; C: int } +""" + +[] +let ``type definition with a spread and an xml doc`` () = + formatSourceString + """ +/// Some documentation +type T = { ...Src; C: int } +""" + config + |> prepend newline + |> should + equal + """ +/// Some documentation +type T = { ...Src; C: int } +""" + +[] +let ``type definition with a spread and a member`` () = + formatSourceString + """ +type T = + { + ...Src + C: int + } + + member this.Total = this.C +""" + config + |> prepend newline + |> should + equal + """ +type T = + { + ...Src + C: int + } + + member this.Total = this.C +""" + +[] +let ``type definition with a spread, stroustrup`` () = + formatSourceString + """ +type LongerRecordName = { + ...SomeSourceRecordType + FirstAdditionalField: int + SecondAdditionalField: string +} +""" + { config with + MultilineBracketStyle = Stroustrup } + |> prepend newline + |> should + equal + """ +type LongerRecordName = { + ...SomeSourceRecordType + FirstAdditionalField: int + SecondAdditionalField: string +} +""" + +[] +let ``type definition with a comment before the spread`` () = + formatSourceString + """ +type T = + { // comment before the spread + ...Src + A: int + } +""" + config + |> prepend newline + |> should + equal + """ +type T = + { // comment before the spread + ...Src + A: int + } +""" + +[] +let ``type definition with a comment after the spread`` () = + formatSourceString + """ +type T = + { + ...Src // comment after the spread + A: int + } +""" + config + |> prepend newline + |> should + equal + """ +type T = + { + ...Src // comment after the spread + A: int + } +""" + +[] +let ``type definition with a comment between the spread and a field`` () = + formatSourceString + """ +type T = + { + ...Src + // comment between + A: int + } +""" + config + |> prepend newline + |> should + equal + """ +type T = + { + ...Src + // comment between + A: int + } +""" + +// ------------------------------------------------------------------------------------------- +// Record type definition in a signature file +// ------------------------------------------------------------------------------------------- + +[] +let ``signature file, type definition with only a spread`` () = + formatSignatureString + """ +module Foo + +type T = { ...Src } +""" + config + |> prepend newline + |> should + equal + """ +module Foo + +type T = { ...Src } +""" + +[] +let ``signature file, type definition with a spread and fields`` () = + formatSignatureString + """ +module Foo + +type T = { ...Src; A: int } +""" + config + |> prepend newline + |> should + equal + """ +module Foo + +type T = { ...Src; A: int } +""" + +[] +let ``signature file, multiline type definition with a spread`` () = + formatSignatureString + """ +module Foo + +type LongerRecordName = + { + ...SomeSourceRecordType + FirstAdditionalField: int + SecondAdditionalField: string + } +""" + config + |> prepend newline + |> should + equal + """ +module Foo + +type LongerRecordName = + { + ...SomeSourceRecordType + FirstAdditionalField: int + SecondAdditionalField: string + } +""" + +[] +let ``signature file, type definition with a spread and a member`` () = + formatSignatureString + """ +module Foo + +type T = + { + ...Src + C: int + } + + member Total: int +""" + config + |> prepend newline + |> should + equal + """ +module Foo + +type T = + { + ...Src + C: int + } + + member Total: int +""" + +[] +let ``signature file, type definition with a comment before the spread`` () = + formatSignatureString + """ +module Foo + +type T = + { // comment before the spread + ...Src + A: int + } +""" + config + |> prepend newline + |> should + equal + """ +module Foo + +type T = + { // comment before the spread + ...Src + A: int + } +""" + +[] +let ``signature file, type definition with a comment after the spread`` () = + formatSignatureString + """ +module Foo + +type T = + { + ...Src // comment after the spread + A: int + } +""" + config + |> prepend newline + |> should + equal + """ +module Foo + +type T = + { + ...Src // comment after the spread + A: int + } +""" + +// ------------------------------------------------------------------------------------------- +// Record expression, SynExprRecordFieldOrSpread.Spread +// ------------------------------------------------------------------------------------------- + +[] +let ``record expression with only a spread`` () = + formatSourceString """let r = { ...source }""" config + |> should + equal + """let r = { ...source } +""" + +[] +let ``record expression with a spread before a field`` () = + formatSourceString """let r = { ...source; B = 2 }""" config + |> should + equal + """let r = { ...source; B = 2 } +""" + +[] +let ``record expression with a spread after a field`` () = + formatSourceString """let r = { A = 1; ...source }""" config + |> should + equal + """let r = { A = 1; ...source } +""" + +[] +let ``record expression with multiple spreads`` () = + formatSourceString """let r = { ...first; ...second }""" config + |> should + equal + """let r = { ...first; ...second } +""" + +[] +let ``record expression with a record literal as spread source`` () = + formatSourceString """let r = { ...{ A = 1; B = 2 }; B = 9 }""" config + |> should + equal + """let r = { ...{ A = 1; B = 2 }; B = 9 } +""" + +[] +let ``record expression with an application as spread source`` () = + formatSourceString """let r = { ...makeSource arg; B = 9 }""" config + |> should + equal + """let r = { ...makeSource arg; B = 9 } +""" + +[] +let ``record expression with a parenthesized property get as spread source`` () = + formatSourceString """let r = { ...(Holder()).P; B = 9 }""" config + |> should + equal + """let r = { ...(Holder()).P; B = 9 } +""" + +[] +let ``multiline record expression with a spread`` () = + formatSourceString + """ +let r = + { + ...someRatherLongSourceExpression + FirstAdditionalField = 1 + SecondAdditionalField = "two" + } +""" + config + |> prepend newline + |> should + equal + """ +let r = + { + ...someRatherLongSourceExpression + FirstAdditionalField = 1 + SecondAdditionalField = "two" + } +""" + +[] +let ``multiline record expression with a trailing spread`` () = + formatSourceString + """ +let r = + { + FirstAdditionalField = 1 + SecondAdditionalField = "two" + ...someRatherLongSourceExpression + } +""" + config + |> prepend newline + |> should + equal + """ +let r = + { + FirstAdditionalField = 1 + SecondAdditionalField = "two" + ...someRatherLongSourceExpression + } +""" + +[] +let ``record expression with a multiline application as spread source`` () = + formatSourceString + """ +let r = + { + ...someRatherLongFunctionName firstArgument secondArgument thirdArgument fourthArgument + B = 2 + } +""" + config + |> prepend newline + |> should + equal + """ +let r = + { + ...someRatherLongFunctionName firstArgument secondArgument thirdArgument fourthArgument + B = 2 + } +""" + +[] +let ``record expression with a conditional as spread source`` () = + formatSourceString + """ +let r = + { + ...(if useDefaults then defaultSource else customSource) + B = 2 + } +""" + config + |> prepend newline + |> should + equal + """ +let r = + { + ...(if useDefaults then defaultSource else customSource) + B = 2 + } +""" + +[] +let ``record expression with a spread source that has to break`` () = + formatSourceString + """ +let r = + { + ...(someRatherLongFunctionName + aFairlyLongArgumentName + anotherFairlyLongArgumentName + aThirdFairlyLongArgumentName) + B = 2 + } +""" + config + |> prepend newline + |> should + equal + """ +let r = + { + ...(someRatherLongFunctionName + aFairlyLongArgumentName + anotherFairlyLongArgumentName + aThirdFairlyLongArgumentName) + B = 2 + } +""" + +[] +let ``anonymous record expression with a multiline application as spread source`` () = + formatSourceString + """ +let r = + {| + ...someRatherLongFunctionName firstArgument secondArgument thirdArgument fourthArg + B = 2 + |} +""" + config + |> prepend newline + |> should + equal + """ +let r = + {| + ...someRatherLongFunctionName firstArgument secondArgument thirdArgument fourthArg + B = 2 + |} +""" + +[] +let ``copy and update expression with a spread binding`` () = + formatSourceString """let r = { original with ...source }""" config + |> should + equal + """let r = { original with ...source } +""" + +[] +let ``record expression with a spread, stroustrup`` () = + formatSourceString + """ +let r = { + ...someRatherLongSourceExpression + FirstAdditionalField = 1 + SecondAdditionalField = "two" +} +""" + { config with + MultilineBracketStyle = Stroustrup } + |> prepend newline + |> should + equal + """ +let r = { + ...someRatherLongSourceExpression + FirstAdditionalField = 1 + SecondAdditionalField = "two" +} +""" + +// ------------------------------------------------------------------------------------------- +// Anonymous record expression, SynExprAnonRecordFieldOrSpread.Spread +// ------------------------------------------------------------------------------------------- + +[] +let ``anonymous record expression with only a spread`` () = + formatSourceString """let r = {| ...source |}""" config + |> should + equal + """let r = {| ...source |} +""" + +[] +let ``anonymous record expression with a spread before a field`` () = + formatSourceString """let r = {| ...source; B = 2 |}""" config + |> should + equal + """let r = {| ...source; B = 2 |} +""" + +[] +let ``anonymous record expression with a spread after a field`` () = + formatSourceString """let r = {| A = 1; ...source |}""" config + |> should + equal + """let r = {| A = 1; ...source |} +""" + +[] +let ``anonymous record expression with multiple spreads`` () = + formatSourceString """let r = {| ...first; ...second |}""" config + |> should + equal + """let r = {| ...first; ...second |} +""" + +[] +let ``struct anonymous record expression with a spread`` () = + formatSourceString """let r = struct {| ...source; B = 2 |}""" config + |> should + equal + """let r = struct {| ...source; B = 2 |} +""" + +[] +let ``anonymous record expression with an anonymous record as spread source`` () = + formatSourceString """let r = {| ...{| A = 5; B = 6 |}; A = 7 |}""" config + |> should + equal + """let r = {| ...{| A = 5; B = 6 |}; A = 7 |} +""" + +[] +let ``multiline anonymous record expression with a spread`` () = + formatSourceString + """ +let r = + {| + ...someRatherLongSourceExpression + FirstAdditionalField = 1 + SecondAdditionalField = "two" + |} +""" + config + |> prepend newline + |> should + equal + """ +let r = + {| + ...someRatherLongSourceExpression + FirstAdditionalField = 1 + SecondAdditionalField = "two" + |} +""" + +[] +let ``anonymous record expression with a spread, stroustrup`` () = + formatSourceString + """ +let r = {| + ...someRatherLongSourceExpression + FirstAdditionalField = 1 + SecondAdditionalField = "two" +|} +""" + { config with + MultilineBracketStyle = Stroustrup } + |> prepend newline + |> should + equal + """ +let r = {| + ...someRatherLongSourceExpression + FirstAdditionalField = 1 + SecondAdditionalField = "two" +|} +""" + +// ------------------------------------------------------------------------------------------- +// Spreads nested in other constructs +// ------------------------------------------------------------------------------------------- + +[] +let ``spread inside a computation expression`` () = + formatSourceString + """ +let xs = seq { for i in 1..2 -> { ...b; A = i } } +""" + config + |> prepend newline + |> should + equal + """ +let xs = seq { for i in 1..2 -> { ...b; A = i } } +""" + +[] +let ``spread inside a lambda`` () = + formatSourceString """let f = fun x -> { ...x; A = 1 }""" config + |> should + equal + """let f = fun x -> { ...x; A = 1 } +""" + +[] +let ``spread inside a quotation`` () = + formatSourceString """let q = <@ { ...p; Y = 3 } @>""" config + |> should + equal + """let q = <@ { ...p; Y = 3 } @> +""" + +[] +let ``record expression with a comment after the spread`` () = + formatSourceString + """ +let r = + { + ...source // comment after the spread + B = 2 + } +""" + config + |> prepend newline + |> should + equal + """ +let r = + { + ...source // comment after the spread + B = 2 + } +""" + +[] +let ``anonymous record expression with a comment before the spread`` () = + formatSourceString + """ +let r = + {| // comment before the spread + ...source + B = 2 + |} +""" + config + |> prepend newline + |> should + equal + """ +let r = + {| // comment before the spread + ...source + B = 2 + |} +""" + +[] +let ``anonymous record expression with a comment after the spread`` () = + formatSourceString + """ +let r = + {| + ...source // comment after the spread + B = 2 + |} +""" + config + |> prepend newline + |> should + equal + """ +let r = + {| + ...source // comment after the spread + B = 2 + |} +""" + +[] +let ``spread with a comment before the source`` () = + formatSourceString + """ +let r = + { // leading comment + ...source + B = 2 + } +""" + config + |> prepend newline + |> should + equal + """ +let r = + { // leading comment + ...source + B = 2 + } +""" diff --git a/src/Fantomas.Core/ASTTransformer.fs b/src/Fantomas.Core/ASTTransformer.fs index e78e13ae6c..656dbf677b 100644 --- a/src/Fantomas.Core/ASTTransformer.fs +++ b/src/Fantomas.Core/ASTTransformer.fs @@ -1211,6 +1211,36 @@ let mkAtomicExpr (creationAide: CreationAide) (expr: SynExpr) : Expr = | ChainExpr links -> mkChainFromLinks creationAide links ChainTerminal.NoSpaceAllowed expr.Range | _ -> mkExpr creationAide expr +let mkExprSpread (creationAide: CreationAide) (SynExprSpread(spreadRange = mDots; expr = expr; range = m)) = + ExprSpreadNode(stn "..." mDots, mkExpr creationAide expr, m) + +/// An item of a nominal record expression, `X = expr` or `...expr`. +/// A field without a name or without a value only arises from error recovery, and is dropped. +let mkExprRecordFieldOrSpread (creationAide: CreationAide) (item: SynExprRecordFieldOrSpread) = + match item with + | SynExprRecordFieldOrSpread.Field(SynExprRecordField((fieldName, _), Some mEq, Some expr, m), _) -> + Some( + ExprRecordFieldOrSpread.Field( + RecordFieldNode(mkSynLongIdent creationAide fieldName, stn "=" mEq, mkExpr creationAide expr, m) + ) + ) + | SynExprRecordFieldOrSpread.Field _ -> None + | SynExprRecordFieldOrSpread.Spread(spread, _) -> + Some(ExprRecordFieldOrSpread.Spread(mkExprSpread creationAide spread)) + +/// An item of an anonymous record expression, `X = expr` or `...expr`. +let mkExprAnonRecordFieldOrSpread (creationAide: CreationAide) (item: SynExprAnonRecordFieldOrSpread) = + match item with + | SynExprAnonRecordFieldOrSpread.Field(SynExprAnonRecordField(sli, Some mEq, e, m), _) -> + Some( + ExprRecordFieldOrSpread.Field( + RecordFieldNode(mkSynLongIdent creationAide sli, stn "=" mEq, mkExpr creationAide e, m) + ) + ) + | SynExprAnonRecordFieldOrSpread.Field _ -> None + | SynExprAnonRecordFieldOrSpread.Spread(spread, _) -> + Some(ExprRecordFieldOrSpread.Spread(mkExprSpread creationAide spread)) + let mkExpr (creationAide: CreationAide) (e: SynExpr) : Expr = let exprRange = e.Range @@ -1293,14 +1323,7 @@ let mkExpr (creationAide: CreationAide) (e: SynExpr) : Expr = ExprArrayOrListNode(o, [ mkExpr creationAide singleExpr ], c, exprRange) |> Expr.ArrayOrList | SynExpr.Record(baseInfo, copyInfo, recordFields, StartEndRange 1 (mOpen, _, mClose)) -> - let fieldNodes = - recordFields - |> List.choose (function - | SynExprRecordField((fieldName, _), Some mEq, Some expr, m, _) -> - Some( - RecordFieldNode(mkSynLongIdent creationAide fieldName, stn "=" mEq, mkExpr creationAide expr, m) - ) - | _ -> None) + let fieldNodes = List.choose (mkExprRecordFieldOrSpread creationAide) recordFields match baseInfo, copyInfo with | Some _, Some _ -> @@ -1323,15 +1346,7 @@ let mkExpr (creationAide: CreationAide) (e: SynExpr) : Expr = recordFields, (StartRange 6 (mStruct, _) & EndRange 2 (mClose, _)), { OpeningBraceRange = mOpen }) -> - let fields = - recordFields - |> List.choose (function - | sli, Some mEq, e -> - let m = unionRanges sli.Range e.Range - let longIdent = mkSynLongIdent creationAide sli - - Some(RecordFieldNode(longIdent, stn "=" mEq, mkExpr creationAide e, m)) - | _ -> None) + let fields = List.choose (mkExprAnonRecordFieldOrSpread creationAide) recordFields ExprAnonStructRecordNode( stn "struct" mStruct, @@ -1343,14 +1358,7 @@ let mkExpr (creationAide: CreationAide) (e: SynExpr) : Expr = ) |> Expr.AnonStructRecord | SynExpr.AnonRecd(false, copyInfo, recordFields, EndRange 2 (mClose, _), { OpeningBraceRange = mOpen }) -> - let fields = - recordFields - |> List.choose (function - | sli, Some mEq, e -> - let m = unionRanges sli.Range e.Range - let longIdent = mkSynLongIdent creationAide sli - Some(RecordFieldNode(longIdent, stn "=" mEq, mkExpr creationAide e, m)) - | _ -> None) + let fields = List.choose (mkExprAnonRecordFieldOrSpread creationAide) recordFields ExprRecordNode( stn "{|" mOpen, @@ -2752,6 +2760,13 @@ let mkSynLeadingKeyword (lk: SynLeadingKeyword) = | SynLeadingKeyword.Do doRange -> mtn [ "do", doRange ] | SynLeadingKeyword.Synthetic -> invariantViolation lk.Range "a synthetic leading keyword reached the transformer" +/// An item of the record representation of a type definition, `X: int` or `...Source`. +let mkTypeDefnRecordFieldOrSpread (creationAide: CreationAide) (item: SynFieldOrSpread) = + match item with + | SynFieldOrSpread.Field field -> TypeDefnRecordFieldOrSpread.Field(mkSynField creationAide field) + | SynFieldOrSpread.Spread(SynTypeSpread(spreadRange = mDots; ty = t; range = m)) -> + TypeDefnRecordFieldOrSpread.Spread(TypeSpreadNode(stn "..." mDots, mkType creationAide t, m)) + let mkSynField (creationAide: CreationAide) (SynField(ats, @@ -2929,7 +2944,7 @@ let mkTypeDefn | SynTypeDefnRepr.Simple( simpleRepr = SynTypeDefnSimpleRepr.Record(ao, fs, StartEndRange 1 (openingBrace, _, closingBrace))) -> - let fields = List.map (mkSynField creationAide) fs + let fields = List.map (mkTypeDefnRecordFieldOrSpread creationAide) fs TypeDefnRecordNode( typeNameNode, @@ -3754,7 +3769,7 @@ let mkTypeDefnSig (creationAide: CreationAide) (SynTypeDefnSig(typeInfo, typeRep | SynTypeDefnSigRepr.Simple( repr = SynTypeDefnSimpleRepr.Record(ao, fs, StartEndRange 1 (openingBrace, _, closingBrace))) -> - let fields = List.map (mkSynField creationAide) fs + let fields = List.map (mkTypeDefnRecordFieldOrSpread creationAide) fs TypeDefnRecordNode( typeNameNode, diff --git a/src/Fantomas.Core/CodePrinter.fs b/src/Fantomas.Core/CodePrinter.fs index 51d131161e..1eaf9d6ae8 100644 --- a/src/Fantomas.Core/CodePrinter.fs +++ b/src/Fantomas.Core/CodePrinter.fs @@ -1164,7 +1164,7 @@ let genExpr (e: Expr) = | Expr.Record node -> let smallRecordExpr = genSmallRecordNode node let multilineRecordExpr = genMultilineRecord node - genRecord smallRecordExpr multilineRecordExpr node + genRecordExpression smallRecordExpr multilineRecordExpr node | Expr.AnonStructRecord node -> let genStructPrefix = genSingleTextNodeWithSpaceSuffix sepSpace node.Struct let smallRecordExpr = genStructPrefix +> genSmallRecordNode node @@ -1175,7 +1175,7 @@ let genExpr (e: Expr) = else genStructPrefix +> genMultilineRecord node - genRecord smallRecordExpr multilineRecordExpr node + genRecordExpression smallRecordExpr multilineRecordExpr node | Expr.InheritRecord node -> let genSmallInheritRecordExpr = genSmallRecordBaseExpr @@ -1183,7 +1183,7 @@ let genExpr (e: Expr) = +> sepSpace +> genInheritConstructor node.InheritConstructor |> genNode (InheritConstructor.Node node.InheritConstructor)) - +> onlyIf node.HasFields sepSemi) + +> onlyIf node.HasItems sepSemi) node let genMultilineInheritRecordExpr = @@ -1194,7 +1194,7 @@ let genExpr (e: Expr) = (genSingleTextNode node.InheritConstructor.InheritKeyword +> sepSpaceOrIndentAndNlnIfExpressionExceedsPageWidth (genInheritConstructor node.InheritConstructor) |> genNode (InheritConstructor.Node node.InheritConstructor)) - +> onlyIf node.HasFields sepNln + +> onlyIf node.HasItems sepNln let genMultilineAlignBrackets = genSingleTextNode node.OpeningBrace @@ -1219,14 +1219,14 @@ let genExpr (e: Expr) = // Add spaces to ensure the record field (incl trivia) starts at the right column. addFixedSpaces targetColumn // Potential indentations will be in relation to the opening curly brace. - +> genRecordFieldNameCramped false e) + +> genExprRecordFieldOrSpread (genRecordFieldNameCramped false) e) +> addSpaceIfSpaceAroundDelimiter +> genSingleTextNode node.ClosingBrace) ctx ifAlignOrStroustrupBrackets genMultilineAlignBrackets genMultilineCramped - genRecord genSmallInheritRecordExpr genMultilineInheritRecordExpr node + genRecordExpression genSmallInheritRecordExpr genMultilineInheritRecordExpr node | Expr.ObjExpr node -> let param = optSingle genExpr node.Expr @@ -2198,11 +2198,26 @@ let genRecordFieldNameAligned (node: RecordFieldNode) = +> sepSpaceOrIndentAndNlnIfExpressionExceedsPageWidthUnlessStroustrup genExpr node.Expr +> leaveNode node +/// Print `...source`. Nothing may come between the dots and the expression, so unlike a field +/// there is no break to decide here, the source expression breaks on its own if it has to. +let genExprSpreadNode (node: ExprSpreadNode) = + genSingleTextNode node.Dots +> genExpr node.Expr |> genNode node + +/// Print one item of a record or anonymous record expression, using the given field printer for +/// fields. Every field printer variant shares the same spread printer. +let genExprRecordFieldOrSpread + (genRecordField: RecordFieldNode -> Context -> Context) + (item: ExprRecordFieldOrSpread) + : Context -> Context = + match item with + | ExprRecordFieldOrSpread.Field node -> genRecordField node + | ExprRecordFieldOrSpread.Spread node -> genExprSpreadNode node + let genMultilineRecordFieldsExpr (genRecordField: RecordFieldNode -> Context -> Context) (node: ExprRecordBaseNode) : Context -> Context = - col sepNln node.Fields genRecordField + col sepNln node.Fields (genExprRecordFieldOrSpread genRecordField) /// /// Print a (anonymous) record with additional information as a single line. @@ -2213,13 +2228,16 @@ let genSmallRecordBaseExpr genExtra (node: ExprRecordBaseNode) = genSingleTextNode node.OpeningBrace +> addSpaceIfSpaceAroundDelimiter +> genExtra - +> coli sepSemi node.Fields (fun _i rf -> - genIdentListNode rf.FieldName - +> sepSpace - +> genSingleTextNode rf.Equals - +> sepSpace - +> genExpr rf.Expr - |> genNode rf) + +> coli sepSemi node.Fields (fun _i item -> + match item with + | ExprRecordFieldOrSpread.Field rf -> + genIdentListNode rf.FieldName + +> sepSpace + +> genSingleTextNode rf.Equals + +> sepSpace + +> genExpr rf.Expr + |> genNode rf + | ExprRecordFieldOrSpread.Spread node -> genExprSpreadNode node) +> addSpaceIfSpaceAroundDelimiter +> genSingleTextNode node.ClosingBrace @@ -2296,7 +2314,7 @@ let genMultilineRecord (node: ExprRecordNode) (ctx: Context) = // Add spaces to ensure the record field (incl trivia) starts at the right column. addFixedSpaces targetColumn // Potential indentations will be in relation to the opening curly brace. - +> genRecordFieldNameCramped false e) + +> genExprRecordFieldOrSpread (genRecordFieldNameCramped false) e) ctx // Edge case scenario to make sure that the closing brace is not before the opening one @@ -2324,8 +2342,31 @@ let genMultilineRecord (node: ExprRecordNode) (ctx: Context) = ifAlignOrStroustrupBrackets genMultilineAlignBrackets genMultilineCramped ctx -let genRecord smallRecordExpr multilineRecordExpr (node: ExprRecordBaseNode) ctx = - let fieldExprs = node.Fields |> List.map (fun rf -> rf.Expr) +/// +/// Print a record or anonymous record expression, choosing between the single line and the +/// multiline rendering. This is the expression form, { A = 1 }, not the record representation +/// of a type definition. +/// +/// +/// The choice is not purely about width. When any item holds an expression that would change meaning +/// on one line, the multiline rendering is forced regardless of how short the record is. Spread items +/// take part in that check through their source expression. +/// +/// The single line rendering, including the braces. +/// The multiline rendering, including the braces. +/// The record, anonymous record or inherit-record expression node. +/// Context +let genRecordExpression + (smallRecordExpr: Context -> Context) + (multilineRecordExpr: Context -> Context) + (node: ExprRecordBaseNode) + (ctx: Context) + : Context = + let fieldExprs = + node.Fields + |> List.map (function + | ExprRecordFieldOrSpread.Field rf -> rf.Expr + | ExprRecordFieldOrSpread.Spread spread -> spread.Expr) if requiresMultilineToPreserveSemantics fieldExprs then genNode node multilineRecordExpr ctx @@ -4059,7 +4100,7 @@ let genTypeDefn (td: TypeDefn) = let multilineExpression (ctx: Context) = let genRecordFields = genSingleTextNode node.OpeningBrace - +> indentSepNlnUnindent (col sepNlnUnlessLastEventIsNewline node.Fields genField) + +> indentSepNlnUnindent (col sepNlnUnlessLastEventIsNewline node.Fields genTypeDefnRecordFieldOrSpread) +> sepNlnUnlessLastEventIsNewline +> genSingleTextNode node.ClosingBrace @@ -4067,7 +4108,10 @@ let genTypeDefn (td: TypeDefn) = onlyIf hasMembers (sepNln +> sepNlnBetweenTypeAndMembers typeDefnNode +> genMemberDefnList members) let anyFieldHasXmlDoc = - List.exists (fun (fieldNode: FieldNode) -> fieldNode.XmlDoc.IsSome) node.Fields + node.Fields + |> List.exists (function + | TypeDefnRecordFieldOrSpread.Field fieldNode -> fieldNode.XmlDoc.IsSome + | TypeDefnRecordFieldOrSpread.Spread _ -> false) let aligned = opt (indent +> sepNln) node.Accessibility genSingleTextNode @@ -4089,7 +4133,10 @@ let genTypeDefn (td: TypeDefn) = sepNlnUnlessLastEventIsNewline +> opt (indent +> sepNln) node.Accessibility genSingleTextNode +> genSingleTextNodeSuffixDelimiter node.OpeningBrace - +> atCurrentColumn (sepNlnWhenWriteBeforeNewlineNotEmpty +> col sepNln node.Fields genField) + +> atCurrentColumn ( + sepNlnWhenWriteBeforeNewlineNotEmpty + +> col sepNln node.Fields genTypeDefnRecordFieldOrSpread + ) +> addSpaceIfSpaceAroundDelimiter +> genSingleTextNode node.ClosingBrace +> optSingle (fun _ -> unindent) node.Accessibility @@ -4111,7 +4158,7 @@ let genTypeDefn (td: TypeDefn) = +> sepSpace +> genSingleTextNode node.OpeningBrace +> addSpaceIfSpaceAroundDelimiter - +> col sepSemi node.Fields genField + +> col sepSemi node.Fields genTypeDefnRecordFieldOrSpread +> addSpaceIfSpaceAroundDelimiter +> genSingleTextNode node.ClosingBrace @@ -4264,6 +4311,16 @@ let genTypeInSignature (t: Type) = | Type.Funs funsNode -> autoIndentAndNlnIfExpressionExceedsPageWidth (genTypeList funsNode) | _ -> autoIndentAndNlnIfExpressionExceedsPageWidth (genType t) +/// Print `...Source` in the record representation of a type definition. +let genTypeSpreadNode (node: TypeSpreadNode) = + genSingleTextNode node.Dots +> genType node.Type |> genNode node + +/// Print one item of the record representation of a type definition. +let genTypeDefnRecordFieldOrSpread (item: TypeDefnRecordFieldOrSpread) : Context -> Context = + match item with + | TypeDefnRecordFieldOrSpread.Field node -> genField node + | TypeDefnRecordFieldOrSpread.Spread node -> genTypeSpreadNode node + let genField (node: FieldNode) = let genAccessAndFieldContent = genAccessOpt node.Accessibility diff --git a/src/Fantomas.Core/SyntaxOak.fs b/src/Fantomas.Core/SyntaxOak.fs index 2267b6d817..ef3c022624 100644 --- a/src/Fantomas.Core/SyntaxOak.fs +++ b/src/Fantomas.Core/SyntaxOak.fs @@ -975,16 +975,37 @@ type RecordFieldNode(fieldName: IdentListNode, equals: SingleTextNode, expr: Exp member val Equals = equals member val Expr = expr -/// Abstract base for all record expression nodes, providing shared access to the braces and fields. +/// Example: `...source` — a spread of an existing value into a record or anonymous record expression. +/// The source is an arbitrary expression, the same grammar as the right-hand side of a field. +type ExprSpreadNode(dots: SingleTextNode, expr: Expr, range) = + inherit NodeBase(range) + + override val Children: Node array = [| yield dots; yield Expr.Node expr |] + member val Dots = dots + member val Expr = expr + +/// A single item inside a record or anonymous record expression, in source order. +[] +type ExprRecordFieldOrSpread = + | Field of RecordFieldNode + | Spread of ExprSpreadNode + + static member Node(item: ExprRecordFieldOrSpread) : Node = + match item with + | Field n -> n + | Spread n -> n + +/// Abstract base for all record expression nodes, providing shared access to the braces and content. [] -type ExprRecordBaseNode(openingBrace: SingleTextNode, fields: RecordFieldNode list, closingBrace: SingleTextNode, range) - = +type ExprRecordBaseNode + (openingBrace: SingleTextNode, fields: ExprRecordFieldOrSpread list, closingBrace: SingleTextNode, range) = inherit NodeBase(range) member val OpeningBrace = openingBrace member val Fields = fields member val ClosingBrace = closingBrace - member x.HasFields = List.isNotEmpty x.Fields + /// True when the braces hold anything at all, a field assignment or a spread. + member x.HasItems = List.isNotEmpty x.Fields /// /// Represents a record instance, parsed from both `SynExpr.Record` and `SynExpr.AnonRecd`. @@ -994,7 +1015,7 @@ type ExprRecordNode ( openingBrace: SingleTextNode, copyInfo: Expr option, - fields: RecordFieldNode list, + fields: ExprRecordFieldOrSpread list, closingBrace: SingleTextNode, range ) = @@ -1005,11 +1026,9 @@ type ExprRecordNode override val Children: Node array = [| yield openingBrace yield! copyInfo |> Option.map Expr.Node |> noa - yield! nodes fields + yield! List.map ExprRecordFieldOrSpread.Node fields yield closingBrace |] - member x.HasFields = List.isNotEmpty x.Fields - /// Example: `struct {| Name = "Alice"; Age = 30 |}` — an anonymous struct record expression. /// Extends `ExprRecordNode` by prepending the `struct` keyword. type ExprAnonStructRecordNode @@ -1017,7 +1036,7 @@ type ExprAnonStructRecordNode structNode: SingleTextNode, openingBrace: SingleTextNode, copyInfo: Expr option, - fields: RecordFieldNode list, + fields: ExprRecordFieldOrSpread list, closingBrace: SingleTextNode, range ) = @@ -1028,7 +1047,7 @@ type ExprAnonStructRecordNode [| yield structNode yield openingBrace yield! copyInfo |> Option.map Expr.Node |> noa - yield! nodes fields + yield! List.map ExprRecordFieldOrSpread.Node fields yield closingBrace |] /// Example: `{ inherit Base(args); Field = value }` — a record with an `inherit` constructor call. @@ -1036,7 +1055,7 @@ type ExprInheritRecordNode ( openingBrace: SingleTextNode, inheritConstructor: InheritConstructor, - fields: RecordFieldNode list, + fields: ExprRecordFieldOrSpread list, closingBrace: SingleTextNode, range ) = @@ -1047,7 +1066,7 @@ type ExprInheritRecordNode override val Children: Node array = [| yield openingBrace yield InheritConstructor.Node inheritConstructor - yield! nodes fields + yield! List.map ExprRecordFieldOrSpread.Node fields yield closingBrace |] /// Example: `interface IDisposable with member _.Dispose() = ()` — an interface implementation clause inside an object expression or type definition. @@ -2386,6 +2405,25 @@ type FieldNode member val Name = name member val Type = t +/// Example: `...Source` — a spread of an existing record type into a record type definition. +type TypeSpreadNode(dots: SingleTextNode, t: Type, range) = + inherit NodeBase(range) + + override val Children: Node array = [| yield dots; yield Type.Node t |] + member val Dots = dots + member val Type = t + +/// A single item inside the record representation of a type definition, in source order. +[] +type TypeDefnRecordFieldOrSpread = + | Field of FieldNode + | Spread of TypeSpreadNode + + static member Node(item: TypeDefnRecordFieldOrSpread) : Node = + match item with + | Field n -> n + | Spread n -> n + /// Example: `| MyCase of int * string` — a discriminated union case declaration. type UnionCaseNode ( @@ -2528,7 +2566,7 @@ type TypeDefnRecordNode typeNameNode, accessibility: SingleTextNode option, openingBrace: SingleTextNode, - fields: FieldNode list, + fields: TypeDefnRecordFieldOrSpread list, closingBrace: SingleTextNode, members, range @@ -2539,7 +2577,7 @@ type TypeDefnRecordNode [| yield typeNameNode yield! noa accessibility yield openingBrace - yield! nodes fields + yield! List.map TypeDefnRecordFieldOrSpread.Node fields yield closingBrace yield! nodes (List.map MemberDefn.Node members) |] From 80ee576642f761a69e740cb66257769fa8afe42f Mon Sep 17 00:00:00 2001 From: nojaf Date: Fri, 21 Aug 2026 14:24:16 +0200 Subject: [PATCH 05/11] Give me the link --- .claude/commands/update-fcs.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.claude/commands/update-fcs.md b/.claude/commands/update-fcs.md index 537b1aadbe..84020a98c2 100644 --- a/.claude/commands/update-fcs.md +++ b/.claude/commands/update-fcs.md @@ -21,6 +21,13 @@ vendored copy has caught up. Most steps will need no repo change at all, the has everything still builds. A few will need real work. Treat a clean step as the normal case and say so plainly, do not go looking for something to fix. +**Whenever you name an upstream commit, give the link to the PR that introduced it**, as +`https://github.com/dotnet/fsharp/pull/`. Not only in the target summary: any commit you +discuss, skip, blame for a regression or point at as future work gets its link the first time it +appears in a reply. The sha alone is not something a person can reason about, and the PR is where +the intent and the review discussion live. Take the number from the commit subject, upstream ends +every squashed subject with `(#NNNNN)`. + Every run must open and close with the same progress line, so the user can see the catch-up shrinking across invocations: From 5944e0819bfac8060a0f0badb5bb03d05b7fde52 Mon Sep 17 00:00:00 2001 From: nojaf Date: Fri, 21 Aug 2026 14:31:26 +0200 Subject: [PATCH 06/11] Update FCS to 'Implement interpolated strings via String.Concat' Bump the vendored compiler sources to d3403caee0e62ae3a64964f19fc93f20a50d6aae, dotnet/fsharp#19971. The second field of SynInterpolatedStringPart.FillExpr changed from `qualifiers: Ident option` to a SynInterpolationFormatting, which models the alignment of `{x,10}` and the specifier of `%d{x}` separately instead of leaving them inside the fill expression and the preceding string part. Nothing is lost from the tree, it carries more than before, but Fantomas read the old locations. Absorb the change in ASTTransformer so the printer and the Oak model stay as they are, and so the formatted output does not move. The alignment is folded back into the expression as the tuple the parser used to hand us, which is why `{x, 10}` keeps its existing space. The comma has no node of its own, its range is the gap between the expression and the alignment. A printf specifier no longer appears in the text of the preceding string part, though that part's range still covers it. The source text path is therefore unaffected, but the fallback used when there is no source text has to look ahead and re-append the specifier, otherwise `%d{x}` prints as `{x}` when formatting from an AST. Negative alignment, `$"{value,-10}"`, previously failed to parse and now formats. Add tests for alignment in string interpolation, which nothing covered, including one that goes through the AST rather than the source text so the reconstructed comma is exercised. --- CHANGELOG.md | 1 + Directory.Build.props | 2 +- .../InterpolatedStringTests.fs | 60 +++++++++++++++++++ src/Fantomas.Core/ASTTransformer.fs | 37 +++++++++--- 4 files changed, 92 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 00e3128495..e8464aac50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Added - Support for the record spread syntax introduced in F# preview, [RFC FS-1151](https://github.com/fsharp/fslang-design/pull/805). A spread can appear in a record expression, `{ ...source; Field = value }`, in an anonymous record expression, `{| ...source; Field = value |}`, and in the record representation of a type definition, `type Target = { ...Source; Field: int }`, in both implementation and signature files. [#3400](https://github.com/fsprojects/fantomas/pull/3400) +- Interpolated strings with a negative alignment, `$"{value,-10}"`, now format instead of failing with a parse error. Alignment and format specifiers keep their existing layout, so `$"{value,10:N2}"` is unaffected. [#3400](https://github.com/fsprojects/fantomas/pull/3400) ### Changed diff --git a/Directory.Build.props b/Directory.Build.props index ab614557be..f319c7ef9f 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -45,7 +45,7 @@ Some common use cases include: - f4b785f189aedc4a0f1ec22182e3653a0b9dd142 + d3403caee0e62ae3a64964f19fc93f20a50d6aae diff --git a/src/Fantomas.Core.Tests/InterpolatedStringTests.fs b/src/Fantomas.Core.Tests/InterpolatedStringTests.fs index 13e9a3eddb..5421494e11 100644 --- a/src/Fantomas.Core.Tests/InterpolatedStringTests.fs +++ b/src/Fantomas.Core.Tests/InterpolatedStringTests.fs @@ -165,6 +165,66 @@ $\"\"\"one: {1}< >two: {2}\"\"\" " +// The parser reports the alignment of `{x,10}` separately from the expression since +// dotnet/fsharp#19971. Fantomas has always printed a space after the comma, because the alignment +// used to arrive as part of a tuple expression. These pin that existing output. +[] +let ``alignment in string interpolation`` () = + formatSourceString + """ +let s = $"value {x,10}" +""" + config + |> prepend newline + |> should + equal + """ +let s = $"value {x, 10}" +""" + +[] +let ``negative alignment in string interpolation`` () = + formatSourceString + """ +let s = $"value {x,-10}" +""" + config + |> prepend newline + |> should + equal + """ +let s = $"value {x, -10}" +""" + +[] +let ``alignment and format in string interpolation`` () = + formatSourceString + """ +let s = $"{x,10:N2} then {y}" +""" + config + |> prepend newline + |> should + equal + """ +let s = $"{x, 10:N2} then {y}" +""" + +[] +let ``alignment in string interpolation from AST`` () = + formatAST + false + """ +$"{x,10:N2} then {y}" +""" + config + |> prepend newline + |> should + equal + """ +$"{x, 10:N2} then {y}" +""" + [] let ``format in FillExpr, 1549`` () = formatSourceString diff --git a/src/Fantomas.Core/ASTTransformer.fs b/src/Fantomas.Core/ASTTransformer.fs index 656dbf677b..f11717d30b 100644 --- a/src/Fantomas.Core/ASTTransformer.fs +++ b/src/Fantomas.Core/ASTTransformer.fs @@ -1804,26 +1804,49 @@ let mkExpr (creationAide: CreationAide) (e: SynExpr) : Expr = |> List.mapi (fun idx part -> match part with | SynInterpolatedStringPart.String(v, r) -> + // `%d{x}` puts the specifier on the following fill, not in this part's text, + // even though this part's range still covers it. Only the fallback needs it. + let specifier = + match List.tryItem (idx + 1) parts with + | Some(SynInterpolatedStringPart.FillExpr(_, SynInterpolationFormatting.Printf(specifier, _))) -> + specifier + | _ -> "" + stn (creationAide.TextFromSource (fun () -> if idx = 0 && not (String.startsWithOrdinal "$" v) then - $"$\"%s{v}{{" + $"$\"%s{v}%s{specifier}{{" elif idx = lastIndex && not (String.endsWithOrdinal "\"" v) then $"}}%s{v}\"" else - $"}}%s{v}{{") + $"}}%s{v}%s{specifier}{{") r) r |> Choice1Of2 - | SynInterpolatedStringPart.FillExpr(fillExpr, qualifiers) -> + | SynInterpolatedStringPart.FillExpr(fillExpr, formatting) -> + // The parser reports the alignment of `{x,10}` separately from the expression. + // A fill is printed as an expression optionally followed by `:format`, so fold the + // alignment back into the expression as the tuple the parser used to hand us. The + // comma is the gap between the two, it has no node of its own. + let expr, format = + match formatting with + | SynInterpolationFormatting.Printf _ -> mkExpr creationAide fillExpr, None + | SynInterpolationFormatting.DotNet(None, format) -> mkExpr creationAide fillExpr, format + | SynInterpolationFormatting.DotNet(Some alignment, format) -> + let mComma = + mkRange fillExpr.Range.FileName fillExpr.Range.End alignment.Range.Start + + let mTuple = unionRanges fillExpr.Range alignment.Range + + mkTuple creationAide [ fillExpr; alignment ] [ mComma ] mTuple |> Expr.Tuple, format + let m = - match qualifiers with - | None -> fillExpr.Range + match format with | Some ident -> unionRanges fillExpr.Range ident.idRange + | None -> unionRanges fillExpr.Range (Expr.Node expr).Range - FillExprNode(mkExpr creationAide fillExpr, Option.map mkIdent qualifiers, m) - |> Choice2Of2) + FillExprNode(expr, Option.map mkIdent format, m) |> Choice2Of2) ExprInterpolatedStringExprNode(parts, exprRange) |> Expr.InterpolatedStringExpr | SynExpr.IndexRange(None, _, None, _, _, _) -> stn "*" exprRange |> Expr.IndexRangeWildcard From 8cd57c7fe29e8b22b6dba7de695450da0321b5d0 Mon Sep 17 00:00:00 2001 From: nojaf Date: Fri, 21 Aug 2026 14:31:42 +0200 Subject: [PATCH 07/11] Improve some test names in RecordSpreadTests --- src/Fantomas.Core.Tests/RecordSpreadTests.fs | 48 ++++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/src/Fantomas.Core.Tests/RecordSpreadTests.fs b/src/Fantomas.Core.Tests/RecordSpreadTests.fs index e72c7d34d9..230cea3e4d 100644 --- a/src/Fantomas.Core.Tests/RecordSpreadTests.fs +++ b/src/Fantomas.Core.Tests/RecordSpreadTests.fs @@ -603,29 +603,7 @@ let r = """ [] -let ``anonymous record expression with a multiline application as spread source`` () = - formatSourceString - """ -let r = - {| - ...someRatherLongFunctionName firstArgument secondArgument thirdArgument fourthArg - B = 2 - |} -""" - config - |> prepend newline - |> should - equal - """ -let r = - {| - ...someRatherLongFunctionName firstArgument secondArgument thirdArgument fourthArg - B = 2 - |} -""" - -[] -let ``copy and update expression with a spread binding`` () = +let ``copy and update record expression with a spread`` () = formatSourceString """let r = { original with ...source }""" config |> should equal @@ -731,6 +709,28 @@ let r = |} """ +[] +let ``anonymous record expression with a multiline application as spread source`` () = + formatSourceString + """ +let r = + {| + ...someRatherLongFunctionName firstArgument secondArgument thirdArgument fourthArg + B = 2 + |} +""" + config + |> prepend newline + |> should + equal + """ +let r = + {| + ...someRatherLongFunctionName firstArgument secondArgument thirdArgument fourthArg + B = 2 + |} +""" + [] let ``anonymous record expression with a spread, stroustrup`` () = formatSourceString @@ -855,7 +855,7 @@ let r = """ [] -let ``spread with a comment before the source`` () = +let ``record expression with a comment before the spread`` () = formatSourceString """ let r = From bc9ff6b1f0c1716d0bd9bd1094bf888c01d4bb94 Mon Sep 17 00:00:00 2001 From: nojaf Date: Fri, 21 Aug 2026 14:34:26 +0200 Subject: [PATCH 08/11] Update FCS to 'LexFilter: drop non-strict mode', commit ea778bb4 Bump the vendored compiler sources to ea778bb414648883fca4219bf7a69286bf82dd6b, dotnet/fsharp#20106. Non-strict indentation is gone from the lexer, so the `strictIndentation` parameter disappears from the lexbuf constructors in UnicodeLexing, ParseHelpers and prim-lexing, along with its language feature flag. Drop the argument at the one call site. Formatting is unaffected. Fantomas already passed `Some true` there, so it had opted into strict indentation, which is exactly the mode that survives upstream. --- Directory.Build.props | 2 +- src/Fantomas.FCS/Parse.fs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Directory.Build.props b/Directory.Build.props index f319c7ef9f..a274bb5338 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -45,7 +45,7 @@ Some common use cases include: - d3403caee0e62ae3a64964f19fc93f20a50d6aae + ea778bb414648883fca4219bf7a69286bf82dd6b diff --git a/src/Fantomas.FCS/Parse.fs b/src/Fantomas.FCS/Parse.fs index 37521a30ff..59393d605a 100644 --- a/src/Fantomas.FCS/Parse.fs +++ b/src/Fantomas.FCS/Parse.fs @@ -382,7 +382,7 @@ let EmptyParsedInput (filename, isLastCompiland) = let createLexbuf langVersion (sourceText: ISourceText) = let lexbuf = - UnicodeLexing.SourceTextAsLexbuf(true, LanguageVersion(langVersion), Some true, sourceText) + UnicodeLexing.SourceTextAsLexbuf(true, LanguageVersion(langVersion), sourceText) lexbuf.BufferLocalStore["SourceText"] <- (sourceText :> obj) lexbuf From 362d18ad7d8356a8421616172f8cd32868d4989b Mon Sep 17 00:00:00 2001 From: nojaf Date: Fri, 21 Aug 2026 14:38:17 +0200 Subject: [PATCH 09/11] Update FCS to 'Add support for XML doc tag', commit 5260b68c Bump the vendored compiler sources to 5260b68c6db2a56b973a0fba0d614cadf3599682, dotnet/fsharp#19186. The commit adds SyntaxTree/XmlDocIncludeExpander.fs and its signature, and XmlDoc.fs now calls into them, so both vendored file lists need the pair. Neither list globs, so a new upstream file has to be added by hand to the Init download list and to the Compile items, in the same position upstream compiles it, between UnicodeLexing and XmlDoc. Formatting is unaffected. XmlDoc gained an expansion of tags used while validating a doc comment, and that validation reads files from disk, but Fantomas asks for the doc comment without checking it, so the expander is never invoked. --- Directory.Build.props | 2 +- build.fsx | 2 ++ src/Fantomas.FCS/Fantomas.FCS.fsproj | 6 ++++++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Directory.Build.props b/Directory.Build.props index a274bb5338..5971570351 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -45,7 +45,7 @@ Some common use cases include: - ea778bb414648883fca4219bf7a69286bf82dd6b + 5260b68c6db2a56b973a0fba0d614cadf3599682 diff --git a/build.fsx b/build.fsx index 58f36961cc..43103228ed 100755 --- a/build.fsx +++ b/build.fsx @@ -359,6 +359,8 @@ pipeline "Init" { "src/Compiler/pars.fsy" "src/Compiler/SyntaxTree/UnicodeLexing.fsi" "src/Compiler/SyntaxTree/UnicodeLexing.fs" + "src/Compiler/SyntaxTree/XmlDocIncludeExpander.fsi" + "src/Compiler/SyntaxTree/XmlDocIncludeExpander.fs" "src/Compiler/SyntaxTree/XmlDoc.fsi" "src/Compiler/SyntaxTree/XmlDoc.fs" "src/Compiler/SyntaxTree/SyntaxTrivia.fsi" diff --git a/src/Fantomas.FCS/Fantomas.FCS.fsproj b/src/Fantomas.FCS/Fantomas.FCS.fsproj index e1480191a1..fa84516150 100644 --- a/src/Fantomas.FCS/Fantomas.FCS.fsproj +++ b/src/Fantomas.FCS/Fantomas.FCS.fsproj @@ -255,6 +255,12 @@ SyntaxTree\UnicodeLexing.fs + + SyntaxTree\XmlDocIncludeExpander.fsi + + + SyntaxTree\XmlDocIncludeExpander.fs + SyntaxTree\XmlDoc.fsi From 364b85a385746733005e9c7db449e431249d5b68 Mon Sep 17 00:00:00 2001 From: nojaf Date: Fri, 21 Aug 2026 14:45:54 +0200 Subject: [PATCH 10/11] Update FCS to 'Remove always-on WildCardInForLoop language feature flag' Bump the vendored compiler sources from 5260b68c to afe45bfeda02e5eab2b9e8a959174a95fdf9ae5e, spanning two upstream commits that touch src/Compiler/SyntaxTree: 34a3053b Allow closing '>' of multiline nested type arguments to align with the opener (#20003) afe45bfe Remove always-on WildCardInForLoop language feature flag (#20221) The first makes the lexer treat a closing '>' as a sequence block element continuator, so a type argument list may close on a line aligned with its opener. That widens what parses and changes nothing about what Fantomas prints. The vendored compiler runs ahead of the compiler in a published SDK, so output that only the newer one accepts would not compile for users. Adopting the layout is a separate decision for later. The second drops a language feature flag that was already always on, which removes the parseState parameter from idOfPat and makes its wildcard case unconditional. Both ParseHelpers and the pars.fsy call site are vendored, so they move together. Add a test for the wildcard identifier in a for loop, `for _ = 1 to 10`. It is the syntax the second commit is about and nothing in the suite covered it, so it was passing on luck rather than on a guarantee. --- Directory.Build.props | 2 +- .../ControlStructureTests.fs | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/Directory.Build.props b/Directory.Build.props index 5971570351..d5bd4accf8 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -45,7 +45,7 @@ Some common use cases include: - 5260b68c6db2a56b973a0fba0d614cadf3599682 + afe45bfeda02e5eab2b9e8a959174a95fdf9ae5e diff --git a/src/Fantomas.Core.Tests/ControlStructureTests.fs b/src/Fantomas.Core.Tests/ControlStructureTests.fs index be6d4fb285..c3cd1e8908 100644 --- a/src/Fantomas.Core.Tests/ControlStructureTests.fs +++ b/src/Fantomas.Core.Tests/ControlStructureTests.fs @@ -45,6 +45,28 @@ if age < 10 then printfn "You are only %d years old and already learning F#? Wow!" age """ +[] +let ``wildcard identifier in for loop`` () = + formatSourceString + """ +for _ = 1 to 10 do + printfn "hi" + +for _ = 10 downto 1 do + () +""" + config + |> prepend newline + |> should + equal + """ +for _ = 1 to 10 do + printfn "hi" + +for _ = 10 downto 1 do + () +""" + [] let ``for loops`` () = formatSourceString From 0ff57212f56552184d721a0bce63dd08102e98b8 Mon Sep 17 00:00:00 2001 From: nojaf Date: Fri, 21 Aug 2026 15:42:07 +0200 Subject: [PATCH 11/11] Update FCS to 'Parser: recover on missing when conditions' Bump the vendored compiler sources from afe45bfe to d05075e098278aedcea3379159504d664628a495, spanning three upstream commits that touch src/Compiler/SyntaxTree: 208b7b4b Add symbol and type highlighting to F# diagnostics (#20097) d26c842f Remove always-on RelaxWhitespace language feature flag (#20226) d05075e0 Parser: recover on missing 'when' conditions (#20071) The first of those makes diagnostic messages carry classified text, so every SR accessor generated from FSComp.txt returns RichText instead of string. The task that generates them, FSharpEmbedResourceText, ships in the .NET SDK and is older than the sources we vendor, so it cannot emit the classified accessors and the compiler no longer builds against it. Upstream avoids this by bootstrapping its own FSharp.Build. Vendor that one task file instead and compile it in a new Fantomas.FCS.BuildTasks project. FSComp.txt moves out of the EmbeddedText item group into one of our own, which keeps the SDK's generator away from it, and a target mirroring the SDK's GenerateFSharpTextResources runs the vendored task over it. RichText and the sources it needs are marked CompileFirst so they precede the generated file, matching what upstream does in its own project. The namespace rewrite that already turns FSharp.Compiler into Fantomas.FCS on download also rewrites the namespace the task emits, so the generated SR opens Fantomas.FCS.Text. The task itself is renamed to Fantomas.FCS.Build so it cannot be confused with the one in the SDK. Parse.fs reads the text back out of the diagnostics it maps to Fantomas diagnostics, since those are reported as plain strings. Only numbered diagnostics became rich, unnumbered messages are still strings. Formatting is unaffected. Layout, indentation, when clauses and the text of parse errors were all checked against the previous hash. --- CHANGELOG.md | 1 + Directory.Build.props | 2 +- Directory.Packages.props | 4 ++ build.fsx | 13 ++++- .../Fantomas.FCS.BuildTasks.fsproj | 46 ++++++++++++++++ src/Fantomas.FCS/Fantomas.FCS.fsproj | 53 ++++++++++++++++++- src/Fantomas.FCS/Parse.fs | 6 ++- 7 files changed, 118 insertions(+), 7 deletions(-) create mode 100644 src/Fantomas.FCS.BuildTasks/Fantomas.FCS.BuildTasks.fsproj diff --git a/CHANGELOG.md b/CHANGELOG.md index e8464aac50..5b7b785057 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ ### Changed - Breaking: warnings and errors are written to standard error instead of standard out. Informational output stays on standard out, including `--version` and the files `--check` reports as needing formatting, so a caller can tell the tool's output apart from its diagnostics by stream. Scripts that capture standard out to detect failures need to capture standard error as well. [#3399](https://github.com/fsprojects/fantomas/pull/3399) +- Update FCS to 'Parser: recover on missing when conditions', commit d05075e098278aedcea3379159504d664628a495 [#3400](https://github.com/fsprojects/fantomas/pull/3400) ### Fixed diff --git a/Directory.Build.props b/Directory.Build.props index d5bd4accf8..11385b77da 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -45,7 +45,7 @@ Some common use cases include: - afe45bfeda02e5eab2b9e8a959174a95fdf9ae5e + d05075e098278aedcea3379159504d664628a495 diff --git a/Directory.Packages.props b/Directory.Packages.props index 66c978de0f..6eb04e547c 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -11,6 +11,10 @@ + + + + diff --git a/build.fsx b/build.fsx index 43103228ed..5a7a26b2a4 100755 --- a/build.fsx +++ b/build.fsx @@ -256,7 +256,9 @@ let updateFileRaw (file: FileInfo) = let updatedLines = lines |> Array.map (fun line -> - if line.Contains("FSharp.Compiler") then + if line.StartsWith("namespace FSharp.Build") then + line.Replace("namespace FSharp.Build", "namespace Fantomas.FCS.Build") + elif line.Contains("FSharp.Compiler") then line.Replace("FSharp.Compiler", "Fantomas.FCS") elif line.Contains("[]") then line.Replace("[]", "[]") @@ -292,7 +294,12 @@ pipeline "Init" { workingDir __SOURCE_DIRECTORY__ stage "Download FCS files" { run (fun _ -> - [| "src/Compiler/FSComp.txt" + [| + // Not a compiler source. This is the MSBuild task that turns FSComp.txt into the SR + // module. Since dotnet/fsharp#20097 the generated diagnostic accessors return RichText + // instead of string, and the task shipped in the .NET SDK cannot generate those yet. + "src/FSharp.Build/FSharpEmbedResourceText.fs" + "src/Compiler/FSComp.txt" "src/Compiler/FSStrings.resx" "src/Compiler/Utilities/NullHelpers.fs" "src/Compiler/Utilities/Activity.fsi" @@ -303,6 +310,8 @@ pipeline "Init" { "src/Compiler/Utilities/sformat.fs" "src/Compiler/Utilities/sr.fsi" "src/Compiler/Utilities/sr.fs" + "src/Compiler/Facilities/RichText.fsi" + "src/Compiler/Facilities/RichText.fs" "src/Compiler/Utilities/ResizeArray.fsi" "src/Compiler/Utilities/ResizeArray.fs" "src/Compiler/Utilities/HashMultiMap.fsi" diff --git a/src/Fantomas.FCS.BuildTasks/Fantomas.FCS.BuildTasks.fsproj b/src/Fantomas.FCS.BuildTasks/Fantomas.FCS.BuildTasks.fsproj new file mode 100644 index 0000000000..fb40afb0c5 --- /dev/null +++ b/src/Fantomas.FCS.BuildTasks/Fantomas.FCS.BuildTasks.fsproj @@ -0,0 +1,46 @@ + + + + + + netstandard2.0 + false + + true + + + + $(NoWarn);FS0025;FS1182;FS3390;FS1178;FS0064;FS0040 + + false + false + + false + + + + + FSharpEmbedResourceText.fs + + + + + + + + + + + + + + + diff --git a/src/Fantomas.FCS/Fantomas.FCS.fsproj b/src/Fantomas.FCS/Fantomas.FCS.fsproj index fa84516150..5a79b3dcfc 100644 --- a/src/Fantomas.FCS/Fantomas.FCS.fsproj +++ b/src/Fantomas.FCS/Fantomas.FCS.fsproj @@ -17,15 +17,23 @@ - + + FSComp.txt - + true + FSStrings.resx FSStrings.resources Utilities\NullHelpers.fs + CompileFirst Utilities\Activity.fsi @@ -41,15 +49,27 @@ Utilities\sformat.fsi + CompileFirst Utilities\sformat.fs + CompileFirst Utilities\sr.fsi + CompileFirst Utilities\sr.fs + CompileFirst + + + Facilities\RichText.fsi + CompileFirst + + + Facilities\RichText.fs + CompileFirst Utilities\ResizeArray.fsi @@ -354,4 +374,33 @@ + + + $(MSBuildThisFileDirectory)..\..\artifacts\bin\Fantomas.FCS.BuildTasks\$(Configuration.ToLowerInvariant())\Fantomas.FCS.BuildTasks.dll + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/Fantomas.FCS/Parse.fs b/src/Fantomas.FCS/Parse.fs index 59393d605a..8d67ac3ecd 100644 --- a/src/Fantomas.FCS/Parse.fs +++ b/src/Fantomas.FCS/Parse.fs @@ -1052,9 +1052,11 @@ let parseFile match p.Exception with | :? IndentationProblem as ip -> Some ip.Data1, ip.Data0, Some 58 | :? SyntaxError as se -> Some se.range, (getSyntaxErrorMessage se.Data0), Some 10 + // Diagnostic messages carry classified text since dotnet/fsharp#20097. Fantomas + // reports plain strings, so read the text back out. | :? LibraryUseOnly as luo -> Some luo.range, LibraryUseOnlyE().Format, Some 42 - | :? DiagnosticWithText as dwt -> Some dwt.range, dwt.message, Some dwt.number - | :? ReservedKeyword as rkw -> Some rkw.Data1, rkw.Data0, Some 46 + | :? DiagnosticWithText as dwt -> Some dwt.range, dwt.message.Text, Some dwt.number + | :? ReservedKeyword as rkw -> Some rkw.Data1, rkw.Data0.Text, Some 46 | _ -> None, p.Exception.Message, None { Severity = p.Severity