Make matched brace highlighting work exactly as in C# editor (#5049)

* make matched brace highlighting work exactly as in C# editor * fix tests * Add optional param for formatting * revert matching braces logic in FSharpEditorFormattingService * add missing test attributes * Revert "revert matching braces logic in FSharpEditorFormattingService" This reverts commit 5ddaba3.
dotnet · Jun 5, 2018 · e6c2a38 · e6c2a38
1 parent 86661b6
commit e6c2a38
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 22 deletions.
diff --git a/vsintegration/src/FSharp.Editor/Formatting/BraceMatchingService.fs b/vsintegration/src/FSharp.Editor/Formatting/BraceMatchingService.fs
@@ -2,10 +2,10 @@
 
 namespace Microsoft.VisualStudio.FSharp.Editor
 
-open System
 open System.ComponentModel.Composition
 open Microsoft.CodeAnalysis.Editor
 open Microsoft.FSharp.Compiler.SourceCodeServices
+open System.Runtime.InteropServices
 
 [<ExportBraceMatcher(FSharpConstants.FSharpLanguageName)>]
 type internal FSharpBraceMatchingService 
@@ -14,19 +14,21 @@ type internal FSharpBraceMatchingService
         checkerProvider: FSharpCheckerProvider,
         projectInfoManager: FSharpProjectOptionsManager
     ) =
-
 
     static let defaultUserOpName = "BraceMatching"
 
-    static member GetBraceMatchingResult(checker: FSharpChecker, sourceText, fileName, parsingOptions: FSharpParsingOptions, position: int, userOpName: string) = 
+    static member GetBraceMatchingResult(checker: FSharpChecker, sourceText, fileName, parsingOptions: FSharpParsingOptions, position: int, userOpName: string, [<Optional; DefaultParameterValue(false)>] forFormatting: bool) = 
         async {
             let! matchedBraces = checker.MatchBraces(fileName, sourceText.ToString(), parsingOptions, userOpName)
             let isPositionInRange range = 
                 match RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, range) with
                 | None -> false
-                | Some range ->
-                    let length = position - range.Start
-                    length >= 0 && length <= range.Length
+                | Some span ->
+                    if forFormatting then
+                        let length = position - span.Start
+                        length >= 0 && length <= span.Length
+                    else
+                        span.Contains position
             return matchedBraces |> Array.tryFind(fun (left, right) -> isPositionInRange left || isPositionInRange right)
         }
 

diff --git a/vsintegration/src/FSharp.Editor/Formatting/EditorFormattingService.fs b/vsintegration/src/FSharp.Editor/Formatting/EditorFormattingService.fs
@@ -50,7 +50,7 @@ type internal FSharpEditorFormattingService
                     x.Tag <> FSharpTokenTag.LINE_COMMENT)
 
             let! (left, right) =
-                FSharpBraceMatchingService.GetBraceMatchingResult(checker, sourceText, filePath, parsingOptions, position, "FormattingService")
+                FSharpBraceMatchingService.GetBraceMatchingResult(checker, sourceText, filePath, parsingOptions, position, "FormattingService", forFormatting=true)
 
             if right.StartColumn = firstMeaningfulToken.LeftColumn then
                 // Replace the indentation on this line with the indentation of the left bracket

diff --git a/vsintegration/tests/UnitTests/BraceMatchingServiceTests.fs b/vsintegration/tests/UnitTests/BraceMatchingServiceTests.fs
@@ -161,26 +161,23 @@ let main argv =
     0 // return an integer exit code"""
         this.VerifyBraceMatch(code, "(printfn", ")endBrace")
 
-    [<TestCase ("let a1 = [ 0 .. 100 ]", [|9;10;20;21|])>]
-    [<TestCase ("let a2 = [| 0 .. 100 |]", [|9;10;11;21;22;23|])>]
-    [<TestCase ("let a3 = <@ 0 @>", [|9;10;11;14;15;16|])>]
-    [<TestCase ("let a4 = <@@ 0 @@>", [|9;10;11;12;15;15;16;17|])>]
-    [<TestCase ("let a6 = (  ()  )", [|9;10;16;17|])>]
-    [<TestCase ("[<ReflectedDefinition>]\nlet a7 = 70", [|0;1;2;21;22;23|])>]
-    [<TestCase ("let a8 = seq { yield() }", [|13;14;23;24|])>]
-    member this.BraceMatchingBothSides_Bug2092(fileContents: string, matchingPositions: int[]) =
-        // https://github.com/Microsoft/visualfsharp/issues/2092
+    [<TestCase ("let a1 = [ 0 .. 100 ]", [|9;20|])>]
+    [<TestCase ("let a2 = [| 0 .. 100 |]", [|9;10;22|])>]
+    [<TestCase ("let a3 = <@ 0 @>", [|9;10;15|])>]
+    [<TestCase ("let a4 = <@@ 0 @@>", [|9;10;11;15;16|])>]
+    [<TestCase ("let a6 = (  ()  )", [|9;16|])>]
+    [<TestCase ("[<ReflectedDefinition>]\nlet a7 = 70", [|0;1;22|])>]
+    [<TestCase ("let a8 = seq { yield() }", [|13;23|])>]
+    member this.DoNotMatchOnInnerSide(fileContents: string, matchingPositions: int[]) =
         let sourceText = SourceText.From(fileContents)
-
         let parsingOptions, _ = checker.GetParsingOptionsFromProjectOptions projectOptions
-        matchingPositions
-        |> Array.iter (fun position ->
+
+        for position in matchingPositions do
             match FSharpBraceMatchingService.GetBraceMatchingResult(checker, sourceText, fileName, parsingOptions, position, "UnitTest") |> Async.RunSynchronously with
             | Some _ -> ()
             | None ->
                 match position with
                 | 0 -> ""
                 | _ -> fileContents.[position - 1] |> sprintf " (previous character '%c')"
-                |> sprintf "Didn't find a matching brace at position '%d', character '%c'%s" position fileContents.[position]
+                |> sprintf "Didn't find a matching brace at position '%d' %s" position
                 |> Assert.Fail
-            )
diff --git a/vsintegration/tests/UnitTests/IndentationServiceTests.fs b/vsintegration/tests/UnitTests/IndentationServiceTests.fs
@@ -170,6 +170,7 @@ while true do
         |> Array.map (fun (lineNumber, expectedIndentation) ->
             ( Some(expectedIndentation), lineNumber, autoIndentTemplate ))
 
+    [<Test>]
     member this.TestIndentation() = 
         for (expectedIndentation, lineNumber, template) in testCases do 
             let sourceText = SourceText.From(template)
@@ -179,7 +180,8 @@ while true do
             match expectedIndentation with
             | None -> Assert.IsTrue(actualIndentation.IsNone, "No indentation was expected at line {0}", lineNumber)
             | Some indentation -> Assert.AreEqual(expectedIndentation.Value, actualIndentation.Value, "Indentation on line {0} doesn't match", lineNumber)
-
+
+    [<Test>]    
     member this.TestAutoIndentation() = 
         for (expectedIndentation, lineNumber, template) in autoIndentTestCases do