Skip to content

Commit bba8e2a

Browse files
Keep parens when removal would result in shadowing
1 parent 6071d00 commit bba8e2a

File tree

2 files changed

+103
-26
lines changed

2 files changed

+103
-26
lines changed

src/Compiler/Service/SynExpr.fs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -812,6 +812,33 @@ module SynExpr =
812812
->
813813
true
814814

815+
// Keep parens if a name in the outer scope is rebound
816+
// in the inner scope and would shadow the outer binding
817+
// if the parens were removed, e.g.:
818+
//
819+
// let x = 3
820+
// (
821+
// let x = 4
822+
// printfn $"{x}"
823+
// )
824+
// x
825+
| SynExpr.Sequential(expr1 = SynExpr.Paren(expr = Is inner); expr2 = expr2), _ ->
826+
let identsBoundInInner =
827+
(Set.empty, [ SyntaxNode.SynExpr inner ])
828+
||> SyntaxNodes.fold (fun idents _path node ->
829+
match node with
830+
| SyntaxNode.SynPat(SynPat.Named(ident = SynIdent(ident = ident))) -> idents.Add ident.idText
831+
| _ -> idents)
832+
833+
if identsBoundInInner.IsEmpty then
834+
false
835+
else
836+
(outer.Range.End, [ SyntaxNode.SynExpr expr2 ])
837+
||> SyntaxNodes.exists (fun _path node ->
838+
match node with
839+
| SyntaxNode.SynExpr(SynExpr.Ident ident) -> identsBoundInInner.Contains ident.idText
840+
| _ -> false)
841+
815842
| SynExpr.InterpolatedString _, SynExpr.Sequential _ -> true
816843

817844
| SynExpr.InterpolatedString(contents = contents), (SynExpr.Tuple(isStruct = false) | Dangling.Problematic _) ->

vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveUnnecessaryParenthesesTests.fs

Lines changed: 76 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -618,32 +618,6 @@ in x
618618
y - x
619619
"
620620

621-
(*
622-
Bug: removing parens shadows `y` in `x + y`.
623-
To address this, we'd need to ensure that any name bound in the inner scope
624-
is never used again in the outer scope.
625-
*)
626-
//"
627-
//let _ =
628-
// let y = 100
629-
// let x = 3
630-
// (
631-
// let y = 99
632-
// ignore (y - x)
633-
// )
634-
// x + y
635-
//",
636-
//"
637-
//let _ =
638-
// let y = 100
639-
// let x = 3
640-
// (
641-
// let y = 99
642-
// ignore (y - x)
643-
// )
644-
// x + y
645-
//"
646-
647621
// TryWith
648622
"try (raise null) with _ -> reraise ()", "try raise null with _ -> reraise ()"
649623
"try raise null with (_) -> reraise ()", "try raise null with _ -> reraise ()"
@@ -670,6 +644,82 @@ in x
670644
""" printfn "1"; (printfn "2") """, """ printfn "1"; printfn "2" """
671645
"let x = 3; (5) in x", "let x = 3; 5 in x"
672646

647+
"
648+
let _ =
649+
let y = 100
650+
let x = 3
651+
(
652+
let y = 99
653+
ignore (y - x)
654+
)
655+
x + y
656+
",
657+
"
658+
let _ =
659+
let y = 100
660+
let x = 3
661+
(
662+
let y = 99
663+
ignore (y - x)
664+
)
665+
x + y
666+
"
667+
668+
"
669+
let _ =
670+
let y = 100
671+
let x = 3
672+
(
673+
let y = 99
674+
ignore (y - x)
675+
)
676+
x
677+
",
678+
"
679+
let _ =
680+
let y = 100
681+
let x = 3
682+
let y = 99
683+
ignore (y - x)
684+
x
685+
"
686+
687+
"
688+
let f y =
689+
let x = 3
690+
(
691+
let y = 99
692+
ignore (y - x)
693+
)
694+
x + y
695+
",
696+
"
697+
let f y =
698+
let x = 3
699+
(
700+
let y = 99
701+
ignore (y - x)
702+
)
703+
x + y
704+
"
705+
706+
"
707+
let f y =
708+
let x = 3
709+
(
710+
let y = 99
711+
ignore (y - x)
712+
)
713+
x
714+
",
715+
"
716+
let f y =
717+
let x = 3
718+
let y = 99
719+
ignore (y - x)
720+
x
721+
"
722+
673723
// IfThenElse
674724
"if (3 = 3) then 3 else 3", "if 3 = 3 then 3 else 3"
675725
"if 3 = 3 then (3) else 3", "if 3 = 3 then 3 else 3"

0 commit comments

Comments
 (0)