From e2cf221c6eab25e4b55cd6cffe14edc7881131a5 Mon Sep 17 00:00:00 2001 From: nojaf Date: Fri, 31 Jul 2026 15:41:05 +0200 Subject: [PATCH 01/26] Redesign ExprChain around a head, segments and a terminal call The old model held a chain as a flat `ChainLink list` with an `isLastLink` flag deciding whether a space could precede a call's opening paren. Position and permission to add a space are only incidentally the same thing, and they came apart inside a dot-lambda body: with `space_before_uppercase_invocation` enabled, `_.Substring(0, 16).ToLower()` gained a space and stopped compiling, because the printer had no way to know it was inside a `_.` lambda (#3364). A chain is now a head, a list of `ChainSegment`, and a `ChainTerminal`. Each segment carries its own dot, since a dot always belongs with what follows it, which makes two adjacent dots unrepresentable. Only the terminal negotiates a space. That is a grammar constraint rather than a style choice: a space mid-chain reparses `a.Foo (x).Bar()` as `a.Foo ((x).Bar())`. `ChainTerminal.NoSpaceAllowed` therefore makes 'no space is permitted here' a property of the node, instead of something the printer has to rediscover from context. Nodes that were chains in all but name are absorbed, leaving `Expr.Chain` as the single representation of a dotted expression: - ExprDotLambda - ExprAppLongIdentAndSingleParenArg - ExprDotIndexedGet - AppWithLambda with no prefix arguments ExprNestedIndexWithoutDot is removed as dead; nothing ever constructed it. This breaks the public Oak API: a dotted long ident such as `a.b.c` now yields `Expr.Chain` rather than `Expr.OptVar` in expression position. Layout changes follow from the model. A chain whose only call is its last step keeps the receiver, navigation and method name together and breaks the call's arguments, exactly as a call with no receiver would. Two or more calls, or a call followed by further navigation, use the leading-dot pipeline. A chain of pure property access fills lines greedily rather than fanning out one member per line. docs/docs/contributors/Chains.md states the rules in full; they are intended for the F# style guide and live under Contributors until they are adopted there. Fixes a second source of invalid F#, unrelated to spacing. A conditional directive attached to an intermediate call's argument pushed the opening paren onto its own line, which reparses the chain (FCS error 597). The paren now stays welded to the member name and the break lands after it. Branches in the chain transformer that are unreachable by construction now raise the new InvariantViolationException rather than silently returning an empty list. It derives from FormatException, which matters because the CLI selects its message by exception type and a bare failwith fell through to an empty message at normal verbosity. Each carries the source range of the construct involved and points at fantomas-tools. Adds a Coverage pipeline (dotnet fsi build.fsx -- -p Coverage) built on AltCover and ReportGenerator. It is how the unreachable branches were identified, and how two branches that looked dead were shown to be reachable but untested, which would have turned valid F# into a crash. global.json rolls forward to the latest SDK feature band so the repo builds with only 10.0.301 installed. --- .config/dotnet-tools.json | 7 + .gitignore | 5 + Directory.Packages.props | 1 + build.fsx | 43 ++ docs/docs/contributors/Chains.md | 310 ++++++++ global.json | 2 +- scripts/chain.fsx | 83 +++ .../ASTTransformerTests.fs | 680 ++++++++++++++++++ .../ChainFormattingTests.fs | 493 +++++++++++++ src/Fantomas.Core.Tests/ChainTests.fs | 275 ++++++- src/Fantomas.Core.Tests/DotIndexedGetTests.fs | 42 +- src/Fantomas.Core.Tests/DotLambdaTests.fs | 15 + .../DynamicOperatorTests.fs | 51 ++ .../Fantomas.Core.Tests.fsproj | 2 + src/Fantomas.Core.Tests/LetBindingTests.fs | 4 +- src/Fantomas.Core.Tests/SynLongIdentTests.fs | 4 +- .../TypeAnnotationTests.fs | 20 +- src/Fantomas.Core.Tests/TypeAppTests.fs | 20 +- src/Fantomas.Core.Tests/ValidationTests.fs | 31 + src/Fantomas.Core.Tests/packages.lock.json | 6 + src/Fantomas.Core/ASTTransformer.fs | 598 +++++++++------ src/Fantomas.Core/CodePrinter.fs | 670 ++++++++++------- src/Fantomas.Core/Context.fsi | 5 + src/Fantomas.Core/FormatConfig.fs | 19 + src/Fantomas.Core/Selection.fs | 9 - src/Fantomas.Core/SyntaxOak.fs | 160 ++--- 26 files changed, 2929 insertions(+), 626 deletions(-) create mode 100644 docs/docs/contributors/Chains.md create mode 100644 scripts/chain.fsx create mode 100644 src/Fantomas.Core.Tests/ChainFormattingTests.fs diff --git a/.config/dotnet-tools.json b/.config/dotnet-tools.json index 6330d0f089..6f2c08cf5a 100644 --- a/.config/dotnet-tools.json +++ b/.config/dotnet-tools.json @@ -29,6 +29,13 @@ "fsharp-analyzers" ], "rollForward": false + }, + "dotnet-reportgenerator-globaltool": { + "version": "5.5.11", + "commands": [ + "reportgenerator" + ], + "rollForward": false } } } \ No newline at end of file diff --git a/.gitignore b/.gitignore index 1b018e8243..b4e9a9cbe1 100644 --- a/.gitignore +++ b/.gitignore @@ -206,3 +206,8 @@ tests/.repositories/** # LLM stuff .claude/settings.local.json + +# Coverage reports (see the Coverage pipeline in build.fsx) +coverage.xml +coveragereport/ +coverage.xml.*.acv diff --git a/Directory.Packages.props b/Directory.Packages.props index 0529d925ec..faa198db65 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -38,5 +38,6 @@ + diff --git a/build.fsx b/build.fsx index cdd7ff1a8c..eb8ee6872b 100644 --- a/build.fsx +++ b/build.fsx @@ -61,6 +61,10 @@ let pushPackage nupkg = let analysisReportsDir = "analysisreports" +let coverageReportDir = "coveragereport" + +let coverageXml = "src" "Fantomas.Core.Tests" "coverage.xml" + pipeline "Build" { workingDir __SOURCE_DIRECTORY__ stage "RestoreTools" { run "dotnet tool restore" } @@ -87,6 +91,45 @@ pipeline "Benchmark" { runIfOnlySpecified true } +// Line and branch coverage for Fantomas.Core, via AltCover's MSBuild integration. +// +// The filter is a negative lookahead: instrument Fantomas.Core and nothing else, which keeps the +// generated Fantomas.FCS parser (and the test assembly itself) out of the report and makes the +// run fast. AltCover writes OpenCover XML, which is for tooling rather than reading, so +// ReportGenerator turns it into a browsable HTML report afterwards. +// +// Produces: +// src/Fantomas.Core.Tests/coverage.xml raw OpenCover XML +// coveragereport/index.html browsable report, per file and per line +pipeline "Coverage" { + workingDir __SOURCE_DIRECTORY__ + stage "RestoreTools" { run "dotnet tool restore" } + stage "Clean" { run (cleanFolders [| coverageReportDir |]) } + + stage "Coverage" { + run + @"dotnet test src/Fantomas.Core.Tests/Fantomas.Core.Tests.fsproj -c Release /p:AltCover=true ""/p:AltCoverAssemblyFilter=^(?!Fantomas\.Core$)""" + } + + stage "Report" { + run + $"dotnet reportgenerator -reports:{coverageXml} -targetdir:{coverageReportDir} -reporttypes:Html;TextSummary" + run (fun _ -> + async { + let summary = coverageReportDir "Summary.txt" + let index = coverageReportDir "index.html" + + if File.Exists summary then + printfn "%s" (File.ReadAllText summary) + + printfn $"Browse the full report at {index}" + return 0 + }) + } + + runIfOnlySpecified true +} + let runGitCommand (arguments: string) = async { let! result = diff --git a/docs/docs/contributors/Chains.md b/docs/docs/contributors/Chains.md new file mode 100644 index 0000000000..d897792708 --- /dev/null +++ b/docs/docs/contributors/Chains.md @@ -0,0 +1,310 @@ +--- +category: Contributors +categoryindex: 2 +index: 19 +--- +# Chains + +A *chain* is one of the few shapes where Fantomas has a real choice to make about line breaks. +This page states the rules it follows and the reasoning behind each one, in plain language and without reference to any internal types. + +## Status: a proposal, backed by an implementation + +The [F# style guide](https://learn.microsoft.com/en-us/dotnet/fsharp/style-guide/formatting) currently says very little about how to lay out a long chain. +The rules described here are meant to fill that gap and to eventually become part of that guide. +This page lives under Contributors for that reason: until the rules are officially adopted, they are a proposal we are testing, not settled guidance to hand to end-users. + +As noted in the [Fantomas style guide page](../end-users/StyleGuide.html), the style itself is not decided in the Fantomas repository. +Those conversations happen at [fsharp/fslang-design](https://github.com/fsharp/fslang-design#style-guide), and they go much better when there is something concrete to react to. +A written proposal invites arguments about hypothetical snippets. +A proposal that is already implemented lets everyone run it over a real code base and see what it does to code they care about. + +That is the order of work here: implement the rules in Fantomas first, use the implementation to find the awkward cases and settle them, then pitch the result upstream. +So treat this page as the current best answer rather than a settled one. +If you disagree with a rule, the discussion belongs at [fsharp/fslang-design](https://github.com/fsharp/fslang-design#style-guide), and having the implementation in hand is exactly what makes that discussion productive. + +## A reminder of how Fantomas works + +Fantomas does not edit your text. +Think of it like a word processor: it re-types your entire file from scratch, following its own rules. + +When it re-types a piece of code, it asks one question first: + +> Does this fit on the current line? + +If the answer is yes, it stays on one line and there is nothing more to decide. +Everything below is about what happens when the answer is **no**. + +## What counts as a chain + +A chain is a starting value followed by a series of steps, where **every step is reached through a dot**. + +```fsharp +document.Body.FirstChild.AppendChild(newNode) +``` + +Here `document` is the starting value, and `.Body`, `.FirstChild` and `.AppendChild(newNode)` are the steps. + +The dot is what matters. If there is no dot, there is no step: + +```fsharp +xs[i] // not a chain, there is no dot +f (args) // not a chain, there is no dot +arr.[i] // a chain, this indexing syntax does have a dot +``` + +An expression without any dots is laid out by other rules, not the ones on this page. + +## Two kinds of step + +Once Fantomas has to break a chain, it sorts the steps into two weights. + +**Navigation** is a step that just gets you somewhere: + +```fsharp +.Name // a plain member +.[0] // a short index +.Cast // short type arguments +``` + +**Action** is a step where something happens, meaning a call: + +```fsharp +.Foo(x) +.Bar() +.Cast() // a generic call is still a call +``` + +A call is always an action, no matter how short it is. +Type arguments make no difference to this: what makes `.Cast()` an action is the `()`, not the ``. + +That is the one pair worth keeping straight: + +```fsharp +.Cast // navigation, this only names something +.Cast() // action, this calls something +``` + +An index or a set of type arguments is navigation while it stays on one line, and becomes an action if its contents grow big enough to need several lines. + +## The rule + +```text +Does the whole chain fit on one line? +├── Yes Leave it on one line. +└── No Is there exactly one action, and is it the last step? + ├── Yes Keep everything up to the method name together, + │ and break the call's ARGUMENTS. + └── No Give each action its own line, led by its dot. +``` + +In one sentence: + +> A single `receiver.Method(args)` breaks its arguments, like any other call. +> As soon as there are two or more calls, the chain is a pipeline and each call gets its own line. + +Navigation is never worth a line of its own. It rides at the front of the line belonging to the action it introduces. + +## Examples + +The examples below assume a narrower [max line length](../end-users/Configuration.html) than the default, so the breaks are visible on this page. + +### One call at the end: the arguments break + +```fsharp +config.GetConnectionString( + "primary-database-readonly-replica-connection-string" +) +``` + +There is a single action and it is the last step, so `config.GetConnectionString(` stays together and only the argument moves. +This is exactly how an ordinary call breaks. Having a receiver in front of it changes nothing. + +### Navigation in front of a single call: still just the arguments + +```fsharp +response.Content.Headers.GetValues( + "Content-Type-And-Transfer-Encoding-Header" +) +``` + +`.Content` and `.Headers` are navigation, so there is still only one action. +The chain is not a pipeline and the navigation stays with the receiver. + +### Two or more calls: a pipeline + +```fsharp +serviceCollection + .AddSingleton(systemClock) + .AddOptions(configureOptions) +``` + +Two actions, so each one gets its own line led by its dot. + +### Navigation between calls rides along + +```fsharp +document.Body.FirstChild + .AppendChild(newNode) + .ParentElement.RemoveChild(oldNode) +``` + +`.Body` and `.FirstChild` lead the first line. +`.ParentElement` is navigation introducing `.RemoveChild(oldNode)`, so it rides at the front of that line instead of claiming one of its own. + +### A chain that ends in navigation + +```fsharp +lookupTable + .GetBucketForHash(hashOfTheKeyValue) + .Entries.[indexWithinTheBucket] +``` + +There is only one call here, but it is **not** the last step. +Breaking only the arguments would strand `.Entries.[indexWithinTheBucket]`, so the pipeline layout is used instead. + +### A chain with no calls at all + +```fsharp +this.Configuration.Database.PrimaryConnection.Settings + .IdleTimeoutInSeconds +``` + +With no actions to lead any lines, the steps simply fill the line and break before a dot when the line is full. + +### A receiver that is itself a call + +```fsharp +getConfiguredServiceBuilder() + .AddLogging(loggingOptions) + .Build() +``` + +The opening call stays glued to its `()` and acts as the starting value. +It has to stay glued: a space there would change what the code means. + +## Arguments are not the chain's business + +It is worth stating the boundary explicitly, because it is what keeps the rules above so short: + +> The chain rules decide where lines break **between** steps. +> They say nothing about what happens **inside** a call's parentheses. + +Everything between `(` and `)` is laid out by the ordinary rules for call arguments, exactly as it would be if that call had no receiver in front of it. +A chain never overrides them. +That is the same idea as "break the call's arguments" in the rule above: at that point Fantomas stops making chain decisions and hands the argument to the normal machinery. + +The practical consequence is that **every setting that governs argument layout keeps working unchanged inside a chain**. +The one you are most likely to notice is [`multi_line_lambda_closing_newline`](../end-users/Configuration.html), which decides where the closing `)` lands when a lambda argument needs several lines. + +With the default (`false`), the `)` trails the last line of the lambda: + +```fsharp +storage.SetConfigurationSettingPublisher(fun configName publisher -> + publish configName publisher) +``` + +With `true`, it drops to its own line: + +```fsharp +storage.SetConfigurationSettingPublisher(fun configName publisher -> + publish configName publisher +) +``` + +Because the setting belongs to the argument and not to the chain, it is honoured wherever the call sits. +Above it was the single trailing call. Here it is three calls inside a pipeline, with the same `true` setting: + +```fsharp +builder + .FirstThing(fun lambda -> + processFirst lambda + ) + .SecondThing(fun next -> + processSecond next + ) + .ThirdThing() + .Result +``` + +For the same reason, each call decides independently whether it needs several lines. +A long call does not drag the short ones open: + +```fsharp +repo + .Where(fun customer -> + customer.IsActive && customer.Region = targetRegion) + .Select(projector) + .ToList() +``` + +### The one place position does matter + +Match lambdas, written `(function`, are the single shape where a call's position in the chain changes the layout. + +`(function` is kept together only when all of the following hold: the call is the **last** step, `multi_line_lambda_closing_newline` is `false`, and you did not already break after the `(` yourself. + +```fsharp +builder + .Build() + .Configure(function + | Some v -> handleSome v + | None -> handleNone ()) +``` + +If any one of those is not true, `function` moves down to its own line and the closing `)` goes to its own line too: + +```fsharp +builder + .Configure( + function + | Some v -> handleSome v + | None -> handleNone () + ) + .Build() + .Result +``` + +The reason position is involved at all: mid-pipeline, the clauses would run on underneath a line that still continues with further steps, which reads poorly. +So this is a readability judgement rather than a rule forced by the language. + +It is also a good illustration of the boundary above rather than an exception to it. +`multi_line_lambda_closing_newline` is still being respected here; it simply has a visible effect on the opener as well as on the closing `)` for this particular argument shape. +And the `(` itself never leaves `.Configure`, for the parsing reason given at the end of this page. + +## The `_.` shorthand lambda + +F# lets you write `_.Property` as a short lambda. Fantomas treats it as a chain whose starting value is `_`: + +```fsharp +"yow" |> _.Substring(0, 16).ToLower() +``` + +There is one thing that makes it special. +The F# compiler requires the body of a `_.` lambda to stay tightly joined, so Fantomas will **never** insert a space before a `(` here. +This holds even when [`space_before_uppercase_invocation`](../end-users/Configuration.html) is enabled, which would otherwise produce `ToLower ()` and fail to compile. + +Apart from that, it follows the same rule as any other chain. +The example above has two calls, so if it does not fit, it becomes a pipeline: + +```fsharp +_ + .Substring(0, 16) + .ToLower() +``` + +## Why intermediate calls never get a space + +Settings such as [`space_before_uppercase_invocation`](../end-users/Configuration.html) can ask Fantomas to write `obj.Bar ()` with a space. +You may notice that this never happens to a call in the middle of a chain. +That is not a style preference. In F#, a space in the middle of a chain changes how the code is parsed: + +```fsharp +a.Foo (x).Bar() // means a.Foo ((x).Bar()) +a.Foo(x).Bar() // means what you intended +``` + +So only the very last call in a chain can be given a space, and only when a setting asks for one. +Every call before it stays tight, always. + + diff --git a/global.json b/global.json index 971b5004e2..f72210cac2 100644 --- a/global.json +++ b/global.json @@ -1,6 +1,6 @@ { "sdk": { "version": "10.0.100", - "rollForward": "latestPatch" + "rollForward": "latestFeature" } } \ No newline at end of file diff --git a/scripts/chain.fsx b/scripts/chain.fsx new file mode 100644 index 0000000000..fc74cf3849 --- /dev/null +++ b/scripts/chain.fsx @@ -0,0 +1,83 @@ +#load "shared.fsx" + +open System.IO +open Fantomas.Core +open Fantomas.Core.SyntaxOak +open Shared + +// Best-effort: extract a short display name from the member expression of a segment. +let rec exprName (e: Expr) : string = + match e with + | Expr.Ident n -> n.Text + | Expr.OptVar n -> + n.Identifier.Content + |> List.choose (function + | IdentifierOrDot.Ident i -> Some i.Text + | _ -> None) + |> String.concat "." + | Expr.TypeApp n -> $"{exprName n.Identifier}<...>" + | _ -> e.GetType().Name + +let printChain (chain: ExprChain) = + printfn "Chain:" + printfn " Head : %s" (exprName chain.Head) + + printfn " Segments: %d" chain.Segments.Length + + chain.Segments + |> List.iteri (fun i seg -> + match seg with + | ChainSegment.DotMember(_, expr) -> + printfn " [%02d] simple .%s" i (exprName expr) + | ChainSegment.DotApplication(_, expr, ChainCall.Unit _) -> + printfn " [%02d] complex .%s()" i (exprName expr) + | ChainSegment.DotApplication(_, expr, ChainCall.Paren _) -> + printfn " [%02d] complex .%s(...)" i (exprName expr) + | ChainSegment.DotIndex(_, idx) -> + printfn " [%02d] simple .[%s]" i (exprName idx)) + + let terminalStr = + match chain.Terminal with + | ChainTerminal.NoTerminal -> "(none)" + | ChainTerminal.SpaceAllowed(ChainCall.Unit _) -> "SpaceAllowed ()" + | ChainTerminal.SpaceAllowed(ChainCall.Paren _) -> "SpaceAllowed (...)" + | ChainTerminal.NoSpaceAllowed(ChainCall.Unit _) -> "NoSpaceAllowed ()" + | ChainTerminal.NoSpaceAllowed(ChainCall.Paren _) -> "NoSpaceAllowed (...)" + + printfn " Terminal: %s" terminalStr + +// Recursively collect all ExprChain nodes in the Oak's Node tree. +let rec collectChains (node: Node) : ExprChain list = + [ + match node with + | :? ExprChain as chain -> yield chain + | _ -> () + for child in node.Children do + yield! collectChains child + ] + +match Array.tryHead fsi.CommandLineArgs with +| Some scriptPath -> + let scriptFile = FileInfo(scriptPath) + let sourceFile = FileInfo(Path.Combine(__SOURCE_DIRECTORY__, __SOURCE_FILE__)) + + if scriptFile.FullName = sourceFile.FullName then + let source, _, _, _ = parseArgs fsi.CommandLineArgs.[1..] + let oak = + CodeFormatter.ParseOakAsync(false, source) + |> Async.RunSynchronously + |> Array.head + |> fst + + let chains = collectChains oak + + if chains.IsEmpty then + printfn "No chain expression found in input." + else + printfn "Found %d chain(s):\n" chains.Length + chains |> List.iteri (fun i chain -> + printfn "--- Chain #%d ---" (i + 1) + printChain chain + printfn "") +| _ -> + printfn "Usage: dotnet fsi scripts/chain.fsx " diff --git a/src/Fantomas.Core.Tests/ASTTransformerTests.fs b/src/Fantomas.Core.Tests/ASTTransformerTests.fs index 2cb7409581..f41397cd88 100644 --- a/src/Fantomas.Core.Tests/ASTTransformerTests.fs +++ b/src/Fantomas.Core.Tests/ASTTransformerTests.fs @@ -6,6 +6,50 @@ open Fantomas.FCS.Xml open Fantomas.FCS.Syntax open Fantomas.FCS.SyntaxTrivia open Fantomas.Core +open Fantomas.Core.SyntaxOak + +let private parseOak source = + CodeFormatter.ParseOakAsync(false, source) + |> Async.RunSynchronously + |> Array.head + |> fst + +let private getExprFromBinding (oak: Oak) = + match oak.ModulesOrNamespaces.[0].Declarations.[0] with + | ModuleDecl.TopLevelBinding binding -> binding.Expr + | _ -> + Assert.Fail "Expected TopLevelBinding" + raise (System.Exception()) + +let private assertIdent (text: string) (expr: Expr) = + match expr with + | Expr.Ident node -> Assert.That(node.Text, Is.EqualTo text) + | Expr.OptVar node -> + match List.tryLast node.Identifier.Content with + | Some(IdentifierOrDot.Ident identNode) -> Assert.That(identNode.Text, Is.EqualTo text) + | _ -> Assert.Fail $"Expected OptVar ending with Ident '%s{text}', got %A{expr}" + | _ -> Assert.Fail $"Expected Ident or OptVar '%s{text}', got %A{expr}" + +let private assertCount (expected: int) (list: 'a list) = + Assert.That(List.length list, Is.EqualTo expected) + +let private assertText (expected: string) (node: SingleTextNode) = + Assert.That(node.Text, Is.EqualTo expected) + +let private lastIdentText (expr: Expr) : string = + match expr with + | Expr.Ident node -> node.Text + | Expr.OptVar node -> + match List.tryLast node.Identifier.Content with + | Some(IdentifierOrDot.Ident identNode) -> identNode.Text + | _ -> failwith "no ident" + | _ -> failwithf "not an ident or optvar: %A" expr + +/// Like lastIdentText, but also looks through a type application (e.g. `OfType` -> "OfType"). +let rec private memberName (expr: Expr) : string = + match expr with + | Expr.TypeApp node -> memberName node.Identifier + | _ -> lastIdentText expr [] let ``avoid stack-overflow in long array/list, 2485`` () = @@ -59,3 +103,639 @@ let ``avoid stack-overflow in long array/list, 2485`` () = let _rootNode = ASTTransformer.mkOak None ast Assert.Pass() + +// ============================================================ +// Chain transformation tests — worked examples from chain-formatting-rationale.md +// ============================================================ + +[] +let ``a.Foo(x).Bar() — regular chain with intermediate call and terminal unit call`` () = + let oak = parseOak "let x = a.Foo(x).Bar()" + let expr = getExprFromBinding oak + + match expr with + | Expr.Chain node -> + // head = Identifier(a) + assertIdent "a" node.Head + + // segments = [DotApplication(., Foo, Paren(x)), DotMember(., Bar)] + assertCount 2 node.Segments + + match node.Segments with + | [ ChainSegment.DotApplication(dot1, fooExpr, ChainCall.Paren _); ChainSegment.DotMember(dot2, barExpr) ] -> + assertText "." dot1 + Assert.That(lastIdentText fooExpr, Is.EqualTo "Foo") + assertText "." dot2 + Assert.That(lastIdentText barExpr, Is.EqualTo "Bar") + | _ -> Assert.Fail $"Unexpected segments: %A{node.Segments}" + + // terminal = SpaceAllowed(Unit) + match node.Terminal with + | ChainTerminal.SpaceAllowed(ChainCall.Unit _) -> () + | _ -> Assert.Fail "Expected SpaceAllowed(Unit)" + | _ -> Assert.Fail "Expected Chain expression" + +[] +let ``a.b.c — pure property access chain`` () = + let oak = parseOak "let x = a.b.c" + let expr = getExprFromBinding oak + + match expr with + | Expr.Chain node -> + assertIdent "a" node.Head + assertCount 2 node.Segments + + match node.Segments with + | [ ChainSegment.DotMember(_, b); ChainSegment.DotMember(_, c) ] -> + Assert.That(lastIdentText b, Is.EqualTo "b") + Assert.That(lastIdentText c, Is.EqualTo "c") + | _ -> Assert.Fail $"Unexpected segments: %A{node.Segments}" + + match node.Terminal with + | ChainTerminal.NoTerminal -> () + | _ -> Assert.Fail "Expected NoTerminal" + | _ -> Assert.Fail $"Expected Chain expression, got %A{expr}" + +[] +let ``_.Substring(0,16).ToLower() — DotLambda becomes NoSpaceAllowed terminal`` () = + let oak = parseOak "let x = arr |> _.Substring(0,16).ToLower()" + let expr = getExprFromBinding oak + + // The pipe right side should be a Chain + let rec findChain (e: Expr) : ExprChain = + match e with + | Expr.Chain node -> node + | Expr.InfixApp app -> findChain app.RightHandSide + | _ -> + Assert.Fail "Expected to find Chain in expression" + raise (System.Exception()) + + let chainNode = findChain expr + + // head = Identifier(_) + assertIdent "_" chainNode.Head + + // segments = [DotApplication(., Substring, Paren(...)), DotMember(., ToLower)] + assertCount 2 chainNode.Segments + + match chainNode.Segments with + | [ ChainSegment.DotApplication(_, sub, ChainCall.Paren _); ChainSegment.DotMember(_, lower) ] -> + Assert.That(lastIdentText sub, Is.EqualTo "Substring") + Assert.That(lastIdentText lower, Is.EqualTo "ToLower") + | _ -> Assert.Fail $"Unexpected segments: %A{chainNode.Segments}" + + // terminal = NoSpaceAllowed(Unit) + match chainNode.Terminal with + | ChainTerminal.NoSpaceAllowed(ChainCall.Unit _) -> () + | _ -> Assert.Fail "Expected NoSpaceAllowed(Unit)" + +[] +let ``path.Replace("x","y") — absorbed from AppLongIdentAndSingleParenArg`` () = + let oak = parseOak """let x = path.Replace("x","y")""" + let expr = getExprFromBinding oak + + match expr with + | Expr.Chain node -> + assertIdent "path" node.Head + + // segments = [DotMember(., Replace)] + assertCount 1 node.Segments + + match node.Segments with + | [ ChainSegment.DotMember(_, replace) ] -> Assert.That(lastIdentText replace, Is.EqualTo "Replace") + | _ -> Assert.Fail $"Unexpected segments: %A{node.Segments}" + + // terminal = SpaceAllowed(Paren(...)) + match node.Terminal with + | ChainTerminal.SpaceAllowed(ChainCall.Paren paren) -> + // paren.Expr should be a tuple of the two string args + match paren.Expr with + | Expr.Tuple _ -> () + | _ -> Assert.Fail "Expected tuple paren for terminal call" + | _ -> Assert.Fail "Expected SpaceAllowed(Paren)" + | _ -> Assert.Fail $"Expected Chain expression, got %A{expr}" + +[] +let ``List.map (fun x -> x+1) — absorbed from AppWithLambda with no prefix args`` () = + let oak = parseOak "let x = List.map (fun x -> x+1)" + let expr = getExprFromBinding oak + + match expr with + | Expr.Chain node -> + assertIdent "List" node.Head + + // segments = [DotMember(., map)] + assertCount 1 node.Segments + + match node.Segments with + | [ ChainSegment.DotMember(_, map) ] -> Assert.That(lastIdentText map, Is.EqualTo "map") + | _ -> Assert.Fail $"Unexpected segments: %A{node.Segments}" + + // terminal = SpaceAllowed(Paren(lambda)) + match node.Terminal with + | ChainTerminal.SpaceAllowed(ChainCall.Paren paren) -> + match paren.Expr with + | Expr.Lambda _ -> () + | _ -> Assert.Fail "Expected lambda inside paren" + | _ -> Assert.Fail "Expected SpaceAllowed(Paren)" + | _ -> Assert.Fail $"Expected Chain expression, got %A{expr}" + +[] +let ``arr.[0] — standalone dot-indexed becomes single-segment chain`` () = + let oak = parseOak "let x = arr.[0]" + let expr = getExprFromBinding oak + + match expr with + | Expr.Chain node -> + assertIdent "arr" node.Head + + // segments = [DotIndex(., [0])] + assertCount 1 node.Segments + + match node.Segments with + | [ ChainSegment.DotIndex(dot, idx) ] -> + assertText "." dot + + match idx with + | Expr.Constant _ -> () + | _ -> Assert.Fail "Expected constant index expression" + | _ -> Assert.Fail $"Unexpected segments: %A{node.Segments}" + + match node.Terminal with + | ChainTerminal.NoTerminal -> () + | _ -> Assert.Fail "Expected NoTerminal" + | _ -> Assert.Fail "Expected Chain expression" + +[] +let ``arr.[0].Foo() — dot-indexed then chained`` () = + let oak = parseOak "let x = arr.[0].Foo()" + let expr = getExprFromBinding oak + + match expr with + | Expr.Chain node -> + assertIdent "arr" node.Head + + // segments = [DotIndex(., [0]), DotMember(., Foo)] + assertCount 2 node.Segments + + match node.Segments with + | [ ChainSegment.DotIndex(dot1, _); ChainSegment.DotMember(dot2, foo) ] -> + assertText "." dot1 + assertText "." dot2 + Assert.That(lastIdentText foo, Is.EqualTo "Foo") + | _ -> Assert.Fail $"Unexpected segments: %A{node.Segments}" + + // terminal = SpaceAllowed(Unit) + match node.Terminal with + | ChainTerminal.SpaceAllowed(ChainCall.Unit _) -> () + | _ -> Assert.Fail "Expected SpaceAllowed(Unit)" + | _ -> Assert.Fail "Expected Chain expression" + +[] +let ``getBuilder().Configure(opts).Build() — chain whose head is itself a call chain`` () = + let oak = parseOak "let x = getBuilder().Configure(opts).Build()" + let expr = getExprFromBinding oak + + match expr with + | Expr.Chain node -> + // head = Chain(getBuilder()) — a tight call receiver + match node.Head with + | Expr.Chain headChain -> + assertIdent "getBuilder" headChain.Head + assertCount 0 headChain.Segments + + match headChain.Terminal with + | ChainTerminal.NoSpaceAllowed(ChainCall.Unit _) -> () + | _ -> Assert.Fail "Expected NoSpaceAllowed(Unit) for head chain" + | _ -> Assert.Fail $"Expected Chain head, got %A{node.Head}" + + // segments = [DotApplication(., Configure, Paren), DotMember(., Build)] + assertCount 2 node.Segments + + match node.Segments with + | [ ChainSegment.DotApplication(_, configure, ChainCall.Paren _); ChainSegment.DotMember(_, build) ] -> + Assert.That(memberName configure, Is.EqualTo "Configure") + Assert.That(memberName build, Is.EqualTo "Build") + | _ -> Assert.Fail $"Unexpected segments: %A{node.Segments}" + + match node.Terminal with + | ChainTerminal.SpaceAllowed(ChainCall.Unit _) -> () + | _ -> Assert.Fail "Expected SpaceAllowed(Unit)" + | _ -> Assert.Fail $"Expected Chain expression, got %A{expr}" + +[] +let ``build(config).Run() — chain whose head is a call with a paren argument`` () = + let oak = parseOak "let x = build(config).Run()" + let expr = getExprFromBinding oak + + match expr with + | Expr.Chain node -> + // head = Chain(build(config)) — a tight call receiver with a paren arg + match node.Head with + | Expr.Chain headChain -> + assertIdent "build" headChain.Head + assertCount 0 headChain.Segments + + match headChain.Terminal with + | ChainTerminal.NoSpaceAllowed(ChainCall.Paren _) -> () + | _ -> Assert.Fail "Expected NoSpaceAllowed(Paren) for head chain" + | _ -> Assert.Fail $"Expected Chain head, got %A{node.Head}" + + assertCount 1 node.Segments + + match node.Segments with + | [ ChainSegment.DotMember(_, run) ] -> Assert.That(memberName run, Is.EqualTo "Run") + | _ -> Assert.Fail $"Unexpected segments: %A{node.Segments}" + + match node.Terminal with + | ChainTerminal.SpaceAllowed(ChainCall.Unit _) -> () + | _ -> Assert.Fail "Expected SpaceAllowed(Unit)" + | _ -> Assert.Fail $"Expected Chain expression, got %A{expr}" + +[] +let ``query.OfType().Where(p).Cast() — type-application call segments and terminal`` () = + let oak = parseOak "let x = query.OfType().Where(p).Cast()" + let expr = getExprFromBinding oak + + match expr with + | Expr.Chain node -> + assertIdent "query" node.Head + + // segments = [DotApplication(., OfType<_>, Unit), DotApplication(., Where, Paren), DotMember(., Cast<_>)] + assertCount 3 node.Segments + + match node.Segments with + | [ ChainSegment.DotApplication(_, ofType, ChainCall.Unit _) + ChainSegment.DotApplication(_, where, ChainCall.Paren _) + ChainSegment.DotMember(_, cast) ] -> + Assert.That(memberName ofType, Is.EqualTo "OfType") + Assert.That(memberName where, Is.EqualTo "Where") + Assert.That(memberName cast, Is.EqualTo "Cast") + + // The type-application segments are genuinely TypeApp expressions. + match ofType with + | Expr.TypeApp _ -> () + | _ -> Assert.Fail "Expected OfType to be a TypeApp" + + match cast with + | Expr.TypeApp _ -> () + | _ -> Assert.Fail "Expected Cast to be a TypeApp" + | _ -> Assert.Fail $"Unexpected segments: %A{node.Segments}" + + match node.Terminal with + | ChainTerminal.SpaceAllowed(ChainCall.Unit _) -> () + | _ -> Assert.Fail "Expected SpaceAllowed(Unit)" + | _ -> Assert.Fail $"Expected Chain expression, got %A{expr}" + +[] +let ``a.b.c.Print() — type-application navigation segment (no call)`` () = + let oak = parseOak "let x = a.b.c.Print()" + let expr = getExprFromBinding oak + + match expr with + | Expr.Chain node -> + assertIdent "a" node.Head + + // segments = [DotMember(., b), DotMember(., c), DotMember(., Print)] + assertCount 3 node.Segments + + match node.Segments with + | [ ChainSegment.DotMember(_, b); ChainSegment.DotMember(_, c); ChainSegment.DotMember(_, print) ] -> + Assert.That(memberName b, Is.EqualTo "b") + Assert.That(memberName c, Is.EqualTo "c") + Assert.That(memberName print, Is.EqualTo "Print") + + // `.b` is a navigation segment that carries a type application. + match b with + | Expr.TypeApp _ -> () + | _ -> Assert.Fail "Expected b to be a TypeApp" + | _ -> Assert.Fail $"Unexpected segments: %A{node.Segments}" + + match node.Terminal with + | ChainTerminal.SpaceAllowed(ChainCall.Unit _) -> () + | _ -> Assert.Fail "Expected SpaceAllowed(Unit)" + | _ -> Assert.Fail $"Expected Chain expression, got %A{expr}" + +[] +let ``x().y[0].Foo() — indexed member becomes a segment whose expr is IndexWithoutDot`` () = + let oak = parseOak "let x = x().y[0].Foo()" + let expr = getExprFromBinding oak + + match expr with + | Expr.Chain node -> + // head = Chain(x()) + match node.Head with + | Expr.Chain headChain -> + assertIdent "x" headChain.Head + assertCount 0 headChain.Segments + | _ -> Assert.Fail $"Expected Chain head, got %A{node.Head}" + + // segments = [DotMember(., IndexWithoutDot(y, [0])), DotMember(., Foo)] + assertCount 2 node.Segments + + match node.Segments with + | [ ChainSegment.DotMember(_, yIndexed); ChainSegment.DotMember(_, foo) ] -> + // `.y[0]` is a DotMember whose expr is an IndexWithoutDot (member with a dotless index suffix). + match yIndexed with + | Expr.IndexWithoutDot _ -> () + | _ -> Assert.Fail $"Expected IndexWithoutDot expr for .y[0], got %A{yIndexed}" + + Assert.That(memberName foo, Is.EqualTo "Foo") + | _ -> Assert.Fail $"Unexpected segments: %A{node.Segments}" + + match node.Terminal with + | ChainTerminal.SpaceAllowed(ChainCall.Unit _) -> () + | _ -> Assert.Fail "Expected SpaceAllowed(Unit)" + | _ -> Assert.Fail $"Expected Chain expression, got %A{expr}" + +[] +let ``node.Children[0].Render() — leading indexed member becomes the head`` () = + // Contrast with `x().y[0]`: when the indexed member is at the START of the chain, + // the whole `node.Children[0]` is the opaque head (an IndexWithoutDot), not a segment. + let oak = parseOak "let x = node.Children[0].Render()" + let expr = getExprFromBinding oak + + match expr with + | Expr.Chain node -> + // head = IndexWithoutDot(Chain(node.Children), [0]) + match node.Head with + | Expr.IndexWithoutDot idx -> + match idx.Identifier with + | Expr.Chain inner -> assertIdent "node" inner.Head + | _ -> Assert.Fail $"Expected Chain inside IndexWithoutDot head, got %A{idx.Identifier}" + | _ -> Assert.Fail $"Expected IndexWithoutDot head, got %A{node.Head}" + + // segments = [DotMember(., Render)] + assertCount 1 node.Segments + + match node.Segments with + | [ ChainSegment.DotMember(_, render) ] -> Assert.That(memberName render, Is.EqualTo "Render") + | _ -> Assert.Fail $"Unexpected segments: %A{node.Segments}" + + match node.Terminal with + | ChainTerminal.SpaceAllowed(ChainCall.Unit _) -> () + | _ -> Assert.Fail "Expected SpaceAllowed(Unit)" + | _ -> Assert.Fail $"Expected Chain expression, got %A{expr}" + +[] +let ``a.Foo(x).b.c.Bar(y) — multiple navigation segments between two calls`` () = + let oak = parseOak "let x = a.Foo(x).b.c.Bar(y)" + let expr = getExprFromBinding oak + + match expr with + | Expr.Chain node -> + assertIdent "a" node.Head + + // segments = [DotApplication(., Foo, Paren), DotMember(., b), DotMember(., c), DotMember(., Bar)] + assertCount 4 node.Segments + + match node.Segments with + | [ ChainSegment.DotApplication(_, foo, ChainCall.Paren _) + ChainSegment.DotMember(_, b) + ChainSegment.DotMember(_, c) + ChainSegment.DotMember(_, bar) ] -> + Assert.That(memberName foo, Is.EqualTo "Foo") + Assert.That(memberName b, Is.EqualTo "b") + Assert.That(memberName c, Is.EqualTo "c") + Assert.That(memberName bar, Is.EqualTo "Bar") + | _ -> Assert.Fail $"Unexpected segments: %A{node.Segments}" + + // The trailing call `Bar(y)` is the terminal. + match node.Terminal with + | ChainTerminal.SpaceAllowed(ChainCall.Paren _) -> () + | _ -> Assert.Fail "Expected SpaceAllowed(Paren)" + | _ -> Assert.Fail $"Expected Chain expression, got %A{expr}" + +[] +let ``a.Foo(x).[2].Bar(y) — dot-index segment between two calls`` () = + let oak = parseOak "let x = a.Foo(x).[2].Bar(y)" + let expr = getExprFromBinding oak + + match expr with + | Expr.Chain node -> + assertIdent "a" node.Head + + // segments = [DotApplication(., Foo, Paren), DotIndex(., [2]), DotMember(., Bar)] + assertCount 3 node.Segments + + match node.Segments with + | [ ChainSegment.DotApplication(_, foo, ChainCall.Paren _) + ChainSegment.DotIndex(_, idx) + ChainSegment.DotMember(_, bar) ] -> + Assert.That(memberName foo, Is.EqualTo "Foo") + Assert.That(memberName bar, Is.EqualTo "Bar") + + match idx with + | Expr.Constant _ -> () + | _ -> Assert.Fail "Expected constant index expression" + | _ -> Assert.Fail $"Unexpected segments: %A{node.Segments}" + + match node.Terminal with + | ChainTerminal.SpaceAllowed(ChainCall.Paren _) -> () + | _ -> Assert.Fail "Expected SpaceAllowed(Paren)" + | _ -> Assert.Fail $"Expected Chain expression, got %A{expr}" + +[] +let ``repo.Where(fun a -> a.B).Select(f).ToList() — intermediate call carrying a lambda`` () = + let oak = parseOak "let x = repo.Where(fun a -> a.B).Select(f).ToList()" + let expr = getExprFromBinding oak + + match expr with + | Expr.Chain node -> + assertIdent "repo" node.Head + + // segments = [DotApplication(., Where, Paren(lambda)), DotApplication(., Select, Paren), DotMember(., ToList)] + assertCount 3 node.Segments + + match node.Segments with + | [ ChainSegment.DotApplication(_, where, ChainCall.Paren wherePack) + ChainSegment.DotApplication(_, select, ChainCall.Paren _) + ChainSegment.DotMember(_, toList) ] -> + Assert.That(memberName where, Is.EqualTo "Where") + Assert.That(memberName select, Is.EqualTo "Select") + Assert.That(memberName toList, Is.EqualTo "ToList") + + // The intermediate `.Where(...)` call carries a lambda argument. + match wherePack.Expr with + | Expr.Lambda _ -> () + | _ -> Assert.Fail $"Expected lambda inside Where paren, got %A{wherePack.Expr}" + | _ -> Assert.Fail $"Unexpected segments: %A{node.Segments}" + + match node.Terminal with + | ChainTerminal.SpaceAllowed(ChainCall.Unit _) -> () + | _ -> Assert.Fail "Expected SpaceAllowed(Unit)" + | _ -> Assert.Fail $"Expected Chain expression, got %A{expr}" + +[] +let ``a.Foo()[0].Bar() — new-style index (no dot) as part of the head`` () = + // Note: a.Foo()[0] requires dot-index syntax to parse: (a.Foo()).[0].Bar() + let oak = parseOak "let x = (a.Foo()).[0].Bar()" + let expr = getExprFromBinding oak + + match expr with + | Expr.Chain node -> + // head = Paren(Chain(a.Foo())) — the paren wraps the inner chain + match node.Head with + | Expr.Paren parenNode -> + match parenNode.Expr with + | Expr.Chain innerChain -> + assertIdent "a" innerChain.Head + // inner chain: a.Foo() — Foo is a segment, () is the terminal + match innerChain.Segments with + | [ ChainSegment.DotMember(_, foo) ] -> Assert.That(lastIdentText foo, Is.EqualTo "Foo") + | _ -> Assert.Fail "Expected single Foo segment in head chain" + + match innerChain.Terminal with + | ChainTerminal.SpaceAllowed(ChainCall.Unit _) -> () + | _ -> Assert.Fail "Expected SpaceAllowed(Unit) for inner chain" + | _ -> Assert.Fail "Expected chain inside paren" + | _ -> Assert.Fail "Expected Paren as head" + + // segments = [DotIndex(., [0]), DotMember(., Bar)] + assertCount 2 node.Segments + + match node.Segments with + | [ ChainSegment.DotIndex(dot1, idx); ChainSegment.DotMember(dot2, bar) ] -> + assertText "." dot1 + assertText "." dot2 + Assert.That(lastIdentText bar, Is.EqualTo "Bar") + + match idx with + | Expr.Constant _ -> () + | _ -> Assert.Fail "Expected constant index" + | _ -> Assert.Fail $"Unexpected segments: %A{node.Segments}" + + // terminal = SpaceAllowed(Unit) + match node.Terminal with + | ChainTerminal.SpaceAllowed(ChainCall.Unit _) -> () + | _ -> Assert.Fail "Expected SpaceAllowed(Unit)" + | _ -> Assert.Fail "Expected Chain expression" + +// ============================================================ +// Negative routing — expressions that must NOT become chains +// ============================================================ +// +// A chain is a sequence of DOT-separated steps. These three shapes have no dot to +// separate anything, so the transformer must route them to their own nodes. If a +// future FCS change alters how they are grouped, these tests fail rather than +// silently widening what a chain is. + +[] +let ``List.map f (fun x -> x+1) — prefix args mean AppWithLambda, not a chain`` () = + let oak = parseOak "let x = List.map f (fun y -> y + 1)" + let expr = getExprFromBinding oak + + match expr with + | Expr.AppWithLambda node -> + // The lambda is the trailing argument; `f` sits in front of it as a prefix arg. + // That prefix arg is exactly what disqualifies this from the chain terminal model. + assertCount 1 node.Arguments + | _ -> Assert.Fail $"Expected AppWithLambda, got %A{expr}" + +[] +let ``xs[i] — new-style index has no dot, so it is not a chain`` () = + let oak = parseOak "let x = xs[i]" + let expr = getExprFromBinding oak + + match expr with + | Expr.IndexWithoutDot node -> assertIdent "xs" node.Identifier + | _ -> Assert.Fail $"Expected IndexWithoutDot, got %A{expr}" + +[] +let ``f (args) — an undotted call is AppSingleParenArg, not a chain`` () = + let oak = parseOak "let x = f (args)" + let expr = getExprFromBinding oak + + match expr with + | Expr.AppSingleParenArg node -> assertIdent "f" node.FunctionExpr + | _ -> Assert.Fail $"Expected AppSingleParenArg, got %A{expr}" + +// ============================================================ +// Zero-segment chains — the tight-receiver shape +// ============================================================ + +[] +let ``X().Y — the receiver call is a chain with zero segments and a tight terminal`` () = + let oak = parseOak "let x = X().Y" + let expr = getExprFromBinding oak + + match expr with + | Expr.Chain node -> + // The outer chain is `.Y`. + assertCount 1 node.Segments + + match node.Segments with + | [ ChainSegment.DotMember(_, y) ] -> Assert.That(lastIdentText y, Is.EqualTo "Y") + | _ -> Assert.Fail $"Unexpected segments: %A{node.Segments}" + + match node.Terminal with + | ChainTerminal.NoTerminal -> () + | _ -> Assert.Fail "Expected NoTerminal on the outer chain" + + // The head is itself a chain: `X()` with NO segments at all. It exists only to + // pair the callee with its argument under a terminal that forbids the space, + // because `X ().Y` would parse as `X (().Y)`. + match node.Head with + | Expr.Chain headChain -> + assertIdent "X" headChain.Head + assertCount 0 headChain.Segments + + match headChain.Terminal with + | ChainTerminal.NoSpaceAllowed(ChainCall.Unit _) -> () + | _ -> Assert.Fail $"Expected NoSpaceAllowed(Unit), got %A{headChain.Terminal}" + | _ -> Assert.Fail $"Expected the head to be a zero-segment Chain, got %A{node.Head}" + | _ -> Assert.Fail $"Expected Chain expression, got %A{expr}" + +[] +let ``X(a).Y — a paren receiver call is also a zero-segment tight chain`` () = + let oak = parseOak "let x = X(a).Y" + let expr = getExprFromBinding oak + + match expr with + | Expr.Chain node -> + match node.Head with + | Expr.Chain headChain -> + assertIdent "X" headChain.Head + assertCount 0 headChain.Segments + + match headChain.Terminal with + | ChainTerminal.NoSpaceAllowed(ChainCall.Paren _) -> () + | _ -> Assert.Fail $"Expected NoSpaceAllowed(Paren), got %A{headChain.Terminal}" + | _ -> Assert.Fail $"Expected the head to be a zero-segment Chain, got %A{node.Head}" + | _ -> Assert.Fail $"Expected Chain expression, got %A{expr}" + +// ============================================================ +// Dot-lambda whose body ends in an indexed member +// ============================================================ + +[] +let ``_.Values[0] — indexed member arrives as one opaque segment with no terminal`` () = + // `Values[0]` has no dot of its own, so it cannot be split into a member plus an + // index. It reaches the chain as a single opaque step, and because there is no call + // there is nothing to negotiate a space for. + let oak = parseOak "let x = xs |> List.map _.Values[0]" + let expr = getExprFromBinding oak + + // Walk the whole tree looking for the `_` chain. + let rec search (node: Node) : ExprChain option = + let isUnderscoreChain (chain: ExprChain) = + match chain.Head with + | Expr.Ident n -> n.Text = "_" + | _ -> false + + match node with + | :? ExprChain as chain when isUnderscoreChain chain -> Some chain + | _ -> node.Children |> Array.tryPick search + + match search (Expr.Node expr) with + | Some chain -> + assertCount 1 chain.Segments + + match chain.Segments with + | [ ChainSegment.DotMember(dot, _) ] -> assertText "." dot + | _ -> Assert.Fail $"Unexpected segments: %A{chain.Segments}" + + match chain.Terminal with + | ChainTerminal.NoTerminal -> () + | _ -> Assert.Fail $"Expected NoTerminal, got %A{chain.Terminal}" + | None -> Assert.Fail "Expected to find the `_` dot-lambda chain" diff --git a/src/Fantomas.Core.Tests/ChainFormattingTests.fs b/src/Fantomas.Core.Tests/ChainFormattingTests.fs new file mode 100644 index 0000000000..78c562dc31 --- /dev/null +++ b/src/Fantomas.Core.Tests/ChainFormattingTests.fs @@ -0,0 +1,493 @@ +module Fantomas.Core.Tests.ChainFormattingTests + +// This file encodes the specification in `chain-formatting-rationale.md`. +// Each test mirrors one before/after example from that document, at a narrow +// page width so the intended line breaks are visible. Where the spec shows a +// setting explicitly, the test uses it (default vs. MultiLineLambdaClosingNewline). +// +// These tests are the north star for the chain redesign: they describe the +// output the design must produce. + +open NUnit.Framework +open FsUnit +open Fantomas.Core.Tests.TestHelpers + +// ── A single call at the end — break the arguments ────────────────────────── + +[] +let ``single call at the end breaks the arguments`` () = + formatSourceString + """ +config.GetConnectionString("primary-database-readonly-replica-connection-string") +""" + { config with MaxLineLength = 70 } + |> prepend newline + |> should + equal + """ +config.GetConnectionString( + "primary-database-readonly-replica-connection-string" +) +""" + +// ── Navigation before a single call — still break the arguments ───────────── + +[] +let ``navigation before a single call still breaks the arguments`` () = + formatSourceString + """ +response.Content.Headers.GetValues("Content-Type-And-Transfer-Encoding-Header") +""" + { config with MaxLineLength = 60 } + |> prepend newline + |> should + equal + """ +response.Content.Headers.GetValues( + "Content-Type-And-Transfer-Encoding-Header" +) +""" + +// ── A single call whose argument is a lambda ──────────────────────────────── + +[] +let ``single trailing lambda call, closing newline false`` () = + formatSourceString + """ +storage.SetConfigurationSettingPublisher(fun configName publisher -> publish configName publisher) +""" + { config with MaxLineLength = 80 } + |> prepend newline + |> should + equal + """ +storage.SetConfigurationSettingPublisher(fun configName publisher -> + publish configName publisher) +""" + +[] +let ``single trailing lambda call, closing newline true`` () = + formatSourceString + """ +storage.SetConfigurationSettingPublisher(fun configName publisher -> publish configName publisher) +""" + { config with + MaxLineLength = 80 + MultiLineLambdaClosingNewline = true } + |> prepend newline + |> should + equal + """ +storage.SetConfigurationSettingPublisher(fun configName publisher -> + publish configName publisher +) +""" + +// ── Two or more calls — a pipeline ────────────────────────────────────────── + +[] +let ``two or more calls form a pipeline`` () = + formatSourceString + """ +serviceCollection.AddSingleton(systemClock).AddOptions(configureOptions) +""" + { config with MaxLineLength = 60 } + |> prepend newline + |> should + equal + """ +serviceCollection + .AddSingleton(systemClock) + .AddOptions(configureOptions) +""" + +// ── Navigation between calls rides at the front of the line ───────────────── + +[] +let ``navigation between calls rides at the front of the line`` () = + formatSourceString + """ +document.Body.FirstChild.AppendChild(newNode).ParentElement.RemoveChild(oldNode) +""" + { config with MaxLineLength = 60 } + |> prepend newline + |> should + equal + """ +document.Body.FirstChild + .AppendChild(newNode) + .ParentElement.RemoveChild(oldNode) +""" + +// ── A long pipeline with lambdas — MultiLineLambdaClosingNewline = true ────── + +[] +let ``long pipeline with lambdas, closing newline true`` () = + formatSourceString + """ +builder.FirstThing(fun lambda -> processFirst lambda).SecondThing(fun next -> processSecond next).ThirdThing().Result +""" + { config with + MaxLineLength = 40 + MultiLineLambdaClosingNewline = true } + |> prepend newline + |> should + equal + """ +builder + .FirstThing(fun lambda -> + processFirst lambda + ) + .SecondThing(fun next -> + processSecond next + ) + .ThirdThing() + .Result +""" + +// ── A chain that ends in navigation, not a call ───────────────────────────── + +[] +let ``chain that ends in navigation, not a call`` () = + formatSourceString + """ +lookupTable.GetBucketForHash(hashOfTheKeyValue).Entries.[indexWithinTheBucket] +""" + { config with MaxLineLength = 60 } + |> prepend newline + |> should + equal + """ +lookupTable + .GetBucketForHash(hashOfTheKeyValue) + .Entries.[indexWithinTheBucket] +""" + +// ── A chain with no calls at all ──────────────────────────────────────────── + +[] +let ``chain with no calls at all fills greedily`` () = + formatSourceString + """ +this.Configuration.Database.PrimaryConnection.Settings.IdleTimeoutInSeconds +""" + { config with MaxLineLength = 60 } + |> prepend newline + |> should + equal + """ +this.Configuration.Database.PrimaryConnection.Settings + .IdleTimeoutInSeconds +""" + +// ── A dot-lambda body (_.…) ───────────────────────────────────────────────── + +[] +let ``dot-lambda body stays tight and short`` () = + formatSourceString + """ +"yow" |> _.Substring(0, 16).ToLower() +""" + config + |> prepend newline + |> should + equal + """ +"yow" |> _.Substring(0, 16).ToLower() +""" + +[] +let ``dot-lambda body too long follows leading-dot`` () = + formatSourceString + """ +_.Substring(0, 16).ToLower() +""" + { config with MaxLineLength = 25 } + |> prepend newline + |> should + equal + """ +_ + .Substring(0, 16) + .ToLower() +""" + +// ── Exotic and combined shapes ────────────────────────────────────────────── + +// A pipeline where one call is multiline and the others are not. + +[] +let ``pipeline with one multiline call, closing newline false`` () = + formatSourceString + """ +repo.Where(fun customer -> customer.IsActive && customer.Region = targetRegion).Select(projector).ToList() +""" + { config with MaxLineLength = 70 } + |> prepend newline + |> should + equal + """ +repo + .Where(fun customer -> + customer.IsActive && customer.Region = targetRegion) + .Select(projector) + .ToList() +""" + +[] +let ``pipeline with one multiline call, closing newline true`` () = + formatSourceString + """ +repo.Where(fun customer -> customer.IsActive && customer.Region = targetRegion).Select(projector).ToList() +""" + { config with + MaxLineLength = 70 + MultiLineLambdaClosingNewline = true } + |> prepend newline + |> should + equal + """ +repo + .Where(fun customer -> + customer.IsActive && customer.Region = targetRegion + ) + .Select(projector) + .ToList() +""" + +// An intermediate call whose tuple arguments break. + +[] +let ``intermediate call whose tuple arguments break`` () = + formatSourceString + """ +client.Post(endpointUrl, serializedRequestPayload, requestHeaders).EnsureSuccessStatusCode().Content +""" + { config with MaxLineLength = 40 } + |> prepend newline + |> should + equal + """ +client + .Post( + endpointUrl, + serializedRequestPayload, + requestHeaders + ) + .EnsureSuccessStatusCode() + .Content +""" + +// Several navigation steps between two calls. + +[] +let ``several navigation steps between two calls`` () = + formatSourceString + """ +store.Items.Active.Filter(predicate).Results.Page.First.Render(renderContext) +""" + { config with MaxLineLength = 60 } + |> prepend newline + |> should + equal + """ +store.Items.Active + .Filter(predicate) + .Results.Page.First.Render(renderContext) +""" + +// Generic (type-application) methods. + +[] +let ``generic type-application methods`` () = + formatSourceString + """ +query.OfType().Where(activePredicate).Cast() +""" + { config with MaxLineLength = 60 } + |> prepend newline + |> should + equal + """ +query + .OfType() + .Where(activePredicate) + .Cast() +""" + +// A chain whose receiver is itself a call. + +[] +let ``chain whose receiver is itself a call`` () = + formatSourceString + """ +getConfiguredServiceBuilder().AddLogging(loggingOptions).Build() +""" + { config with MaxLineLength = 50 } + |> prepend newline + |> should + equal + """ +getConfiguredServiceBuilder() + .AddLogging(loggingOptions) + .Build() +""" + +// An index between two calls. + +[] +let ``index between two calls`` () = + formatSourceString + """ +spreadsheet.GetRow(rowIndex).[targetColumnIndex].FormatWith(cultureInfo) +""" + { config with MaxLineLength = 60 } + |> prepend newline + |> should + equal + """ +spreadsheet + .GetRow(rowIndex) + .[targetColumnIndex].FormatWith(cultureInfo) +""" + +// ── The receiver has a vote ───────────────────────────────────────────────── +// +// Keeping a chain together is only offered when the receiver is a plain value. +// A compound receiver leads a pipeline even when there is a single trailing call. + +[] +let ``plain value receiver with a single trailing call breaks the arguments`` () = + formatSourceString + """ +config.GetConnectionString(primaryDatabaseReadonlyReplica) +""" + { config with MaxLineLength = 50 } + |> prepend newline + |> should + equal + """ +config.GetConnectionString( + primaryDatabaseReadonlyReplica +) +""" + +[] +let ``call receiver with a single trailing call leads a pipeline`` () = + formatSourceString + """ +getBuilder().GetConnectionString(primaryDatabaseReplica) +""" + { config with MaxLineLength = 50 } + |> prepend newline + |> should + equal + """ +getBuilder() + .GetConnectionString(primaryDatabaseReplica) +""" + +[] +let ``parenthesised receiver with a single trailing call leads a pipeline`` () = + formatSourceString + """ +(thing :> IProvider).GetConnectionString(primaryDbReplica) +""" + { config with MaxLineLength = 50 } + |> prepend newline + |> should + equal + """ +(thing :> IProvider) + .GetConnectionString(primaryDbReplica) +""" + +[] +let ``generic receiver with a single trailing call leads a pipeline`` () = + formatSourceString + """ +Animal.GetConnectionString(primaryDbReplica) +""" + { config with MaxLineLength = 50 } + |> prepend newline + |> should + equal + """ +Animal + .GetConnectionString(primaryDbReplica) +""" + +// ── Exception: a parenthesised value that is only indexed ─────────────────── + +[] +let ``multiline parenthesised receiver keeps a lone dot-index welded`` () = + formatSourceString + """ +let first = + (line.Split([| ":" |], StringSplitOptions.RemoveEmptyEntries)).[0] +""" + { config with MaxLineLength = 60 } + |> prepend newline + |> should + equal + """ +let first = + (line.Split( + [| ":" |], + StringSplitOptions.RemoveEmptyEntries + )).[0] +""" + +[] +let ``multiline parenthesised receiver followed by a member does break`` () = + formatSourceString + """ +let first = + (line.Split([| ":" |], StringSplitOptions.RemoveEmptyEntries)).Length +""" + { config with MaxLineLength = 60 } + |> prepend newline + |> should + equal + """ +let first = + (line.Split( + [| ":" |], + StringSplitOptions.RemoveEmptyEntries + )) + .Length +""" + +// ── A generic call is still a call ────────────────────────────────────────── + +[] +let ``generic call is an action, so a lone trailing one breaks its arguments`` () = + formatSourceString + """ +repository.Cast(someArgument) +""" + { config with MaxLineLength = 50 } + |> prepend newline + |> should + equal + """ +repository.Cast( + someArgument +) +""" + +[] +let ``bare generic member is navigation and rides with the receiver`` () = + formatSourceString + """ +repository.Cast.GetConnectionString(primaryReplica) +""" + { config with MaxLineLength = 50 } + |> prepend newline + |> should + equal + """ +repository.Cast.GetConnectionString( + primaryReplica +) +""" diff --git a/src/Fantomas.Core.Tests/ChainTests.fs b/src/Fantomas.Core.Tests/ChainTests.fs index bfac68858e..725ddde987 100644 --- a/src/Fantomas.Core.Tests/ChainTests.fs +++ b/src/Fantomas.Core.Tests/ChainTests.fs @@ -289,8 +289,25 @@ Map |> should equal """ -Map.empty<_, obj> - .Add("headerAction", modifyHeader.Action.ArmValue) +Map.empty<_, obj>.Add( + "headerAction", + modifyHeader.Action.ArmValue +) +""" + +[] +let ``dotlambda chain with simple segments`` () = + formatSourceString + """ +_.Name.Length +""" + { config with MaxLineLength = 12 } + |> prepend newline + |> should + equal + """ +_.Name + .Length """ [] @@ -429,3 +446,257 @@ Animal< "Spot" ) """ + +// ── Tight receivers ───────────────────────────────────────────────────────── +// +// `mkAtomicExpr` in the ASTTransformer marks a chain that has to stay one indivisible unit, +// because a prefix operator, an index or a `?member` binds directly to it. Each test below +// covers one call site of it. They all run with `SpaceBeforeUppercaseInvocation = true`, +// because that is the setting that would otherwise introduce the space: compare with +// `obj.Bar()` on its own, which correctly becomes `obj.Bar ()` under the same config. + +[] +let ``tight receiver control case, a plain terminal call does take the space`` () = + formatSourceString + """ +obj.Bar() +""" + { config with + SpaceBeforeUppercaseInvocation = true } + |> prepend newline + |> should + equal + """ +obj.Bar () +""" + +[] +let ``tight receiver, leading expression of a dynamic chain`` () = + formatSourceString + """ +obj?A()?B() +""" + { config with + SpaceBeforeUppercaseInvocation = true } + |> prepend newline + |> should + equal + """ +obj?A()?B() +""" + +[] +let ``tight receiver, prefix operator applied to a unit call`` () = + formatSourceString + """ +-obj.Bar() +""" + { config with + SpaceBeforeUppercaseInvocation = true } + |> prepend newline + |> should + equal + """ +-obj.Bar() +""" + +[] +let ``tight receiver, prefix operator applied to a paren call`` () = + formatSourceString + """ +-obj.Bar(a) +""" + { config with + SpaceBeforeUppercaseInvocation = true } + |> prepend newline + |> should + equal + """ +-obj.Bar(a) +""" + +[] +let ``tight receiver, identifier of a new-style index`` () = + formatSourceString + """ +a.Foo()[0] +""" + { config with + SpaceBeforeUppercaseInvocation = true } + |> prepend newline + |> should + equal + """ +a.Foo()[0] +""" + +// ── Intermediate calls stay welded to their opening paren ─────────────────── +// +// An intermediate call may never be separated from its `(`: `a.Foo (x).Bar()` parses as +// `a.Foo ((x).Bar())`. A conditional directive attached to the argument pushes that +// argument onto its own lines, and the break has to land AFTER the `(`, not before it. + +[] +let ``directive inside an intermediate call argument keeps the opening paren tight`` () = + formatSourceString + """ +let x = + builder.Configure( +#if DEBUG + debugOptions +#else + releaseOptions +#endif + ).Build().Result +""" + config + |> prepend newline + |> should + equal + """ +let x = + builder + .Configure( +#if DEBUG + debugOptions +#else + releaseOptions +#endif + ) + .Build() + .Result +""" + +[] +let ``match lambda as an intermediate call argument breaks after the opening paren`` () = + formatSourceString + """ +let x = + builder.Configure(function + | Some v -> handleSome v + | None -> handleNone ()).Build().Result +""" + config + |> prepend newline + |> should + equal + """ +let x = + builder + .Configure( + function + | Some v -> handleSome v + | None -> handleNone () + ) + .Build() + .Result +""" + +[] +let ``match lambda as a terminal call argument keeps the function keyword attached`` () = + formatSourceString + """ +let x = + builder.Build().Configure(function + | Some v -> handleSome v + | None -> handleNone ()) +""" + config + |> prepend newline + |> should + equal + """ +let x = + builder + .Build() + .Configure(function + | Some v -> handleSome v + | None -> handleNone ()) +""" + +// ── Casing of the terminal is decided by the LAST segment ─────────────────── +// +// `SpaceBeforeUppercaseInvocation` looks at the name the terminal call is made on, +// which is always the final segment. Intermediate calls earlier in the chain have no +// say — and stay tight regardless of their own casing. + +[] +let ``uppercase terminal after an uppercase intermediate call takes the space`` () = + formatSourceString + """ +a.Foo(x).Bar(y) +""" + { config with + SpaceBeforeUppercaseInvocation = true } + |> prepend newline + |> should + equal + """ +a.Foo(x).Bar (y) +""" + +[] +let ``lowercase terminal after an uppercase intermediate call does not take the space`` () = + formatSourceString + """ +a.Foo(x).bar(y) +""" + { config with + SpaceBeforeUppercaseInvocation = true + SpaceBeforeLowercaseInvocation = false } + |> prepend newline + |> should + equal + """ +a.Foo(x).bar(y) +""" + +[] +let ``match lambda as a terminal call argument breaks when closing newline is set`` () = + formatSourceString + """ +let x = + builder.Build().Configure(function + | Some v -> handleSome v + | None -> handleNone ()) +""" + { config with + MultiLineLambdaClosingNewline = true } + |> prepend newline + |> should + equal + """ +let x = + builder + .Build() + .Configure( + function + | Some v -> handleSome v + | None -> handleNone () + ) +""" + +[] +let ``match lambda as a terminal call argument honours a user break after the paren`` () = + formatSourceString + """ +let x = + builder.Build().Configure( + function + | Some v -> handleSome v + | None -> handleNone ()) +""" + config + |> prepend newline + |> should + equal + """ +let x = + builder + .Build() + .Configure( + function + | Some v -> handleSome v + | None -> handleNone () + ) +""" diff --git a/src/Fantomas.Core.Tests/DotIndexedGetTests.fs b/src/Fantomas.Core.Tests/DotIndexedGetTests.fs index 9a73e8f719..79fcb86183 100644 --- a/src/Fantomas.Core.Tests/DotIndexedGetTests.fs +++ b/src/Fantomas.Core.Tests/DotIndexedGetTests.fs @@ -20,11 +20,13 @@ foo.Bar( |> should equal """ -foo.Bar( - ziggy, - jiggy, - "looooooooooooooooooooooooooooooooonStringValue" -).[5] +foo + .Bar( + ziggy, + jiggy, + "looooooooooooooooooooooooooooooooonStringValue" + ) + .[5] """ [] @@ -38,14 +40,13 @@ myList.[7].SomeFunctionCallOnSeven("looooooooooooooooooooooooooooooooonnggggStri |> should equal """ -myList.[7] - .SomeFunctionCallOnSeven( - "looooooooooooooooooooooooooooooooonnggggStringArgument", - otherArg1, - otherArg2, - otherArg3, - otherArgument4 - ) +myList.[7].SomeFunctionCallOnSeven( + "looooooooooooooooooooooooooooooooonnggggStringArgument", + otherArg1, + otherArg2, + otherArg3, + otherArgument4 +) """ [] @@ -59,14 +60,13 @@ myList.[7].lowerSomeFunctionCallOnSeven("looooooooooooooooooooooooooooooooonnggg |> should equal """ -myList.[7] - .lowerSomeFunctionCallOnSeven ( - "looooooooooooooooooooooooooooooooonnggggStringArgument", - otherArg1, - otherArg2, - otherArg3, - otherArgument4 - ) +myList.[7].lowerSomeFunctionCallOnSeven ( + "looooooooooooooooooooooooooooooooonnggggStringArgument", + otherArg1, + otherArg2, + otherArg3, + otherArgument4 +) """ [] diff --git a/src/Fantomas.Core.Tests/DotLambdaTests.fs b/src/Fantomas.Core.Tests/DotLambdaTests.fs index 67c001b824..9223e92c15 100644 --- a/src/Fantomas.Core.Tests/DotLambdaTests.fs +++ b/src/Fantomas.Core.Tests/DotLambdaTests.fs @@ -174,3 +174,18 @@ workstations category ) """ + +[] +let ``no space before unit invocation in chained dot lambda body, 3364`` () = + formatSourceString + """ +"yow" |> _.Substring(0, 16).ToLower() +""" + { config with + SpaceBeforeUppercaseInvocation = true } + |> prepend newline + |> should + equal + """ +"yow" |> _.Substring(0, 16).ToLower() +""" diff --git a/src/Fantomas.Core.Tests/DynamicOperatorTests.fs b/src/Fantomas.Core.Tests/DynamicOperatorTests.fs index c0a16b11e5..e9b4ac214f 100644 --- a/src/Fantomas.Core.Tests/DynamicOperatorTests.fs +++ b/src/Fantomas.Core.Tests/DynamicOperatorTests.fs @@ -153,3 +153,54 @@ Jest.expect(json)?oMatchSnapshot () """ Jest.expect(json)?oMatchSnapshot () """ + +// A lambda or match-lambda argument is NOT fused into the `?` chain: unlike a plain +// argument (`x?y (a)`), it stays a normal application so the argument can use the +// ordinary multiline lambda layout. + +[] +let ``dynamic operator with a lambda argument`` () = + formatSourceString + """ +let a = x?y (fun a -> a) +""" + config + |> prepend newline + |> should + equal + """ +let a = x?y (fun a -> a) +""" + +[] +let ``dynamic operator with a match lambda argument`` () = + formatSourceString + """ +let b = x?y (function + | Some v -> v + | None -> 0) +""" + config + |> prepend newline + |> should + equal + """ +let b = + x?y (function + | Some v -> v + | None -> 0) +""" + +[] +let ``dynamic chain with a lambda argument`` () = + formatSourceString + """ +let c = obj?y?z (fun a -> a) +""" + config + |> prepend newline + |> should + equal + """ +let c = obj?y?z (fun a -> a) +""" diff --git a/src/Fantomas.Core.Tests/Fantomas.Core.Tests.fsproj b/src/Fantomas.Core.Tests/Fantomas.Core.Tests.fsproj index f1903212d9..b490310d89 100644 --- a/src/Fantomas.Core.Tests/Fantomas.Core.Tests.fsproj +++ b/src/Fantomas.Core.Tests/Fantomas.Core.Tests.fsproj @@ -126,6 +126,7 @@ + @@ -149,5 +150,6 @@ + \ No newline at end of file diff --git a/src/Fantomas.Core.Tests/LetBindingTests.fs b/src/Fantomas.Core.Tests/LetBindingTests.fs index 681c12e260..b95fbdfaa1 100644 --- a/src/Fantomas.Core.Tests/LetBindingTests.fs +++ b/src/Fantomas.Core.Tests/LetBindingTests.fs @@ -1321,9 +1321,7 @@ let ``app tuple inside dotget expression`` () = """ (st :> IProvidedCustomAttributeProvider) .GetHasTypeProviderEditorHideMethodsAttribute( - info - .ProvidedType - .TypeProvider + info.ProvidedType.TypeProvider .PUntaintNoFailure(id) ) """ diff --git a/src/Fantomas.Core.Tests/SynLongIdentTests.fs b/src/Fantomas.Core.Tests/SynLongIdentTests.fs index ef99c6e95a..d8afc82978 100644 --- a/src/Fantomas.Core.Tests/SynLongIdentTests.fs +++ b/src/Fantomas.Core.Tests/SynLongIdentTests.fs @@ -312,9 +312,7 @@ Rollbar |> should equal """ -Rollbar - .RollbarLocator - .RollbarInstance +Rollbar.RollbarLocator.RollbarInstance // .AsBlockingLogger(System.TimeSpan.FromSeconds 5) .Error(package, custom) """ diff --git a/src/Fantomas.Core.Tests/TypeAnnotationTests.fs b/src/Fantomas.Core.Tests/TypeAnnotationTests.fs index bb3a1f6b69..e6a80807e1 100644 --- a/src/Fantomas.Core.Tests/TypeAnnotationTests.fs +++ b/src/Fantomas.Core.Tests/TypeAnnotationTests.fs @@ -574,15 +574,17 @@ XYZ.app int -> int -> string>.[tellMeWhy { return wouldSomeoneWriteThisCo |> should equal """ -XYZ.app< - int - -> int - -> int - -> string - >.[tellMeWhy { - return - wouldSomeoneWriteThisCode -}] +XYZ + .app< + int + -> int + -> int + -> string + > + .[tellMeWhy { + return + wouldSomeoneWriteThisCode + }] """ [] diff --git a/src/Fantomas.Core.Tests/TypeAppTests.fs b/src/Fantomas.Core.Tests/TypeAppTests.fs index 4f80885552..c1247e1e55 100644 --- a/src/Fantomas.Core.Tests/TypeAppTests.fs +++ b/src/Fantomas.Core.Tests/TypeAppTests.fs @@ -460,15 +460,17 @@ XYZ.app int -> int -> string>.[tellMeWhy { return wouldSomeoneWriteThisCo |> should equal """ -XYZ.app< - int - -> int - -> int - -> string - >.[tellMeWhy { - return - wouldSomeoneWriteThisCode -}] +XYZ + .app< + int + -> int + -> int + -> string + > + .[tellMeWhy { + return + wouldSomeoneWriteThisCode + }] """ [] diff --git a/src/Fantomas.Core.Tests/ValidationTests.fs b/src/Fantomas.Core.Tests/ValidationTests.fs index 5f45b4b66a..56fdda6198 100644 --- a/src/Fantomas.Core.Tests/ValidationTests.fs +++ b/src/Fantomas.Core.Tests/ValidationTests.fs @@ -33,3 +33,34 @@ let ``should fail on uncompilable extern functions`` () = [] let GetWindowLong hwnd : System.IntPtr, index : int : int = failwith )""" |> should equal false + +// InvariantViolationException marks a state the transformer's own model says is impossible. +// It must derive from FormatException: the CLI matches on that type to decide what to print, +// and anything else falls through to an empty message at normal verbosity. + +let private sampleRange = + Fantomas.FCS.Text.Range.mkRange + "Sample.fs" + (Fantomas.FCS.Text.Position.mkPos 7 4) + (Fantomas.FCS.Text.Position.mkPos 7 20) + +[] +let ``InvariantViolationException is reported as a FormatException`` () = + let ex = Fantomas.Core.InvariantViolationException("chain head is Foo", sampleRange) + ex |> should be instanceOfType + +[] +let ``InvariantViolationException keeps the bare invariant and points at the issue tracker`` () = + let ex = Fantomas.Core.InvariantViolationException("chain head is Foo", sampleRange) + ex.Invariant |> should equal "chain head is Foo" + ex.Message |> should haveSubstring "chain head is Foo" + ex.Message |> should haveSubstring "fsprojects.github.io/fantomas-tools" + +[] +let ``InvariantViolationException reports where in the source the violation happened`` () = + let ex = Fantomas.Core.InvariantViolationException("chain head is Foo", sampleRange) + ex.Range |> should equal sampleRange + // The location has to survive into the message, because that is all the CLI prints. + ex.Message |> should haveSubstring "line 7" + ex.Message |> should haveSubstring "column 4" + ex.Message |> should haveSubstring "Sample.fs" diff --git a/src/Fantomas.Core.Tests/packages.lock.json b/src/Fantomas.Core.Tests/packages.lock.json index 84764f29c1..ce36580c74 100644 --- a/src/Fantomas.Core.Tests/packages.lock.json +++ b/src/Fantomas.Core.Tests/packages.lock.json @@ -2,6 +2,12 @@ "version": 2, "dependencies": { "net10.0": { + "altcover": { + "type": "Direct", + "requested": "[9.0.102, )", + "resolved": "9.0.102", + "contentHash": "q3Rf5t0M9kXlcO5qhsaAe6NrFSNd5enrhKmF/Ezgmomqw34PbUTbRSYjSDNhS3YGDyUrPTkyPn14EfLDJWztcA==" + }, "FsCheck": { "type": "Direct", "requested": "[3.3.2, )", diff --git a/src/Fantomas.Core/ASTTransformer.fs b/src/Fantomas.Core/ASTTransformer.fs index 74002adc49..a0800f8b53 100644 --- a/src/Fantomas.Core/ASTTransformer.fs +++ b/src/Fantomas.Core/ASTTransformer.fs @@ -11,6 +11,12 @@ open Fantomas.Core.RangePatterns open Fantomas.Core.SyntaxOak open Microsoft.FSharp.Core +/// Raise an with a printf-style +/// message. Use this instead of `failwith` when a branch is unreachable by construction, so +/// that the CLI reports something actionable rather than an empty message. +let invariantViolation (range: range) format = + Printf.kprintf (fun msg -> raise (InvariantViolationException(msg, range))) format + [] type CreationAide = { SourceText: ISourceText option } @@ -669,7 +675,6 @@ type LinkExpr = SynExpr | Dot of range | Expr of SynExpr - | AppParenLambda of functionName: SynExpr * parenLambda: SynExpr | AppParen of functionName: SynExpr * lpr: range * e: SynExpr * rpr: range * pr: range | AppUnit of functionName: SynExpr * unit: range | IndexExpr of indexExpr: SynExpr @@ -685,10 +690,14 @@ let mkLinksFromSynLongIdent (sli: SynLongIdent) : LinkExpr list = | LinkExpr.Identifier identifierExpr -> identifierExpr.Range.StartLine, identifierExpr.Range.StartColumn | LinkExpr.Dot m -> m.StartLine, m.StartColumn | LinkExpr.Expr _ - | LinkExpr.AppParenLambda _ | LinkExpr.AppParen _ | LinkExpr.AppUnit _ - | LinkExpr.IndexExpr _ -> -1, -1) + // mkLinksFromSynLongIdent only ever builds Identifier and Dot links, so no other + // case can reach the sort key. + | LinkExpr.IndexExpr _ -> + invariantViolation + sli.Range + "mkLinksFromSynLongIdent produced a link that is neither an Identifier nor a Dot") [] let (|UnitExpr|_|) e = @@ -704,6 +713,8 @@ let (|DynamicChainArg|_|) (e: SynExpr) = | SynExpr.Const(constant = SynConst.Unit) -> ValueSome e | SynExpr.Paren(expr = inner) -> match inner with + // A lambda argument (`x?y (fun a -> a)`) is not fused into the `?` chain: it is a + // normal application whose argument needs the ordinary multiline lambda layout. | SynExpr.Lambda _ | SynExpr.MatchLambda _ -> ValueNone | _ -> ValueSome e @@ -750,8 +761,10 @@ let mkLinksFromFunctionName (mkLinkFromExpr: SynExpr -> LinkExpr) (functionName: typeArgsRange, _) -> match sli.IdentsWithTrivia with + // A generic function name only reaches here from a dotted chain, so it always has + // at least two identifiers (`a.Foo`); `Foo` on its own is not a chain. | [] - | [ _ ] -> [ mkLinkFromExpr functionName ] + | [ _ ] -> invariantViolation functionName.Range "generic function name %A has fewer than two identifiers" sli | synIdents -> let leftLinks = mkLinksFromSynLongIdent sli let lastSynIdent = List.last synIdents @@ -775,8 +788,10 @@ let mkLinksFromFunctionName (mkLinkFromExpr: SynExpr -> LinkExpr) (functionName: | SynExpr.LongIdent(longDotId = sli) -> match sli.IdentsWithTrivia with + // Likewise a plain function name: an undotted `f(x)` never becomes a chain, so by the + // time we are building links there are always at least two identifiers. | [] - | [ _ ] -> [ mkLinkFromExpr functionName ] + | [ _ ] -> invariantViolation functionName.Range "function name %A has fewer than two identifiers" sli | synIdents -> let leftLinks = mkLinksFromSynLongIdent sli let lastSynIdent = List.last synIdents @@ -785,188 +800,348 @@ let mkLinksFromFunctionName (mkLinkFromExpr: SynExpr -> LinkExpr) (functionName: yield (mkLongIdentExprFromSynIdent lastSynIdent |> mkLinkFromExpr) ] | e -> [ mkLinkFromExpr e ] -[] -let (|ChainExpr|_|) (e: SynExpr) : LinkExpr list voption = - let rec visit (e: SynExpr) (continuation: LinkExpr list -> LinkExpr list) = - match e with - | SynExpr.App( - isInfix = false - funcExpr = SynExpr.TypeApp(SynExpr.DotGet _ as funcExpr, - lessRange, - typeArgs, - commaRanges, - Some greaterRange, - typeArgsRange, - _) - argExpr = ParenExpr _ | UnitExpr _ as argExpr) -> - visit funcExpr (fun leftLinks -> - let lastLink = - match List.tryLast leftLinks with - | Some(LinkExpr.Identifier identifierExpr) -> - let typeApp = - SynExpr.TypeApp( - identifierExpr, - lessRange, - typeArgs, - commaRanges, - Some greaterRange, - typeArgsRange, - unionRanges identifierExpr.Range greaterRange - ) - - match argExpr with - | UnitExpr mUnit -> [ LinkExpr.AppUnit(typeApp, mUnit) ] - | ParenExpr(lpr, innerExpr, rpr, pr) -> [ LinkExpr.AppParen(typeApp, lpr, innerExpr, rpr, pr) ] - | _ -> [] - | _ -> [] - - let leftLinks = List.cutOffLast leftLinks - - continuation [ yield! leftLinks; yield! lastLink ]) - - | SynExpr.TypeApp(SynExpr.DotGet _ as dotGet, +/// Visits a SynExpr and produces a flat LinkExpr list representing the chain links. +/// Exposed at module level so that SynExpr.DotLambda handling can reuse the same logic. +let rec visitChainLinks (e: SynExpr) (continuation: LinkExpr list -> LinkExpr list) = + match e with + | SynExpr.App( + isInfix = false + funcExpr = SynExpr.TypeApp(SynExpr.DotGet _ as funcExpr, + lessRange, + typeArgs, + commaRanges, + Some greaterRange, + typeArgsRange, + _) + argExpr = ParenExpr _ | UnitExpr _ as argExpr) -> + visitChainLinks funcExpr (fun leftLinks -> + let lastLink = + match List.tryLast leftLinks with + | Some(LinkExpr.Identifier identifierExpr) -> + let typeApp = + SynExpr.TypeApp( + identifierExpr, + lessRange, + typeArgs, + commaRanges, + Some greaterRange, + typeArgsRange, + unionRanges identifierExpr.Range greaterRange + ) + + match argExpr with + | UnitExpr mUnit -> [ LinkExpr.AppUnit(typeApp, mUnit) ] + | ParenExpr(lpr, innerExpr, rpr, pr) -> [ LinkExpr.AppParen(typeApp, lpr, innerExpr, rpr, pr) ] + // Unreachable: the outer pattern already constrained argExpr to Paren or Unit. + | _ -> + invariantViolation + e.Range + "generic call argument %A is neither a unit nor a parenthesised expression" + argExpr + // Unreachable: visiting a DotGet always ends the link list with an Identifier. + | _ -> + invariantViolation + e.Range + "generic call has no identifier to attach its type arguments to (links: %A)" + leftLinks + + let leftLinks = List.cutOffLast leftLinks + continuation [ yield! leftLinks; yield! lastLink ]) + + | SynExpr.TypeApp(SynExpr.DotGet _ as dotGet, lessRange, typeArgs, commaRanges, Some greaterRange, typeArgsRange, _) -> + visitChainLinks dotGet (fun leftLinks -> + let lastLink = + match List.tryLast leftLinks with + | Some(LinkExpr.Identifier property) -> + [ SynExpr.TypeApp( + property, lessRange, typeArgs, commaRanges, Some greaterRange, typeArgsRange, - _) -> - visit dotGet (fun leftLinks -> - let lastLink = - match List.tryLast leftLinks with - | Some(LinkExpr.Identifier property) -> - [ SynExpr.TypeApp( - property, - lessRange, - typeArgs, - commaRanges, - Some greaterRange, - typeArgsRange, - unionRanges property.Range greaterRange - ) - |> LinkExpr.Identifier ] - | _ -> [] - - let leftLinks = List.cutOffLast leftLinks - continuation [ yield! leftLinks; yield! lastLink ]) - - // Transform `x().y[0]` into `x()` , `dot`, `y[0]` - | IndexWithoutDot(SynExpr.DotGet(expr, mDot, sli, _), indexExpr) -> - visit expr (fun leftLinks -> - let middleLinks, lastExpr = - match List.tryLast sli.IdentsWithTrivia with - | None -> [], indexExpr - | Some lastMiddleLink -> - let middleLinks = mkLinksFromSynLongIdent sli |> List.cutOffLast - - let indexWithDotExpr = - let identifierExpr = mkLongIdentExprFromSynIdent lastMiddleLink - - // Create an adjacent range for the `[`,`]` in the index expression. - let adjacentRange = - mkRange - indexExpr.Range.FileName - (Position.mkPos - identifierExpr.Range.StartLine - (identifierExpr.Range.StartColumn + 1)) - (Position.mkPos indexExpr.Range.EndLine (indexExpr.Range.EndColumn - 1)) - - SynExpr.App( - ExprAtomicFlag.Atomic, - false, - identifierExpr, - SynExpr.ArrayOrListComputed(false, indexExpr, adjacentRange), - unionRanges identifierExpr.Range indexExpr.Range - ) - - middleLinks, indexWithDotExpr - - continuation - [ yield! leftLinks - yield LinkExpr.Dot mDot - yield! middleLinks - yield LinkExpr.Expr lastExpr ]) - - | SynExpr.App(isInfix = false; funcExpr = SynExpr.DotGet _ as funcExpr; argExpr = argExpr) -> - visit funcExpr (fun leftLinks -> + unionRanges property.Range greaterRange + ) + |> LinkExpr.Identifier ] + // Unreachable: visiting a DotGet always ends the link list with an Identifier. + | _ -> + invariantViolation e.Range "type application has no identifier to attach to (links: %A)" leftLinks + + let leftLinks = List.cutOffLast leftLinks + continuation [ yield! leftLinks; yield! lastLink ]) + + // Transform `x().y[0]` into `x()` , `dot`, `y[0]` + | IndexWithoutDot(SynExpr.DotGet(expr, mDot, sli, _), indexExpr) -> + visitChainLinks expr (fun leftLinks -> + let middleLinks, lastExpr = + match List.tryLast sli.IdentsWithTrivia with + // Unreachable: a DotGet always carries at least one identifier after its dot. + | None -> invariantViolation e.Range "indexed member access has no identifier after the dot (%A)" sli + | Some lastMiddleLink -> + let middleLinks = mkLinksFromSynLongIdent sli |> List.cutOffLast + + let indexWithDotExpr = + let identifierExpr = mkLongIdentExprFromSynIdent lastMiddleLink + + // Create an adjacent range for the `[`,`]` in the index expression. + let adjacentRange = + mkRange + indexExpr.Range.FileName + (Position.mkPos identifierExpr.Range.StartLine (identifierExpr.Range.StartColumn + 1)) + (Position.mkPos indexExpr.Range.EndLine (indexExpr.Range.EndColumn - 1)) + + SynExpr.App( + ExprAtomicFlag.Atomic, + false, + identifierExpr, + SynExpr.ArrayOrListComputed(false, indexExpr, adjacentRange), + unionRanges identifierExpr.Range indexExpr.Range + ) + + middleLinks, indexWithDotExpr + + continuation + [ yield! leftLinks + yield LinkExpr.Dot mDot + yield! middleLinks + yield LinkExpr.Expr lastExpr ]) + + | SynExpr.App(isInfix = false; funcExpr = SynExpr.DotGet _ as funcExpr; argExpr = argExpr) -> + visitChainLinks funcExpr (fun leftLinks -> + match List.tryLast leftLinks with + | Some(LinkExpr.Identifier(identifierExpr)) -> + match argExpr with + | UnitExpr mUnit -> + let leftLinks = List.cutOffLast leftLinks + + // Compose a function application by taking the last identifier of the SynExpr.DotGet + // and the following argument expression. + // Example: X().Y() -> Take `Y` as function name and `()` as argument. + let rightLink = LinkExpr.AppUnit(identifierExpr, mUnit) + + continuation [ yield! leftLinks; yield rightLink ] + + | ParenExpr(lpr, e, rpr, pr) -> + let leftLinks = List.cutOffLast leftLinks + // Example: A().B(fun b -> b) + let rightLink = LinkExpr.AppParen(identifierExpr, lpr, e, rpr, pr) + continuation [ yield! leftLinks; yield rightLink ] + + // Unreachable: `(|ChainExpr|_|)` only routes Unit/Paren arguments here, and a + // dot-lambda body cannot be a space-separated application. + | _ -> + invariantViolation + e.Range + "call argument %A is neither a unit nor a parenthesised expression" + argExpr + // Unreachable: visiting a DotGet always ends the link list with an Identifier. + | _ -> invariantViolation e.Range "call has no identifier to apply its argument to (links: %A)" leftLinks) + + | SynExpr.DotGet(expr, rangeOfDot, longDotId, _) -> + visitChainLinks expr (fun links -> + continuation + [ yield! links + yield LinkExpr.Dot rangeOfDot + yield! mkLinksFromSynLongIdent longDotId ]) + + | SynExpr.App(isInfix = false; funcExpr = funcExpr; argExpr = UnitExpr mUnit) -> + mkLinksFromFunctionName (fun e -> LinkExpr.AppUnit(e, mUnit)) funcExpr + |> continuation + + | SynExpr.App(isInfix = false; funcExpr = funcExpr; argExpr = ParenExpr(lpr, e, rpr, pr)) -> + mkLinksFromFunctionName (fun f -> LinkExpr.AppParen(f, lpr, e, rpr, pr)) funcExpr + |> continuation + + | SynExpr.App(ExprAtomicFlag.Atomic, false, (SynExpr.LongIdent _ as funcExpr), (SynExpr.ArrayOrList _ as argExpr), _) -> + visitChainLinks funcExpr (fun leftLinks -> + let app = match List.tryLast leftLinks with - | Some(LinkExpr.Identifier(identifierExpr)) -> - match argExpr with - | UnitExpr mUnit -> - let leftLinks = List.cutOffLast leftLinks + | Some(LinkExpr.Identifier identifier) -> + [ SynExpr.App( + ExprAtomicFlag.Atomic, + false, + identifier, + argExpr, + unionRanges identifier.Range argExpr.Range + ) + |> LinkExpr.Expr ] + // Unreachable: visiting a LongIdent always ends the link list with an Identifier. + | _ -> + invariantViolation e.Range "indexed application has no identifier to index (links: %A)" leftLinks + + let leftLinks = List.cutOffLast leftLinks + continuation [ yield! leftLinks; yield! app ]) + + | SynExpr.TypeApp _ as typeApp -> mkLinksFromFunctionName LinkExpr.Identifier typeApp |> continuation + + | SynExpr.LongIdent(longDotId = sli) -> continuation (mkLinksFromSynLongIdent sli) + + | SynExpr.Ident _ -> continuation [ LinkExpr.Identifier e ] + + | SynExpr.DotIndexedGet(objectExpr, indexArgs, dotRange, _) -> + visitChainLinks objectExpr (fun leftLinks -> + continuation + [ yield! leftLinks + yield LinkExpr.Dot dotRange + yield LinkExpr.IndexExpr indexArgs ]) + + | other -> continuation [ LinkExpr.Expr other ] + +/// Convert a LinkExpr list (produced by visitChainLinks) into an ExprChain. +/// mkTerminal maps a ChainCall to the appropriate ChainTerminal case — +/// use ChainTerminal.SpaceAllowed for regular chains, ChainTerminal.NoSpaceAllowed for DotLambda bodies. +/// +/// This is the ONLY place an ExprChain with segments is built, so it is also the only place +/// that has to uphold the chain invariant: **the head contains no dotted content**. Every dot +/// in the source becomes a ChainSegment, so a receiver like `List.map` is split into +/// `head = List` plus `DotMember(., map)` rather than being kept as one opaque LongIdent. +/// The CodePrinter relies on this — it decides where to break by counting segments, and a +/// dot hidden inside the head would be invisible to that count. +let mkChainFromLinks + (creationAide: CreationAide) + (links: LinkExpr list) + (mkTerminal: ChainCall -> ChainTerminal) + (range: range) + : Expr = + + // The first link is the receiver; everything after it is dotted content. Because + // visitChainLinks has already peeled every dot out into its own `Dot` link, whatever + // lands here is dot-free and can be used as the head as-is. + let head, rest = + match links with + | LinkExpr.Identifier e :: rest -> mkExpr creationAide e, rest + | LinkExpr.Expr e :: rest -> mkExpr creationAide e, rest + | LinkExpr.AppUnit(f, mUnit) :: rest -> + // A function call like X() is the receiver of a dotted chain (e.g. X().Y). + // Use NoSpaceAllowed so the head always prints tight — a space before () would + // change the parse (X ().Y parses as X (().Y), not (X ()).Y). + // The result is a chain with ZERO segments: it exists only to pair a callee with + // its argument under a terminal that forbids the space. + let headChain = + ExprChain( + mkExpr creationAide f, + [], + ChainTerminal.NoSpaceAllowed(ChainCall.Unit(mkUnit mUnit)), + unionRanges f.Range mUnit + ) + + Expr.Chain headChain, rest + | LinkExpr.AppParen(f, lpr, e, rpr, pr) :: rest -> + // Similarly, X(args) as the chain receiver must be tight. + let headChain = + ExprChain( + mkExpr creationAide f, + [], + ChainTerminal.NoSpaceAllowed(ChainCall.Paren(mkParenExpr creationAide lpr e rpr pr)), + unionRanges f.Range pr + ) - // Compose a function application by taking the last identifier of the SynExpr.DotGet - // and the following argument expression. - // Example: X().Y() -> Take `Y` as function name and `()` as argument. - let rightLink = LinkExpr.AppUnit(identifierExpr, mUnit) + Expr.Chain headChain, rest + | other -> invariantViolation range "chain head is %A, which is not a receiver the model allows" other + + // Expected input: the links *after* the head, which `visitChainLinks` always emits as a + // repeating `Dot` + one content link (`Identifier` / `AppUnit` / `AppParen` / `IndexExpr` / + // `Expr`). Two dots never follow each other and two content links never sit adjacent — a + // dot always introduces exactly one thing. `processLinks` consumes that pairing two links + // at a time; the two-element arms come first because the *last* pair is what decides the + // terminal, and only there may a call negotiate a space before its `(`. + // + // A malformed list (e.g. `[Identifier; Identifier]`, no dot between them) means + // `visitChainLinks` produced something this function does not model, so it fails loudly + // rather than silently dropping links. + let rec processLinks (remainingLinks: LinkExpr list) : ChainSegment list * ChainTerminal = + match remainingLinks with + // Only reachable for a chain with no segments at all. `visitChainLinks` never + // produces this (every chain it matches has at least one dot) and the recursion + // below always stops at a two-element arm, so this is the base case for + // completeness rather than a path exercised by real source. + // Unreachable: every chain has at least one dot, and the recursion below always stops + // at one of the two-element arms, so an empty remainder is never passed in. + | [] -> invariantViolation range "a chain was built with no dotted content at all" + + | [ LinkExpr.Dot mDot; LinkExpr.Identifier f ] -> + [ ChainSegment.DotMember(stn "." mDot, mkExpr creationAide f) ], ChainTerminal.NoTerminal + + | [ LinkExpr.Dot mDot; LinkExpr.AppUnit(f, mUnit) ] -> + [ ChainSegment.DotMember(stn "." mDot, mkExpr creationAide f) ], mkTerminal (ChainCall.Unit(mkUnit mUnit)) + + | [ LinkExpr.Dot mDot; LinkExpr.AppParen(f, lpr, e, rpr, pr) ] -> + [ ChainSegment.DotMember(stn "." mDot, mkExpr creationAide f) ], + mkTerminal (ChainCall.Paren(mkParenExpr creationAide lpr e rpr pr)) + + | [ LinkExpr.Dot mDot; LinkExpr.IndexExpr idx ] -> + [ ChainSegment.DotIndex(stn "." mDot, mkExpr creationAide idx) ], ChainTerminal.NoTerminal + + // A chain ending in an opaque expression. Reached when a dot-lambda body is an indexed + // member access (`_.Values[0]`): the `[0]` has no dot of its own, so `Values[0]` arrives + // as a single `Expr` link. There is no call to negotiate, hence no terminal. + | [ LinkExpr.Dot mDot; LinkExpr.Expr e ] -> + [ ChainSegment.DotMember(stn "." mDot, mkExpr creationAide e) ], ChainTerminal.NoTerminal + + | LinkExpr.Dot mDot :: LinkExpr.AppUnit(f, mUnit) :: rest -> + let seg = + ChainSegment.DotApplication(stn "." mDot, mkExpr creationAide f, ChainCall.Unit(mkUnit mUnit)) + + let segs, terminal = processLinks rest + seg :: segs, terminal + + | LinkExpr.Dot mDot :: LinkExpr.AppParen(f, lpr, e, rpr, pr) :: rest -> + let seg = + ChainSegment.DotApplication( + stn "." mDot, + mkExpr creationAide f, + ChainCall.Paren(mkParenExpr creationAide lpr e rpr pr) + ) - continuation [ yield! leftLinks; yield rightLink ] + let segs, terminal = processLinks rest + seg :: segs, terminal - | ParenExpr(lpr, e, rpr, pr) -> - let leftLinks = List.cutOffLast leftLinks - // Example: A().B(fun b -> b) - let rightLink = LinkExpr.AppParen(identifierExpr, lpr, e, rpr, pr) - continuation [ yield! leftLinks; yield rightLink ] + | LinkExpr.Dot mDot :: LinkExpr.Identifier f :: rest -> + let seg = ChainSegment.DotMember(stn "." mDot, mkExpr creationAide f) + let segs, terminal = processLinks rest + seg :: segs, terminal - | _ -> visit argExpr (fun rightLinks -> continuation [ yield! leftLinks; yield! rightLinks ]) - | _ -> visit argExpr (fun rightLinks -> continuation [ yield! leftLinks; yield! rightLinks ])) + | LinkExpr.Dot mDot :: LinkExpr.IndexExpr idx :: rest -> + let seg = ChainSegment.DotIndex(stn "." mDot, mkExpr creationAide idx) + let segs, terminal = processLinks rest + seg :: segs, terminal - | SynExpr.DotGet(expr, rangeOfDot, longDotId, _) -> - visit expr (fun links -> - continuation - [ yield! links - yield LinkExpr.Dot rangeOfDot - yield! mkLinksFromSynLongIdent longDotId ]) + | LinkExpr.Dot mDot :: LinkExpr.Expr e :: rest -> + let seg = ChainSegment.DotMember(stn "." mDot, mkExpr creationAide e) + let segs, terminal = processLinks rest + seg :: segs, terminal - | SynExpr.App(isInfix = false; funcExpr = funcExpr; argExpr = UnitExpr mUnit) -> - mkLinksFromFunctionName (fun e -> LinkExpr.AppUnit(e, mUnit)) funcExpr - |> continuation + | other -> + invariantViolation + range + "unexpected link pattern in chain.\nExpected a `Dot` followed by exactly one content link, but got:\n%A\nFull link list for this chain:\n%A" + other + links - | SynExpr.App(isInfix = false; funcExpr = funcExpr; argExpr = ParenExpr(lpr, e, rpr, pr)) -> - mkLinksFromFunctionName (fun f -> LinkExpr.AppParen(f, lpr, e, rpr, pr)) funcExpr - |> continuation + let segments, terminal = processLinks rest + ExprChain(head, segments, terminal, range) |> Expr.Chain - | SynExpr.App(ExprAtomicFlag.Atomic, - false, - (SynExpr.LongIdent _ as funcExpr), - (SynExpr.ArrayOrList _ as argExpr), - _) -> - visit funcExpr (fun leftLinks -> - let app = - match List.tryLast leftLinks with - | Some(LinkExpr.Identifier identifier) -> - [ SynExpr.App( - ExprAtomicFlag.Atomic, - false, - identifier, - argExpr, - unionRanges identifier.Range argExpr.Range - ) - |> LinkExpr.Expr ] - | _ -> [] - - let leftLinks = List.cutOffLast leftLinks - continuation [ yield! leftLinks; yield! app ]) - - | SynExpr.TypeApp _ as typeApp -> mkLinksFromFunctionName LinkExpr.Identifier typeApp |> continuation - - | SynExpr.LongIdent(longDotId = sli) -> continuation (mkLinksFromSynLongIdent sli) - - | SynExpr.Ident _ -> continuation [ LinkExpr.Identifier e ] - - | SynExpr.DotIndexedGet(objectExpr, indexArgs, dotRange, _) -> - visit objectExpr (fun leftLinks -> - continuation - [ yield! leftLinks - yield LinkExpr.Dot dotRange - yield LinkExpr.IndexExpr indexArgs ]) - - | other -> continuation [ LinkExpr.Expr other ] +[] +let (|ChainExpr|_|) (e: SynExpr) : LinkExpr list voption = + let (|DottedLongIdent|_|) (sli: SynLongIdent) = + match sli.IdentsWithTrivia with + | _ :: _ :: _ when sli.Dots.Length > 0 -> ValueSome() + | _ -> ValueNone match e with - // An identifier only application with a parenthesis lambda expression. - // ex: `List.map (fun n -> n)` or `MailboxProcessor.Start (fun n -> n) - // Because the identifier is not complex we don't consider it a chain. + // A plain LongIdent with dots (e.g. `a.b.c`) is a chain of property accesses. + // An *optional* one (`?a.b`) is not: the `?` belongs to the whole path and the chain + // links would drop it, so leave it as a single (OptVar) expression. + | SynExpr.LongIdent(isOptional = false; longDotId = DottedLongIdent) -> ValueSome(visitChainLinks e id) + // An App(LongIdent, Paren _ | Unit) where the LongIdent has dots (e.g. `path.Replace("x","y")`) + // is a chain with a terminal call. `ParenExpr _` is deliberately unrestricted, so this also + // covers a paren carrying a lambda (`List.map (fun x -> x+1)`) — the lambda is just another + // argument shape and needs no arm of its own. + | SynExpr.App( + isInfix = false; funcExpr = SynExpr.LongIdent(longDotId = DottedLongIdent); argExpr = (ParenExpr _ | UnitExpr _)) -> + ValueSome(visitChainLinks e id) + // An identifier-only application with a paren lambda (e.g. `List.map (fun n -> n)`) + // is NOT considered a chain because the identifier is not complex. | SynExpr.App( isInfix = false funcExpr = SynExpr.LongIdent _ | SynExpr.Ident _ | SynExpr.DotGet(expr = SynExpr.TypeApp(expr = SynExpr.Ident _)) @@ -977,14 +1152,15 @@ let (|ChainExpr|_|) (e: SynExpr) : LinkExpr list voption = argExpr = UnitExpr _ | ParenExpr _) | SynExpr.DotGet _ | SynExpr.TypeApp(expr = SynExpr.DotGet _) - | SynExpr.DotIndexedGet(objectExpr = SynExpr.App(funcExpr = SynExpr.DotGet _) | SynExpr.DotGet _) -> - ValueSome(visit e id) + | SynExpr.DotIndexedGet _ -> ValueSome(visitChainLinks e id) | _ -> ValueNone [] +// Note: no guard is needed against a dotted function expression (`a.Foo(x, y)`). The only +// use site sits directly after the `ChainExpr` arm in `mkExpr`, and a chain claims every +// `App(DotGet, Paren _)` before this pattern is ever tried. let (|AppSingleParenArg|_|) = function - | App(SynExpr.DotGet _, [ (SynExpr.Paren(expr = SynExpr.Tuple _)) ]) -> ValueNone | App(e, [ UnitExpr _ as px ]) -> ValueSome(e, px) | App(e, [ SynExpr.Paren(expr = singleExpr) as px ]) -> match singleExpr with @@ -996,6 +1172,28 @@ let (|AppSingleParenArg|_|) = let mkParenExpr creationAide lpr e rpr m = ExprParenNode(stn "(" lpr, mkExpr creationAide e, stn ")" rpr, m) +/// Build `expr` for a position where it has to stay one indivisible unit, because something +/// binds directly to it with no whitespace allowed in between: +/// +/// * a prefix operator in front of it — `-x.Foo(a)` +/// * an index behind it — `x.Foo()[0]`, `x.Foo().[0] <- v` +/// * a `?member` behind it — `x.Foo()?Bar`, `x?a()?b()` +/// +/// The F# parser calls such an expression *atomic*, and it only stays atomic while its final +/// call is written tight: `x.Foo()[0]` parses the call as `App(Atomic, ...)`, whereas +/// `x.Foo ()[0]` parses it as `App(NonAtomic, ...)` and the index no longer binds to the call. +/// +/// The one thing that could introduce that space is a `SpaceBefore*Invocation` setting acting +/// on a chain's final call, so a chain built here is marked `NoSpaceAllowed`. Every other +/// expression is already indivisible and is built normally. +/// +/// Note this does not *make* an expression atomic (it adds no parentheses); it only avoids the +/// one formatting choice that would stop it being atomic. +let mkAtomicExpr (creationAide: CreationAide) (expr: SynExpr) : Expr = + match expr with + | ChainExpr links -> mkChainFromLinks creationAide links ChainTerminal.NoSpaceAllowed expr.Range + | _ -> mkExpr creationAide expr + let mkExpr (creationAide: CreationAide) (e: SynExpr) : Expr = let exprRange = e.Range @@ -1304,10 +1502,10 @@ let mkExpr (creationAide: CreationAide) (e: SynExpr) : Expr = ExprDynamicChainItemNode(memberExpr', parenArg', itemRange)) - ExprDynamicChainNode(mkExpr creationAide leading, chainItems, exprRange) + ExprDynamicChainNode(mkAtomicExpr creationAide leading, chainItems, exprRange) |> Expr.DynamicChain | SynExpr.Dynamic(funcExpr, _, argExpr, _) -> - ExprDynamicNode(mkExpr creationAide funcExpr, mkExpr creationAide argExpr, exprRange) + ExprDynamicNode(mkAtomicExpr creationAide funcExpr, mkExpr creationAide argExpr, exprRange) |> Expr.Dynamic | SynExpr.App(_, false, @@ -1321,7 +1519,9 @@ let mkExpr (creationAide: CreationAide) (e: SynExpr) : Expr = PrettyNaming.ConvertValLogicalNameToDisplayNameCore ident.idText ) -> - ExprPrefixAppNode(stn operatorName ident.idRange, mkExpr creationAide e2, exprRange) + // A prefix operator needs an atomic operand (`!-foo.bar(x)`), so a chain argument keeps + // its final call tight regardless of the space-before-invocation settings. + ExprPrefixAppNode(stn operatorName ident.idRange, mkAtomicExpr creationAide e2, exprRange) |> Expr.PrefixApp | NewlineInfixApps(head, xs) @@ -1337,7 +1537,7 @@ let mkExpr (creationAide: CreationAide) (e: SynExpr) : Expr = |> Expr.InfixApp | IndexWithoutDot(identifierExpr, indexExpr) -> - ExprIndexWithoutDotNode(mkExpr creationAide identifierExpr, mkExpr creationAide indexExpr, exprRange) + ExprIndexWithoutDotNode(mkAtomicExpr creationAide identifierExpr, mkExpr creationAide indexExpr, exprRange) |> Expr.IndexWithoutDot | EmptyComputationExpr(expr, mOpen, mClose, m) -> @@ -1350,31 +1550,8 @@ let mkExpr (creationAide: CreationAide) (e: SynExpr) : Expr = ) |> Expr.NamedComputation - | ChainExpr links -> - let chainLinks = - links - |> List.map (function - | LinkExpr.Identifier identifierExpr -> mkExpr creationAide identifierExpr |> ChainLink.Identifier - | LinkExpr.Dot mDot -> stn "." mDot |> ChainLink.Dot - | LinkExpr.Expr e -> mkExpr creationAide e |> ChainLink.Expr - | LinkExpr.AppUnit(f, mUnit) -> - LinkSingleAppUnit(mkExpr creationAide f, mkUnit mUnit, unionRanges f.Range mUnit) - |> ChainLink.AppUnit - | LinkExpr.AppParen(f, lpr, e, rpr, pr) -> - LinkSingleAppParen( - mkExpr creationAide f, - mkParenExpr creationAide lpr e rpr pr, - unionRanges f.Range pr - ) - |> ChainLink.AppParen - | LinkExpr.IndexExpr e -> mkExpr creationAide e |> ChainLink.IndexExpr - | link -> failwithf "cannot map %A" link) - - ExprChain(chainLinks, exprRange) |> Expr.Chain + | ChainExpr links -> mkChainFromLinks creationAide links ChainTerminal.SpaceAllowed exprRange - | AppSingleParenArg(SynExpr.LongIdent(longDotId = longDotId), px) -> - ExprAppLongIdentAndSingleParenArgNode(mkSynLongIdent creationAide longDotId, mkExpr creationAide px, exprRange) - |> Expr.AppLongIdentAndSingleParenArg | AppSingleParenArg(e, px) -> ExprAppSingleParenArgNode(mkExpr creationAide e, mkExpr creationAide px, exprRange) |> Expr.AppSingleParenArg @@ -1531,13 +1708,9 @@ let mkExpr (creationAide: CreationAide) (e: SynExpr) : Expr = | SynExpr.LongIdentSet(synLongIdent, e, _) -> ExprLongIdentSetNode(mkSynLongIdent creationAide synLongIdent, mkExpr creationAide e, exprRange) |> Expr.LongIdentSet - | SynExpr.DotIndexedGet(objectExpr, indexArgs, _, _) -> - ExprDotIndexedGetNode(mkExpr creationAide objectExpr, mkExpr creationAide indexArgs, exprRange) - |> Expr.DotIndexedGet - | SynExpr.DotIndexedSet(objectExpr, indexExpr, valueExpr, _, _, _) -> ExprDotIndexedSetNode( - mkExpr creationAide objectExpr, + mkAtomicExpr creationAide objectExpr, mkExpr creationAide indexExpr, mkExpr creationAide valueExpr, exprRange @@ -1693,8 +1866,15 @@ let mkExpr (creationAide: CreationAide) (e: SynExpr) : Expr = expr = e trivia = { DotRange = mDot UnderscoreRange = mUnderscore }) -> - ExprDotLambda(stn "_" mUnderscore, stn "." mDot, mkExpr creationAide e, exprRange) - |> Expr.DotLambda + // Build the chain with `_` as head and the body links as segments. + // Terminal uses NoSpaceAllowed — the F# compiler requires DotLambda bodies to be atomic. + let bodyLinks = visitChainLinks e id + let underscore = SynExpr.Ident(Ident("_", mUnderscore)) + + let allLinks = + [ LinkExpr.Identifier underscore; LinkExpr.Dot mDot; yield! bodyLinks ] + + mkChainFromLinks creationAide allLinks ChainTerminal.NoSpaceAllowed exprRange | _ -> failwithf "todo for %A" e let mkExprQuote creationAide isRaw e range : ExprQuoteNode = diff --git a/src/Fantomas.Core/CodePrinter.fs b/src/Fantomas.Core/CodePrinter.fs index d793de426b..bab484e4cd 100644 --- a/src/Fantomas.Core/CodePrinter.fs +++ b/src/Fantomas.Core/CodePrinter.fs @@ -54,20 +54,24 @@ let rec (|UppercaseExpr|LowercaseExpr|) (expr: Expr) = | Expr.Ident ident -> upperOrLower ident.Text | Expr.OptVar node -> lastFragmentInList node.Identifier | Expr.Chain node -> - match List.tryLast node.Links with - | None - | Some(ChainLink.Dot _) -> LowercaseExpr - | Some(ChainLink.Identifier e) - | Some(ChainLink.Expr e) -> (|UppercaseExpr|LowercaseExpr|) e - | Some(ChainLink.AppParen appParen) -> (|UppercaseExpr|LowercaseExpr|) appParen.FunctionName - | Some(ChainLink.AppUnit appUnit) -> (|UppercaseExpr|LowercaseExpr|) appUnit.FunctionName - // Questionable - | Some(ChainLink.IndexExpr _) -> LowercaseExpr - | Expr.DotIndexedGet node -> (|UppercaseExpr|LowercaseExpr|) node.ObjectExpr + // The casing that governs `SpaceBefore*Invocation` is the casing of the name the call + // is made on, which is always the LAST segment — `a.foo(x).Bar` is uppercase because of + // `Bar`, not `a` or `foo`. The terminal itself carries no name, so it does not take part. + // + // In practice only the DotMember arm is ever reached: a chain that has a terminal call + // always ends in a DotMember (the member being called), and the other shapes are either + // impossible or answered before we get here — a chain ending in an index is short- + // circuited by `chainEndsInIndex` in `sepSpaceBeforeParenInFuncInvocation`, and a + // zero-segment chain only ever exists as the head of another chain. They are kept so + // the active pattern stays total, and each returns the answer its shape deserves. + match List.tryLast node.Segments with + | Some(ChainSegment.DotMember(_, e)) + | Some(ChainSegment.DotApplication(_, e, _)) -> (|UppercaseExpr|LowercaseExpr|) e + | Some(ChainSegment.DotIndex _) -> LowercaseExpr + | None -> (|UppercaseExpr|LowercaseExpr|) node.Head | Expr.TypeApp node -> (|UppercaseExpr|LowercaseExpr|) node.Identifier | Expr.Dynamic node -> (|UppercaseExpr|LowercaseExpr|) node.FuncExpr | Expr.DynamicChain node -> (|UppercaseExpr|LowercaseExpr|) node.LeadingExpr - | Expr.AppLongIdentAndSingleParenArg node -> lastFragmentInList node.FunctionName | Expr.AppSingleParenArg node -> (|UppercaseExpr|LowercaseExpr|) node.FunctionExpr | Expr.Paren node -> (|UppercaseExpr|LowercaseExpr|) node.Expr | Expr.App node -> (|UppercaseExpr|LowercaseExpr|) node.FunctionExpr @@ -396,6 +400,347 @@ let requiresMultilineToPreserveSemantics (exprs: Expr list) = |> List.take (exprs.Length - 1) // skip the last item: it can't swallow anything to its right |> List.exists isOpenEndedExpression +// Chain printing helpers + +/// Where a call sits in its chain. This governs exactly two things about the layout of the +/// call's parenthesised argument, and nothing else: +/// * whether the opening `(` is allowed to leave the member name, which is a grammar +/// constraint rather than a style choice, and +/// * whether a `(function` argument is forced onto its own line, which is a readability call. +[] +type ChainCallPosition = + /// A call with more chain after it, e.g. `.Foo(x)` in `a.Foo(x).Bar()`. + | Intermediate + /// The last call of the chain, with nothing following it. + | Terminal + +/// Layout for an argument carrying a directive or comment before it (e.g. `(` on its own line +/// above a `#if`): the argument is pushed onto its own indented lines and the closing `)` +/// returns to the method's column. +let genParenArgWithContentBefore (position: ChainCallPosition) (p: ExprParenNode) : Context -> Context = + // An INTERMEDIATE call may not be separated from its `(` — `a.Foo (x).Bar()` parses as + // `a.Foo ((x).Bar())` — so there the paren stays welded to the member name and the break + // happens after it. Only a terminal call may put the `(` on the indented line. + let genOpeningParen: Context -> Context = + match position with + | ChainCallPosition.Intermediate -> genSingleTextNode p.OpeningParen +> indent +> sepNln + | ChainCallPosition.Terminal -> indent +> sepNln +> genSingleTextNode p.OpeningParen +> sepNln + + genOpeningParen + +> genExpr p.Expr + +> unindent + +> sepNlnUnlessLastEventIsNewline + +> genSingleTextNode p.ClosingParen + |> genNode p + +/// Layout for `(fun params -> body)`. Identical wherever the call sits: `MultiLineLambdaClosingNewline` +/// decides where the closing `)` lands, exactly as it would for a call with no receiver. +let genLambdaParenArg (p: ExprParenNode) (lambdaNode: ExprLambdaNode) : Context -> Context = + // `startColumn` is remembered so the closing `)` can be indented back to it — important + // when the body ends at a shallow column (e.g. a verbatim string in column 0), where a `)` + // left there breaks the offside rule and produces invalid F#. + let genParenLambda (ctx: Context) : Context = + let startColumn: int = ctx.WriterModel.Indent + + (genSingleTextNode p.OpeningParen + +> genLambdaWithParen lambdaNode + +> ifElseCtx + (fun ctx -> + ctx.Config.MultiLineLambdaClosingNewline + && not (isStroustrupStyleExpr ctx.Config lambdaNode.Expr)) + sepNln + (sepNlnWhenWriteBeforeNewlineNotEmpty +> addFixedSpaces startColumn) + +> genSingleTextNode p.ClosingParen) + ctx + + // Move the whole lambda argument onto its own indented line when leaving `(fun` at the + // margin would read badly: + // * a single parameter that is itself multiline (a record pattern), or + // * even the bare `(fun` no longer fits, because the method name in front of it is + // already too long. (A long *list* of simple parameters is NOT a reason — those align + // under `fun` on the same line — so we probe only up to `fun`.) + fun (ctx: Context) -> + let singleMultilineParameter: bool = + match lambdaNode.Parameters with + | [ singleParam ] -> fst (futureNlnCheckMem (genPat singleParam, ctx)) + | _ -> false + + let openerDoesNotFit: bool = + futureNlnCheck (genSingleTextNode p.OpeningParen +> genSingleTextNode lambdaNode.Fun) ctx + + if singleMultilineParameter || openerDoesNotFit then + indentSepNlnUnindent genParenLambda ctx + else + genParenLambda ctx + +/// Layout for `(function | ... -> ...)`. This is the one argument shape whose layout depends on +/// where the call sits: mid-chain the clauses would run on underneath a line that still +/// continues with further steps, so `function` is given its own line. +let genMatchLambdaParenArg + (position: ChainCallPosition) + (p: ExprParenNode) + (matchLambdaNode: ExprMatchLambdaNode) + : Context -> Context = + let keepFunctionWithParen: Context -> Context = + genSingleTextNode p.OpeningParen + +> (genSingleTextNode matchLambdaNode.Function + +> indentSepNlnUnindent (genClauses matchLambdaNode.Clauses) + |> genNode matchLambdaNode) + +> sepNlnWhenWriteBeforeNewlineNotEmpty + +> genSingleTextNode p.ClosingParen + + let breakAfterParen: Context -> Context = + genSingleTextNode p.OpeningParen + +> indentSepNlnUnindent ( + genSingleTextNode matchLambdaNode.Function + +> sepNln + +> genClauses matchLambdaNode.Clauses + |> genNode matchLambdaNode + ) + +> sepNln + +> genSingleTextNode p.ClosingParen + + // The user already broke after the `(` themselves — `function` starts on a line below it. + let userBrokeAfterParen: bool = + matchLambdaNode.Range.StartLine > p.OpeningParen.Range.EndLine + + fun (ctx: Context) -> + let breakOpener: bool = + match position with + // Mid-chain the other considerations do not get a say. + | ChainCallPosition.Intermediate -> true + | ChainCallPosition.Terminal -> ctx.Config.MultiLineLambdaClosingNewline || userBrokeAfterParen + + if breakOpener then + breakAfterParen ctx + else + keepFunctionWithParen ctx + +/// Multiline layout of a call's parenthesised argument `( ... )`, shared by intermediate calls +/// (genSegment) and the terminal call (genTerminal). Everything between `(` and `)` is laid out +/// by the ordinary argument rules; the chain only gets a say in the two places named on +/// `ChainCallPosition`. +let genMultilineParenArg (position: ChainCallPosition) (p: ExprParenNode) : Context -> Context = + // Content attached before the argument is decided ahead of the argument's shape, so this is + // an `if` rather than another match arm. + if p.HasContentBefore || (Expr.Node p.Expr).HasContentBefore then + genParenArgWithContentBefore position p + else + match p.Expr with + | Expr.Lambda lambdaNode -> genLambdaParenArg p lambdaNode + | Expr.MatchLambda matchLambdaNode -> genMatchLambdaParenArg position p matchLambdaNode + | _ -> genMultilineFunctionApplicationArguments (Expr.Paren p) + +let genSegment (segment: ChainSegment) : Context -> Context = + match segment with + | ChainSegment.DotMember(dot, expr) -> genSingleTextNode dot +> genExpr expr + | ChainSegment.DotApplication(dot, expr, call) -> + genSingleTextNode dot + +> genExpr expr + +> match call with + | ChainCall.Unit u -> genUnit u + | ChainCall.Paren p -> + // Intermediate paren calls are always tight — space would change the parse tree. + expressionFitsOnRestOfLine + (genExpr (Expr.Paren p)) + (genMultilineParenArg ChainCallPosition.Intermediate p) + | ChainSegment.DotIndex(dot, idx) -> + // DotIndex (.[i]) is structurally like DotMember with no call: the dot is explicit + // for trivia, but the [ ] brackets are not part of the index expression and must + // be added here rather than delegated to genExpr. + genSingleTextNode dot +> sepOpenLFixed +> genExpr idx +> sepCloseLFixed + +let genTerminal (node: ExprChain) : Context -> Context = + match node.Terminal with + | ChainTerminal.NoTerminal -> sepNone + | ChainTerminal.SpaceAllowed call + | ChainTerminal.NoSpaceAllowed call -> + let addSpace: Context -> Context = + match node.Terminal with + | ChainTerminal.NoSpaceAllowed _ -> sepNone + | _ -> + match List.tryLast node.Segments with + | Some(ChainSegment.DotMember(_, funcExpr)) -> + match call with + | ChainCall.Unit u -> sepSpaceBeforeParenInFuncInvocation funcExpr (Expr.Constant(Constant.Unit u)) + | ChainCall.Paren p -> sepSpaceBeforeParenInFuncInvocation funcExpr (Expr.Paren p) + | _ -> sepNone + + match call with + | ChainCall.Unit u -> addSpace +> genUnit u + | ChainCall.Paren p -> + let short: Context -> Context = addSpace +> genExpr (Expr.Paren p) + + let long: Context -> Context = + addSpace +> genMultilineParenArg ChainCallPosition.Terminal p + + expressionFitsOnRestOfLine short long + +let genChain (node: ExprChain) : Context -> Context = + // The receiver, every navigation step and the method name are rendered together in + // source order with no line breaks between them; only the terminal call's arguments may + // wrap (`genTerminal` breaks them like an ordinary `f(args)`). This same rendering + // doubles as the "does the whole chain fit on one line?" candidate — when it fits, it + // *is* the one-liner. (The design notes in `chain-formatting-rationale.md` call this + // layout "Option A".) + let genChainKeptTogether: Context -> Context = + genExpr node.Head +> col sepNone node.Segments genSegment +> genTerminal node + + // A segment is HEAVY — it earns its own line in a pipeline — when it is an + // intermediate call (`.Foo(x)`), or a member / index whose bracket payload is + // large enough to render on multiple lines (a long `.Cast<..>`, a big `.[expr]`). + // Everything else is light NAVIGATION (`.Name`, a short `.[0]`) and rides at the + // front of the next line instead of taking one of its own. + let isHeavy (seg: ChainSegment) (ctx: Context) : bool = + // Heavy only when the member's own bracket payload renders on multiple lines. + // We probe the payload alone (not the whole segment) so a comment attached to a + // plain `.Name` does not masquerade as a multiline type application, and so mere + // right-margin overflow does not count either. + let payloadIsMultiline (payload: Context -> Context) = fst (futureNlnCheckMem (payload, ctx)) + + match seg with + | ChainSegment.DotApplication _ -> true + | ChainSegment.DotMember(_, expr) -> + match expr with + | Expr.TypeApp _ -> payloadIsMultiline (genExpr expr) + | _ -> false + | ChainSegment.DotIndex(_, idx) -> payloadIsMultiline (genExpr idx) + + // A `#if`/`#else` directive before a segment's dot pins that segment to its own line. + // Directly after the receiver this makes the navigation split from the call it would + // otherwise lead; deeper in the chain the segment already starts a fresh line, so the + // directive changes nothing and the following call still rides with it. + let segmentBeginsWithDirective (seg: ChainSegment) : bool = + let dot = + match seg with + | ChainSegment.DotMember(dot, _) + | ChainSegment.DotApplication(dot, _, _) + | ChainSegment.DotIndex(dot, _) -> dot + + dot.ContentBefore + |> Seq.exists (fun trivia -> + match trivia.Content with + | TriviaContent.Directive _ -> true + | _ -> false) + + let terminalIsCall: bool = + match node.Terminal with + | ChainTerminal.SpaceAllowed _ + | ChainTerminal.NoSpaceAllowed _ -> true + | ChainTerminal.NoTerminal -> false + + // Is the receiver a plain value — a bare identifier or dotted path (`config`, + // `this`, the dot-lambda `_`)? Only then may it share its line with the navigation + // and method name of a single trailing call. A compound receiver — a call `Mock()`, + // a parenthesised expression `(x :> T)`, a generic `Animal` — is not eligible to + // be kept together and leads a pipeline instead. + let headIsSimple: bool = + match node.Head with + | Expr.Ident _ + | Expr.OptVar _ -> true + | _ -> false + + // Leading-dot pipeline: two or more actions, or a chain that ends in + // navigation. Walk the segments and place each one: + // * every ACTION starts its own line, led by its dot; + // * light NAVIGATION rides at the front of the line of the action it + // introduces — or, directly after the receiver, on the receiver's line; + // * a run of navigation with no action fills the line greedily, breaking + // before a dot when the current line is full. + let genLeadingDotPipeline (ctx: Context) : Context = + // onHeadLine — still on the receiver's line; no break has been emitted + // yet, so the first break must also `indent`. + // mustBreak — the previous segment was an action, so whatever comes + // next has to start on a fresh line. + let rec place (onHeadLine: bool) (mustBreak: bool) (segs: ChainSegment list) (ctx: Context) : Context = + match segs with + | [] -> (genTerminal node +> onlyIfNot onHeadLine unindent) ctx + | seg :: rest -> + if isHeavy seg ctx || (onHeadLine && segmentBeginsWithDirective seg) then + // An action starts a new line when it opens the pipeline + // (leaving the receiver's line) or follows another action; + // otherwise it rides after the navigation that introduced it. + let placeAction: Context -> Context = + if onHeadLine then indent +> sepNln +> genSegment seg + elif mustBreak then sepNln +> genSegment seg + else genSegment seg + + place false true rest (placeAction ctx) + elif mustBreak then + // Navigation that introduces the next action: give it a fresh line. + place false false rest ((onlyIf onHeadLine indent +> sepNln +> genSegment seg) ctx) + elif not (futureNlnCheck (genSegment seg) ctx) then + // Navigation that still fits: ride on the current line. + place onHeadLine false rest (genSegment seg ctx) + else + // Navigation that no longer fits: break before its dot. + place false false rest ((onlyIf onHeadLine indent +> sepNln +> genSegment seg) ctx) + + // Indexing a parenthesised value — `(expr).[0]` — is a plain access, not a + // pipeline: the index rides tight onto the closing paren instead of breaking. + let isParenThenIndex: bool = + (match node.Head with + | Expr.Paren _ -> true + | _ -> false) + && (match node.Segments with + | [ ChainSegment.DotIndex _ ] -> true + | _ -> false) + && not terminalIsCall + + // Otherwise the first segment starts a fresh line when the receiver is compound + // (it leads a pipeline), or when the receiver carries a trailing comment — else + // that comment would be stranded in the middle of the first call. + let receiverForcesBreak: bool = + (Expr.Node node.Head).HasContentAfter + || (not headIsSimple && not isParenThenIndex) + + place true receiverForcesBreak node.Segments (genExpr node.Head ctx) + + // Chosen when the whole chain does not fit on one line: either keep the chain together + // (and let the terminal's arguments wrap) or fall back to the leading-dot pipeline. + let long (ctx: Context) : Context = + // Keeping the chain together is only allowed for a single call at the very end: + // every segment is light navigation and the terminal is the one and only action. + let everySegmentIsLight: bool = + node.Segments |> List.forall (fun seg -> not (isHeavy seg ctx)) + + // Keeping the chain together holds the receiver, navigation and method name on one + // line. It gives way to the leading-dot pipeline when a comment between the steps + // forces a break, or when breaking actually *helps* the width. With several segments, + // spreading them across lines always helps. With a single method, breaking only helps + // when that method would fit on its own line — if the method itself is the overflow (a + // very long name), there is nothing to gain, so it stays together and the arguments wrap. + // + // Hence the two arms below ask genuinely different questions, and it is worth being + // explicit about why: + // * ONE segment — `receiver.Method(args)`. The pipeline has exactly one line to + // offer (`.Method(args)` under the receiver), so it is only an improvement if + // `.Method` actually fits there. `methodOverflowsOnItsOwn` asks that. When the + // method name alone already overruns the margin, moving it down changes nothing + // and we would have traded a readable `receiver.Method(` opener for a lone dotted + // line that still overflows. + // * MANY segments — the pipeline can spread the segments over several lines, so it + // always reduces width. There is no "would it even help?" question to ask; we only + // need to know whether the combined head + segments overflow in the first place. + let headAndSegmentsFit: bool = + match node.Segments with + | [ single ] -> + let combinedForcesBreak: bool = + futureNlnCheck (genExpr node.Head +> genSegment single) ctx + + let methodOverflowsOnItsOwn: bool = snd (futureNlnCheckMem (genSegment single, ctx)) + not (combinedForcesBreak && not methodOverflowsOnItsOwn) + | _ -> not (futureNlnCheck (genExpr node.Head +> col sepNone node.Segments genSegment) ctx) + + if terminalIsCall && everySegmentIsLight && headIsSimple && headAndSegmentsFit then + genChainKeptTogether ctx + else + genLeadingDotPipeline ctx + + // Try the whole chain on one line first; otherwise `long` decides between keeping the + // chain together (with wrapped arguments) and the leading-dot pipeline. + expressionFitsOnRestOfLine genChainKeptTogether long + let genExpr (e: Expr) = match e with | Expr.Lazy node -> @@ -765,14 +1110,10 @@ let genExpr (e: Expr) = +> genSingleTextNode node.ClosingParen |> genNode node | Expr.Dynamic node -> - // Use sepNone for AppLongIdentAndSingleParenArg to preserve atomic application (no space before paren). - // Adding a space would change the AST from `(Jest.expect(json))?oMatchSnapshot` to `Jest.expect ((json)?oMatchSnapshot)`. See #3135. - let genFuncExpr = - match node.FuncExpr with - | Expr.AppLongIdentAndSingleParenArg appNode -> genAppLongIdentAndSingleParenArgExpr sepNone appNode - | _ -> genExpr node.FuncExpr - - genFuncExpr +> !-"?" +> genExpr node.ArgExpr |> genNode node + // Chain expressions that serve as the function expression (e.g. Jest.expect(json)) are + // already printed tight — no space before their terminal paren — so genExpr is sufficient. + // See #3135. + genExpr node.FuncExpr +> !-"?" +> genExpr node.ArgExpr |> genNode node | Expr.DynamicChain node -> // A chain of `?` accesses is printed tight (no space before paren args). // Adding a space changes the parsing of the next `?member`. See #3159. @@ -813,13 +1154,6 @@ let genExpr (e: Expr) = genSingleTextNode node.Operator +> genExpr appNode.FunctionExpr +> genExpr appNode.ArgExpr - // E.g. !-Foo.Meh(a) - | Expr.AppLongIdentAndSingleParenArg appNode -> - let mOptVarNode = appNode.FunctionName.Range - - genSingleTextNode node.Operator - +> genExpr (Expr.OptVar(ExprOptVarNode(false, appNode.FunctionName, mOptVarNode))) - +> genExpr appNode.ArgExpr | _ -> genWithoutSpace |> genNode node | Expr.SameInfixApps node -> @@ -916,7 +1250,6 @@ let genExpr (e: Expr) = | Expr.IndexWithoutDot node -> let genIdentifierExpr = match node.Identifier with - | Expr.AppLongIdentAndSingleParenArg appNode -> genAppLongIdentAndSingleParenArgExpr sepNone appNode | Expr.AppSingleParenArg appNode -> genAppSingleParenArgExpr sepNone appNode | _ -> genExpr node.Identifier @@ -928,161 +1261,8 @@ let genExpr (e: Expr) = +> sepCloseLFixed |> genNode node - | Expr.Chain node -> - let genLink (isLastLink: bool) (link: ChainLink) = - match link with - | ChainLink.Identifier expr -> genExpr expr - | ChainLink.Dot stn -> genSingleTextNode stn - | ChainLink.Expr expr -> - match expr with - | Expr.App appNode -> - match appNode.Arguments with - | [ Expr.ArrayOrList _ as arrayOrList ] -> - // Edge case for something like .G[]. - genExpr appNode.FunctionExpr +> genExpr arrayOrList - | _ -> genExpr expr - | _ -> genExpr expr - | ChainLink.AppUnit appUnitNode -> - genExpr appUnitNode.FunctionName - +> onlyIf - isLastLink - (sepSpaceBeforeParenInFuncInvocation - appUnitNode.FunctionName - (Expr.Constant(Constant.Unit appUnitNode.Unit))) - +> genUnit appUnitNode.Unit - |> genNode appUnitNode - | ChainLink.AppParen appParen -> - let short = - genExpr appParen.FunctionName - +> onlyIf - isLastLink - (sepSpaceBeforeParenInFuncInvocation appParen.FunctionName (Expr.Paren appParen.Paren)) - +> genExpr (Expr.Paren appParen.Paren) - - let long = - match appParen.Paren.Expr with - | Expr.Lambda lambdaNode -> - genExpr appParen.FunctionName - +> onlyIf - isLastLink - (sepSpaceBeforeParenInFuncInvocation appParen.FunctionName (Expr.Paren appParen.Paren)) - +> genSingleTextNode appParen.Paren.OpeningParen - +> genLambdaWithParen lambdaNode - +> onlyIfCtx - (fun ctx -> - ctx.Config.MultiLineLambdaClosingNewline - && (not (isStroustrupStyleExpr ctx.Config lambdaNode.Expr))) - sepNln - +> genSingleTextNode appParen.Paren.ClosingParen - | _ -> - genExpr appParen.FunctionName - +> onlyIf - isLastLink - (sepSpaceBeforeParenInFuncInvocation appParen.FunctionName (Expr.Paren appParen.Paren)) - +> genMultilineFunctionApplicationArguments (Expr.Paren appParen.Paren) - - expressionFitsOnRestOfLine short long |> genNode appParen - | ChainLink.IndexExpr e -> sepOpenLFixed +> genExpr e +> sepCloseLFixed - - let lastIndex = node.Links.Length - 1 - let short = coli sepNone node.Links (fun idx -> genLink (idx = lastIndex)) - - let long = - let (|SimpleChain|_|) (link: ChainLink) = - match link with - | ChainLink.Identifier _ - | ChainLink.IndexExpr _ -> Some link - | _ -> None - - let (|LeadingSimpleChain|_|) (links: ChainLink list) = - let leading = System.Collections.Generic.Queue(links.Length) - let rest = System.Collections.Generic.Queue(links.Length) + | Expr.Chain node -> genChain node |> genNode node - (None, links) - ||> List.fold (fun lastDot link -> - if not (Seq.isEmpty rest) then - rest.Enqueue link - None - else - match link with - | SimpleChain _ -> - Option.iter leading.Enqueue lastDot - leading.Enqueue link - None - | ChainLink.Dot _ as dot -> Some dot - | _ -> - Option.iter rest.Enqueue lastDot - rest.Enqueue link - None) - |> (fun _ -> - if Seq.isEmpty leading then - None - else - Some(Seq.toList leading, Seq.toList rest)) - - let rec genIndentedLinks (lastLinkWasSimple: bool) (links: ChainLink list) (ctx: Context) : Context = - match links with - | [] -> ctx - | ChainLink.Dot dot :: link :: rest -> - let isLast = List.isEmpty rest - let genDotAndLink = genSingleTextNode dot +> genLink isLast link - let currentIsSimple = ((|SimpleChain|_|) >> Option.isSome) link - let currentLinkFitsOnRestOfLine = not (futureNlnCheck genDotAndLink ctx) - - if lastLinkWasSimple && currentLinkFitsOnRestOfLine then - // The last link was an identifier and the current link fits on the remainder of the current line. - genIndentedLinks currentIsSimple rest (genDotAndLink ctx) - else - let ctx' = - onlyIf - (not // Last link was `.Foo()` - lastLinkWasSimple - // `.Foo.Bar` but `Bar` crossed the max_line_length - || (lastLinkWasSimple && currentIsSimple && not currentLinkFitsOnRestOfLine)) - sepNlnUnlessLastEventIsNewline - ctx - - genIndentedLinks - currentIsSimple - rest - // Print the current link - (genDotAndLink ctx') - | _ -> failwith "Expected dot in chain at this point" - - let genFirstLinkAndIndentOther (firstLink: ChainLink) (others: ChainLink list) = - genLink false firstLink +> indentSepNlnUnindent (genIndentedLinks false others) - - match node.Links with - | [] -> sepNone - | LeadingSimpleChain(leadingChain, links) -> - match links with - | [] -> - expressionFitsOnRestOfLine - short - (match leadingChain with - | [] -> sepNone - | head :: links -> genLink false head +> indent +> genIndentedLinks true links +> unindent) - | _ -> - expressionFitsOnRestOfLine - (coli sepNone leadingChain (fun idx -> genLink (idx = lastIndex))) - (match leadingChain with - | [] -> sepNone - | [ head ] -> genLink false head - | head :: rest -> genLink false head +> indentSepNlnUnindent (genIndentedLinks true rest)) - +> indentSepNlnUnindent (genIndentedLinks true links) - - | head :: links -> genFirstLinkAndIndentOther head links - - expressionFitsOnRestOfLine short long |> genNode node - - // path.Replace("../../../", "....") - | Expr.AppLongIdentAndSingleParenArg node -> - let addSpace = - sepSpaceBeforeParenInFuncInvocation - (Expr.OptVar(ExprOptVarNode(false, node.FunctionName, node.FunctionName.Range))) - node.ArgExpr - - genAppLongIdentAndSingleParenArgExpr addSpace node // fn (a, b, c) | Expr.AppSingleParenArg node when (match node.FunctionExpr with @@ -1111,13 +1291,6 @@ let genExpr (e: Expr) = | _ -> sepSpace genAppWithLambda sepSpaceAfterFunctionName node - | Expr.NestedIndexWithoutDot node -> - genExpr node.Identifier - +> sepOpenLFixed - +> genExpr node.Index - +> sepCloseLFixed - +> genExpr node.Argument - |> genNode node | Expr.App node -> fun ctx -> @@ -1451,41 +1624,6 @@ let genExpr (e: Expr) = +> sepArrowRev +> autoIndentAndNlnIfExpressionExceedsPageWidthUnlessStroustrup genExpr node.Expr |> genNode node - | Expr.DotIndexedGet node -> - let genDotIndexedGet = - let isParen = - match node.ObjectExpr with - | Expr.Paren _ -> true - | _ -> false - - ifElse isParen (genExpr node.ObjectExpr) (addParenIfAutoNln node.ObjectExpr genExpr) - +> !-"." - +> sepOpenLFixed - +> genExpr node.IndexExpr - +> sepCloseLFixed - - let genDotIndexedGetWithApp funcExpr argExpr (appNode: Node) = - let short = funcExpr +> genExpr argExpr |> genNode appNode - - let long = - funcExpr +> genMultilineFunctionApplicationArguments argExpr |> genNode appNode - - let idx = !-"." +> sepOpenLFixed +> genExpr node.IndexExpr +> sepCloseLFixed - expressionFitsOnRestOfLine (short +> idx) (long +> idx) - - match node.ObjectExpr with - | Expr.App appNode -> - match appNode.Arguments with - | [ Expr.Constant(Constant.Unit _) as ux ] -> - genDotIndexedGetWithApp (genExpr appNode.FunctionExpr) ux appNode - | _ -> genDotIndexedGet - | Expr.AppSingleParenArg appNode -> - genDotIndexedGetWithApp (genExpr appNode.FunctionExpr) appNode.ArgExpr appNode - - | Expr.AppLongIdentAndSingleParenArg appNode -> - genDotIndexedGetWithApp (genIdentListNode appNode.FunctionName) appNode.ArgExpr appNode - | _ -> genDotIndexedGet - |> genNode node | Expr.DotIndexedSet node -> let genDotIndexedSet = addParenIfAutoNln node.ObjectExpr genExpr @@ -1525,8 +1663,16 @@ let genExpr (e: Expr) = | Expr.AppSingleParenArg appNode -> genDotIndexedSetWithApp (genExpr appNode.FunctionExpr) appNode.ArgExpr appNode - | Expr.AppLongIdentAndSingleParenArg appNode -> - genDotIndexedSetWithApp (genIdentListNode appNode.FunctionName) appNode.ArgExpr appNode + // An indexed set on a chain ending in a paren call (`foo.Bar(args).[i] <- v`): + // treat the receiver + method as the function and the terminal parens as the + // argument, so the arguments can break when the whole statement is too long. + | Expr.Chain chainNode -> + match chainNode.Terminal with + | ChainTerminal.SpaceAllowed(ChainCall.Paren p) + | ChainTerminal.NoSpaceAllowed(ChainCall.Paren p) -> + let funcExpr = genExpr chainNode.Head +> col sepNone chainNode.Segments genSegment + genDotIndexedSetWithApp funcExpr (Expr.Paren p) chainNode + | _ -> genDotIndexedSet | _ -> genDotIndexedSet |> genNode node @@ -1544,7 +1690,10 @@ let genExpr (e: Expr) = match node.Index with | Expr.Constant _ | Expr.Ident _ - | Expr.OptVar _ -> sepSpace + | Expr.OptVar _ + // A dotted access index (`a.B c.DE`) is now a chain; it still needs the + // separating space, since a chain never opens with a bracket. + | Expr.Chain _ -> sepSpace | _ -> sepNone genIdentListNode node.Identifier @@ -1638,17 +1787,6 @@ let genExpr (e: Expr) = |> genNode node | Expr.IndexFromEnd node -> !-"^" +> genExpr node.Expr |> genNode node | Expr.Typar node -> genSingleTextNode node - | Expr.DotLambda node -> - let genDotLambdaExpr expr = - match expr with - | Expr.AppSingleParenArg p -> genAppSingleParenArgExpr sepNone p // be always atomic, see 3050 - | Expr.AppLongIdentAndSingleParenArg p -> genAppLongIdentAndSingleParenArgExpr sepNone p // see 3120 - | _ -> genExpr expr - - genSingleTextNode node.Underscore - +> genSingleTextNode node.Dot - +> genDotLambdaExpr node.Expr - |> genNode node | Expr.BeginEnd node -> let short = genSingleTextNode node.Begin @@ -2050,22 +2188,6 @@ let genLambdaAux (includeClosingParen: bool) (node: ExprLambdaNode) = let genLambda = genLambdaAux false let genLambdaWithParen = genLambdaAux true -let genAppLongIdentAndSingleParenArgExpr (addSpace: Context -> Context) (node: ExprAppLongIdentAndSingleParenArgNode) = - let shortLids = genIdentListNode node.FunctionName - let short = shortLids +> addSpace +> genExpr node.ArgExpr - - let long = - let args = - addSpace - +> expressionFitsOnRestOfLine (genExpr node.ArgExpr) (genMultilineFunctionApplicationArguments node.ArgExpr) - - ifElseCtx - (futureNlnCheck shortLids) - (genFunctionNameWithMultilineLids args node.FunctionName) - (shortLids +> args) - - expressionFitsOnRestOfLine short long |> genNode node - let genAppSingleParenArgExpr (addSpace: Context -> Context) (node: ExprAppSingleParenArgNode) = let short = genExpr node.FunctionExpr +> addSpace +> genExpr node.ArgExpr @@ -2383,25 +2505,6 @@ let genPrefixApp +> genSingleTextNode greaterThan) ctx -let genFunctionNameWithMultilineLids (trailing: Context -> Context) (longIdent: IdentListNode) = - match longIdent.Content with - | IdentifierOrDot.Ident identNode :: t -> - genSingleTextNode identNode - +> indentSepNlnUnindent ( - colEx - (function - | IdentifierOrDot.Ident _ -> sepNone - | IdentifierOrDot.KnownDot _ - | IdentifierOrDot.UnknownDot -> sepNln) - t - (function - | IdentifierOrDot.Ident identNode -> genSingleTextNode identNode - | IdentifierOrDot.KnownDot dot -> genSingleTextNode dot - | IdentifierOrDot.UnknownDot -> sepDot) - +> trailing - ) - | _ -> sepNone - let (|EndsWithDualListApp|_|) (config: FormatConfig) (appNode: ExprAppNode) = if not (config.ExperimentalElmish || config.IsStroustrupStyle) then None @@ -2605,9 +2708,30 @@ let genAppWithLambda sep (node: ExprAppWithLambdaNode) = expressionFitsOnRestOfLine short long |> genNode node let sepSpaceBeforeParenInFuncInvocation (functionExpr: Expr) (argExpr: Expr) ctx = + // A chain that ends in an index access (`a.A.[0]`) invokes the indexed value — + // like new-style `xs[0]`, the `(` stays tight regardless of the space-before + // settings, since a space would read as applying the index to the parens. + let chainEndsInIndex = + match functionExpr with + | Expr.Chain node -> + match node.Terminal, List.tryLast node.Segments with + | ChainTerminal.NoTerminal, Some(ChainSegment.DotIndex _) -> true + | _ -> false + | _ -> false + + // An empty index written tight against its receiver (`x.G[]`) is index syntax, not an + // application to an empty list — keep it tight. Adjacency distinguishes it from a real + // empty-list argument `f []`, which keeps its space. + let isEmptyIndex = + match argExpr with + | Expr.ArrayOrList arrayNode -> + List.isEmpty arrayNode.Elements + && RangeHelpers.isAdjacentTo (Expr.Node functionExpr).Range arrayNode.Range + | _ -> false + match functionExpr, argExpr with - | Expr.DotLambda _, _ -> ctx | Expr.IndexWithoutDot _, ParenExpr _ -> ctx + | _ when chainEndsInIndex || isEmptyIndex -> ctx | Expr.Constant _, _ -> sepSpace ctx | ParenExpr _, _ -> sepSpace ctx | UppercaseExpr, ParenExpr _ -> onlyIf ctx.Config.SpaceBeforeUppercaseInvocation sepSpace ctx diff --git a/src/Fantomas.Core/Context.fsi b/src/Fantomas.Core/Context.fsi index 3459c9f072..95dcf6df7f 100644 --- a/src/Fantomas.Core/Context.fsi +++ b/src/Fantomas.Core/Context.fsi @@ -272,6 +272,11 @@ val sepSpaceOrDoubleIndentAndNlnIfExpressionExceedsPageWidth: expr: (Context -> val autoParenthesisIfExpressionExceedsPageWidth: expr: (Context -> Context) -> ctx: Context -> Context val futureNlnCheck: f: (Context -> Context) -> ctx: Context -> bool +/// Probe `f` and report `(isMultiline, isLong)` separately: whether it spans +/// multiple lines, and whether it overflows the right margin. `futureNlnCheck` +/// is `isMultiline || isLong`; callers that care only about multiline layout +/// (not width) use the first component. +val futureNlnCheckMem: f: (Context -> Context) * ctx: Context -> bool * bool /// similar to futureNlnCheck but // validates whether the expression is going over the max page width /// This functions is does not use any caching val exceedsWidth: maxWidth: int -> f: (Context -> Context) -> ctx: Context -> bool diff --git a/src/Fantomas.Core/FormatConfig.fs b/src/Fantomas.Core/FormatConfig.fs index b96dda2856..74ee762e2e 100644 --- a/src/Fantomas.Core/FormatConfig.fs +++ b/src/Fantomas.Core/FormatConfig.fs @@ -3,6 +3,7 @@ namespace Fantomas.Core open System open System.ComponentModel open Fantomas.FCS.Parse +open Fantomas.FCS.Text /// Raised when the F# parser produces errors for source code without conditional directives. exception ParseException of diagnostics: FSharpParserDiagnostic list @@ -11,6 +12,24 @@ exception ParseException of diagnostics: FSharpParserDiagnostic list type FormatException(msg: string) = inherit Exception(msg) +/// Raised when Fantomas reaches a state that its own model says is impossible, for example a +/// chain whose parts do not fit the shape the transformer guarantees. +/// +/// Unlike the other exceptions here, this never indicates a problem with the code being +/// formatted: it is a bug in Fantomas, or a change in how the F# parser groups expressions. +/// Failing loudly is deliberate — the alternative is silently dropping parts of the source. +type InvariantViolationException(msg: string, range: range) = + inherit + FormatException( + $"%s{msg}\nAt line %i{range.StartLine}, column %i{range.StartColumn} in %s{range.FileName}.\nThis is a bug in Fantomas. Please report it via https://fsprojects.github.io/fantomas-tools/" + ) + + /// The invariant that was violated, without the location or "please report" suffix. + member _.Invariant = msg + + /// The source range of the construct that triggered the violation. + member _.Range = range + /// Raised when one or more conditional compilation define combinations produce invalid syntax trees. type DefineParseException(combinations: string list) = inherit diff --git a/src/Fantomas.Core/Selection.fs b/src/Fantomas.Core/Selection.fs index a62e98f97e..59efac4f2a 100644 --- a/src/Fantomas.Core/Selection.fs +++ b/src/Fantomas.Core/Selection.fs @@ -237,18 +237,12 @@ let mkTreeWithSingleNode (node: Node) : TreeForSelection = | :? ExprChain as node -> let expr = Expr.Chain node mkOakFromModuleDecl (ModuleDecl.DeclExpr expr) - | :? ExprAppLongIdentAndSingleParenArgNode as node -> - let expr = Expr.AppLongIdentAndSingleParenArg node - mkOakFromModuleDecl (ModuleDecl.DeclExpr expr) | :? ExprAppSingleParenArgNode as node -> let expr = Expr.AppSingleParenArg node mkOakFromModuleDecl (ModuleDecl.DeclExpr expr) | :? ExprAppWithLambdaNode as node -> let expr = Expr.AppWithLambda node mkOakFromModuleDecl (ModuleDecl.DeclExpr expr) - | :? ExprNestedIndexWithoutDotNode as node -> - let expr = Expr.NestedIndexWithoutDot node - mkOakFromModuleDecl (ModuleDecl.DeclExpr expr) | :? ExprAppNode as node -> let expr = Expr.App node mkOakFromModuleDecl (ModuleDecl.DeclExpr expr) @@ -279,9 +273,6 @@ let mkTreeWithSingleNode (node: Node) : TreeForSelection = | :? ExprLongIdentSetNode as node -> let expr = Expr.LongIdentSet node mkOakFromModuleDecl (ModuleDecl.DeclExpr expr) - | :? ExprDotIndexedGetNode as node -> - let expr = Expr.DotIndexedGet node - mkOakFromModuleDecl (ModuleDecl.DeclExpr expr) | :? ExprDotIndexedSetNode as node -> let expr = Expr.DotIndexedSet node mkOakFromModuleDecl (ModuleDecl.DeclExpr expr) diff --git a/src/Fantomas.Core/SyntaxOak.fs b/src/Fantomas.Core/SyntaxOak.fs index 641616b502..a11ba40418 100644 --- a/src/Fantomas.Core/SyntaxOak.fs +++ b/src/Fantomas.Core/SyntaxOak.fs @@ -1380,57 +1380,81 @@ type ExprIndexWithoutDotNode(identifierExpr: Expr, indexExpr: Expr, range) = member val Identifier = identifierExpr member val Index = indexExpr -/// A chain link where a function is applied to a parenthesised argument, e.g. `foo(x)` in `a.foo(x).bar`. -type LinkSingleAppParen(functionName: Expr, parenExpr: ExprParenNode, range) = - inherit NodeBase(range) - override val Children: Node array = [| yield Expr.Node functionName; yield parenExpr |] - member val FunctionName = functionName - member val Paren = parenExpr - -/// A chain link where a function is applied to a unit argument, e.g. `Dispose()` in `x.Dispose()`. -type LinkSingleAppUnit(functionName: Expr, unit: UnitNode, range) = - inherit NodeBase(range) - override val Children: Node array = [| yield Expr.Node functionName; yield unit |] - member val FunctionName = functionName - member val Unit = unit +/// The argument of a call within a chain — either a parenthesised expression or unit. +[] +type ChainCall = + | Paren of ExprParenNode + | Unit of UnitNode -/// A single link in a method-call or property-access chain (e.g. a.b().c[0]). -/// The chain is represented as an ordered list of ChainLink values so that the -/// printer can decide whether to keep the chain on one line or break at each dot. +/// A single dot-prefixed step in a member-access or call chain. +/// Every step is reached through a .; the dot is an explicit +/// so that trivia (comments, blank lines) attached to it are preserved during formatting. +/// +/// A segment is always intermediate: the final call of a chain is the +/// ExprChain.Terminal, never a segment here. That is why DotApplication (an +/// intermediate call) is distinct from DotMember (plain access) — the distinction the +/// layout cares about (navigation vs. action) is then visible in the shape of the data itself. [] -type ChainLink = - | Identifier of Expr - | Dot of SingleTextNode - | Expr of Expr - | AppParen of - // There should only be one argument - LinkSingleAppParen - | AppUnit of LinkSingleAppUnit - // [ expr ] from DotIndexedGet - | IndexExpr of Expr // e.[f] - - static member Node(link: ChainLink) : Node = - match link with - | Identifier e -> Expr.Node e - | Dot n -> n - | Expr e -> Expr.Node e - | AppParen n -> n - | AppUnit n -> n - | IndexExpr e -> Expr.Node e - -/// Example: `person.Address.City.ToUpper()` — a chain of dot-separated member accesses and calls -type ExprChain(links: ChainLink list, range) = - inherit NodeBase(range) - override val Children: Node array = List.map ChainLink.Node links |> List.toArray - member val Links = links - -/// Example: `List.map(f)` — a qualified name (dotted identifier) applied to a single parenthesised argument -type ExprAppLongIdentAndSingleParenArgNode(functionName: IdentListNode, argExpr: Expr, range) = - inherit NodeBase(range) - - override val Children: Node array = [| yield functionName; yield Expr.Node argExpr |] - member val FunctionName = functionName - member val ArgExpr = argExpr +type ChainSegment = + /// .Foo — plain property access (navigation) + /// .Items[0] — expr = IndexWithoutDot(Items, [0]), no dedicated case needed + | DotMember of dot: SingleTextNode * expr: Expr + /// .Foo(x) — intermediate call — always tight, never a space before ( + /// .Foo() — intermediate unit call — always tight + /// e.g. the `.Foo(x)` in `a.Foo(x).Bar`. The terminal call of a chain is not a segment. + | DotApplication of dot: SingleTextNode * expr: Expr * call: ChainCall + /// Old dot-bracket index syntax: arr.[i]. + /// DotIndex is a separate case rather than DotMember because the [ and + /// ] brackets are not part of indexExpr in the AST — genExpr + /// indexExpr produces only the content (e.g. 0), not [0]. No existing + /// Expr type naturally renders bracketed index content, so the printer must add + /// [ and ] explicitly. From a layout perspective DotIndex is + /// identical to DotMember — both are navigation segments with no call. + | DotIndex of dot: SingleTextNode * indexExpr: Expr + +/// Controls whether and how a space is emitted before the terminal call's parenthesis. +/// The terminal call is the only position in a chain where a space is negotiable; +/// intermediate calls are always tight (adding a space before their parens changes the parse tree). +[] +type ChainTerminal = + | SpaceAllowed of ChainCall // regular chain — space governed by SpaceBeforeUppercaseInvocation + | NoSpaceAllowed of ChainCall // DotLambda body — space never permitted (compiler constraint) + | NoTerminal // chain ends with a property access or index, no invocation + +/// Example: `person.Address.City.ToUpper()` — a chain of dot-separated member accesses and calls. +/// Head is the leading receiver expression (fully decomposed; no dotted content remains in it). +/// Segments are the dot-paired steps. Terminal is the optional outermost call. +type ExprChain(head: Expr, segments: ChainSegment list, terminal: ChainTerminal, range) = + inherit NodeBase(range) + member val Head = head + member val Segments = segments + member val Terminal = terminal + + override val Children: Node array = + [| yield Expr.Node head + + for segment in segments do + match segment with + | ChainSegment.DotMember(dot, expr) -> + yield dot + yield Expr.Node expr + | ChainSegment.DotApplication(dot, expr, call) -> + yield dot + yield Expr.Node expr + + match call with + | ChainCall.Paren p -> yield p + | ChainCall.Unit u -> yield u + | ChainSegment.DotIndex(dot, idx) -> + yield dot + yield Expr.Node idx + + match terminal with + | ChainTerminal.SpaceAllowed(ChainCall.Paren p) + | ChainTerminal.NoSpaceAllowed(ChainCall.Paren p) -> yield p + | ChainTerminal.SpaceAllowed(ChainCall.Unit u) + | ChainTerminal.NoSpaceAllowed(ChainCall.Unit u) -> yield u + | ChainTerminal.NoTerminal -> () |] /// Example: `f(a)` — a general expression (not a simple dotted name) applied to a single parenthesised argument type ExprAppSingleParenArgNode(functionExpr: Expr, argExpr: Expr, range) = @@ -1470,20 +1494,6 @@ type ExprAppWithLambdaNode member val Lambda = lambda member val ClosingParen = closingParen -/// Example: `xs[i] arg` — an indexed expression (`xs[i]`) immediately applied to an argument. -/// Used for cases like `arr[0] + 1` where the index result is itself called or applied. -type ExprNestedIndexWithoutDotNode(identifierExpr: Expr, indexExpr: Expr, argumentExpr: Expr, range) = - inherit NodeBase(range) - - override val Children: Node array = - [| yield Expr.Node identifierExpr - yield Expr.Node indexExpr - yield Expr.Node argumentExpr |] - - member val Identifier = identifierExpr - member val Index = indexExpr - member val Argument = argumentExpr - /// Example: `List.map f xs` — a function applied to two or more space-separated arguments type ExprAppNode(functionExpr: Expr, arguments: Expr list, range) = inherit NodeBase(range) @@ -1716,13 +1726,6 @@ type ExprLongIdentSetNode(identifier: IdentListNode, rhs: Expr, range) = member val Identifier = identifier member val Expr = rhs -/// Example: `arr.[i]` — indexed get using the older dot-bracket syntax (deprecated in F# 6). -type ExprDotIndexedGetNode(objectExpr: Expr, indexExpr: Expr, range) = - inherit NodeBase(range) - override val Children: Node array = [| yield Expr.Node objectExpr; yield Expr.Node indexExpr |] - member val ObjectExpr = objectExpr - member val IndexExpr = indexExpr - /// Example: `arr.[i] <- value` — indexed set using the older dot-bracket syntax (deprecated in F# 6). type ExprDotIndexedSetNode(objectExpr: Expr, indexExpr: Expr, valueExpr: Expr, range) = inherit NodeBase(range) @@ -1874,15 +1877,6 @@ type ExprIndexFromEndNode(expr: Expr, range) = override val Children: Node array = [| Expr.Node expr |] member val Expr = expr -/// Example: `_.ToUpper()` or `_.Length` — a dot-lambda shorthand (F# 8+). -/// `Underscore` is the `_` placeholder, `Dot` is the `.`, and `Expr` is the member access or call. -type ExprDotLambda(underscore: SingleTextNode, dot: SingleTextNode, expr: Expr, range: range) = - inherit NodeBase(range) - override val Children: Node array = [| underscore; dot; Expr.Node expr |] - member val Underscore = underscore - member val Dot = dot - member val Expr = expr - /// Example: `begin expr end` — explicit `begin`/`end` block delimiters (equivalent to parentheses). type ExprBeginEndNode(beginNode: SingleTextNode, expr: Expr, endNode: SingleTextNode, range) = inherit NodeBase(range) @@ -1941,10 +1935,8 @@ type Expr = | SameInfixApps of ExprSameInfixAppsNode | InfixApp of ExprInfixAppNode | IndexWithoutDot of ExprIndexWithoutDotNode - | AppLongIdentAndSingleParenArg of ExprAppLongIdentAndSingleParenArgNode | AppSingleParenArg of ExprAppSingleParenArgNode | AppWithLambda of ExprAppWithLambdaNode - | NestedIndexWithoutDot of ExprNestedIndexWithoutDotNode | App of ExprAppNode | TypeApp of ExprTypeAppNode | TryWithSingleClause of ExprTryWithSingleClauseNode @@ -1956,7 +1948,6 @@ type Expr = | Ident of SingleTextNode | OptVar of ExprOptVarNode | LongIdentSet of ExprLongIdentSetNode - | DotIndexedGet of ExprDotIndexedGetNode | DotIndexedSet of ExprDotIndexedSetNode | NamedIndexedPropertySet of ExprNamedIndexedPropertySetNode | DotNamedIndexedPropertySet of ExprDotNamedIndexedPropertySetNode @@ -1969,7 +1960,6 @@ type Expr = | IndexFromEnd of ExprIndexFromEndNode | Typar of SingleTextNode | Chain of ExprChain - | DotLambda of ExprDotLambda | BeginEnd of ExprBeginEndNode | ExplicitConstructorThenExpr of ExprExplicitConstructorThenExpr @@ -2010,10 +2000,8 @@ type Expr = | SameInfixApps n -> n | InfixApp n -> n | IndexWithoutDot n -> n - | AppLongIdentAndSingleParenArg n -> n | AppSingleParenArg n -> n | AppWithLambda n -> n - | NestedIndexWithoutDot n -> n | App n -> n | TypeApp n -> n | TryWithSingleClause n -> n @@ -2025,7 +2013,6 @@ type Expr = | Ident n -> n | OptVar n -> n | LongIdentSet n -> n - | DotIndexedGet n -> n | DotIndexedSet n -> n | NamedIndexedPropertySet n -> n | DotNamedIndexedPropertySet n -> n @@ -2038,7 +2025,6 @@ type Expr = | IndexFromEnd n -> n | Typar n -> n | Chain n -> n - | DotLambda n -> n | BeginEnd n -> n | ExplicitConstructorThenExpr n -> n From cc0122da7cc70de607729a71e3cf8c9588949eef Mon Sep 17 00:00:00 2001 From: nojaf Date: Fri, 31 Jul 2026 15:57:18 +0200 Subject: [PATCH 02/26] Add struct to ChainCallPosition --- src/Fantomas.Core/CodePrinter.fs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Fantomas.Core/CodePrinter.fs b/src/Fantomas.Core/CodePrinter.fs index bab484e4cd..844568ca5e 100644 --- a/src/Fantomas.Core/CodePrinter.fs +++ b/src/Fantomas.Core/CodePrinter.fs @@ -407,7 +407,7 @@ let requiresMultilineToPreserveSemantics (exprs: Expr list) = /// * whether the opening `(` is allowed to leave the member name, which is a grammar /// constraint rather than a style choice, and /// * whether a `(function` argument is forced onto its own line, which is a readability call. -[] +[] type ChainCallPosition = /// A call with more chain after it, e.g. `.Foo(x)` in `a.Foo(x).Bar()`. | Intermediate From 4eb29c1882912826d1c4f8fa47b4dcf167477ffe Mon Sep 17 00:00:00 2001 From: nojaf Date: Fri, 31 Jul 2026 16:01:21 +0200 Subject: [PATCH 03/26] Clarify the two changes more obviously --- docs/docs/contributors/Chains.md | 59 ++++++++++++++++++++++---------- 1 file changed, 41 insertions(+), 18 deletions(-) diff --git a/docs/docs/contributors/Chains.md b/docs/docs/contributors/Chains.md index d897792708..466602bc89 100644 --- a/docs/docs/contributors/Chains.md +++ b/docs/docs/contributors/Chains.md @@ -55,6 +55,43 @@ arr.[i] // a chain, this indexing syntax does have a dot An expression without any dots is laid out by other rules, not the ones on this page. +## Two decisions, not one + +Formatting a chain settles two questions that have nothing to do with each other: + +1. **Where do the line breaks go?** A style decision, and the bulk of this page. +2. **May a space sit between a method name and its `(`?** Mostly *not* a style decision, and the shorter of the two, so it is settled first. + +## Only the last call may have a space before its parentheses + +Fantomas has settings that ask for a space before the parentheses of a call, [`space_before_uppercase_invocation`](../end-users/Configuration.html) and [`space_before_lowercase_invocation`](../end-users/Configuration.html). + +In a chain, **those settings apply to the final call only**. Every earlier call stays tight, whatever the settings say: + +```fsharp +// both examples with space_before_uppercase_invocation = true + +obj.Bar () // a call on its own: the setting applies + +a.Foo(x).Bar (y) // in a chain: only the final .Bar takes the space, + // the intermediate .Foo stays tight +``` + +This is not Fantomas being inconsistent. A space in the middle of a chain changes what the code *means*: + +```fsharp +a.Foo (x).Bar() // parsed as a.Foo ((x).Bar()) +a.Foo(x).Bar() // parsed the way you intended +``` + +The parser reads `(x).Bar()` as a single parenthesised argument handed to `a.Foo`. So for every call except the last one, tightness is a grammar requirement rather than a preference, and no setting can override it. + +The same applies wherever an expression has to stay glued to its neighbour: a receiver that is itself a call (`getBuilder().Build()`), an indexed call (`x.Foo()[0]`), or a `?` access. In each case a space would rebind the parentheses to the wrong thing. + +There is one place where even the last call stays tight: the body of a `_.` shorthand lambda, covered in the final section of this page. + +Everything from here on is about the first question, where the line breaks go. + ## Two kinds of step Once Fantomas has to break a chain, it sorts the steps into two weights. @@ -87,7 +124,7 @@ That is the one pair worth keeping straight: An index or a set of type arguments is navigation while it stays on one line, and becomes an action if its contents grow big enough to need several lines. -## The rule +## The rule for line breaks ```text Does the whole chain fit on one line? @@ -280,9 +317,9 @@ F# lets you write `_.Property` as a short lambda. Fantomas treats it as a chain "yow" |> _.Substring(0, 16).ToLower() ``` -There is one thing that makes it special. -The F# compiler requires the body of a `_.` lambda to stay tightly joined, so Fantomas will **never** insert a space before a `(` here. -This holds even when [`space_before_uppercase_invocation`](../end-users/Configuration.html) is enabled, which would otherwise produce `ToLower ()` and fail to compile. +There is one thing that makes it special, and it concerns [the space before a call](#only-the-last-call-may-have-a-space-before-its-parentheses). +Everywhere else the *last* call of a chain may take a space when a setting asks for one. Here it may not: the F# compiler requires the body of a `_.` lambda to stay atomic, so `ToLower ()` would not compile. +Fantomas therefore keeps it tight even with [`space_before_uppercase_invocation`](../end-users/Configuration.html) enabled. This was [issue 3364](https://github.com/fsprojects/fantomas/issues/3364). Apart from that, it follows the same rule as any other chain. The example above has two calls, so if it does not fit, it becomes a pipeline: @@ -293,18 +330,4 @@ _ .ToLower() ``` -## Why intermediate calls never get a space - -Settings such as [`space_before_uppercase_invocation`](../end-users/Configuration.html) can ask Fantomas to write `obj.Bar ()` with a space. -You may notice that this never happens to a call in the middle of a chain. -That is not a style preference. In F#, a space in the middle of a chain changes how the code is parsed: - -```fsharp -a.Foo (x).Bar() // means a.Foo ((x).Bar()) -a.Foo(x).Bar() // means what you intended -``` - -So only the very last call in a chain can be given a space, and only when a setting asks for one. -Every call before it stays tight, always. - From 031c03a58f9832433589591f6f9d2baaf9a89ab0 Mon Sep 17 00:00:00 2001 From: nojaf Date: Fri, 31 Jul 2026 16:14:14 +0200 Subject: [PATCH 04/26] Clarify which parentheses stay tight in a chain --- docs/docs/contributors/Chains.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/contributors/Chains.md b/docs/docs/contributors/Chains.md index 466602bc89..b4d33e8a80 100644 --- a/docs/docs/contributors/Chains.md +++ b/docs/docs/contributors/Chains.md @@ -86,7 +86,7 @@ a.Foo(x).Bar() // parsed the way you intended The parser reads `(x).Bar()` as a single parenthesised argument handed to `a.Foo`. So for every call except the last one, tightness is a grammar requirement rather than a preference, and no setting can override it. -The same applies wherever an expression has to stay glued to its neighbour: a receiver that is itself a call (`getBuilder().Build()`), an indexed call (`x.Foo()[0]`), or a `?` access. In each case a space would rebind the parentheses to the wrong thing. +The same constraint turns up wherever an expression has to stay glued to its neighbour. In `getBuilder().Build()` the receiver keeps its own `()` tight, in `x.Foo()[0]` the indexed call keeps its own `()` tight, and a `?` access does the same. In each case a space there would rebind the parentheses to the wrong thing. (The *final* call is still free to take a space: with the setting on, that first example formats as `getBuilder().Build ()`.) There is one place where even the last call stays tight: the body of a `_.` shorthand lambda, covered in the final section of this page. From 935d5031edad43b1e7e4c051977f13eed6d68112 Mon Sep 17 00:00:00 2001 From: nojaf Date: Fri, 31 Jul 2026 16:46:19 +0200 Subject: [PATCH 05/26] More usage of InvariantViolation --- src/Fantomas.Core/ASTTransformer.fs | 81 +++++++++++-------- src/Fantomas.Core/CodePrinter.fs | 16 +++- src/Fantomas.Core/FormatConfig.fs | 2 +- src/Fantomas.Core/Selection.fs | 3 + .../EditorConfigurationTests.fs | 4 +- 5 files changed, 70 insertions(+), 36 deletions(-) diff --git a/src/Fantomas.Core/ASTTransformer.fs b/src/Fantomas.Core/ASTTransformer.fs index a0800f8b53..2a601260dc 100644 --- a/src/Fantomas.Core/ASTTransformer.fs +++ b/src/Fantomas.Core/ASTTransformer.fs @@ -291,7 +291,7 @@ let mkInheritConstructor (creationAide: CreationAide) (t: SynType) (e: SynExpr) let mkTuple (creationAide: CreationAide) (exprs: SynExpr list) (commas: range list) (m: range) = match exprs with - | [] -> failwith "SynExpr.Tuple with no elements" + | [] -> invariantViolation m "a tuple expression has no elements" | head :: tail -> let rest = assert (tail.Length = commas.Length) @@ -393,7 +393,7 @@ let mkSynMatchClause creationAide (SynMatchClause(p, eo, e, range, _, trivia)) : let arrowRange = match trivia.ArrowRange with | Some r -> r - | None -> failwith $"unable to get the arrow range from trivia in %s{nameof mkSynMatchClause}" + | None -> invariantViolation range $"unable to get the arrow range from trivia in %s{nameof mkSynMatchClause}" MatchClauseNode( Option.map (stn "|") trivia.BarRange, @@ -604,7 +604,10 @@ let (|ElIf|_|) expr = | Some mElse -> match ifNode with | Choice1Of2 ifNode -> Choice2Of2(mElse, ifNode.Range) - | Choice2Of2 _ -> failwith "Cannot merge a second else keyword into existing else if" + | Choice2Of2 _ -> + invariantViolation + expr.Range + "cannot merge a second else keyword into existing else if" | None -> ifNode (node, e1, thenKw, e2)) @@ -1260,7 +1263,7 @@ let mkExpr (creationAide: CreationAide) (e: SynExpr) : Expr = let mTuple = match List.tryHead exprs, List.tryLast exprs with | Some e1, Some e2 -> unionRanges e1.Range e2.Range - | _ -> failwith "SynExpr.Tuple with no elements" + | _ -> invariantViolation exprRange "a struct tuple expression has no elements" ExprStructTupleNode(stn "struct" mStruct, mkTuple creationAide exprs commas mTuple, stn ")" mClosing, exprRange) |> Expr.StructTuple @@ -1286,7 +1289,8 @@ let mkExpr (creationAide: CreationAide) (e: SynExpr) : Expr = | _ -> None) match baseInfo, copyInfo with - | Some _, Some _ -> failwith "Unexpected that both baseInfo and copyInfo are present in SynExpr.Record" + | Some _, Some _ -> + invariantViolation exprRange "a record expression has both an inherit and a copy-from source" | Some(t, e, m, _, mInherit), None -> let inheritCtor = mkInheritConstructor creationAide t e mInherit m @@ -1875,7 +1879,7 @@ let mkExpr (creationAide: CreationAide) (e: SynExpr) : Expr = [ LinkExpr.Identifier underscore; LinkExpr.Dot mDot; yield! bodyLinks ] mkChainFromLinks creationAide allLinks ChainTerminal.NoSpaceAllowed exprRange - | _ -> failwithf "todo for %A" e + | _ -> invariantViolation exprRange "no Oak node is defined for this expression: %A" e let mkExprQuote creationAide isRaw e range : ExprQuoteNode = let startToken, endToken = @@ -1909,11 +1913,12 @@ let mkUnit (StartEndRange 1 (lpr, m, rpr)) = UnitNode(stn "(" lpr, stn ")" rpr, let mkTuplePat (creationAide: CreationAide) (pats: SynPat list) (commas: range list) (m: range) = match pats with - | [] -> failwith "SynPat.Tuple with no elements" + | [] -> invariantViolation m "a tuple pattern has no elements" | head :: tail -> let rest = if tail.Length <> commas.Length then - failwith + invariantViolation + m $"Number of elements in tail of tuple (%i{tail.Length}) was not equal to number of commas (%i{commas.Length}), at range %O{m}." List.zip commas tail @@ -2032,7 +2037,7 @@ let mkPat (creationAide: CreationAide) (p: SynPat) = |> Pattern.IsInst | SynPat.QuoteExpr(SynExpr.Quote(_, isRaw, e, _, _), _) -> mkExprQuote creationAide isRaw e patternRange |> Pattern.QuoteExpr - | pat -> failwith $"unexpected pattern: %A{pat}" + | pat -> invariantViolation pat.Range $"no Oak node is defined for this pattern: %A{pat}" let mkBindingReturnInfo creationAide (returnInfo: SynBindingReturnInfo option) = Option.bind @@ -2091,7 +2096,7 @@ let mkBinding let equalsRange = match trivia.EqualsRange with | Some r -> r - | None -> failwith $"failed to get equals range in %s{nameof mkBinding}" + | None -> invariantViolation pat.Range $"failed to get equals range in %s{nameof mkBinding}" stn "=" equalsRange @@ -2155,11 +2160,11 @@ let mkExternBinding let externNode = match trivia.LeadingKeyword with | SynLeadingKeyword.Extern mExtern -> stn "extern" mExtern - | _ -> failwith "Leading keyword should be extern" + | _ -> invariantViolation range "an extern binding has a leading keyword other than `extern`" let attributesOfReturnType, returnType = match returnInfo with - | None -> failwith "return info in extern binding should be present" + | None -> invariantViolation range "an extern binding has no return type" | Some(SynBindingReturnInfo(typeName = t; attributes = a)) -> let attrs = mkAttributes creationAide a @@ -2231,7 +2236,7 @@ let mkExternBinding longDotId = longDotId argPats = SynArgPats.Pats [ SynPat.Tuple(_, ps, _, StartEndRange 1 (mOpen, _, mClose)) ]) -> mkSynLongIdent creationAide longDotId, stn "(" mOpen, List.map mkExternPat ps, stn ")" mClose - | _ -> failwith "expecting a SynPat.LongIdent for extern binding" + | _ -> invariantViolation pat.Range "an extern binding has a head pattern that is not a long identifier" ExternBindingNode( mkXmlDoc xmlDoc, @@ -2302,7 +2307,7 @@ let mkModuleDecl (creationAide: CreationAide) (decl: SynModuleDecl) = declRange ) |> ModuleDecl.NestedModule - | decl -> failwithf $"Failed to create ModuleDecl for %A{decl}" + | decl -> invariantViolation decl.Range $"no Oak node is defined for this module declaration: %A{decl}" let mkSynTyparDecl (creationAide: CreationAide) @@ -2315,7 +2320,9 @@ let mkSynTyparDecl let intersectionConstraintNodes = if intersectionConstraints.Length <> trivia.AmpersandRanges.Length then - failwith "Unexpected mismatch in SynTyparDecl between intersectionConstraints and AmpersandRanges" + invariantViolation + typar.Range + "a typar declaration has a different number of intersection constraints than ampersands" else (trivia.AmpersandRanges, intersectionConstraints) ||> List.zip @@ -2381,7 +2388,8 @@ let mkSynRationalConst (creationAide: CreationAide) rc = | SynRationalConst.Paren(innerRc, _) -> visit innerRc | SynRationalConst.Negate(innerRc, range) -> RationalConstNode.Negate(NegateRationalNode(stn "-" range.StartRange, visit innerRc, range)) - | SynRationalConst.Rational _ -> failwith "SynRationalConst.Rational not wrapped in SynRationalConst.Paren" + | SynRationalConst.Rational(range = m) -> + invariantViolation m "a rational constant is not wrapped in parentheses" visit rc @@ -2579,7 +2587,8 @@ let mkType (creationAide: CreationAide) (t: SynType) : Type = Type.Var(mkSynTypar typar), ts | None -> match ts with - | [] -> failwith "SynType.Intersection does not contain typar or any intersectionConstraints" + | [] -> + invariantViolation t.Range "a type intersection contains neither a typar nor any constraints" | head :: tail -> mkType creationAide head, tail assert (ts.Length = trivia.AmpersandRanges.Length) @@ -2596,7 +2605,7 @@ let mkType (creationAide: CreationAide) (t: SynType) : Type = let nullType = stn "null" mNull |> Type.Var TypeOrNode(mkType creationAide innerType, stn "|" mBar, nullType, m) |> Type.Or - | t -> failwith $"unexpected type: %A{t}" + | t -> invariantViolation t.Range $"no Oak node is defined for this type: %A{t}" [] let rec visitOpenL acc decls = @@ -2670,7 +2679,7 @@ let mkSynLeadingKeyword (lk: SynLeadingKeyword) = | SynLeadingKeyword.Val valRange -> mtn [ "val", valRange ] | SynLeadingKeyword.New newRange -> mtn [ "new", newRange ] | SynLeadingKeyword.Do doRange -> mtn [ "do", doRange ] - | SynLeadingKeyword.Synthetic -> failwith "Unexpected SynLeadingKeyword.Synthetic" + | SynLeadingKeyword.Synthetic -> invariantViolation lk.Range "a synthetic leading keyword reached the transformer" let mkSynField (creationAide: CreationAide) @@ -2774,7 +2783,8 @@ let mkTypeDefn | SynTypeDefnLeadingKeyword.Type mType -> stn "type" mType | SynTypeDefnLeadingKeyword.And mAnd -> stn "and" mAnd | SynTypeDefnLeadingKeyword.StaticType _ - | SynTypeDefnLeadingKeyword.Synthetic -> failwithf "unexpected %A" trivia.LeadingKeyword + | SynTypeDefnLeadingKeyword.Synthetic -> + invariantViolation range "unexpected leading keyword %A" trivia.LeadingKeyword let implicitConstructorNode = match implicitConstructor with @@ -2875,7 +2885,7 @@ let mkTypeDefn | SynTypeDefnKind.Class, StartRange 5 (mClass, _) -> stn "class" mClass | SynTypeDefnKind.Interface, StartRange 9 (mInterface, _) -> stn "interface" mInterface | SynTypeDefnKind.Struct, StartRange 6 (mStruct, _) -> stn "struct" mStruct - | _ -> failwith "unexpected kind" + | _ -> invariantViolation range "unexpected type definition kind" let objectMembers = objectMembers @@ -2932,7 +2942,7 @@ let mkTypeDefn [ yield! objectMembers; yield! members ] TypeDefnRegularNode(typeNameNode, allMembers, typeDefnRange) |> TypeDefn.Regular - | _ -> failwithf $"Could not create a TypeDefn for %A{typeRepr}" + | _ -> invariantViolation range $"no Oak node is defined for this type definition: %A{typeRepr}" let mkWithGetSet (withKeyword: range option) @@ -3024,7 +3034,10 @@ let mkPropertyGetSetBinding [ tuplePat match List.tryLast ps with - | None -> failwith "" + | None -> + invariantViolation + binding.RangeOfBindingWithRhs + "an indexed property setter has no indexer pattern" | Some indexerPat -> mkPat creationAide indexerPat ] | [ SynPat.Tuple(false, [ p1; p2 ], _, _) ] -> [ mkPat creationAide p1; mkPat creationAide p2 ] | ps -> List.map (mkPat creationAide) ps @@ -3042,7 +3055,10 @@ let mkPropertyGetSetBinding mkExpr creationAide e, range ) - | _ -> failwith "SynBinding does not expected information for PropertyGetSetBinding" + | _ -> + invariantViolation + binding.RangeOfBindingWithRhs + "a property get/set binding is missing the information the printer needs" let mkMemberDefn (creationAide: CreationAide) (md: SynMemberDefn) = let memberDefinitionRange = md.Range @@ -3117,7 +3133,7 @@ let mkMemberDefn (creationAide: CreationAide) (md: SynMemberDefn) = memberDefinitionRange ) |> MemberDefn.Inherit - | None -> failwith "successful parse shouldn't have any unfinished inherit" + | None -> invariantViolation md.Range "a successful parse produced an unfinished inherit member" | SynMemberDefn.ValField(f, _) -> mkSynField creationAide f |> MemberDefn.ValField | SynMemberDefn.LetBindings( bindings = [ SynBinding(trivia = { LeadingKeyword = SynLeadingKeyword.Extern _ }) as binding ]) -> @@ -3354,8 +3370,8 @@ let mkMemberDefn (creationAide: CreationAide) (md: SynMemberDefn) = memberDefinitionRange ) |> MemberDefn.PropertyGetSet - | _ -> failwith "SynMemberDefn.GetSetMember cannot exist with get and without set" - | _ -> failwithf "Unexpected SynMemberDefn: %A" md + | _ -> invariantViolation md.Range "a get/set member has a getter but no setter" + | _ -> invariantViolation md.Range "no Oak node is defined for this member definition: %A" md let mkVal (creationAide: CreationAide) @@ -3403,7 +3419,7 @@ let mkMemberSig (creationAide: CreationAide) (ms: SynMemberSig) = MemberDefnInheritNode(stn "inherit" mInherit, mkType creationAide t, memberSigRange) |> MemberDefn.Inherit | SynMemberSig.ValField(f, _) -> mkSynField creationAide f |> MemberDefn.ValField - | _ -> failwithf "Cannot construct node for %A" ms + | _ -> invariantViolation ms.Range "no Oak node is defined for this member signature: %A" ms [] let rec mkModuleDecls @@ -3599,7 +3615,7 @@ let mkModuleSigDecl (creationAide: CreationAide) (decl: SynModuleSigDecl) = ) |> ModuleDecl.NestedModule | SynModuleSigDecl.Val(valSig, _) -> mkVal creationAide valSig |> ModuleDecl.Val - | decl -> failwithf $"Failed to create ModuleDecl for %A{decl}" + | decl -> invariantViolation decl.Range $"no Oak node is defined for this signature declaration: %A{decl}" let mkTypeDefnSig (creationAide: CreationAide) (SynTypeDefnSig(typeInfo, typeRepr, members, range, trivia)) : TypeDefn = let typeNameNode = @@ -3613,7 +3629,8 @@ let mkTypeDefnSig (creationAide: CreationAide) (SynTypeDefnSig(typeInfo, typeRep | SynTypeDefnLeadingKeyword.Type mType -> stn "type" mType | SynTypeDefnLeadingKeyword.And mAnd -> stn "and" mAnd | SynTypeDefnLeadingKeyword.StaticType _ - | SynTypeDefnLeadingKeyword.Synthetic -> failwithf "unexpected %A" trivia.LeadingKeyword + | SynTypeDefnLeadingKeyword.Synthetic -> + invariantViolation range "unexpected leading keyword %A" trivia.LeadingKeyword let m = if not px.IsEmpty then @@ -3713,7 +3730,7 @@ let mkTypeDefnSig (creationAide: CreationAide) (SynTypeDefnSig(typeInfo, typeRep | SynTypeDefnKind.Class, StartRange 5 (mClass, _) -> stn "class" mClass | SynTypeDefnKind.Interface, StartRange 9 (mInterface, _) -> stn "interface" mInterface | SynTypeDefnKind.Struct, StartRange 6 (mStruct, _) -> stn "struct" mStruct - | _ -> failwith "unexpected kind" + | _ -> invariantViolation range "unexpected type definition kind" let objectMembers = objectMembers |> List.map (mkMemberSig creationAide) @@ -3759,7 +3776,7 @@ let mkTypeDefnSig (creationAide: CreationAide) (SynTypeDefnSig(typeInfo, typeRep [ yield! objectMembers; yield! members ] TypeDefnRegularNode(typeNameNode, allMembers, typeDefnRange) |> TypeDefn.Regular - | _ -> failwithf "Could not create a TypeDefn for %A" typeRepr + | _ -> invariantViolation range "no Oak node is defined for this type definition: %A" typeRepr [] let rec mkModuleSigDecls diff --git a/src/Fantomas.Core/CodePrinter.fs b/src/Fantomas.Core/CodePrinter.fs index 844568ca5e..48846757e4 100644 --- a/src/Fantomas.Core/CodePrinter.fs +++ b/src/Fantomas.Core/CodePrinter.fs @@ -35,7 +35,13 @@ let rec (|UppercaseType|LowercaseType|) (t: Type) : Choice = | Type.Var node -> upperOrLower (node.Text.Substring(1)) | Type.AppPostfix node -> (|UppercaseType|LowercaseType|) node.First | Type.AppPrefix node -> (|UppercaseType|LowercaseType|) node.Identifier - | _ -> failwithf $"Cannot determine if synType %A{t} is uppercase or lowercase" + | _ -> + raise ( + InvariantViolationException( + $"cannot tell whether this type is uppercase or lowercase: %A{t}", + (Type.Node t).Range + ) + ) let rec (|UppercaseExpr|LowercaseExpr|) (expr: Expr) = let upperOrLower (v: string) = @@ -76,7 +82,13 @@ let rec (|UppercaseExpr|LowercaseExpr|) (expr: Expr) = | Expr.Paren node -> (|UppercaseExpr|LowercaseExpr|) node.Expr | Expr.App node -> (|UppercaseExpr|LowercaseExpr|) node.FunctionExpr | Expr.IndexWithoutDot node -> (|UppercaseExpr|LowercaseExpr|) node.Identifier - | _ -> failwithf "cannot determine if Expr %A is uppercase or lowercase" expr + | _ -> + raise ( + InvariantViolationException( + $"cannot tell whether this expression is uppercase or lowercase: %A{expr}", + (Expr.Node expr).Range + ) + ) let (|ParenExpr|_|) (e: Expr) = match e with diff --git a/src/Fantomas.Core/FormatConfig.fs b/src/Fantomas.Core/FormatConfig.fs index 74ee762e2e..d415302900 100644 --- a/src/Fantomas.Core/FormatConfig.fs +++ b/src/Fantomas.Core/FormatConfig.fs @@ -103,7 +103,7 @@ type EndOfLineStyle = static member OfConfigString(eolString: string) = match eolString with | "lf" -> Some EndOfLineStyle.LF - | "cr" -> failwith "Carriage returns are not valid for F# code, please use one of 'lf' or 'crlf'" + | "cr" -> raise (FormatException "Carriage returns are not valid for F# code, please use one of 'lf' or 'crlf'") | "crlf" -> Some EndOfLineStyle.CRLF | _ -> None diff --git a/src/Fantomas.Core/Selection.fs b/src/Fantomas.Core/Selection.fs index 59efac4f2a..e86d3387e6 100644 --- a/src/Fantomas.Core/Selection.fs +++ b/src/Fantomas.Core/Selection.fs @@ -336,6 +336,9 @@ let mkTreeWithSingleNode (node: Node) : TreeForSelection = mkOakFromModuleDecl (ModuleDecl.TypeDefn tdn) | :? ValNode as node -> mkOakFromModuleDecl (ModuleDecl.Val node) | _ -> + // Deliberately not an InvariantViolationException: this is not an impossible state but a + // node kind selection does not support yet, and release builds degrade to `Unsupported` + // rather than failing. The DEBUG-only throw exists to make the gap loud while developing. #if DEBUG failwithf $"%s{node.GetType().Name} is currently unsupported" #endif diff --git a/src/Fantomas.Tests/EditorConfigurationTests.fs b/src/Fantomas.Tests/EditorConfigurationTests.fs index b292deaaeb..ecd903f016 100644 --- a/src/Fantomas.Tests/EditorConfigurationTests.fs +++ b/src/Fantomas.Tests/EditorConfigurationTests.fs @@ -344,8 +344,10 @@ end_of_line = cr use fsharpFile = new FSharpFile(rootDir) + // A FormatException rather than a bare exception, because that is the type the CLI matches + // on to decide what to print. Anything else reports an empty message at normal verbosity. let ex = - Assert.Throws(fun () -> EditorConfig.readConfiguration fsharpFile.FSharpFile |> ignore) + Assert.Throws(fun () -> EditorConfig.readConfiguration fsharpFile.FSharpFile |> ignore) ex.Message == "Carriage returns are not valid for F# code, please use one of 'lf' or 'crlf'" From 097eb3f47b1ae729b52a5f3f46b4f4ba93cc1fb6 Mon Sep 17 00:00:00 2001 From: nojaf Date: Tue, 4 Aug 2026 14:45:55 +0200 Subject: [PATCH 06/26] improve wording --- docs/docs/contributors/Chains.md | 35 +++++++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/docs/docs/contributors/Chains.md b/docs/docs/contributors/Chains.md index b4d33e8a80..337f695ca9 100644 --- a/docs/docs/contributors/Chains.md +++ b/docs/docs/contributors/Chains.md @@ -28,9 +28,10 @@ If you disagree with a rule, the discussion belongs at [fsharp/fslang-design](ht Fantomas does not edit your text. Think of it like a word processor: it re-types your entire file from scratch, following its own rules. -When it re-types a piece of code, it asks one question first: +Most of that re-typing is mechanical: spacing, indentation and parentheses follow fixed rules with nothing to decide. +The one real choice is **where to put line breaks**, and it comes down to a single question: -> Does this fit on the current line? +> Does this fit within the max line length? If the answer is yes, it stays on one line and there is nothing more to decide. Everything below is about what happens when the answer is **no**. @@ -86,7 +87,7 @@ a.Foo(x).Bar() // parsed the way you intended The parser reads `(x).Bar()` as a single parenthesised argument handed to `a.Foo`. So for every call except the last one, tightness is a grammar requirement rather than a preference, and no setting can override it. -The same constraint turns up wherever an expression has to stay glued to its neighbour. In `getBuilder().Build()` the receiver keeps its own `()` tight, in `x.Foo()[0]` the indexed call keeps its own `()` tight, and a `?` access does the same. In each case a space there would rebind the parentheses to the wrong thing. (The *final* call is still free to take a space: with the setting on, that first example formats as `getBuilder().Build ()`.) +The same constraint turns up wherever an expression has to stay glued to its neighbour. In `getBuilder().Build()` the receiver keeps its own `()` tight, in `x.Foo()[0]` the indexed call keeps its own `()` tight, and the dynamic-access operator `?` behaves the same way: `settings?Section("db")?ConnectionString` stays tight throughout. In each case a space there would rebind the parentheses to the wrong thing. (The *final* call is still free to take a space: with the setting on, that first example formats as `getBuilder().Build ()`.) There is one place where even the last call stays tight: the body of a `_.` shorthand lambda, covered in the final section of this page. @@ -122,8 +123,6 @@ That is the one pair worth keeping straight: .Cast() // action, this calls something ``` -An index or a set of type arguments is navigation while it stays on one line, and becomes an action if its contents grow big enough to need several lines. - ## The rule for line breaks ```text @@ -142,6 +141,10 @@ In one sentence: Navigation is never worth a line of its own. It rides at the front of the line belonging to the action it introduces. +Riding along only works while the navigation itself stays on one line. +If an index (or a set of type arguments) has to break across several lines, it can no longer be a passenger, and the question above then counts it as an action: it claims a line of its own, and the chain around it becomes a pipeline. +It is still navigation in what it *does*; it has simply grown too big to ride along. + ## Examples The examples below assume a narrower [max line length](../end-users/Configuration.html) than the default, so the breaks are visible on this page. @@ -189,6 +192,28 @@ document.Body.FirstChild `.Body` and `.FirstChild` lead the first line. `.ParentElement` is navigation introducing `.RemoveChild(oldNode)`, so it rides at the front of that line instead of claiming one of its own. +### An index too big to ride along + +```fsharp +lookupTable.[0].AppendEntry( + newEntryForTheBucket +) +``` + +A short index is navigation, so there is a single action at the end and only its arguments break. + +Grow the index until it needs several lines of its own and it can no longer ride along: + +```fsharp +lookupTable + .[computeBucketIndex + hashOfTheKeyValue + tableSizeInBuckets] + .AppendEntry(newEntry) +``` + +Nothing about the index started executing. It just stopped fitting on someone else's line. + ### A chain that ends in navigation ```fsharp From ca30bd386494368e08a19bf520aacf2341a571c4 Mon Sep 17 00:00:00 2001 From: nojaf Date: Wed, 5 Aug 2026 10:09:19 +0200 Subject: [PATCH 07/26] Add code review feedback --- .claude/commands/chain.md | 20 +++++++++++++++++ AGENTS.md | 2 +- build.fsx | 4 ++-- scripts/chain.fsx | 34 ++++++++++++++--------------- src/Fantomas.Core/ASTTransformer.fs | 8 +++++-- src/Fantomas.Core/SyntaxOak.fs | 2 +- 6 files changed, 47 insertions(+), 23 deletions(-) create mode 100644 .claude/commands/chain.md diff --git a/.claude/commands/chain.md b/.claude/commands/chain.md new file mode 100644 index 0000000000..9ad83916e4 --- /dev/null +++ b/.claude/commands/chain.md @@ -0,0 +1,20 @@ +--- +description: Show the ExprChain structure (head, segments, terminal) of F# source code +allowed-tools: Bash(dotnet fsi:*), Bash(echo:*), Bash(dotnet build:*) +--- + +First build the project: `dotnet build src/Fantomas.Core/Fantomas.Core.fsproj` + +Then run the chain script. Pass a file path as argument: + +``` +dotnet fsi scripts/chain.fsx [--signature] +``` + +Or pipe inline source via stdin: + +``` +echo '' | dotnet fsi scripts/chain.fsx [--signature] +``` + +$ARGUMENTS diff --git a/AGENTS.md b/AGENTS.md index 776e5e9467..c476da2074 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -31,7 +31,7 @@ Run these after completing a task, not during iterative development — analyzer ### Format ```bash -dotnet fantomas src docs build.fsx +dotnet fsi build.fsx -- -p FormatAll ``` ### Analyzers diff --git a/build.fsx b/build.fsx index eb8ee6872b..060efcc81a 100644 --- a/build.fsx +++ b/build.fsx @@ -69,7 +69,7 @@ pipeline "Build" { workingDir __SOURCE_DIRECTORY__ stage "RestoreTools" { run "dotnet tool restore" } stage "Clean" { run (cleanFolders [| analysisReportsDir; "artifacts" |]) } - stage "CheckFormat" { run "dotnet fantomas src docs build.fsx --check" } + stage "CheckFormat" { run "dotnet fantomas src docs scripts build.fsx --check" } stage "Build" { run "dotnet build -c Release --tl" } stage "UnitTests" { run "dotnet test -c Release --tl" } stage "Pack" { run "dotnet pack --no-restore -c Release --tl" } @@ -222,7 +222,7 @@ pipeline "Docs" { pipeline "FormatAll" { workingDir __SOURCE_DIRECTORY__ - stage "Fantomas" { run "dotnet fantomas src docs build.fsx" } + stage "Fantomas" { run "dotnet fantomas src docs scripts build.fsx" } runIfOnlySpecified true } diff --git a/scripts/chain.fsx b/scripts/chain.fsx index fc74cf3849..7438d46d58 100644 --- a/scripts/chain.fsx +++ b/scripts/chain.fsx @@ -27,14 +27,12 @@ let printChain (chain: ExprChain) = chain.Segments |> List.iteri (fun i seg -> match seg with - | ChainSegment.DotMember(_, expr) -> - printfn " [%02d] simple .%s" i (exprName expr) + | ChainSegment.DotMember(_, expr) -> printfn " [%02d] navigation .%s" i (exprName expr) | ChainSegment.DotApplication(_, expr, ChainCall.Unit _) -> - printfn " [%02d] complex .%s()" i (exprName expr) + printfn " [%02d] action .%s()" i (exprName expr) | ChainSegment.DotApplication(_, expr, ChainCall.Paren _) -> - printfn " [%02d] complex .%s(...)" i (exprName expr) - | ChainSegment.DotIndex(_, idx) -> - printfn " [%02d] simple .[%s]" i (exprName idx)) + printfn " [%02d] action .%s(...)" i (exprName expr) + | ChainSegment.DotIndex(_, idx) -> printfn " [%02d] navigation .[%s]" i (exprName idx)) let terminalStr = match chain.Terminal with @@ -48,13 +46,11 @@ let printChain (chain: ExprChain) = // Recursively collect all ExprChain nodes in the Oak's Node tree. let rec collectChains (node: Node) : ExprChain list = - [ - match node with - | :? ExprChain as chain -> yield chain - | _ -> () - for child in node.Children do - yield! collectChains child - ] + [ match node with + | :? ExprChain as chain -> yield chain + | _ -> () + for child in node.Children do + yield! collectChains child ] match Array.tryHead fsi.CommandLineArgs with | Some scriptPath -> @@ -62,9 +58,10 @@ match Array.tryHead fsi.CommandLineArgs with let sourceFile = FileInfo(Path.Combine(__SOURCE_DIRECTORY__, __SOURCE_FILE__)) if scriptFile.FullName = sourceFile.FullName then - let source, _, _, _ = parseArgs fsi.CommandLineArgs.[1..] + let source, isSignature, _, _ = parseArgs fsi.CommandLineArgs.[1..] + let oak = - CodeFormatter.ParseOakAsync(false, source) + CodeFormatter.ParseOakAsync(isSignature, source) |> Async.RunSynchronously |> Array.head |> fst @@ -75,9 +72,12 @@ match Array.tryHead fsi.CommandLineArgs with printfn "No chain expression found in input." else printfn "Found %d chain(s):\n" chains.Length - chains |> List.iteri (fun i chain -> + + chains + |> List.iteri (fun i chain -> printfn "--- Chain #%d ---" (i + 1) printChain chain printfn "") | _ -> - printfn "Usage: dotnet fsi scripts/chain.fsx " + printfn "Usage: dotnet fsi chain.fsx [--signature] []" + printfn " source code is read from stdin when no input file is given" diff --git a/src/Fantomas.Core/ASTTransformer.fs b/src/Fantomas.Core/ASTTransformer.fs index 2a601260dc..96f0d910d3 100644 --- a/src/Fantomas.Core/ASTTransformer.fs +++ b/src/Fantomas.Core/ASTTransformer.fs @@ -1143,8 +1143,12 @@ let (|ChainExpr|_|) (e: SynExpr) : LinkExpr list voption = | SynExpr.App( isInfix = false; funcExpr = SynExpr.LongIdent(longDotId = DottedLongIdent); argExpr = (ParenExpr _ | UnitExpr _)) -> ValueSome(visitChainLinks e id) - // An identifier-only application with a paren lambda (e.g. `List.map (fun n -> n)`) - // is NOT considered a chain because the identifier is not complex. + // An application with a paren lambda and a dot-less function expression (e.g. `map (fun n -> n)`) + // is NOT considered a chain: there is nothing to chain. The dotted `List.map (fun n -> n)` + // never reaches here — the arm above already claimed it. + // The one dotted shape that does land here is `DotGet(TypeApp(Ident))` + // (`MailboxProcessor.Start (fun ...)`): it deliberately stays an `AppWithLambda` + // rather than becoming a chain, so it keeps the space before its argument. | SynExpr.App( isInfix = false funcExpr = SynExpr.LongIdent _ | SynExpr.Ident _ | SynExpr.DotGet(expr = SynExpr.TypeApp(expr = SynExpr.Ident _)) diff --git a/src/Fantomas.Core/SyntaxOak.fs b/src/Fantomas.Core/SyntaxOak.fs index da0c6b8225..22b463a009 100644 --- a/src/Fantomas.Core/SyntaxOak.fs +++ b/src/Fantomas.Core/SyntaxOak.fs @@ -1439,7 +1439,7 @@ type ChainSegment = /// intermediate calls are always tight (adding a space before their parens changes the parse tree). [] type ChainTerminal = - | SpaceAllowed of ChainCall // regular chain — space governed by SpaceBeforeUppercaseInvocation + | SpaceAllowed of ChainCall // regular chain — space governed by SpaceBeforeUppercaseInvocation / SpaceBeforeLowercaseInvocation, depending on the casing of the called identifier | NoSpaceAllowed of ChainCall // DotLambda body — space never permitted (compiler constraint) | NoTerminal // chain ends with a property access or index, no invocation From a9a610bad1dd8ad6536b3afab1dace62731a3d82 Mon Sep 17 00:00:00 2001 From: nojaf Date: Thu, 6 Aug 2026 14:04:18 +0200 Subject: [PATCH 08/26] Use regular mermaid syntax in markdown --- .config/dotnet-tools.json | 4 ++-- docs/_body.html | 20 ++++++++++++++++++- .../Conditional Compilation Directives.md | 4 ++-- docs/docs/contributors/Formatted Code.md | 4 ++-- docs/docs/contributors/Solution Structure.md | 4 ++-- docs/docs/contributors/Transforming.md | 12 +++++------ docs/docs/contributors/Traverse.md | 4 ++-- 7 files changed, 35 insertions(+), 17 deletions(-) diff --git a/.config/dotnet-tools.json b/.config/dotnet-tools.json index 6f2c08cf5a..71468c1251 100644 --- a/.config/dotnet-tools.json +++ b/.config/dotnet-tools.json @@ -17,7 +17,7 @@ "rollForward": false }, "fsdocs-tool": { - "version": "22.0.0", + "version": "22.1.0", "commands": [ "fsdocs" ], @@ -38,4 +38,4 @@ "rollForward": false } } -} \ No newline at end of file +} diff --git a/docs/_body.html b/docs/_body.html index 5dddc4cdcb..8793272d6c 100644 --- a/docs/_body.html +++ b/docs/_body.html @@ -7,7 +7,25 @@ })