Merge pull request #3352 from nojaf/fix-3159

Fix soundness in `?` operator chains via Expr.DynamicChain node

Author: nojaf

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## [Unreleased]
+
+### Fixed
+
+- Chained `?` operator accesses (e.g. `x?a("")?b(t)`) no longer add a space before parenthesised arguments, which previously changed how the next `?member` was parsed. Detected during AST→Oak transformation and represented as a new `Expr.DynamicChain` node so the printer can keep the chain tight; lone `?` calls still respect `SpaceBefore(Upper|Lower)caseInvocation`. [#3159](https://github.com/fsprojects/fantomas/issues/3159)
+
 ## [8.0.0-alpha-011] - 2026-04-15
 
 ### Fixed

src/Fantomas.Core.Tests/DynamicOperatorTests.fs

@@ -88,13 +88,27 @@ let doc = x?a("")?b(t)?b(t)
     |> should
         equal
         """
-let doc = x?a ("")?b (t)?b (t)
+let doc = x?a("")?b(t)?b(t)
+"""
+
+[<Test>]
+let ``no space before paren args in dynamic operator chain, 3159`` () =
+    formatSourceString
+        """
+x?a("")?b(t)
+"""
+        config
+    |> prepend newline
+    |> should
+        equal
+        """
+x?a("")?b(t)
 """
 
 [<Test>]
 let ``case determination issue with ExprAppSingleParenArgNode uppercase with config lower, 3088`` () =
-    // We want to disobey SpaceBefore(Upper|Lower)caseInvocation inside of the ? chain because mixing it up can generate invalid code like x?a("arg")?B ("barg")?c("carg")
-    // The space config that is used (Upper or Lower) depends on the case of the dynamic object, here x
+    // Space before paren args of a `?` result is never added, regardless of SpaceBefore(Upper|Lower)caseInvocation.
+    // Adding a space changes the AST when followed by another `?`, e.g. `X?a ("arg")?B`. See #3159.
     formatSourceString
         """
 let doc1 = x?a("arg")?B("barg")?c("carg")
@@ -108,7 +122,7 @@ let doc2 = X?a("arg")?B("barg")?c("carg")
         equal
         """
 let doc1 = x?a("arg")?B("barg")?c("carg")
-let doc2 = X?a ("arg")?B ("barg")?c ("carg")
+let doc2 = X?a("arg")?B("barg")?c("carg")
 """
 
 [<Test>]

src/Fantomas.Core/ASTTransformer.fs

@@ -696,6 +696,41 @@ let (|UnitExpr|_|) e =
     | SynExpr.Const(constant = SynConst.Unit) -> ValueSome e.Range
     | _ -> ValueNone
 
+/// Matches the argument of a dynamic-chain item: a paren expression (excluding lambdas)
+/// or a unit literal.
+[<return: Struct>]
+let (|DynamicChainArg|_|) (e: SynExpr) =
+    match e with
+    | SynExpr.Const(constant = SynConst.Unit) -> ValueSome e
+    | SynExpr.Paren(expr = inner) ->
+        match inner with
+        | SynExpr.Lambda _
+        | SynExpr.MatchLambda _ -> ValueNone
+        | _ -> ValueSome e
+    | _ -> ValueNone
+
+/// Walks an expression outermost-to-innermost, prepending each `?member` (and any paren arg)
+/// onto the accumulator. Because we recurse from outer to inner, the resulting list is in
+/// source order without a reversal step.
+[<TailCall>]
+let rec visitDynamicChain acc e =
+    match e with
+    | SynExpr.App(_, false, SynExpr.Dynamic(funcExpr, _, memberExpr, _), (DynamicChainArg _ as parenArg), _) ->
+        visitDynamicChain ((memberExpr, Some parenArg) :: acc) funcExpr
+    | SynExpr.Dynamic(funcExpr, _, memberExpr, _) -> visitDynamicChain ((memberExpr, None) :: acc) funcExpr
+    | _ -> e, acc
+
+/// Detects two or more consecutive `?` operator accesses (e.g. `x?a("")?b(t)`),
+/// returning the leading expression and the ordered list of items.
+/// Each item is a member expression and an optional paren/unit argument.
+[<return: Struct>]
+let (|DynamicChain|_|) (e: SynExpr) =
+    let leading, items = visitDynamicChain [] e
+
+    match items with
+    | _ :: _ :: _ -> ValueSome(leading, items)
+    | _ -> ValueNone
+
 [<return: Struct>]
 let (|ParenExpr|_|) e =
     match e with
@@ -1254,6 +1289,23 @@ let mkExpr (creationAide: CreationAide) (e: SynExpr) : Expr =
             |> Expr.BeginEnd
         else
             mkParenExpr creationAide lpr e rpr pr |> Expr.Paren
+    | DynamicChain(leading, items) ->
+        let chainItems =
+            items
+            |> List.map (fun (memberExpr, parenArg) ->
+                let memberExpr' = mkExpr creationAide memberExpr
+
+                let parenArg' = parenArg |> Option.map (mkExpr creationAide)
+
+                let itemRange =
+                    match parenArg with
+                    | Some pa -> unionRanges memberExpr.Range pa.Range
+                    | None -> memberExpr.Range
+
+                ExprDynamicChainItemNode(memberExpr', parenArg', itemRange))
+
+        ExprDynamicChainNode(mkExpr creationAide leading, chainItems, exprRange)
+        |> Expr.DynamicChain
     | SynExpr.Dynamic(funcExpr, _, argExpr, _) ->
         ExprDynamicNode(mkExpr creationAide funcExpr, mkExpr creationAide argExpr, exprRange)
         |> Expr.Dynamic

src/Fantomas.Core/CodePrinter.fs

@@ -66,6 +66,7 @@ let rec (|UppercaseExpr|LowercaseExpr|) (expr: Expr) =
     | Expr.DotIndexedGet node -> (|UppercaseExpr|LowercaseExpr|) node.ObjectExpr
     | 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
@@ -772,6 +773,18 @@ let genExpr (e: Expr) =
             | _ -> genExpr node.FuncExpr
 
         genFuncExpr +> !-"?" +> 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.
+        let genItem (item: ExprDynamicChainItemNode) =
+            !-"?"
+            +> genExpr item.MemberExpr
+            +> (match item.ParenArg with
+                | Some arg -> genExpr arg
+                | None -> sepNone)
+            |> genNode item
+
+        genExpr node.LeadingExpr +> col sepNone node.Items genItem |> genNode node
     | Expr.PrefixApp node ->
         let genWithoutSpace = genSingleTextNode node.Operator +> genExpr node.Expr
         let genWithSpace = genSingleTextNode node.Operator +> sepSpace +> genExpr node.Expr

src/Fantomas.Core/Selection.fs

@@ -219,6 +219,9 @@ let mkTreeWithSingleNode (node: Node) : TreeForSelection =
     | :? ExprDynamicNode as node ->
         let expr = Expr.Dynamic node
         mkOakFromModuleDecl (ModuleDecl.DeclExpr expr)
+    | :? ExprDynamicChainNode as node ->
+        let expr = Expr.DynamicChain node
+        mkOakFromModuleDecl (ModuleDecl.DeclExpr expr)
     | :? ExprPrefixAppNode as node ->
         let expr = Expr.PrefixApp node
         mkOakFromModuleDecl (ModuleDecl.DeclExpr expr)

src/Fantomas.Core/SyntaxOak.fs

@@ -1314,6 +1314,27 @@ type ExprDynamicNode(funcExpr: Expr, argExpr: Expr, range) =
     member val FuncExpr = funcExpr
     member val ArgExpr = argExpr
 
+/// A single `?member` (with optional paren or unit argument) inside a <see cref="ExprDynamicChainNode"/>.
+type ExprDynamicChainItemNode(memberExpr: Expr, parenArg: Expr option, range) =
+    inherit NodeBase(range)
+
+    override val Children: Node array = [| yield Expr.Node memberExpr; yield! noa (Option.map Expr.Node parenArg) |]
+
+    member val MemberExpr = memberExpr
+    member val ParenArg = parenArg
+
+/// Example: `x?a("")?b(t)` — a chain of two or more `?` operator accesses.
+/// Captured as a dedicated node so the printer can keep `?member(arg)` tight,
+/// because adding a space before the paren argument changes parsing of the
+/// following `?member`. See #3159.
+type ExprDynamicChainNode(leadingExpr: Expr, items: ExprDynamicChainItemNode list, range) =
+    inherit NodeBase(range)
+
+    override val Children: Node array = [| yield Expr.Node leadingExpr; yield! nodes items |]
+
+    member val LeadingExpr = leadingExpr
+    member val Items = items
+
 /// Example: `!x`, `-x`, `~~~x` — a prefix (unary) operator applied to an expression.
 type ExprPrefixAppNode(operator: SingleTextNode, expr: Expr, range) =
     inherit NodeBase(range)
@@ -1915,6 +1936,7 @@ type Expr =
     | ParenFunctionNameWithStar of ExprParenFunctionNameWithStarNode
     | Paren of ExprParenNode
     | Dynamic of ExprDynamicNode
+    | DynamicChain of ExprDynamicChainNode
     | PrefixApp of ExprPrefixAppNode
     | SameInfixApps of ExprSameInfixAppsNode
     | InfixApp of ExprInfixAppNode
@@ -1983,6 +2005,7 @@ type Expr =
         | ParenFunctionNameWithStar n -> n
         | Paren n -> n
         | Dynamic n -> n
+        | DynamicChain n -> n
         | PrefixApp n -> n
         | SameInfixApps n -> n
         | InfixApp n -> n