From 29cff48f17d2f783d2d23c56de19e08f881536de Mon Sep 17 00:00:00 2001 From: Vasily Kirichenko Date: Mon, 13 Nov 2017 21:53:56 +0300 Subject: [PATCH 1/4] suggests types and modules only at pattern type position (x: ...) --- src/fsharp/vs/ServiceParseTreeWalk.fs | 27 +++++++++++++++++++++++++-- src/fsharp/vs/ServiceUntypedParse.fs | 8 ++++++++ src/fsharp/vs/ServiceUntypedParse.fsi | 2 ++ src/fsharp/vs/service.fs | 12 ++++++++++++ 4 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/fsharp/vs/ServiceParseTreeWalk.fs b/src/fsharp/vs/ServiceParseTreeWalk.fs index 992570be687..71183e8886a 100755 --- a/src/fsharp/vs/ServiceParseTreeWalk.fs +++ b/src/fsharp/vs/ServiceParseTreeWalk.fs @@ -94,6 +94,9 @@ module internal AstTraversal = abstract VisitSimplePats : SynSimplePat list -> 'T option default this.VisitSimplePats (_) = None + abstract VisitPat : (SynPat -> 'T option) * SynPat -> 'T option + default this.VisitPat (defaultTraverse, pat) = defaultTraverse pat + let dive node range project = range,(fun() -> project node) @@ -448,6 +451,24 @@ module internal AstTraversal = visitor.VisitExpr(path, traverseSynExpr path, defaultTraverse, expr) + and traversePat (pat: SynPat) = + let defaultTraverse p = + match p with + | SynPat.Paren (p, _) -> traversePat p + | SynPat.Or (p1, p2, _) -> [ p1; p2] |> List.tryPick traversePat + | SynPat.Ands (ps, _) + | SynPat.Tuple (ps, _) + | SynPat.ArrayOrList (_, ps, _) -> ps |> List.tryPick traversePat + | SynPat.Attrib (p, _, _) -> traversePat p + | SynPat.LongIdent(_, _, _, args, _, _) -> + match args with + | SynConstructorArgs.Pats ps -> ps |> List.tryPick traversePat + | SynConstructorArgs.NamePatPairs (ps, _) -> + ps |> List.map snd |> List.tryPick traversePat + | _ -> None + + visitor.VisitPat (defaultTraverse, pat) + and normalizeMembersToDealWithPeculiaritiesOfGettersAndSetters path traverseInherit (synMemberDefns:SynMemberDefns) = synMemberDefns // property getters are setters are two members that can have the same range, so do some somersaults to deal with this @@ -566,8 +587,10 @@ module internal AstTraversal = let defaultTraverse b = let path = TraverseStep.Binding b :: path match b with - | (SynBinding.Binding(_synAccessOption, _synBindingKind, _, _, _synAttributes, _preXmlDoc, _synValData, _synPat, _synBindingReturnInfoOption, synExpr, _range, _sequencePointInfoForBinding)) -> - traverseSynExpr path synExpr + | (SynBinding.Binding(_synAccessOption, _synBindingKind, _, _, _synAttributes, _preXmlDoc, _synValData, synPat, _synBindingReturnInfoOption, synExpr, _range, _sequencePointInfoForBinding)) -> + [ traversePat synPat + traverseSynExpr path synExpr ] + |> List.tryPick id visitor.VisitBinding(defaultTraverse,b) match parseTree with diff --git a/src/fsharp/vs/ServiceUntypedParse.fs b/src/fsharp/vs/ServiceUntypedParse.fs index c5e30bf3df0..198315fbfa9 100755 --- a/src/fsharp/vs/ServiceUntypedParse.fs +++ b/src/fsharp/vs/ServiceUntypedParse.fs @@ -79,6 +79,8 @@ type CompletionContext = | ParameterList of pos * HashSet | AttributeApplication | OpenDeclaration + /// completing pattern type (e.g. foo (x: |)) + | PatternType //---------------------------------------------------------------------------- // FSharpParseFileResults @@ -1272,6 +1274,12 @@ module UntypedParseImpl = else None | _ -> defaultTraverse decl + + member __.VisitPat(defaultTraverse, pat) = + match pat with + | SynPat.Typed (_, ty, _) when rangeContainsPos ty.Range pos -> + Some CompletionContext.PatternType + | _ -> defaultTraverse pat } AstTraversal.Traverse(pos, pt, walker) diff --git a/src/fsharp/vs/ServiceUntypedParse.fsi b/src/fsharp/vs/ServiceUntypedParse.fsi index 9a2c8379d53..62485fdefa2 100755 --- a/src/fsharp/vs/ServiceUntypedParse.fsi +++ b/src/fsharp/vs/ServiceUntypedParse.fsi @@ -104,6 +104,8 @@ type internal CompletionContext = | ParameterList of pos * HashSet | AttributeApplication | OpenDeclaration + /// completing pattern type (e.g. foo (x: |)) + | PatternType #if COMPILER_PUBLIC_API type ModuleKind = { IsAutoOpen: bool; HasModuleSuffix: bool } diff --git a/src/fsharp/vs/service.fs b/src/fsharp/vs/service.fs index 10f2c61926e..6596bb93667 100644 --- a/src/fsharp/vs/service.fs +++ b/src/fsharp/vs/service.fs @@ -870,6 +870,18 @@ type TypeCheckInfo |> Option.map (fun (items, denv, m) -> items |> List.filter (fun x -> match x.Item with Item.ModuleOrNamespaces _ -> true | _ -> false), denv, m) + // Completion at '(x: ...)" + | Some (CompletionContext.PatternType) -> + GetDeclaredItems (parseResultsOpt, lineStr, origLongIdentOpt, colAtEndOfNamesAndResidue, residueOpt, lastDotPos, line, loc, filterCtors, resolveOverloads, hasTextChangedSinceLastTypecheck, false, getAllSymbols) + |> Option.map (fun (items, denv, m) -> + items + |> List.filter (fun cItem -> + match cItem.Item with + | Item.ModuleOrNamespaces _ + | Item.Types _ + | Item.UnqualifiedType _ -> true + | _ -> false), denv, m) + // Other completions | cc -> match residueOpt |> Option.bind Seq.tryHead with From c22dcf3efc67e908b27fd8d02f44db762d6b388e Mon Sep 17 00:00:00 2001 From: Vasily Kirichenko Date: Tue, 14 Nov 2017 20:35:58 +0300 Subject: [PATCH 2/4] handle all places where type can be used --- src/fsharp/vs/ServiceParseTreeWalk.fs | 37 ++++++++++++++++++++++++--- src/fsharp/vs/ServiceUntypedParse.fs | 8 +++--- 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/fsharp/vs/ServiceParseTreeWalk.fs b/src/fsharp/vs/ServiceParseTreeWalk.fs index 71183e8886a..9dddedb6276 100755 --- a/src/fsharp/vs/ServiceParseTreeWalk.fs +++ b/src/fsharp/vs/ServiceParseTreeWalk.fs @@ -90,12 +90,15 @@ module internal AstTraversal = /// VisitLetOrUse allows overriding behavior when visiting module or local let or use bindings abstract VisitLetOrUse : SynBinding list * range -> 'T option default this.VisitLetOrUse (_, _) = None - + /// VisitType allows overriding behavior when visiting simple pats abstract VisitSimplePats : SynSimplePat list -> 'T option default this.VisitSimplePats (_) = None - + /// VisitPat allows overriding behavior when visiting patterns abstract VisitPat : (SynPat -> 'T option) * SynPat -> 'T option default this.VisitPat (defaultTraverse, pat) = defaultTraverse pat + /// VisitType allows overriding behavior when visiting type hints (x: ..., etc.) + abstract VisitType : (SynType -> 'T option) * SynType -> 'T option + default this.VisitType (defaultTraverse, ty) = defaultTraverse ty let dive node range project = range,(fun() -> project node) @@ -192,12 +195,12 @@ module internal AstTraversal = dive synExpr2 synExpr2.Range traverseSynExpr] |> pick expr | SynExpr.Const(_synConst, _range) -> None - | SynExpr.Typed(synExpr, _synType, _range) -> traverseSynExpr synExpr + | SynExpr.Typed(synExpr, synType, _range) -> [ traverseSynExpr synExpr; traverseSynType synType ] |> List.tryPick id | SynExpr.Tuple(synExprList, _, _range) | SynExpr.StructTuple(synExprList, _, _range) -> synExprList |> List.map (fun x -> dive x x.Range traverseSynExpr) |> pick expr | SynExpr.ArrayOrList(_, synExprList, _range) -> synExprList |> List.map (fun x -> dive x x.Range traverseSynExpr) |> pick expr | SynExpr.Record(inheritOpt,copyOpt,fields, _range) -> - [ + [ let diveIntoSeparator offsideColumn scPosOpt copyOpt = match scPosOpt with | Some scPos -> @@ -458,6 +461,7 @@ module internal AstTraversal = | SynPat.Or (p1, p2, _) -> [ p1; p2] |> List.tryPick traversePat | SynPat.Ands (ps, _) | SynPat.Tuple (ps, _) + | SynPat.StructTuple (ps, _) | SynPat.ArrayOrList (_, ps, _) -> ps |> List.tryPick traversePat | SynPat.Attrib (p, _, _) -> traversePat p | SynPat.LongIdent(_, _, _, args, _, _) -> @@ -465,10 +469,35 @@ module internal AstTraversal = | SynConstructorArgs.Pats ps -> ps |> List.tryPick traversePat | SynConstructorArgs.NamePatPairs (ps, _) -> ps |> List.map snd |> List.tryPick traversePat + | SynPat.Typed (p, ty, _) -> + [ traversePat p; traverseSynType ty ] |> List.tryPick id | _ -> None visitor.VisitPat (defaultTraverse, pat) + and traverseSynType (ty: SynType) = + let defaultTraverse ty = + match ty with + | SynType.App (typeName, _, typeArgs, _, _, _, _) + | SynType.LongIdentApp (typeName, _, _, typeArgs, _, _, _) -> + [ yield typeName + yield! typeArgs ] + |> List.tryPick traverseSynType + | SynType.Fun (ty1, ty2, _) -> [ty1; ty2] |> List.tryPick traverseSynType + | SynType.MeasurePower (ty, _, _) + | SynType.HashConstraint (ty, _) + | SynType.WithGlobalConstraints (ty, _, _) + | SynType.Array (_, ty, _) -> traverseSynType ty + | SynType.StaticConstantNamed (ty1, ty2, _) + | SynType.MeasureDivide (ty1, ty2, _) -> [ty1; ty2] |> List.tryPick traverseSynType + | SynType.Tuple (tys, _) + | SynType.StructTuple (tys, _) -> tys |> List.map snd |> List.tryPick traverseSynType + | SynType.StaticConstantExpr (expr, _) -> traverseSynExpr [] expr + | SynType.Anon _ -> None + | _ -> None + + visitor.VisitType (defaultTraverse, ty) + and normalizeMembersToDealWithPeculiaritiesOfGettersAndSetters path traverseInherit (synMemberDefns:SynMemberDefns) = synMemberDefns // property getters are setters are two members that can have the same range, so do some somersaults to deal with this diff --git a/src/fsharp/vs/ServiceUntypedParse.fs b/src/fsharp/vs/ServiceUntypedParse.fs index 198315fbfa9..dff684fe526 100755 --- a/src/fsharp/vs/ServiceUntypedParse.fs +++ b/src/fsharp/vs/ServiceUntypedParse.fs @@ -1275,11 +1275,11 @@ module UntypedParseImpl = None | _ -> defaultTraverse decl - member __.VisitPat(defaultTraverse, pat) = - match pat with - | SynPat.Typed (_, ty, _) when rangeContainsPos ty.Range pos -> + member __.VisitType(defaultTraverse, ty) = + match ty with + | SynType.LongIdent _ when rangeContainsPos ty.Range pos -> Some CompletionContext.PatternType - | _ -> defaultTraverse pat + | _ -> defaultTraverse ty } AstTraversal.Traverse(pos, pt, walker) From 8251a312d233e17c8b8b60c14e17f30e63585eac Mon Sep 17 00:00:00 2001 From: Vasily Kirichenko Date: Wed, 15 Nov 2017 21:13:16 +0300 Subject: [PATCH 3/4] fix --- src/fsharp/vs/service.fs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/fsharp/vs/service.fs b/src/fsharp/vs/service.fs index 6596bb93667..a5fdb1473bc 100644 --- a/src/fsharp/vs/service.fs +++ b/src/fsharp/vs/service.fs @@ -879,7 +879,9 @@ type TypeCheckInfo match cItem.Item with | Item.ModuleOrNamespaces _ | Item.Types _ - | Item.UnqualifiedType _ -> true + | Item.UnqualifiedType _ + | Item.ExnCase _ + | Item.UnionCase _ -> true | _ -> false), denv, m) // Other completions From 28b93573362c1c60a30031196526dc528b30e3cf Mon Sep 17 00:00:00 2001 From: Vasily Kirichenko Date: Wed, 15 Nov 2017 22:06:19 +0300 Subject: [PATCH 4/4] union cases should not appear in completion at type hint position --- src/fsharp/vs/service.fs | 3 +-- .../tests/unittests/Tests.LanguageService.Completion.fs | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/fsharp/vs/service.fs b/src/fsharp/vs/service.fs index a5fdb1473bc..f40cc61dc46 100644 --- a/src/fsharp/vs/service.fs +++ b/src/fsharp/vs/service.fs @@ -880,8 +880,7 @@ type TypeCheckInfo | Item.ModuleOrNamespaces _ | Item.Types _ | Item.UnqualifiedType _ - | Item.ExnCase _ - | Item.UnionCase _ -> true + | Item.ExnCase _ -> true | _ -> false), denv, m) // Other completions diff --git a/vsintegration/tests/unittests/Tests.LanguageService.Completion.fs b/vsintegration/tests/unittests/Tests.LanguageService.Completion.fs index 88d22ecb293..6c5a8596ff0 100644 --- a/vsintegration/tests/unittests/Tests.LanguageService.Completion.fs +++ b/vsintegration/tests/unittests/Tests.LanguageService.Completion.fs @@ -6064,7 +6064,7 @@ let rec f l = let f (x:MyNamespace1.MyModule(*Maftervariable4*)) = 10 let y = int System.IO(*Maftervariable5*)""", marker = "(*Maftervariable4*)", - list = ["DuType";"Tag"]) + list = ["DuType"]) [] member this.``VariableIdentifier.SystemNamespace``() =