Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect AttributeApplication completion context for unfinished attributes #4126

Merged
merged 7 commits into from
Dec 17, 2017
Merged

Detect AttributeApplication completion context for unfinished attributes #4126

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
dae0ad7
detect AttributeApplication completion context for unfinished attributes
vasily-kirichenko Dec 16, 2017
3673d66
fix whitespace sensitiveness, add more tests
vasily-kirichenko Dec 16, 2017
2c90332
more tests
vasily-kirichenko Dec 16, 2017
902a04c
support ":" in attribute names
vasily-kirichenko Dec 16, 2017
80bd40e
fix tests
vasily-kirichenko Dec 16, 2017
11dc2dd
pass ParsedInput instead of ParsedInput option to TryGetCompletionCon…
vasily-kirichenko Dec 16, 2017
25dae9e
refactor tests
vasily-kirichenko Dec 17, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 48 additions & 12 deletions src/fsharp/service/ServiceUntypedParse.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,8 @@ type EntityKind =
override x.ToString() = sprintf "%A" x

module UntypedParseImpl =
open System.Text.RegularExpressions
open Microsoft.FSharp.Compiler.PrettyNaming

let emptyStringSet = HashSet<string>()

Expand Down Expand Up @@ -932,20 +934,13 @@ module UntypedParseImpl =
| ParsedInput.ImplFile input -> walkImplFileInput input

type internal TS = AstTraversal.TraverseStep
/// Matches the most nested [< and >] pair.
let insideAttributeApplicationRegex = Regex(@"(?<=\[\<)(?<attribute>(.*?))(?=\>\])", RegexOptions.Compiled ||| RegexOptions.ExplicitCapture)

/// Try to determine completion context for the given pair (row, columns)
let TryGetCompletionContext (pos, untypedParseOpt: FSharpParseFileResults option, lineStr: string) : CompletionContext option =
let parsedInputOpt =
match untypedParseOpt with
| Some upi -> upi.ParseTree
| None -> None

match parsedInputOpt with
| None -> None
| Some pt ->
let TryGetCompletionContext (pos, parsedInput: ParsedInput, lineStr: string) : CompletionContext option =


match GetEntityKind(pos, pt) with
match GetEntityKind(pos, parsedInput) with
| Some EntityKind.Attribute -> Some CompletionContext.AttributeApplication
| _ ->

Expand Down Expand Up @@ -1282,7 +1277,48 @@ module UntypedParseImpl =
| _ -> defaultTraverse ty
}

AstTraversal.Traverse(pos, pt, walker)
AstTraversal.Traverse(pos, parsedInput, walker)
// Uncompleted attribute applications are not presented in the AST in any way. So, we have to parse source string.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My gut feeling tells me we should try to get something useful from the parse tree instead of trying to do some sort of ad-hoc parsing ourselves to determine what context we are in.

Consider this code:

[<
 (* marker *)
 >]
module Test =

    let beef = 1

Completions do not work here because we are checking lineStr. IMHO, we should avoid lineStr and parsing ourselves to determine what context we are in at all costs.

If we are getting nothing useful from the parse tree, then we need to modify the parser. That way we can handle most if not all cases. The implementation would be much simpler too.

Also, in the future, these big new lines code should probably go into a separate function rather than make an existing function bigger than what it was.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TIHan This also applies for current uses of QuickParse that uses a similar approach of getting a context for possibly unfinished expressions. Moving to AST there as well would be a great move.

|> Option.orElseWith (fun _ ->
let cutLeadingAttributes (str: string) =
// cut off leading attributes, i.e. we cut "[<A1; A2; >]" to " >]"
match str.LastIndexOf ';' with
| -1 -> str
| idx when idx < str.Length -> str.[idx + 1..].TrimStart()
| _ -> ""

let isLongIdent = Seq.forall (fun c -> IsIdentifierPartCharacter c || c = '.' || c = ':') // ':' may occur in "[<type:AnAttribute>]"

// match the most nested paired [< and >] first
let matches =
insideAttributeApplicationRegex.Matches(lineStr)
|> Seq.cast<Match>
|> Seq.filter (fun m -> m.Index <= pos.Column && m.Index + m.Length >= pos.Column)
|> Seq.toArray

if not (Array.isEmpty matches) then
matches
|> Seq.tryPick (fun m ->
let g = m.Groups.["attribute"]
let col = pos.Column - g.Index
if col >= 0 && col < g.Length then
let str = g.Value.Substring(0, col).TrimStart() // cut other rhs attributes
let str = cutLeadingAttributes str
if isLongIdent str then
Some CompletionContext.AttributeApplication
else None
else None)
else
// Paired [< and >] were not found, try to determine that we are after [< without closing >]
match lineStr.LastIndexOf "[<" with
| -1 -> None
| openParenIndex when pos.Column >= openParenIndex + 2 ->
let str = lineStr.[openParenIndex + 2..pos.Column - 1].TrimStart()
let str = cutLeadingAttributes str
if isLongIdent str then
Some CompletionContext.AttributeApplication
else None
| _ -> None)

/// Check if we are at an "open" declaration
let GetFullNameOfSmallestModuleOrNamespaceAtPoint (parsedInput: ParsedInput, pos: pos) =
Expand Down
2 changes: 1 addition & 1 deletion src/fsharp/service/ServiceUntypedParse.fsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ module public UntypedParseImpl =
val TryFindExpressionASTLeftOfDotLeftOfCursor : pos * ParsedInput option -> (pos * bool) option
val GetRangeOfExprLeftOfDot : pos * ParsedInput option -> range option
val TryFindExpressionIslandInPosition : pos * ParsedInput option -> string option
val TryGetCompletionContext : pos * FSharpParseFileResults option * lineStr: string -> CompletionContext option
val TryGetCompletionContext : pos * ParsedInput * lineStr: string -> CompletionContext option
val GetEntityKind: pos * ParsedInput -> EntityKind option
val GetFullNameOfSmallestModuleOrNamespaceAtPoint : ParsedInput * pos -> string[]

Expand Down
6 changes: 5 additions & 1 deletion src/fsharp/service/service.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -780,7 +780,11 @@ type TypeCheckInfo
| otherwise -> otherwise - 1

// Look for a "special" completion context
let completionContext = UntypedParseImpl.TryGetCompletionContext(mkPos line colAtEndOfNamesAndResidue, parseResultsOpt, lineStr)
let completionContext =
parseResultsOpt
|> Option.bind (fun x -> x.ParseTree)
|> Option.bind (fun parseTree -> UntypedParseImpl.TryGetCompletionContext(mkPos line colAtEndOfNamesAndResidue, parseTree, lineStr))

let res =
match completionContext with
// Invalid completion locations
Expand Down
Loading