Fix completion with backticks, underscores, numbers (#10500)

vsintegration/src/FSharp.Editor/Completion/CompletionService.fs

@@ -43,6 +43,13 @@ type internal FSharpCompletionService
             .WithDismissIfLastCharacterDeleted(true)
             .WithDefaultEnterKeyRule(enterKeyRule)
 
+    /// Indicates the text span to be replaced by a committed completion list item.
+    override _.GetDefaultCompletionListSpan(sourceText, caretIndex) =
+        let documentId = workspace.GetDocumentIdInCurrentContext(sourceText.Container)
+        let document = workspace.CurrentSolution.GetDocument(documentId)
+        let defines = projectInfoManager.GetCompilationDefinesForEditingDocument(document)
+        CompletionUtils.getDefaultCompletionListSpan(sourceText, caretIndex, documentId, document.FilePath, defines)
+
 [<Shared>]
 [<ExportLanguageServiceFactory(typeof<CompletionService>, FSharpConstants.FSharpLanguageName)>]
 type internal FSharpCompletionServiceFactory 

vsintegration/src/FSharp.Editor/Completion/CompletionUtils.fs

@@ -11,6 +11,7 @@ open Microsoft.CodeAnalysis.Completion
 open Microsoft.CodeAnalysis.ExternalAccess.FSharp.Completion
 open System.Globalization
 open FSharp.Compiler.SourceCodeServices
+open FSharp.Compiler.PrettyNaming
 
 module internal CompletionUtils =
 
@@ -112,4 +113,59 @@ module internal CompletionUtils =
         | CompletionItemKind.Event -> 4
         | CompletionItemKind.Argument -> 5
         | CompletionItemKind.Other -> 6
-        | CompletionItemKind.Method (isExtension = true) -> 7
+        | CompletionItemKind.Method (isExtension = true) -> 7
+
+    /// Indicates the text span to be replaced by a committed completion list item.
+    let getDefaultCompletionListSpan(sourceText: SourceText, caretIndex, documentId, filePath, defines) =
+
+        // Gets connected identifier-part characters backward and forward from caret.
+        let getIdentifierChars() =
+            let mutable startIndex = caretIndex
+            let mutable endIndex = caretIndex
+            while startIndex > 0 && IsIdentifierPartCharacter sourceText.[startIndex - 1] do startIndex <- startIndex - 1
+            if startIndex <> caretIndex then
+                while endIndex < sourceText.Length && IsIdentifierPartCharacter sourceText.[endIndex] do endIndex <- endIndex + 1
+            TextSpan.FromBounds(startIndex, endIndex)
+
+        let line = sourceText.Lines.GetLineFromPosition(caretIndex)
+        if line.ToString().IndexOf "``" < 0 then
+            // No backticks on the line, capture standard identifier chars.
+            getIdentifierChars()
+        else
+            // Line contains backticks.
+            // Use tokenizer to check for identifier, in order to correctly handle extraneous backticks in comments, strings, etc.
+
+            // If caret is at a backtick-identifier, then that is our span.
+
+            // Else, check if we are after an unclosed ``, to support the common case of a manually typed leading ``.
+            // Tokenizer will not consider this an identifier, it will consider the bare `` a Keyword, followed by
+            // arbitrary tokens (Identifier, Operator, Text, etc.) depending on the trailing text.
+
+            // Else, backticks are not involved in caret location, fall back to standard identifier character scan.
+
+            // There may still be edge cases where backtick related spans are incorrectly captured, such as unclosed
+            // backticks before later valid backticks on a line, this is an acceptable compromise in order to support
+            // the majority of common cases.
+
+            let classifiedSpans = Tokenizer.getClassifiedSpans(documentId, sourceText, line.Span, Some filePath, defines, CancellationToken.None)
+
+            let isBacktickIdentifier (classifiedSpan: ClassifiedSpan) =
+                classifiedSpan.ClassificationType = ClassificationTypeNames.Identifier
+                    && Tokenizer.isDoubleBacktickIdent (sourceText.ToString(classifiedSpan.TextSpan))
+            let isUnclosedBacktick (classifiedSpan: ClassifiedSpan) =
+                classifiedSpan.ClassificationType = ClassificationTypeNames.Keyword
+                    && sourceText.ToString(classifiedSpan.TextSpan) = "``"
+
+            match classifiedSpans |> Seq.tryFind (fun cs -> isBacktickIdentifier cs && cs.TextSpan.IntersectsWith caretIndex) with
+            | Some backtickIdentifier ->
+                // Backtick enclosed identifier found intersecting with caret, use its span.
+                backtickIdentifier.TextSpan
+            | _ ->
+                match classifiedSpans |> Seq.tryFindBack (fun cs -> isUnclosedBacktick cs && caretIndex >= cs.TextSpan.Start) with
+                | Some unclosedBacktick ->
+                    // Unclosed backtick found before caret, use span from backtick to end of line.
+                    let lastSpan = classifiedSpans.[classifiedSpans.Count - 1]
+                    TextSpan.FromBounds(unclosedBacktick.TextSpan.Start, lastSpan.TextSpan.End)
+                | _ ->
+                    // No backticks involved at caret position, fall back to standard identifier chars.
+                    getIdentifierChars()

vsintegration/src/FSharp.Editor/LanguageService/Tokenizer.fs

@@ -1,4 +1,4 @@
-namespace Microsoft.VisualStudio.FSharp.Editor
+namespace Microsoft.VisualStudio.FSharp.Editor
 
 open System
 open System.Collections.Generic
@@ -805,15 +805,16 @@ module internal Tokenizer =
             | -1 | 0 -> span
             | index -> TextSpan(span.Start + index + 1, text.Length - index - 1)
 
+    let private doubleBackTickDelimiter = "``"
+
+    let isDoubleBacktickIdent (s: string) =
+        let doubledDelimiter = 2 * doubleBackTickDelimiter.Length
+        if s.Length > doubledDelimiter && s.StartsWith(doubleBackTickDelimiter, StringComparison.Ordinal) && s.EndsWith(doubleBackTickDelimiter, StringComparison.Ordinal) then
+            let inner = s.AsSpan(doubleBackTickDelimiter.Length, s.Length - doubledDelimiter)
+            not (inner.Contains(doubleBackTickDelimiter.AsSpan(), StringComparison.Ordinal))
+        else false
+
     let isValidNameForSymbol (lexerSymbolKind: LexerSymbolKind, symbol: FSharpSymbol, name: string) : bool =
-        let doubleBackTickDelimiter = "``"
-
-        let isDoubleBacktickIdent (s: string) =
-            let doubledDelimiter = 2 * doubleBackTickDelimiter.Length
-            if s.Length > doubledDelimiter && s.StartsWith(doubleBackTickDelimiter, StringComparison.Ordinal) && s.EndsWith(doubleBackTickDelimiter, StringComparison.Ordinal) then
-                let inner = s.AsSpan(doubleBackTickDelimiter.Length, s.Length - doubledDelimiter)
-                not (inner.Contains(doubleBackTickDelimiter.AsSpan(), StringComparison.Ordinal))
-            else false
 
         let isIdentifier (ident: string) =
             if isDoubleBacktickIdent ident then

vsintegration/tests/UnitTests/CompletionProviderTests.fs

@@ -125,6 +125,13 @@ let VerifyCompletionListExactly(fileContents: string, marker: string, expected:
 let VerifyNoCompletionList(fileContents: string, marker: string) =
     VerifyCompletionListExactly(fileContents, marker, [])
 
+let VerifyCompletionListSpan(fileContents: string, marker: string, expected: string) =
+    let caretPosition = fileContents.IndexOf(marker) + marker.Length
+    let documentId = DocumentId.CreateNewId(ProjectId.CreateNewId())
+    let sourceText = SourceText.From(fileContents)
+    let resultSpan = CompletionUtils.getDefaultCompletionListSpan(sourceText, caretPosition, documentId, filePath, [])
+    Assert.AreEqual(expected, sourceText.ToString(resultSpan))
+
 [<Test>]
 let ShouldTriggerCompletionAtCorrectMarkers() =
     let testCases = 
@@ -687,6 +694,48 @@ module Extensions =
 """
     VerifyCompletionList(fileContents, "wrappedMessage.", ["PrintRef"], [])
 
+[<Test>]
+let ``Completion list span works with underscore in identifier``() =
+    let fileContents = """
+let x = A.B_C
+"""
+    VerifyCompletionListSpan(fileContents, "A.B_C", "B_C")
+
+[<Test>]
+let ``Completion list span works with digit in identifier``() =
+    let fileContents = """
+let x = A.B1C
+"""
+    VerifyCompletionListSpan(fileContents, "A.B1C", "B1C")
+
+[<Test>]
+let ``Completion list span works with enclosed backtick identifier``() =
+    let fileContents = """
+let x = A.``B C``
+"""
+    VerifyCompletionListSpan(fileContents, "A.``B C``", "``B C``")
+
+[<Test>]
+let ``Completion list span works with partial backtick identifier``() =
+    let fileContents = """
+let x = A.``B C
+"""
+    VerifyCompletionListSpan(fileContents, "A.``B C", "``B C")
+
+[<Test>]
+let ``Completion list span works with first of multiple enclosed backtick identifiers``() =
+    let fileContents = """
+let x = A.``B C`` + D.``E F``
+"""
+    VerifyCompletionListSpan(fileContents, "A.``B C``", "``B C``")
+
+[<Test>]
+let ``Completion list span works with last of multiple enclosed backtick identifiers``() =
+    let fileContents = """
+let x = A.``B C`` + D.``E F``
+"""
+    VerifyCompletionListSpan(fileContents, "D.``E F``", "``E F``")
+
 #if EXE
 ShouldDisplaySystemNamespace()
 #endif