-
Notifications
You must be signed in to change notification settings - Fork 471
Add completion for throw keyword #7905
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
cb88ad9
4d7ce2e
497b372
678220d
69bc0d0
5f1e834
5cd2754
d23722d
7dc6ff1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -793,3 +793,77 @@ let fileForModule moduleName ~package = | |
| None -> | ||
Log.log ("No path for module " ^ moduleName); | ||
None | ||
|
||
(* Collect top-level exception constructors from typedtree/CMT file. *) | ||
let exceptionsForCmt ~cmt : (string * bool) list = | ||
match Shared.tryReadCmt cmt with | ||
| None -> [] | ||
| Some infos -> | ||
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 | ||
(* Only collect top-level exception declarations (Tstr_exception/Tsig_exception). | ||
Avoid picking up exceptions from Texp_letexception by tracking context. *) | ||
Comment on lines
+818
to
+819
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In theory I guess the most robust approach would be to collect and use whatever defined exceptions are in scope at the current position. Collecting at the top level is good and simple, but you could also define them in sub modules, and so on. It might be worth exploring making exceptions part of the tracked scope instead, and track them just like we track types and values today when doing completion. Then we could just extend the current mechanism a bit, and hopefully get the correct scope tracking for free. |
||
let in_toplevel_exception = ref false in | ||
let module Iter = TypedtreeIter.MakeIterator (struct | ||
include TypedtreeIter.DefaultIteratorArgument | ||
Comment on lines
+821
to
+822
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we use the functor based approach elsewhere in the code base? Or is there no way for the typed tree to just define a mapper like we do for the AST mapper? I don't remember how this works exactly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this was weird to me as well, it is a departure from what we have on the untyped side, but this seems to be the mapper we have? |
||
|
||
let enter_structure_item (item : Typedtree.structure_item) = | ||
(match item.str_desc with | ||
| Tstr_exception _ -> in_toplevel_exception := true | ||
| _ -> ()); | ||
() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is there an extra unit here...? |
||
|
||
let leave_structure_item (_ : Typedtree.structure_item) = | ||
in_toplevel_exception := false | ||
|
||
let enter_signature_item (item : Typedtree.signature_item) = | ||
(match item.sig_desc with | ||
| Tsig_exception _ -> in_toplevel_exception := true | ||
| _ -> ()); | ||
() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same, extra unit. |
||
|
||
let leave_signature_item (_ : Typedtree.signature_item) = | ||
in_toplevel_exception := false | ||
|
||
let enter_extension_constructor (ext : Typedtree.extension_constructor) = | ||
if !in_toplevel_exception then add_ext ext | ||
end) in | ||
let () = | ||
match infos.cmt_annots with | ||
| Cmt_format.Implementation s -> Iter.iter_structure s | ||
| Interface s -> Iter.iter_signature s | ||
| Partial_implementation parts -> | ||
Array.iter | ||
(function | ||
| Cmt_format.Partial_structure s -> Iter.iter_structure s | ||
| Partial_structure_item si -> Iter.iter_structure_item si | ||
| Partial_signature s -> Iter.iter_signature s | ||
| Partial_signature_item si -> Iter.iter_signature_item si | ||
| _ -> ()) | ||
parts | ||
| Partial_interface parts -> | ||
Array.iter | ||
(function | ||
| Cmt_format.Partial_structure s -> Iter.iter_structure s | ||
| Partial_structure_item si -> Iter.iter_structure_item si | ||
| Partial_signature s -> Iter.iter_signature s | ||
| Partial_signature_item si -> Iter.iter_signature_item si | ||
| _ -> ()) | ||
parts | ||
| _ -> () | ||
in | ||
Hashtbl.fold (fun name hasArgs acc -> (name, hasArgs) :: acc) by_name [] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
exception MyCustomThingToThrow(string) | ||
exception NoArgsToThrow | ||
|
||
// let x = () => thro | ||
// ^com |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
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": "JsError.throwWithMessage", | ||
"kind": 12, | ||
"tags": [], | ||
"detail": "Throw a JavaScript error, example: `throw new Error(str)`", | ||
"documentation": null, | ||
"insertText": "JsError.throwWithMessage(\"$0\")", | ||
"insertTextFormat": 2 | ||
}, { | ||
"label": "JsExn.throw", | ||
"kind": 12, | ||
"tags": [], | ||
"detail": "Throw any JavaScript value, example: `throw 100`", | ||
"documentation": null, | ||
"insertText": "JsExn.throw($0)", | ||
"insertTextFormat": 2 | ||
}, { | ||
"label": "throw(MyCustomThingToThrow)", | ||
"kind": 12, | ||
"tags": [], | ||
"detail": "exn", | ||
"documentation": null, | ||
"filterText": "throw", | ||
"insertText": "throw(MyCustomThingToThrow($0))", | ||
"insertTextFormat": 2 | ||
}, { | ||
"label": "throw(NoArgsToThrow)", | ||
"kind": 12, | ||
"tags": [], | ||
"detail": "exn", | ||
"documentation": null, | ||
"filterText": "throw", | ||
"insertText": "throw(NoArgsToThrow)", | ||
"insertTextFormat": 2 | ||
}] | ||
|
There was a problem hiding this comment.
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?There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you only have
thr
you can't do that.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.