Skip to content

Commit

Permalink
Fix completion with backticks, underscores, numbers (dotnet#10500)
Browse files Browse the repository at this point in the history
  • Loading branch information
zanaptak committed Nov 25, 2020
1 parent 2cd971c commit 0e6f1ee
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 10 deletions.
7 changes: 7 additions & 0 deletions Completion/CompletionService.fs
Expand Up @@ -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
Expand Down
58 changes: 57 additions & 1 deletion Completion/CompletionUtils.fs
Expand Up @@ -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 =

Expand Down Expand Up @@ -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()
19 changes: 10 additions & 9 deletions LanguageService/Tokenizer.fs
@@ -1,4 +1,4 @@
namespace Microsoft.VisualStudio.FSharp.Editor
namespace Microsoft.VisualStudio.FSharp.Editor

open System
open System.Collections.Generic
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 0e6f1ee

Please sign in to comment.