From 3a8e227fd6f41d27ed92e3dcd7770b3e78f531b0 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Wed, 11 Jan 2023 18:52:57 +0100 Subject: [PATCH 1/4] add failing test for optional record fields --- analysis/tests/src/CompletionExpressions.res | 12 +++++++++++ .../expected/CompletionExpressions.res.txt | 20 +++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/analysis/tests/src/CompletionExpressions.res b/analysis/tests/src/CompletionExpressions.res index 2cd24682b..93c492125 100644 --- a/analysis/tests/src/CompletionExpressions.res +++ b/analysis/tests/src/CompletionExpressions.res @@ -96,3 +96,15 @@ let fnTakingOtherRecord = (r: otherRecord) => { // let _ = fnTakingOtherRecord({otherField: }) // ^com + +type recordWithOptionalField = { + someField: int, + someOptField?: string, +} + +let fnTakingRecordWithOptionalField = (r: recordWithOptionalField) => { + ignore(r) +} + +// let _ = fnTakingRecordWithOptionalField({someOptField: }) +// ^com diff --git a/analysis/tests/src/expected/CompletionExpressions.res.txt b/analysis/tests/src/expected/CompletionExpressions.res.txt index 0a9c2fc89..8b2f99015 100644 --- a/analysis/tests/src/expected/CompletionExpressions.res.txt +++ b/analysis/tests/src/expected/CompletionExpressions.res.txt @@ -427,3 +427,23 @@ Completable: Cexpression CArgument Value[fnTakingOtherRecord]($0)->recordField(o "insertTextFormat": 2 }] +Complete src/CompletionExpressions.res 108:57 +posCursor:[108:57] posNoWhite:[108:56] Found expr:[108:11->108:60] +Pexp_apply ...[108:11->108:42] (...[108:43->108:60]) +Completable: Cexpression CArgument Value[fnTakingRecordWithOptionalField]($0)->recordField(someOptField) +[{ + "label": "None", + "kind": 4, + "tags": [], + "detail": "string", + "documentation": null + }, { + "label": "Some(_)", + "kind": 4, + "tags": [], + "detail": "string", + "documentation": null, + "insertText": "Some(${1:_})", + "insertTextFormat": 2 + }] + From 01272fb2979de43eeb509cf6856adb3480a87c97 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Wed, 11 Jan 2023 18:54:23 +0100 Subject: [PATCH 2/4] extract whether record field is optional from cmt --- analysis/src/ProcessCmt.ml | 21 ++++++++++++++++++--- analysis/src/SharedTypes.ml | 7 ++++++- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/analysis/src/ProcessCmt.ml b/analysis/src/ProcessCmt.ml index dab690758..901c19a12 100644 --- a/analysis/src/ProcessCmt.ml +++ b/analysis/src/ProcessCmt.ml @@ -89,13 +89,16 @@ let rec forTypeSignatureItem ~(env : SharedTypes.Env.t) ~(exported : Exported.t) | Type_record (fields, _) -> Record (fields - |> List.map (fun {Types.ld_id; ld_type} -> + |> List.map (fun {Types.ld_id; ld_type; ld_attributes} -> let astamp = Ident.binding_time ld_id in let name = Ident.name ld_id in { stamp = astamp; fname = Location.mknoloc name; typ = ld_type; + optional = + Res_parsetree_viewer.hasOptionalAttribute + ld_attributes; }))); } ~name ~stamp:(Ident.binding_time ident) ~env type_attributes @@ -212,10 +215,22 @@ let forTypeDeclaration ~env ~(exported : Exported.t) (fields |> List.map (fun - {Typedtree.ld_id; ld_name = fname; ld_type = {ctyp_type}} + { + Typedtree.ld_id; + ld_name = fname; + ld_type = {ctyp_type}; + ld_attributes; + } -> let fstamp = Ident.binding_time ld_id in - {stamp = fstamp; fname; typ = ctyp_type}))); + { + stamp = fstamp; + fname; + typ = ctyp_type; + optional = + Res_parsetree_viewer.hasOptionalAttribute + ld_attributes; + }))); } ~name ~stamp ~env typ_attributes (Exported.add exported Exported.Type) diff --git a/analysis/src/SharedTypes.ml b/analysis/src/SharedTypes.ml index a9962c62f..768a7bb7e 100644 --- a/analysis/src/SharedTypes.ml +++ b/analysis/src/SharedTypes.ml @@ -24,7 +24,12 @@ module ModulePath = struct loop modulePath [tipName] end -type field = {stamp: int; fname: string Location.loc; typ: Types.type_expr} +type field = { + stamp: int; + fname: string Location.loc; + typ: Types.type_expr; + optional: bool; +} module Constructor = struct type t = { From b703c7dfb35839587dba6c6b48889658a0fb89c9 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Wed, 11 Jan 2023 18:54:43 +0100 Subject: [PATCH 3/4] unwrap options of optional record fields when following typed path --- analysis/src/CompletionBackEnd.ml | 4 +++- analysis/tests/src/CompletionExpressions.res | 2 +- .../tests/src/expected/CompletionExpressions.res.txt | 12 +++++------- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/analysis/src/CompletionBackEnd.ml b/analysis/src/CompletionBackEnd.ml index bf9bf65f0..1302a374f 100644 --- a/analysis/src/CompletionBackEnd.ml +++ b/analysis/src/CompletionBackEnd.ml @@ -1925,7 +1925,9 @@ let rec resolveNested typ ~env ~package ~nested = |> List.find_opt (fun (field : field) -> field.fname.txt = fieldName) with | None -> None - | Some {typ} -> typ |> resolveNested ~env ~package ~nested) + | Some {typ; optional} -> + let typ = if optional then Utils.unwrapIfOption typ else typ in + typ |> resolveNested ~env ~package ~nested) | NRecordBody {seenFields}, Some (Trecord {env; typeExpr}) -> Some (typeExpr, env, Some (Completable.RecordField {seenFields})) | ( NVariantPayload {constructorName = "Some"; itemNum = 0}, diff --git a/analysis/tests/src/CompletionExpressions.res b/analysis/tests/src/CompletionExpressions.res index 93c492125..408467075 100644 --- a/analysis/tests/src/CompletionExpressions.res +++ b/analysis/tests/src/CompletionExpressions.res @@ -99,7 +99,7 @@ let fnTakingOtherRecord = (r: otherRecord) => { type recordWithOptionalField = { someField: int, - someOptField?: string, + someOptField?: bool, } let fnTakingRecordWithOptionalField = (r: recordWithOptionalField) => { diff --git a/analysis/tests/src/expected/CompletionExpressions.res.txt b/analysis/tests/src/expected/CompletionExpressions.res.txt index 8b2f99015..0d014613d 100644 --- a/analysis/tests/src/expected/CompletionExpressions.res.txt +++ b/analysis/tests/src/expected/CompletionExpressions.res.txt @@ -432,18 +432,16 @@ posCursor:[108:57] posNoWhite:[108:56] Found expr:[108:11->108:60] Pexp_apply ...[108:11->108:42] (...[108:43->108:60]) Completable: Cexpression CArgument Value[fnTakingRecordWithOptionalField]($0)->recordField(someOptField) [{ - "label": "None", + "label": "true", "kind": 4, "tags": [], - "detail": "string", + "detail": "bool", "documentation": null }, { - "label": "Some(_)", + "label": "false", "kind": 4, "tags": [], - "detail": "string", - "documentation": null, - "insertText": "Some(${1:_})", - "insertTextFormat": 2 + "detail": "bool", + "documentation": null }] From f1314d38bedc7114ad188890992041583c44645d Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Wed, 11 Jan 2023 18:56:43 +0100 Subject: [PATCH 4/4] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 98d114866..47a20f1a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ - Add support for pattern completion of unsaved tuples. https://github.com/rescript-lang/rescript-vscode/pull/679 - Add support for completion in typed expressions. https://github.com/rescript-lang/rescript-vscode/pull/682 - Complete for `React.element` creator functions (`React.string` etc) when in JSX context. https://github.com/rescript-lang/rescript-vscode/pull/681 +- Handle optional record fields in expression/pattern completion. https://github.com/rescript-lang/rescript-vscode/pull/691 #### :nail_care: Polish