Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
- Add (dev-)dependencies to build schema. https://github.com/rescript-lang/rescript/pull/7892
- Dedicated error for dict literal spreads. https://github.com/rescript-lang/rescript/pull/7901
- Dedicated error message for when mixing up `:` and `=` in various positions. https://github.com/rescript-lang/rescript/pull/7900
- Add completions for `throw`. https://github.com/rescript-lang/rescript/pull/7905

#### :house: Internal

Expand Down
102 changes: 100 additions & 2 deletions analysis/src/CompletionBackEnd.ml
Original file line number Diff line number Diff line change
Expand Up @@ -756,6 +756,101 @@ let getCompletionsForPath ~debug ~opens ~full ~pos ~exact ~scope
findAllCompletions ~env ~prefix ~exact ~namesUsed ~completionContext
| None -> []))

(* Collect exception constructor names from cmt infos. *)
let exceptions_from_cmt_infos (infos : Cmt_format.cmt_infos) :
Copy link
Member Author

Choose a reason for hiding this comment

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

This part is vibe coded I'm afraid.
Please review it and let me know if there is a better way.

Copy link
Member

Choose a reason for hiding this comment

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

Can a typed tree AST iterator be used here instead of the hand rolled traversal functions below? Would feel more idiomatic, and make the code clearer as you could just add a visitor for the relevant extension nodes.

(string * bool) list =
let by_name : (string, bool) Hashtbl.t = Hashtbl.create 16 in
let add_ext (ext : Typedtree.extension_constructor) : unit =
let name = ext.ext_name.txt in
let hasArgs =
match ext.ext_kind with
| Text_decl (Cstr_tuple args, _ret) -> args <> []
| Text_decl (Cstr_record fields, _ret) -> fields <> []
| Text_rebind _ -> true
in
let prev =
match Hashtbl.find_opt by_name name with
| Some b -> b
| None -> false
in
Hashtbl.replace by_name name (prev || hasArgs)
in
let rec of_structure_items (items : Typedtree.structure_item list) : unit =
match items with
| [] -> ()
| item :: rest ->
(match item.str_desc with
| Tstr_exception ext -> add_ext ext
| _ -> ());
of_structure_items rest
in
let rec of_signature_items (items : Typedtree.signature_item list) : unit =
match items with
| [] -> ()
| item :: rest ->
(match item.sig_desc with
| Tsig_exception ext -> add_ext ext
| _ -> ());
of_signature_items rest
in
let of_parts (parts : Cmt_format.binary_part array) : unit =
Array.iter
(function
| Cmt_format.Partial_structure s -> of_structure_items s.str_items
| Partial_structure_item si -> of_structure_items [si]
| Partial_signature s -> of_signature_items s.sig_items
| Partial_signature_item si -> of_signature_items [si]
| _ -> ())
parts
in
(match infos.cmt_annots with
| Cmt_format.Implementation s -> of_structure_items s.str_items
| Interface s -> of_signature_items s.sig_items
| Partial_implementation parts -> of_parts parts
| Partial_interface parts -> of_parts parts
| _ -> ());
Hashtbl.fold (fun name hasArgs acc -> (name, hasArgs) :: acc) by_name []

(* Predefined Stdlib/Pervasives exceptions. *)
let predefined_exceptions : (string * bool) list =
[
("Not_found", true);
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

I can't help but wonder whether we should be encouraging using these builtins. They're really just heritage from the OCaml era. So I think a case could be made to not include them at all.

("Invalid_argument", true);
("Assert_failure", true);
("Failure", true);
("Match_failure", true);
("Division_by_zero", false);
]

let completionsForThrow ~(env : QueryEnv.t) ~full =
let exn_typ = Ctype.newconstr Predef.path_exn [] in
let names_from_cmt =
let moduleName = env.file.moduleName in
match Hashtbl.find_opt full.package.pathsForModule moduleName with
| None -> []
| Some paths -> (
let uri = getUri paths in
let cmt_path = getCmtPath ~uri paths in
match Shared.tryReadCmt cmt_path with
| None -> []
| Some infos -> exceptions_from_cmt_infos infos)
in
let all = names_from_cmt @ predefined_exceptions in
all
|> List.map (fun (name, hasArgs) ->
let insertText =
if hasArgs then Printf.sprintf "throw(%s($0))" name
else Printf.sprintf "throw(%s)" name
in
let isBuiltin = List.mem (name, hasArgs) predefined_exceptions in
let detail =
if isBuiltin then "Built-in Exception" else "User-defined Exception"
in
Completion.create
(Printf.sprintf "throw(%s)" name)
~env ~kind:(Completion.Value exn_typ) ~includesSnippets:true
~insertText ~filterText:"throw" ~detail)

(** Completions intended for piping, from a completion path. *)
let completionsForPipeFromCompletionPath ~envCompletionIsMadeFrom ~opens ~pos
~scope ~debug ~prefix ~env ~rawOpens ~full completionPath =
Expand Down Expand Up @@ -1010,7 +1105,7 @@ and getCompletionsForContextPath ~debug ~full ~opens ~rawOpens ~pos ~env ~exact
| Some (Tpromise (env, typ), _env) ->
[Completion.create "dummy" ~env ~kind:(Completion.Value typ)]
| _ -> [])
| CPId {path; completionContext; loc} ->
| CPId {path; completionContext; loc} -> (
if Debug.verbose () then print_endline "[ctx_path]--> CPId";
(* Looks up the type of an identifier.

Expand Down Expand Up @@ -1048,7 +1143,10 @@ and getCompletionsForContextPath ~debug ~full ~opens ~rawOpens ~pos ~env ~exact
| _ -> byPath
else byPath
in
result
match (result, path) with
| [], [prefix] when Utils.startsWith "throw" prefix ->
completionsForThrow ~env ~full
| _ -> result)
Comment on lines +1083 to +1086
Copy link
Member

Choose a reason for hiding this comment

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

This should probably be done in a "safer" way that matches by type and not by name. We'd need to check if this thing actually resolves to Pervasives.throw.

Also, why Utils.startsWith? Is it not supposed to be an exact match?

Copy link
Member

Choose a reason for hiding this comment

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

@nojaf this has not been answered I believe.

Copy link
Member Author

Choose a reason for hiding this comment

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

We'd need to check if this thing actually resolves to Pervasives.throw.

If you only have thr you can't do that.

Also, why Utils.startsWith? Is it not supposed to be an exact match?

As mentioned, I want the completions to show up while I'm typing towards the word throw. Only having it once the full word is typed I find unintuitive.

| CPApply (cp, labels) -> (
if Debug.verbose () then print_endline "[ctx_path]--> CPApply";
match
Expand Down
5 changes: 5 additions & 0 deletions tests/analysis_tests/tests/src/Throw.res
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
exception MyCustomThingToThrow(string)
exception NoArgsToThrow

// let x = () => thro
// ^com
10 changes: 10 additions & 0 deletions tests/analysis_tests/tests/src/expected/Completion.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2147,6 +2147,16 @@ Path T
"modulePath": "TableclothMap",
"filePath": "src/Completion.res"
}
}, {
"label": "Throw",
"kind": 9,
"tags": [],
"detail": "module Throw",
"documentation": null,
"data": {
"modulePath": "Throw",
"filePath": "src/Completion.res"
}
}, {
"label": "TypeArgCtx",
"kind": 9,
Expand Down
10 changes: 10 additions & 0 deletions tests/analysis_tests/tests/src/expected/CompletionJsxProps.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,16 @@ Path CompletionSupport.TestComponent.make
"modulePath": "TableclothMap",
"filePath": "src/CompletionJsxProps.res"
}
}, {
"label": "Throw",
"kind": 9,
"tags": [],
"detail": "module Throw",
"documentation": null,
"data": {
"modulePath": "Throw",
"filePath": "src/CompletionJsxProps.res"
}
}, {
"label": "TypeArgCtx",
"kind": 9,
Expand Down
83 changes: 83 additions & 0 deletions tests/analysis_tests/tests/src/expected/Throw.res.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
Complete src/Throw.res 3:21
posCursor:[3:21] posNoWhite:[3:20] Found expr:[3:11->3:21]
posCursor:[3:21] posNoWhite:[3:20] Found expr:[3:17->3:21]
Pexp_ident thro:[3:17->3:21]
Completable: Cpath Value[thro]
Package opens Stdlib.place holder Pervasives.JsxModules.place holder
Resolved opens 1 Stdlib
ContextPath Value[thro]
Path thro
[{
"label": "throw(MyCustomThingToThrow)",
"kind": 12,
"tags": [],
"detail": "User-defined Exception",
"documentation": null,
"filterText": "throw",
"insertText": "throw(MyCustomThingToThrow($0))",
"insertTextFormat": 2
}, {
"label": "throw(NoArgsToThrow)",
"kind": 12,
"tags": [],
"detail": "User-defined Exception",
"documentation": null,
"filterText": "throw",
"insertText": "throw(NoArgsToThrow)",
"insertTextFormat": 2
}, {
"label": "throw(Not_found)",
"kind": 12,
"tags": [],
"detail": "Built-in Exception",
"documentation": null,
"filterText": "throw",
"insertText": "throw(Not_found($0))",
"insertTextFormat": 2
}, {
"label": "throw(Invalid_argument)",
"kind": 12,
"tags": [],
"detail": "Built-in Exception",
"documentation": null,
"filterText": "throw",
"insertText": "throw(Invalid_argument($0))",
"insertTextFormat": 2
}, {
"label": "throw(Assert_failure)",
"kind": 12,
"tags": [],
"detail": "Built-in Exception",
"documentation": null,
"filterText": "throw",
"insertText": "throw(Assert_failure($0))",
"insertTextFormat": 2
}, {
"label": "throw(Failure)",
"kind": 12,
"tags": [],
"detail": "Built-in Exception",
"documentation": null,
"filterText": "throw",
"insertText": "throw(Failure($0))",
"insertTextFormat": 2
}, {
"label": "throw(Match_failure)",
"kind": 12,
"tags": [],
"detail": "Built-in Exception",
"documentation": null,
"filterText": "throw",
"insertText": "throw(Match_failure($0))",
"insertTextFormat": 2
}, {
"label": "throw(Division_by_zero)",
"kind": 12,
"tags": [],
"detail": "Built-in Exception",
"documentation": null,
"filterText": "throw",
"insertText": "throw(Division_by_zero)",
"insertTextFormat": 2
}]

Loading