Skip to content

Commit 1ac6d13

Browse files
auduchinokvzarytovskii
auduchinok
and
vzarytovskii
authored
Completion: empty record fixes (#12872)
Co-authored-by: Vlad Zarytovskii <[email protected]>
1 parent 665f675 commit 1ac6d13

File tree

8 files changed

+136
-38
lines changed

8 files changed

+136
-38
lines changed

‎src/fsharp/NameResolution.fs‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4484,22 +4484,31 @@ let rec ResolvePartialLongIdentInModuleOrNamespaceForRecordFields (ncenv: NameRe
44844484
| _ -> []
44854485
)
44864486

4487+
let getRecordFieldsInScope nenv =
4488+
nenv.eFieldLabels
4489+
|> Seq.collect (fun (KeyValue(_, v)) -> v)
4490+
|> Seq.map (fun fref ->
4491+
let typeInsts = fref.TyconRef.TyparsNoRange |> List.map mkTyparTy
4492+
Item.RecdField(RecdFieldInfo(typeInsts, fref)))
4493+
|> List.ofSeq
4494+
44874495
/// allowObsolete - specifies whether we should return obsolete types & modules
44884496
/// as (no other obsolete items are returned)
4489-
let rec ResolvePartialLongIdentToClassOrRecdFields (ncenv: NameResolver) (nenv: NameResolutionEnv) m ad plid (allowObsolete: bool) =
4490-
ResolvePartialLongIdentToClassOrRecdFieldsImpl ncenv nenv OpenQualified m ad plid allowObsolete
4497+
let rec ResolvePartialLongIdentToClassOrRecdFields (ncenv: NameResolver) (nenv: NameResolutionEnv) m ad plid (allowObsolete: bool) (fieldsOnly: bool) =
4498+
ResolvePartialLongIdentToClassOrRecdFieldsImpl ncenv nenv OpenQualified m ad plid allowObsolete fieldsOnly
44914499

4492-
and ResolvePartialLongIdentToClassOrRecdFieldsImpl (ncenv: NameResolver) (nenv: NameResolutionEnv) fullyQualified m ad plid allowObsolete =
4500+
and ResolvePartialLongIdentToClassOrRecdFieldsImpl (ncenv: NameResolver) (nenv: NameResolutionEnv) fullyQualified m ad plid allowObsolete fieldsOnly =
44934501
let g = ncenv.g
44944502

44954503
match plid with
44964504
| id :: plid when id = "global" -> // this is deliberately not the mangled name
44974505
// dive deeper
4498-
ResolvePartialLongIdentToClassOrRecdFieldsImpl ncenv nenv FullyQualified m ad plid allowObsolete
4506+
ResolvePartialLongIdentToClassOrRecdFieldsImpl ncenv nenv FullyQualified m ad plid allowObsolete fieldsOnly
44994507
| [] ->
45004508

45014509
// empty plid - return namespaces\modules\record types\accessible fields
45024510

4511+
if fieldsOnly then getRecordFieldsInScope nenv else
45034512

45044513
let mods =
45054514
let moduleOrNamespaceRefs =
@@ -4528,12 +4537,7 @@ and ResolvePartialLongIdentToClassOrRecdFieldsImpl (ncenv: NameResolver) (nenv:
45284537
|> Seq.toList
45294538

45304539
let recdFields =
4531-
nenv.eFieldLabels
4532-
|> Seq.collect (fun (KeyValue(_, v)) -> v)
4533-
|> Seq.map (fun fref ->
4534-
let typeInsts = fref.TyconRef.TyparsNoRange |> List.map mkTyparTy
4535-
Item.RecdField(RecdFieldInfo(typeInsts, fref)))
4536-
|> List.ofSeq
4540+
getRecordFieldsInScope nenv
45374541

45384542
mods @ recdTyCons @ recdFields
45394543

@@ -4549,7 +4553,7 @@ and ResolvePartialLongIdentToClassOrRecdFieldsImpl (ncenv: NameResolver) (nenv:
45494553

45504554
let qualifiedFields =
45514555
match rest with
4552-
| [] ->
4556+
| [] when not fieldsOnly ->
45534557
// get record types accessible in given nenv
45544558
let tycons = LookupTypeNameInEnvNoArity OpenQualified id nenv
45554559
tycons

‎src/fsharp/NameResolution.fsi‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -557,8 +557,10 @@ val internal ResolveField : TcResultsSink -> NameResolver -
557557
/// Resolve a long identifier occurring in an expression position
558558
val internal ResolveExprLongIdent : TcResultsSink -> NameResolver -> range -> AccessorDomain -> NameResolutionEnv -> TypeNameResolutionInfo -> Ident list -> ResultOrException<EnclosingTypeInst * Item * Ident list>
559559

560+
val internal getRecordFieldsInScope: NameResolutionEnv -> Item list
561+
560562
/// Resolve a (possibly incomplete) long identifier to a loist of possible class or record fields
561-
val internal ResolvePartialLongIdentToClassOrRecdFields: NameResolver -> NameResolutionEnv -> range -> AccessorDomain -> string list -> bool -> Item list
563+
val internal ResolvePartialLongIdentToClassOrRecdFields: NameResolver -> NameResolutionEnv -> range -> AccessorDomain -> string list -> bool -> bool -> Item list
562564

563565
/// Return the fields for the given class or record
564566
val internal ResolveRecordOrClassFieldsOfType : NameResolver -> range -> AccessorDomain -> TType -> bool -> Item list

‎src/fsharp/service/FSharpCheckerResults.fs‎

Lines changed: 37 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -638,9 +638,9 @@ type internal TypeCheckInfo
638638
GetEnvironmentLookupResolutions(nenv, ad, m, plid, filterCtors, showObsolete)
639639

640640
/// Find record fields in the best naming environment.
641-
let GetClassOrRecordFieldsEnvironmentLookupResolutions(cursorPos, plid) =
641+
let GetClassOrRecordFieldsEnvironmentLookupResolutions(cursorPos, plid, fieldsOnly) =
642642
let (nenv, ad),m = GetBestEnvForPos cursorPos
643-
let items = ResolvePartialLongIdentToClassOrRecdFields ncenv nenv m ad plid false
643+
let items = ResolvePartialLongIdentToClassOrRecdFields ncenv nenv m ad plid false fieldsOnly
644644
let items = items |> List.map ItemWithNoInst
645645
let items = items |> RemoveDuplicateItems g
646646
let items = items |> RemoveExplicitlySuppressed g
@@ -913,6 +913,21 @@ type internal TypeCheckInfo
913913
let toCompletionItems (items: ItemWithInst list, denv: DisplayEnv, m: range ) =
914914
items |> List.map DefaultCompletionItem, denv, m
915915

916+
/// Find record fields in the best naming environment.
917+
let GetEnvironmentLookupResolutionsIncludingRecordFieldsAtPosition cursorPos plid envItems =
918+
// An empty record expression may be completed into something like these:
919+
// { XXX = ... }
920+
// { xxx with XXX ... }
921+
// Provide both expression items in scope and available record fields.
922+
let (nenv, _), m = GetBestEnvForPos cursorPos
923+
924+
let fieldItems, _, _ = GetClassOrRecordFieldsEnvironmentLookupResolutions(cursorPos, plid, true)
925+
let fieldCompletionItems, _, _ as fieldsResult = (fieldItems, nenv.DisplayEnv, m) |> toCompletionItems
926+
927+
match envItems with
928+
| Some(items, denv, m) -> Some(fieldCompletionItems @ items, denv, m)
929+
| _ -> Some(fieldsResult)
930+
916931
/// Get the auto-complete items at a particular location.
917932
let GetDeclItemsForNamesAtPosition(parseResultsOpt: FSharpParseFileResults option, origLongIdentOpt: string list option,
918933
residueOpt:string option, lastDotPos: int option, line:int, lineStr:string, colAtEndOfNamesAndResidue, filterCtors, resolveOverloads,
@@ -963,27 +978,38 @@ type internal TypeCheckInfo
963978
|> Option.map toCompletionItems
964979

965980
// Completion at ' { XXX = ... } "
966-
| Some(CompletionContext.RecordField(RecordContext.New(plid, _))) ->
967-
// { x. } can be either record construction or computation expression. Try to get all visible record fields first
968-
match GetClassOrRecordFieldsEnvironmentLookupResolutions(mkPos line loc, plid) |> toCompletionItems with
969-
| [],_,_ ->
970-
// no record fields found, return completion list as if we were outside any computation expression
971-
GetDeclaredItems (parseResultsOpt, lineStr, origLongIdentOpt, colAtEndOfNamesAndResidue, residueOpt, lastDotPos, line, loc, filterCtors,resolveOverloads, false, fun() -> [])
972-
| result -> Some(result)
981+
| Some(CompletionContext.RecordField(RecordContext.New((plid, _), isFirstField))) ->
982+
if isFirstField then
983+
let cursorPos = mkPos line loc
984+
let envItems = GetDeclaredItems (parseResultsOpt, lineStr, origLongIdentOpt, colAtEndOfNamesAndResidue, residueOpt, lastDotPos, line, loc, filterCtors,resolveOverloads, false, fun () -> [])
985+
GetEnvironmentLookupResolutionsIncludingRecordFieldsAtPosition cursorPos plid envItems
986+
else
987+
// { x. } can be either record construction or computation expression. Try to get all visible record fields first
988+
match GetClassOrRecordFieldsEnvironmentLookupResolutions(mkPos line loc, plid, false) |> toCompletionItems with
989+
| [],_,_ ->
990+
// no record fields found, return completion list as if we were outside any computation expression
991+
GetDeclaredItems (parseResultsOpt, lineStr, origLongIdentOpt, colAtEndOfNamesAndResidue, residueOpt, lastDotPos, line, loc, filterCtors,resolveOverloads, false, fun() -> [])
992+
| result -> Some(result)
993+
994+
// Completion at '{ ... }'
995+
| Some(CompletionContext.RecordField RecordContext.Empty) ->
996+
let cursorPos = mkPos line loc
997+
let envItems = GetDeclaredItems (parseResultsOpt, lineStr, origLongIdentOpt, colAtEndOfNamesAndResidue, residueOpt, lastDotPos, line, loc, filterCtors,resolveOverloads, false, fun () -> [])
998+
GetEnvironmentLookupResolutionsIncludingRecordFieldsAtPosition cursorPos [] envItems
973999

9741000
// Completion at ' { XXX = ... with ... } "
9751001
| Some(CompletionContext.RecordField(RecordContext.CopyOnUpdate(r, (plid, _)))) ->
9761002
match GetRecdFieldsForExpr(r) with
9771003
| None ->
978-
Some (GetClassOrRecordFieldsEnvironmentLookupResolutions(mkPos line loc, plid))
1004+
Some (GetClassOrRecordFieldsEnvironmentLookupResolutions(mkPos line loc, plid, false))
9791005
|> Option.map toCompletionItems
9801006
| Some (items, denv, m) ->
9811007
Some (List.map ItemWithNoInst items, denv, m)
9821008
|> Option.map toCompletionItems
9831009

9841010
// Completion at ' { XXX = ... with ... } "
9851011
| Some(CompletionContext.RecordField(RecordContext.Constructor(typeName))) ->
986-
Some(GetClassOrRecordFieldsEnvironmentLookupResolutions(mkPos line loc, [typeName]))
1012+
Some(GetClassOrRecordFieldsEnvironmentLookupResolutions(mkPos line loc, [typeName], false))
9871013
|> Option.map toCompletionItems
9881014

9891015
// No completion at '...: string'

‎src/fsharp/service/ServiceParsedInputOps.fs‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ type InheritanceContext =
4343
type RecordContext =
4444
| CopyOnUpdate of range: range * path: CompletionPath
4545
| Constructor of typeName: string
46-
| New of path: CompletionPath
46+
| Empty
47+
| New of path: CompletionPath * isFirstField: bool
4748
| Declaration of isInIdentifier: bool
4849

4950
[<RequireQualifiedAccess>]
@@ -956,7 +957,8 @@ module ParsedInput =
956957
Some (CompletionContext.ParameterList args)
957958
| _ ->
958959
defaultTraverse expr
959-
960+
| SynExpr.Record(None, None, [], _) ->
961+
Some(CompletionContext.RecordField RecordContext.Empty)
960962
// Unchecked.defaultof<str$>
961963
| SynExpr.TypeApp (typeArgsRange = range) when rangeContainsPos range pos ->
962964
Some CompletionContext.PatternType
@@ -968,7 +970,22 @@ module ParsedInput =
968970
match path with
969971
| SyntaxNode.SynExpr _ :: SyntaxNode.SynBinding _ :: SyntaxNode.SynMemberDefn _ :: SyntaxNode.SynTypeDefn(SynTypeDefn(typeInfo=SynComponentInfo(longId=[id]))) :: _ ->
970972
RecordContext.Constructor(id.idText)
971-
| _ -> RecordContext.New completionPath
973+
974+
| SyntaxNode.SynExpr(SynExpr.Record(None, _, fields, _)) :: _ ->
975+
let isFirstField =
976+
match field, fields with
977+
| Some contextLid, SynExprRecordField(fieldName = lid, _) :: _ -> contextLid.Range = lid.Range
978+
| _ -> false
979+
980+
RecordContext.New(completionPath, isFirstField)
981+
982+
// Unfinished `{ xxx }` expression considered a record field by the tree walker.
983+
| SyntaxNode.SynExpr(SynExpr.ComputationExpr _) :: _ ->
984+
RecordContext.New(completionPath, true)
985+
986+
| _ ->
987+
RecordContext.New(completionPath, false)
988+
972989
match field with
973990
| Some field ->
974991
match parseLid field with

‎src/fsharp/service/ServiceParsedInputOps.fsi‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ type public InheritanceContext =
1818
type public RecordContext =
1919
| CopyOnUpdate of range: range * path: CompletionPath
2020
| Constructor of typeName: string
21-
| New of path: CompletionPath
21+
| Empty
22+
| New of path: CompletionPath * isFirstField: bool
2223
| Declaration of isInIdentifier: bool
2324

2425
[<RequireQualifiedAccess>]

‎tests/FSharp.Compiler.Service.Tests/FSharp.CompilerService.SurfaceArea.netstandard.expected‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3511,27 +3511,34 @@ FSharp.Compiler.EditorServices.RecordContext+CopyOnUpdate: System.Tuple`2[Micros
35113511
FSharp.Compiler.EditorServices.RecordContext+CopyOnUpdate: System.Tuple`2[Microsoft.FSharp.Collections.FSharpList`1[System.String],Microsoft.FSharp.Core.FSharpOption`1[System.String]] path
35123512
FSharp.Compiler.EditorServices.RecordContext+Declaration: Boolean get_isInIdentifier()
35133513
FSharp.Compiler.EditorServices.RecordContext+Declaration: Boolean isInIdentifier
3514+
FSharp.Compiler.EditorServices.RecordContext+New: Boolean get_isFirstField()
3515+
FSharp.Compiler.EditorServices.RecordContext+New: Boolean isFirstField
35143516
FSharp.Compiler.EditorServices.RecordContext+New: System.Tuple`2[Microsoft.FSharp.Collections.FSharpList`1[System.String],Microsoft.FSharp.Core.FSharpOption`1[System.String]] get_path()
35153517
FSharp.Compiler.EditorServices.RecordContext+New: System.Tuple`2[Microsoft.FSharp.Collections.FSharpList`1[System.String],Microsoft.FSharp.Core.FSharpOption`1[System.String]] path
35163518
FSharp.Compiler.EditorServices.RecordContext+Tags: Int32 Constructor
35173519
FSharp.Compiler.EditorServices.RecordContext+Tags: Int32 CopyOnUpdate
35183520
FSharp.Compiler.EditorServices.RecordContext+Tags: Int32 Declaration
3521+
FSharp.Compiler.EditorServices.RecordContext+Tags: Int32 Empty
35193522
FSharp.Compiler.EditorServices.RecordContext+Tags: Int32 New
35203523
FSharp.Compiler.EditorServices.RecordContext: Boolean Equals(FSharp.Compiler.EditorServices.RecordContext)
35213524
FSharp.Compiler.EditorServices.RecordContext: Boolean Equals(System.Object)
35223525
FSharp.Compiler.EditorServices.RecordContext: Boolean Equals(System.Object, System.Collections.IEqualityComparer)
35233526
FSharp.Compiler.EditorServices.RecordContext: Boolean IsConstructor
35243527
FSharp.Compiler.EditorServices.RecordContext: Boolean IsCopyOnUpdate
35253528
FSharp.Compiler.EditorServices.RecordContext: Boolean IsDeclaration
3529+
FSharp.Compiler.EditorServices.RecordContext: Boolean IsEmpty
35263530
FSharp.Compiler.EditorServices.RecordContext: Boolean IsNew
35273531
FSharp.Compiler.EditorServices.RecordContext: Boolean get_IsConstructor()
35283532
FSharp.Compiler.EditorServices.RecordContext: Boolean get_IsCopyOnUpdate()
35293533
FSharp.Compiler.EditorServices.RecordContext: Boolean get_IsDeclaration()
3534+
FSharp.Compiler.EditorServices.RecordContext: Boolean get_IsEmpty()
35303535
FSharp.Compiler.EditorServices.RecordContext: Boolean get_IsNew()
3536+
FSharp.Compiler.EditorServices.RecordContext: FSharp.Compiler.EditorServices.RecordContext Empty
35313537
FSharp.Compiler.EditorServices.RecordContext: FSharp.Compiler.EditorServices.RecordContext NewConstructor(System.String)
35323538
FSharp.Compiler.EditorServices.RecordContext: FSharp.Compiler.EditorServices.RecordContext NewCopyOnUpdate(FSharp.Compiler.Text.Range, System.Tuple`2[Microsoft.FSharp.Collections.FSharpList`1[System.String],Microsoft.FSharp.Core.FSharpOption`1[System.String]])
35333539
FSharp.Compiler.EditorServices.RecordContext: FSharp.Compiler.EditorServices.RecordContext NewDeclaration(Boolean)
3534-
FSharp.Compiler.EditorServices.RecordContext: FSharp.Compiler.EditorServices.RecordContext NewNew(System.Tuple`2[Microsoft.FSharp.Collections.FSharpList`1[System.String],Microsoft.FSharp.Core.FSharpOption`1[System.String]])
3540+
FSharp.Compiler.EditorServices.RecordContext: FSharp.Compiler.EditorServices.RecordContext NewNew(System.Tuple`2[Microsoft.FSharp.Collections.FSharpList`1[System.String],Microsoft.FSharp.Core.FSharpOption`1[System.String]], Boolean)
3541+
FSharp.Compiler.EditorServices.RecordContext: FSharp.Compiler.EditorServices.RecordContext get_Empty()
35353542
FSharp.Compiler.EditorServices.RecordContext: FSharp.Compiler.EditorServices.RecordContext+Constructor
35363543
FSharp.Compiler.EditorServices.RecordContext: FSharp.Compiler.EditorServices.RecordContext+CopyOnUpdate
35373544
FSharp.Compiler.EditorServices.RecordContext: FSharp.Compiler.EditorServices.RecordContext+Declaration

‎tests/service/CompletionTests.fs‎

Lines changed: 48 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,59 @@
11
module FSharp.Compiler.Service.Tests.CompletionTests
22

33
open FSharp.Compiler.EditorServices
4-
open FsUnit
54
open NUnit.Framework
65

6+
let getCompletionInfo lineText (line, column) source =
7+
let parseResults, checkResults = getParseAndCheckResults source
8+
let plid = QuickParse.GetPartialLongNameEx(lineText, column)
9+
checkResults.GetDeclarationListInfo(Some parseResults, line, lineText, plid)
10+
11+
let getCompletionItemNames (completionInfo: DeclarationListInfo) =
12+
completionInfo.Items |> Array.map (fun item -> item.Name)
13+
14+
let assertHasItemWithNames names (completionInfo: DeclarationListInfo) =
15+
let itemNames = getCompletionItemNames completionInfo |> set
16+
17+
for name in names do
18+
Assert.That(Set.contains name itemNames, name)
19+
20+
721
[<Test>]
822
let ``Expr - record - field 01 - anon module`` () =
9-
let parseResults, checkResults = getParseAndCheckResults """
10-
type Record = { Field: int}
23+
let info = getCompletionInfo "{ Fi }" (4, 3) """
24+
type Record = { Field: int }
1125
1226
{ Fi }
1327
"""
14-
let lineText = "{ Fi }"
15-
let plid = QuickParse.GetPartialLongNameEx(lineText, 3)
16-
let info = checkResults.GetDeclarationListInfo(Some parseResults, 4, lineText, plid)
28+
assertHasItemWithNames ["Field"] info
1729

18-
info.Items |> Array.exists (fun item -> item.Name = "Field") |> shouldEqual true
30+
[<Test>]
31+
let ``Expr - record - field 02 - anon module`` () =
32+
let info = getCompletionInfo "{ Fi }" (6, 3) """
33+
type Record = { Field: int }
34+
35+
let record = { Field = 1 }
36+
37+
{ Fi }
38+
"""
39+
assertHasItemWithNames ["Field"] info
40+
41+
[<Test>]
42+
let ``Expr - record - empty 01`` () =
43+
let info = getCompletionInfo "{ }" (4, 2) """
44+
type Record = { Field: int }
45+
46+
{ }
47+
"""
48+
assertHasItemWithNames ["Field"] info
49+
50+
[<Test>]
51+
let ``Expr - record - empty 02`` () =
52+
let info = getCompletionInfo "{ }" (6, 2) """
53+
type Record = { Field: int }
54+
55+
let record = { Field = 1 }
56+
57+
{ }
58+
"""
59+
assertHasItemWithNames ["Field"; "record"] info

‎vsintegration/tests/UnitTests/LegacyLanguageService/Tests.LanguageService.Completion.fs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -786,15 +786,15 @@ for i in 0..a."]
786786
]
787787
let useCases =
788788
[
789-
"let _ = (* MARKER*){X", "(* MARKER*){X", [], ["XX"]
789+
"let _ = (* MARKER*){X }", "(* MARKER*){X", [], ["XX"]
790790
"let _ = {(* MARKER*)Mod. = 1; O", "(* MARKER*)Mod.", ["XX"; "YY"], ["System"]
791791
"let _ = {(* MARKER*)Mod.Rec. ", "(* MARKER*)Mod.Rec.", ["XX"; "YY"], ["System"]
792+
"let _ = (* MARKER*){Mod.XX = 1; }", "(* MARKER*){Mod.XX = 1; ", ["Mod"], ["XX"; "abs"]
792793
]
793794

794795
for (code, marker, should, shouldnot) in useCases do
795796
let code = prologue @ [code]
796-
let shouldnot = shouldnot @ ["abs"]
797-
AssertCtrlSpaceCompleteContains code marker should ["abs"]
797+
AssertCtrlSpaceCompleteContains code marker should shouldnot
798798

799799
[<Test>]
800800
[<Category("Records")>]

0 commit comments

Comments
 (0)