Skip to content

support completing in/through inline records in variant payloads #695

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

Merged
merged 2 commits into from
Jan 14, 2023
Merged

support completing in/through inline records in variant payloads #695

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
116d259
support completing in/through inline records in variant payloads
zth Jan 13, 2023
37c5870
changelog
zth Jan 13, 2023
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
- Handle optional record fields in expression/pattern completion. https://github.com/rescript-lang/rescript-vscode/pull/691
- Expand options in completion to make working with options a bit more ergonomic. https://github.com/rescript-lang/rescript-vscode/pull/690
- Let `_` trigger completion in patterns. https://github.com/rescript-lang/rescript-vscode/pull/692
- Support inline records in completion. https://github.com/rescript-lang/rescript-vscode/pull/695

#### :nail_care: Polish

Expand Down
95 changes: 67 additions & 28 deletions analysis/src/CompletionBackEnd.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,8 +490,8 @@ let domLabels =
let showConstructor {Constructor.cname = {txt}; args; res} =
txt
^ (match args with
| [] -> ""
| _ ->
| Args [] | InlineRecord _ -> ""
| Args args ->
"("
^ (args
|> List.map (fun (typ, _) -> typ |> Shared.typeToString)
Expand Down Expand Up @@ -1793,29 +1793,35 @@ let printConstructorArgs argsLen ~asSnippet =
if List.length !args > 0 then "(" ^ (!args |> String.concat ", ") ^ ")"
else ""

let rec completeTypedValue (t : Types.type_expr) ~env ~full ~prefix
let rec completeTypedValue (t : SharedTypes.completionType) ~env ~full ~prefix
~completionContext =
match t |> extractType ~env ~package:full.package with
let extractedType =
match t with
| TypeExpr t -> t |> extractType ~env ~package:full.package
| InlineRecord fields -> Some (TinlineRecord {env; fields})
Copy link
Collaborator

Choose a reason for hiding this comment

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

I was a wondering why we need a variant called InlineRecord in two places in SharedTypes.
I guess the question can be rephrased here as: do we need TypeExpr and InlineRecord to exist at the same level here? Somehow an inline record is not a self-standing type.
Not sure I have something concrete to suggest, just an open-ended question.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's a good question for sure, and I don't have a clear/good answer either. One extra bit of context is that I intend to add another constructor there as well soon, for handling completing from Type.t (via type annotations for example).

I guess I view this as the "type source" to derive what to complete from, and this was the fastest/easiest way to handle inline records since they have no type expression of their own. And I guess the same will be true for types derived from type annotations?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Let's come back to this eventually. I'll merge now so I can move on to the next thing.

in
match extractedType with
| Some (Tbool env) ->
[
Completion.create "true" ~kind:(Label (t |> Shared.typeToString)) ~env;
Completion.create "false" ~kind:(Label (t |> Shared.typeToString)) ~env;
Completion.create "true" ~kind:(Label "bool") ~env;
Completion.create "false" ~kind:(Label "bool") ~env;
]
|> filterItems ~prefix
| Some (Tvariant {env; constructors; variantDecl; variantName}) ->
constructors
|> List.map (fun (constructor : Constructor.t) ->
let numArgs =
match constructor.args with
| InlineRecord _ -> 1
| Args args -> List.length args
in
Completion.createWithSnippet
~name:
(constructor.cname.txt
^ printConstructorArgs
(List.length constructor.args)
~asSnippet:false)
^ printConstructorArgs numArgs ~asSnippet:false)
~insertText:
(constructor.cname.txt
^ printConstructorArgs
(List.length constructor.args)
~asSnippet:true)
^ printConstructorArgs numArgs ~asSnippet:true)
~kind:
(Constructor
(constructor, variantDecl |> Shared.declToString variantName))
Expand Down Expand Up @@ -1844,7 +1850,7 @@ let rec completeTypedValue (t : Types.type_expr) ~env ~full ~prefix
| Some (Toption (env, t)) ->
let innerType = Utils.unwrapIfOption t in
let expandedCompletions =
innerType
TypeExpr innerType
|> completeTypedValue ~env ~full ~prefix ~completionContext
|> List.map (fun (c : Completion.t) ->
{
Expand Down Expand Up @@ -1895,6 +1901,24 @@ let rec completeTypedValue (t : Types.type_expr) ~env ~full ~prefix
~sortText:"A" ~kind:(Value typeExpr) ~env ();
]
else [])
| Some (TinlineRecord {env; fields}) -> (
match completionContext with
| Some (Completable.RecordField {seenFields}) ->
fields
|> List.filter (fun (field : field) ->
List.mem field.fname.txt seenFields = false)
|> List.map (fun (field : field) ->
Completion.create field.fname.txt ~kind:(Label "Inline record")
~env)
|> filterItems ~prefix
| None ->
if prefix = "" then
[
Completion.createWithSnippet ~name:"{}"
~insertText:(if !Cfg.supportsSnippets then "{$0}" else "{}")
~sortText:"A" ~kind:(Label "Inline record") ~env ();
]
else [])
| Some (Tarray (env, typeExpr)) ->
if prefix = "" then
[
Expand All @@ -1917,41 +1941,52 @@ let rec completeTypedValue (t : Types.type_expr) ~env ~full ~prefix
| _ -> []

(** This moves through a nested path via a set of instructions, trying to resolve the type at the end of the path. *)
let rec resolveNested typ ~env ~package ~nested =
let rec resolveNested (typ : completionType) ~env ~package ~nested =
match nested with
| [] -> Some (typ, env, None)
| patternPath :: nested -> (
match (patternPath, typ |> extractType ~env ~package) with
let extractedType =
match typ with
| TypeExpr typ -> typ |> extractType ~env ~package
| InlineRecord fields -> Some (TinlineRecord {env; fields})
in
match (patternPath, extractedType) with
| Completable.NTupleItem {itemNum}, Some (Tuple (env, tupleItems, _)) -> (
match List.nth_opt tupleItems itemNum with
| None -> None
| Some typ -> typ |> resolveNested ~env ~package ~nested)
| NFollowRecordField {fieldName}, Some (Trecord {env; fields}) -> (
| Some typ -> TypeExpr typ |> resolveNested ~env ~package ~nested)
| ( NFollowRecordField {fieldName},
Some (TinlineRecord {env; fields} | Trecord {env; fields}) ) -> (
match
fields
|> List.find_opt (fun (field : field) -> field.fname.txt = fieldName)
with
| None -> None
| Some {typ; optional} ->
let typ = if optional then Utils.unwrapIfOption typ else typ in
typ |> resolveNested ~env ~package ~nested)
TypeExpr typ |> resolveNested ~env ~package ~nested)
| NRecordBody {seenFields}, Some (Trecord {env; typeExpr}) ->
Some (typeExpr, env, Some (Completable.RecordField {seenFields}))
Some (TypeExpr typeExpr, env, Some (Completable.RecordField {seenFields}))
| NRecordBody {seenFields}, Some (TinlineRecord {env; fields}) ->
Some
(InlineRecord fields, env, Some (Completable.RecordField {seenFields}))
| ( NVariantPayload {constructorName = "Some"; itemNum = 0},
Some (Toption (env, typ)) ) ->
typ |> resolveNested ~env ~package ~nested
TypeExpr typ |> resolveNested ~env ~package ~nested
| ( NVariantPayload {constructorName; itemNum},
Some (Tvariant {env; constructors}) ) -> (
match
constructors
|> List.find_opt (fun (c : Constructor.t) ->
c.cname.txt = constructorName)
with
| None -> None
| Some constructor -> (
match List.nth_opt constructor.args itemNum with
| Some {args = Args args} -> (
match List.nth_opt args itemNum with
| None -> None
| Some (typ, _) -> typ |> resolveNested ~env ~package ~nested))
| Some (typ, _) -> TypeExpr typ |> resolveNested ~env ~package ~nested)
| Some {args = InlineRecord fields} when itemNum = 0 ->
InlineRecord fields |> resolveNested ~env ~package ~nested
| _ -> None)
| ( NPolyvariantPayload {constructorName; itemNum},
Some (Tpolyvariant {env; constructors}) ) -> (
match
Expand All @@ -1963,9 +1998,9 @@ let rec resolveNested typ ~env ~package ~nested =
| Some constructor -> (
match List.nth_opt constructor.args itemNum with
| None -> None
| Some typ -> typ |> resolveNested ~env ~package ~nested))
| Some typ -> TypeExpr typ |> resolveNested ~env ~package ~nested))
| NArray, Some (Tarray (env, typ)) ->
typ |> resolveNested ~env ~package ~nested
TypeExpr typ |> resolveNested ~env ~package ~nested
| _ -> None)

let rec processCompletable ~debug ~full ~scope ~env ~pos ~forHover
Expand Down Expand Up @@ -2301,7 +2336,9 @@ Note: The `@react.component` decorator requires the react-jsx config to be set i
|> completionsGetTypeEnv
with
| Some (typ, env) -> (
match typ |> resolveNested ~env ~package:full.package ~nested with
match
TypeExpr typ |> resolveNested ~env ~package:full.package ~nested
with
| None -> fallbackOrEmpty ()
| Some (typ, env, completionContext) ->
let items =
Expand All @@ -2318,7 +2355,9 @@ Note: The `@react.component` decorator requires the react-jsx config to be set i
with
| None -> []
| Some (typ, env) -> (
match typ |> resolveNested ~env ~package:full.package ~nested with
match
TypeExpr typ |> resolveNested ~env ~package:full.package ~nested
with
| None -> []
| Some (typ, env, completionContext) -> (
let items =
Expand Down
4 changes: 2 additions & 2 deletions analysis/src/Hover.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,8 @@ let newHover ~full:{file; package} ~supportsMarkdownLinks locItem =
let typeString, docstring = t |> fromType ~docstring in
let argsString =
match args with
| [] -> ""
| _ ->
| InlineRecord _ | Args [] -> ""
| Args args ->
args
|> List.map (fun (t, _) -> Shared.typeToString t)
|> String.concat ", " |> Printf.sprintf "(%s)"
Expand Down
71 changes: 49 additions & 22 deletions analysis/src/ProcessCmt.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,20 @@ let attrsToDocstring attrs =
| None -> []
| Some docstring -> [docstring]

let mapRecordField {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;
docstring =
(match ProcessAttributes.findDocAttribute ld_attributes with
| None -> []
| Some docstring -> [docstring]);
}

let rec forTypeSignatureItem ~(env : SharedTypes.Env.t) ~(exported : Exported.t)
(item : Types.signature_item) =
match item with
Expand Down Expand Up @@ -76,9 +90,11 @@ let rec forTypeSignatureItem ~(env : SharedTypes.Env.t) ~(exported : Exported.t)
args =
(match cd_args with
| Cstr_tuple args ->
args |> List.map (fun t -> (t, Location.none))
(* TODO(406): constructor record args support *)
| Cstr_record _ -> []);
Args
(args
|> List.map (fun t -> (t, Location.none)))
| Cstr_record fields ->
InlineRecord (fields |> List.map mapRecordField));
res = cd_res;
typeDecl = (name, decl);
docstring = attrsToDocstring cd_attributes;
Expand All @@ -93,20 +109,7 @@ let rec forTypeSignatureItem ~(env : SharedTypes.Env.t) ~(exported : Exported.t)
Stamps.addConstructor env.stamps stamp declared;
item))
| Type_record (fields, _) ->
Record
(fields
|> 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;
docstring = attrsToDocstring ld_attributes;
})));
Record (fields |> List.map mapRecordField));
}
~name ~stamp:(Ident.binding_time ident) ~env type_attributes
(Exported.add exported Exported.Type)
Expand Down Expand Up @@ -198,11 +201,35 @@ let forTypeDeclaration ~env ~(exported : Exported.t)
args =
(match cd_args with
| Cstr_tuple args ->
args
|> List.map (fun t ->
(t.Typedtree.ctyp_type, t.ctyp_loc))
(* TODO(406) *)
| Cstr_record _ -> []);
Args
(args
|> List.map (fun t ->
(t.Typedtree.ctyp_type, t.ctyp_loc)))
| Cstr_record fields ->
InlineRecord
(fields
|> List.map
(fun (f : Typedtree.label_declaration) ->
let astamp =
Ident.binding_time f.ld_id
in
let name = Ident.name f.ld_id in
{
stamp = astamp;
fname = Location.mknoloc name;
typ = f.ld_type.ctyp_type;
optional =
Res_parsetree_viewer
.hasOptionalAttribute
f.ld_attributes;
docstring =
(match
ProcessAttributes
.findDocAttribute f.ld_attributes
with
| None -> []
| Some docstring -> [docstring]);
})));
res =
(match cd_res with
| None -> None
Expand Down
9 changes: 8 additions & 1 deletion analysis/src/SharedTypes.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,17 @@ type field = {
docstring: string list;
}

type completionType = TypeExpr of Types.type_expr | InlineRecord of field list

type constructorArgs =
| InlineRecord of field list
| Args of (Types.type_expr * Location.t) list

module Constructor = struct
type t = {
stamp: int;
cname: string Location.loc;
args: (Types.type_expr * Location.t) list;
args: constructorArgs;
res: Types.type_expr option;
typeDecl: string * Types.type_declaration;
docstring: string list;
Expand Down Expand Up @@ -631,6 +637,7 @@ module Completable = struct
fields: field list;
typeExpr: Types.type_expr;
}
| TinlineRecord of {env: QueryEnv.t; fields: field list}

let toString =
let completionContextToString = function
Expand Down
22 changes: 22 additions & 0 deletions analysis/tests/src/CompletionExpressions.res
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,3 +116,25 @@ let fnTakingRecordWithOptVariant = (r: recordWithOptVariant) => {

// let _ = fnTakingRecordWithOptVariant({someVariant: })
// ^com

type variantWithInlineRecord =
WithInlineRecord({someBoolField: bool, otherField: option<bool>, nestedRecord: otherRecord})

let fnTakingInlineRecord = (r: variantWithInlineRecord) => {
ignore(r)
}

// let _ = fnTakingInlineRecord(WithInlineRecord())
// ^com

// let _ = fnTakingInlineRecord(WithInlineRecord({}))
// ^com

// let _ = fnTakingInlineRecord(WithInlineRecord({s}))
// ^com

// let _ = fnTakingInlineRecord(WithInlineRecord({nestedRecord: }))
// ^com

// let _ = fnTakingInlineRecord(WithInlineRecord({nestedRecord: {} }))
// ^com
Loading