diff --git a/.claude/commands/update-fcs.md b/.claude/commands/update-fcs.md new file mode 100644 index 0000000000..84020a98c2 --- /dev/null +++ b/.claude/commands/update-fcs.md @@ -0,0 +1,370 @@ +--- +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. + +**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: + +``` +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. + +### 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: + +``` +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. diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f0d14b695..5b7b785057 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,15 @@ ## [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) +- 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 - 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 2fc126a694..11385b77da 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -45,7 +45,7 @@ Some common use cases include: - ab1f6ceaaec997d2854ac1c07a6c0f107675d95c + 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 58f36961cc..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" @@ -359,6 +368,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.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.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 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/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.Tests/RecordSpreadTests.fs b/src/Fantomas.Core.Tests/RecordSpreadTests.fs new file mode 100644 index 0000000000..230cea3e4d --- /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 ``copy and update record expression with a spread`` () = + 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 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 + """ +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 ``record expression with a comment before the spread`` () = + 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 b545621e17..f11717d30b 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, @@ -1796,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 @@ -2069,11 +2100,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 ] -> @@ -2695,6 +2783,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, @@ -2872,7 +2967,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, @@ -3697,7 +3792,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) |] 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 e1480191a1..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 @@ -255,6 +275,12 @@ SyntaxTree\UnicodeLexing.fs + + SyntaxTree\XmlDocIncludeExpander.fsi + + + SyntaxTree\XmlDocIncludeExpander.fs + SyntaxTree\XmlDoc.fsi @@ -348,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 37521a30ff..8d67ac3ecd 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 @@ -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