Parens: tweak sensitive indentation handling

vsintegration/src/FSharp.Editor/CodeFixes/RemoveUnnecessaryParentheses.fs

@@ -15,61 +15,98 @@ open CancellableTasks
 module private Patterns =
     let inline toPat f x = if f x then ValueSome() else ValueNone
 
-    [<return: Struct>]
-    let inline (|LetterOrDigit|_|) c = toPat Char.IsLetterOrDigit c
-
-    [<return: Struct>]
-    let inline (|Punctuation|_|) c = toPat Char.IsPunctuation c
-
-    [<return: Struct>]
-    let inline (|Symbol|_|) c = toPat Char.IsSymbol c
-
-module private SourceText =
-    /// Returns true if the given span contains an expression
-    /// whose indentation would be made invalid if the open paren
-    /// were removed (because the offside line would be shifted), e.g.,
-    ///
-    ///     // Valid.
-    ///     (let x = 2
-    ///      x)
-    ///
-    ///     // Invalid.
-    ///    ←let x = 2
-    ///      x◌
-    ///
-    ///     // Valid.
-    ///     ◌let x = 2
-    ///      x◌
-    let containsSensitiveIndentation (span: TextSpan) (sourceText: SourceText) =
-        let startLinePosition = sourceText.Lines.GetLinePosition span.Start
-        let endLinePosition = sourceText.Lines.GetLinePosition span.End
-        let startLine = startLinePosition.Line
-        let startCol = startLinePosition.Character
-        let endLine = endLinePosition.Line
-
-        if startLine = endLine then
-            false
-        else
-            let rec loop offsides lineNo startCol =
+    [<AutoOpen>]
+    module Char =
+        [<return: Struct>]
+        let inline (|LetterOrDigit|_|) c = toPat Char.IsLetterOrDigit c
+
+        [<return: Struct>]
+        let inline (|Punctuation|_|) c = toPat Char.IsPunctuation c
+
+        [<return: Struct>]
+        let inline (|Symbol|_|) c = toPat Char.IsSymbol c
+
+    [<AutoOpen>]
+    module SourceText =
+        /// Returns true if the given span contains an expression
+        /// whose indentation would be made invalid if the open paren
+        /// were removed (because the offside line would be shifted), e.g.,
+        ///
+        ///     // Valid.
+        ///     (let x = 2
+        ///      x)
+        ///
+        ///     // Invalid.
+        ///    ←let x = 2
+        ///      x◌
+        ///
+        ///     // Valid.
+        ///     ◌let x = 2
+        ///      x◌
+        let containsSensitiveIndentation (span: TextSpan) (sourceText: SourceText) =
+            let startLinePosition = sourceText.Lines.GetLinePosition span.Start
+            let endLinePosition = sourceText.Lines.GetLinePosition span.End
+            let startLine = startLinePosition.Line
+            let startCol = startLinePosition.Character
+            let endLine = endLinePosition.Line
+
+            if startLine = endLine then
+                false
+            else
+                let rec loop offsides lineNo startCol =
+                    if lineNo <= endLine then
+                        let line = sourceText.Lines[ lineNo ].ToString()
+
+                        match offsides with
+                        | ValueNone ->
+                            let i = line.AsSpan(startCol).IndexOfAnyExcept(' ', ')')
+
+                            if i >= 0 then
+                                loop (ValueSome(i + startCol)) (lineNo + 1) 0
+                            else
+                                loop offsides (lineNo + 1) 0
+
+                        | ValueSome offsidesCol ->
+                            let i = line.AsSpan(0, min offsidesCol line.Length).IndexOfAnyExcept(' ', ')')
+                            i <= offsidesCol || loop offsides (lineNo + 1) 0
+                    else
+                        false
+
+                loop ValueNone startLine startCol
+
+        let hasPrecedingConstructOnSameLine (span: TextSpan) (sourceText: SourceText) =
+            let linePosition = sourceText.Lines.GetLinePosition span.Start
+            let line = (sourceText.Lines.GetLineFromPosition span.Start).ToString()
+            line.AsSpan(0, linePosition.Character).LastIndexOfAnyExcept(' ', '(') >= 0
+
+        let followingLineMovesOffsidesRightward (span: TextSpan) (sourceText: SourceText) =
+            let startLinePosition = sourceText.Lines.GetLinePosition span.Start
+            let startLine = startLinePosition.Line
+            let endLinePosition = sourceText.Lines.GetLinePosition span.End
+            let endLine = endLinePosition.Line
+            let offsides = startLinePosition.Character
+
+            let rec loop lineNo =
                 if lineNo <= endLine then
-                    let line = sourceText.Lines[ lineNo ].ToString()
+                    let line = sourceText.Lines[ lineNo ].ToString().AsSpan()
+                    let i = line.IndexOfAnyExcept("*/%-+:^@><=!|0$.?) ".AsSpan())
+                    i > offsides || loop (lineNo + 1)
+                else
+                    false
 
-                    match offsides with
-                    | ValueNone ->
-                        let i = line.AsSpan(startCol).IndexOfAnyExcept(' ', ')')
+            loop (startLine + 1)
 
-                        if i >= 0 then
-                            loop (ValueSome(i + startCol)) (lineNo + 1) 0
-                        else
-                            loop offsides (lineNo + 1) 0
+        [<return: Struct>]
+        let (|ContainsSensitiveIndentation|_|) span sourceText =
+            toPat (containsSensitiveIndentation span) sourceText
 
-                    | ValueSome offsidesCol ->
-                        let i = line.AsSpan(0, min offsidesCol line.Length).IndexOfAnyExcept(' ', ')')
-                        i <= offsidesCol || loop offsides (lineNo + 1) 0
-                else
-                    false
+        [<return: Struct>]
+        let (|HasPrecedingConstructOnSameLine|_|) span sourceText =
+            toPat (hasPrecedingConstructOnSameLine span) sourceText
 
-            loop ValueNone startLine startCol
+        [<return: Struct>]
+        let (|FollowingLineMovesOffsidesRightward|_|) span sourceText =
+            toPat (followingLineMovesOffsidesRightward span) sourceText
 
 [<ExportCodeFixProvider(FSharpConstants.FSharpLanguageName, Name = CodeFix.RemoveUnnecessaryParentheses); Shared; Sealed>]
 type internal FSharpRemoveUnnecessaryParenthesesCodeFixProvider [<ImportingConstructor>] () =
@@ -130,7 +167,6 @@ type internal FSharpRemoveUnnecessaryParenthesesCodeFixProvider [<ImportingConst
                         | _, LetterOrDigit, '(' -> None
                         | _, (LetterOrDigit | '`'), _ -> Some ShouldPutSpaceBefore
                         | _, (Punctuation | Symbol), (Punctuation | Symbol) -> Some ShouldPutSpaceBefore
-                        | _ when SourceText.containsSensitiveIndentation context.Span sourceText -> Some ShouldPutSpaceBefore
                         | _ -> None
 
                     let (|ShouldPutSpaceAfter|_|) (s: string) =
@@ -142,11 +178,24 @@ type internal FSharpRemoveUnnecessaryParenthesesCodeFixProvider [<ImportingConst
                         | LetterOrDigit, LetterOrDigit -> Some ShouldPutSpaceAfter
                         | _ -> None
 
+                    let (|NewOffsidesOnFirstLine|_|) (s: string) =
+                        let s = s.AsSpan 1 // (…
+                        let newline = s.IndexOfAny('\n', '\r')
+
+                        if newline < 0 || s.Slice(0, newline).IndexOfAnyExcept(@"\r\n ".AsSpan()) >= 0 then
+                            Some NewOffsidesOnFirstLine
+                        else
+                            None
+
                     let newText =
-                        match txt with
-                        | ShouldPutSpaceBefore & ShouldPutSpaceAfter -> " " + txt[1 .. txt.Length - 2] + " "
-                        | ShouldPutSpaceBefore -> " " + txt[1 .. txt.Length - 2]
-                        | ShouldPutSpaceAfter -> txt[1 .. txt.Length - 2] + " "
+                        match txt, sourceText with
+                        | ShouldPutSpaceBefore & ShouldPutSpaceAfter, _ -> " " + txt[1 .. txt.Length - 2] + " "
+                        | ShouldPutSpaceBefore, _ -> " " + txt[1 .. txt.Length - 2]
+                        | ShouldPutSpaceAfter, _ -> txt[1 .. txt.Length - 2] + " "
+                        | NewOffsidesOnFirstLine,
+                          ContainsSensitiveIndentation context.Span & (HasPrecedingConstructOnSameLine context.Span | FollowingLineMovesOffsidesRightward context.Span) ->
+                            txt[ 1 .. txt.Length - 2 ].Replace("\n ", "\n")
+                        | NewOffsidesOnFirstLine, ContainsSensitiveIndentation context.Span -> " " + txt[1 .. txt.Length - 2]
                         | _ -> txt[1 .. txt.Length - 2]
 
                     return

vsintegration/src/FSharp.Editor/Common/Extensions.fs

@@ -578,6 +578,16 @@ open System.Runtime.CompilerServices
 
 [<Sealed; AbstractClass; Extension>]
 type ReadOnlySpanExtensions =
+    [<Extension>]
+    static member IndexOfAnyExcept(span: ReadOnlySpan<char>, value: char) =
+        let mutable i = 0
+        let mutable found = false
+
+        while not found && i < span.Length do
+            if span[i] <> value then found <- true else i <- i + 1
+
+        if found then i else -1
+
     [<Extension>]
     static member IndexOfAnyExcept(span: ReadOnlySpan<char>, value0: char, value1: char) =
         let mutable i = 0
@@ -592,4 +602,32 @@ type ReadOnlySpanExtensions =
                 i <- i + 1
 
         if found then i else -1
+
+    [<Extension>]
+    static member IndexOfAnyExcept(span: ReadOnlySpan<char>, values: ReadOnlySpan<char>) =
+        let mutable i = 0
+        let mutable found = false
+
+        while not found && i < span.Length do
+            if values.IndexOf span[i] < 0 then
+                found <- true
+            else
+                i <- i + 1
+
+        if found then i else -1
+
+    [<Extension>]
+    static member LastIndexOfAnyExcept(span: ReadOnlySpan<char>, value0: char, value1: char) =
+        let mutable i = span.Length - 1
+        let mutable found = false
+
+        while not found && i >= 0 do
+            let c = span[i]
+
+            if c <> value0 && c <> value1 then
+                found <- true
+            else
+                i <- i - 1
+
+        if found then i else -1
 #endif

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

@@ -209,7 +209,7 @@ module Xunit =
         let actual = split actual
 
         try
-            Assert.All(Array.zip expected actual, (fun (expected, actual) -> Assert.Equal(expected, actual)))
+            Assert.All((expected, actual) ||> Array.zip |> Array.rev, (fun (expected, actual) -> Assert.Equal(expected, actual)))
         with :? Xunit.Sdk.AllException as all when all.Failures.Count = 1 ->
             raise all.Failures[0]