From bba8e2a0a15ac7fa1ab38d791dbf2d85c919ee05 Mon Sep 17 00:00:00 2001
From: Brian Rourke Boll <brianrourkeboll@gmail.com>
Date: Wed, 7 Feb 2024 12:00:44 -0500
Subject: [PATCH] Keep parens when removal would result in shadowing

---
 src/Compiler/Service/SynExpr.fs               |  27 +++++
 .../RemoveUnnecessaryParenthesesTests.fs      | 102 +++++++++++++-----
 2 files changed, 103 insertions(+), 26 deletions(-)

diff --git a/src/Compiler/Service/SynExpr.fs b/src/Compiler/Service/SynExpr.fs
index 8ca6dcf435f..a28d8f9ce13 100644
--- a/src/Compiler/Service/SynExpr.fs
+++ b/src/Compiler/Service/SynExpr.fs
@@ -812,6 +812,33 @@ module SynExpr =
                 ->
                 true
 
+            // Keep parens if a name in the outer scope is rebound
+            // in the inner scope and would shadow the outer binding
+            // if the parens were removed, e.g.:
+            //
+            //     let x = 3
+            //     (
+            //         let x = 4
+            //         printfn $"{x}"
+            //     )
+            //     x
+            | SynExpr.Sequential(expr1 = SynExpr.Paren(expr = Is inner); expr2 = expr2), _ ->
+                let identsBoundInInner =
+                    (Set.empty, [ SyntaxNode.SynExpr inner ])
+                    ||> SyntaxNodes.fold (fun idents _path node ->
+                        match node with
+                        | SyntaxNode.SynPat(SynPat.Named(ident = SynIdent(ident = ident))) -> idents.Add ident.idText
+                        | _ -> idents)
+
+                if identsBoundInInner.IsEmpty then
+                    false
+                else
+                    (outer.Range.End, [ SyntaxNode.SynExpr expr2 ])
+                    ||> SyntaxNodes.exists (fun _path node ->
+                        match node with
+                        | SyntaxNode.SynExpr(SynExpr.Ident ident) -> identsBoundInInner.Contains ident.idText
+                        | _ -> false)
+
             | SynExpr.InterpolatedString _, SynExpr.Sequential _ -> true
 
             | SynExpr.InterpolatedString(contents = contents), (SynExpr.Tuple(isStruct = false) | Dangling.Problematic _) ->
diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveUnnecessaryParenthesesTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveUnnecessaryParenthesesTests.fs
index b49000c09da..ff57ffac6d8 100644
--- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveUnnecessaryParenthesesTests.fs
+++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveUnnecessaryParenthesesTests.fs
@@ -618,32 +618,6 @@ in x
                 y - x
             "
 
-            (*
-            Bug: removing parens shadows `y` in `x + y`.
-            To address this, we'd need to ensure that any name bound in the inner scope
-            is never used again in the outer scope.
-            *)
-            //"
-            //let _ =
-            //    let y = 100
-            //    let x = 3
-            //    (
-            //        let y = 99
-            //        ignore (y - x)
-            //    )
-            //    x + y
-            //",
-            //"
-            //let _ =
-            //    let y = 100
-            //    let x = 3
-            //    (
-            //        let y = 99
-            //        ignore (y - x)
-            //    )
-            //    x + y
-            //"
-
             // TryWith
             "try (raise null) with _ -> reraise ()", "try raise null with _ -> reraise ()"
             "try raise null with (_) -> reraise ()", "try raise null with _ -> reraise ()"
@@ -670,6 +644,82 @@ in x
             """ printfn "1"; (printfn "2") """, """ printfn "1"; printfn "2" """
             "let x = 3; (5) in x", "let x = 3; 5 in x"
 
+            "
+            let _ =
+                let y = 100
+                let x = 3
+                (
+                    let y = 99
+                    ignore (y - x)
+                )
+                x + y
+            ",
+            "
+            let _ =
+                let y = 100
+                let x = 3
+                (
+                    let y = 99
+                    ignore (y - x)
+                )
+                x + y
+            "
+
+            "
+            let _ =
+                let y = 100
+                let x = 3
+                (
+                    let y = 99
+                    ignore (y - x)
+                )
+                x
+            ",
+            "
+            let _ =
+                let y = 100
+                let x = 3
+                let y = 99
+                ignore (y - x)
+                x
+            "
+
+            "
+            let f y =
+                let x = 3
+                (
+                    let y = 99
+                    ignore (y - x)
+                )
+                x + y
+            ",
+            "
+            let f y =
+                let x = 3
+                (
+                    let y = 99
+                    ignore (y - x)
+                )
+                x + y
+            "
+
+            "
+            let f y =
+                let x = 3
+                (
+                    let y = 99
+                    ignore (y - x)
+                )
+                x
+            ",
+            "
+            let f y =
+                let x = 3
+                let y = 99
+                ignore (y - x)
+                x
+            "
+
             // IfThenElse
             "if (3 = 3) then 3 else 3", "if 3 = 3 then 3 else 3"
             "if 3 = 3 then (3) else 3", "if 3 = 3 then 3 else 3"