fix(all): eliminate dead bindings left by CE/inline lowering

Author: MangelMaxime

src/Fable.Transforms/Babel/BabelPrinter.fs

@@ -351,6 +351,25 @@ module PrinterExtensions =
             printer.Print(expr)
             printer.Print(")")
 
+        /// True when the expression's leftmost token would otherwise be parsed as the start of
+        /// a block/function/class declaration, e.g. `{ A: 1 };` parses as an invalid block.
+        member printer.NeedsParensAsExpressionStatement(expr: Expression) =
+            match expr with
+            | CommentedExpression(_, e) -> printer.NeedsParensAsExpressionStatement(e)
+            | ObjectExpression _
+            | ClassExpression _
+            | FunctionExpression _ -> true
+            | SequenceExpression(exprs, _) when exprs.Length > 0 -> printer.NeedsParensAsExpressionStatement(exprs[0])
+            | BinaryExpression(left, _, _, _)
+            | LogicalExpression(left, _, _, _)
+            | AssignmentExpression(left, _, _, _) -> printer.NeedsParensAsExpressionStatement(left)
+            | ConditionalExpression(test, _, _, _) -> printer.NeedsParensAsExpressionStatement(test)
+            | MemberExpression(object, _, _, _) -> printer.NeedsParensAsExpressionStatement(object)
+            | CallExpression(callee, _, _, _) -> printer.NeedsParensAsExpressionStatement(callee)
+            | UpdateExpression(false, argument, _, _) -> printer.NeedsParensAsExpressionStatement(argument)
+            | AsExpression(e, _) -> printer.NeedsParensAsExpressionStatement(e)
+            | _ -> false
+
         /// Should the expression be printed with parens when nested?
         member printer.IsComplex(expr: Expression) =
             match expr with
@@ -642,9 +661,15 @@ module PrinterExtensions =
                 printer.PrintOptional(label, " ")
 
             | ExpressionStatement(expr) ->
-                match expr with
-                | UnaryExpression(argument, "void", false, _loc) -> printer.Print(argument)
-                | _ -> printer.Print(expr)
+                let expr =
+                    match expr with
+                    | UnaryExpression(argument, "void", false, _loc) -> argument
+                    | expr -> expr
+
+                if printer.NeedsParensAsExpressionStatement(expr) then
+                    printer.WithParens(expr)
+                else
+                    printer.Print(expr)
 
         member printer.PrintJsDoc(doc: string option) =
             match doc with

src/Fable.Transforms/FableTransforms.fs

@@ -441,6 +441,18 @@ module private Transforms =
             (not com.Options.DebugMode) || ident.IsCompilerGenerated
 
         match e with
+        | Let(ident, value, letBody) when
+            (not ident.IsMutable)
+            && isErasingCandidate ident
+            && countReferencesUntil 1 ident.Name letBody = 0
+            && canHaveSideEffects com value
+            ->
+            // The binding is never read but its value may have side effects (e.g. residue from
+            // inlining CE builder methods like `Combine`/`Run` that discard their argument).
+            // Keep evaluating it for its effects, but drop the now-useless named binding.
+            match letBody with
+            | Sequential exprs -> Sequential(value :: exprs)
+            | letBody -> Sequential [ value; letBody ]
         | Let(ident, value, letBody) when (not ident.IsMutable) && isErasingCandidate ident ->
             match tryInlineBinding com ident value letBody with
             | Some(ident, value) ->

src/Fable.Transforms/Rust/Fable2Rust.fs

@@ -2793,6 +2793,20 @@ module Util =
         let expr = transformLeaveContext com ctx None e
         mkExprStmt expr
 
+    // Only the block's true tail can omit the trailing `;`. A block-like expression (`if`,
+    // `match`, nested `{ }`) in a non-tail statement needs it forced via `Semi`, or a
+    // non-`()`-typed value there is a Rust type error, not just a style choice.
+    let transformExprsAsStmts (com: IRustCompiler) ctx (exprs: Fable.Expr list) : Rust.Stmt list =
+        match List.rev exprs with
+        | [] -> []
+        | last :: revRest ->
+            let nonTailStmts =
+                revRest
+                |> List.rev
+                |> List.map (fun e -> transformLeaveContext com ctx None e |> mkSemiStmt)
+
+            nonTailStmts @ [ transformAsStmt com ctx last ]
+
     // flatten nested Let binding expressions
     let rec flattenLet acc (expr: Fable.Expr) =
         match expr with
@@ -2954,13 +2968,13 @@ module Util =
             match body with
             | Fable.Sequential exprs ->
                 let exprs = flattenSequential body
-                List.map (transformAsStmt com ctx) exprs
+                transformExprsAsStmts com ctx exprs
             | _ -> [ transformAsStmt com ctx body ]
 
         letStmts @ bodyStmts |> mkStmtBlockExpr
 
     let transformSequential (com: IRustCompiler) ctx exprs =
-        exprs |> List.map (transformAsStmt com ctx) |> mkStmtBlockExpr
+        exprs |> transformExprsAsStmts com ctx |> mkStmtBlockExpr
 
     let transformIfThenElse (com: IRustCompiler) ctx range guard thenBody elseBody =
         // transform null checks for nullable value types

tests/Js/Main/InlineIfLambdaTests.fs

@@ -42,6 +42,20 @@ type TwiceBuilder() =
 
 let twice = TwiceBuilder()
 
+// ── CE builder whose members discard their argument ──────────────────────────
+//
+// All members are `inline` and return `()` regardless of their argument, so the
+// inlined argument bindings are never read. `bindingBetaReduction` must turn
+// those into plain sequential evaluation instead of leaving a dead `let` behind.
+type DiscardBuilder() =
+    member inline _.Yield(_: unit) = ()
+    member inline _.Combine(_: unit, _: unit) = ()
+    member inline _.Delay([<InlineIfLambda>] f: unit -> unit) = f ()
+    member inline _.Zero() = ()
+    member inline _.Run(_: unit) = ()
+
+let discard = DiscardBuilder()
+
 // ── tests ────────────────────────────────────────────────────────────────────
 
 let tests =
@@ -115,4 +129,13 @@ let tests =
             callCount |> equal 2
             result   |> equal 2
 
+        testCase "Inline CE builder whose members discard their argument still runs body once, in order" <| fun () ->
+            let mutable log = []
+            let addLog x = log <- x :: log
+            discard {
+                addLog "a"
+                addLog "b"
+            }
+            log |> List.rev |> equal [ "a"; "b" ]
+
     ]

tests/Rust/tests/src/InlineIfLambdaTests.fs

@@ -34,6 +34,23 @@ type TwiceBuilder() =
 
 let twice = TwiceBuilder()
 
+type DiscardBuilder() =
+    member inline _.Yield(_: unit) = ()
+    member inline _.Combine(_: unit, _: unit) = ()
+    member inline _.Delay([<InlineIfLambda>] f: unit -> unit) = f ()
+    member inline _.Zero() = ()
+    member inline _.Run(_: unit) = ()
+
+let discard = DiscardBuilder()
+
+// Inlining this at a discarded call site keeps `r` as a real `let` (referenced
+// twice), so the discarded value compiles to a Rust block ending in a non-`()`
+// tail expression - the shape that needs a forced `;` to stay valid Rust.
+let inline makeAndFill () =
+    let r = ResizeArray()
+    r.Add(1)
+    r
+
 // ── tests ──────────────────────────────────────────────────────────────────────
 
 [<Fact>]
@@ -102,3 +119,18 @@ let ``InlineIfLambda on CE builder Delay inlines body at each call site`` () =
         }
     callCount |> equal 2
     result |> equal 2
+
+[<Fact>]
+let ``Inline CE builder whose members discard their argument still runs body once, in order`` () =
+    let mutable log = []
+    let addLog x = log <- x :: log
+    discard {
+        addLog "a"
+        addLog "b"
+    }
+    log |> List.rev |> equal [ "a"; "b" ]
+
+[<Fact>]
+let ``Discarding a value that compiles to a Rust block does not corrupt codegen`` () =
+    makeAndFill () |> ignore
+    true |> equal true