Skip to content

Print exotic identifiers properly when completing polyvariants #870

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

Merged
merged 5 commits into from
Dec 18, 2023
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -23,6 +23,7 @@
- Fix infinite loop when resolving inferred completions when several values in scope has the same name. https://github.com/rescript-lang/rescript-vscode/pull/869
- Fix crash when trying to print recursive polymorphic variants without a concrete definition. https://github.com/rescript-lang/rescript-vscode/pull/851
- Fix `rescript-language-server --version` command. https://github.com/rescript-lang/rescript-vscode/pull/873
- Print exotic polyvariant constructor names with quotes when doing completion. https://github.com/rescript-lang/rescript-vscode/pull/870

#### :nail_care: Polish

Expand Down
10 changes: 5 additions & 5 deletions analysis/src/CompletionBackEnd.ml
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,8 @@ let kindToDetail name (kind : Completion.kind) =
^ "\n\n" ^ s
else name ^ ": " ^ (typ |> Shared.typeToString) ^ "\n\n" ^ s
| Constructor (c, s) -> showConstructor c ^ "\n\n" ^ s
| PolyvariantConstructor ({name; args}, s) ->
"#" ^ name
| PolyvariantConstructor ({displayName; args}, s) ->
"#" ^ displayName
^ (match args with
| [] -> ""
| typeExprs ->
Expand Down Expand Up @@ -1223,13 +1223,13 @@ let rec completeTypedValue ~full ~prefix ~completionContext ~mode
|> List.map (fun (constructor : polyVariantConstructor) ->
Completion.createWithSnippet
~name:
("#" ^ constructor.name
("#" ^ constructor.displayName
^ printConstructorArgs
(List.length constructor.args)
~asSnippet:false)
~insertText:
((if Utils.startsWith prefix "#" then "" else "#")
^ constructor.name
^ constructor.displayName
^ printConstructorArgs
(List.length constructor.args)
~asSnippet:true)
Expand Down Expand Up @@ -1771,7 +1771,7 @@ let rec processCompletable ~debug ~full ~scope ~env ~pos ~forHover completable =
~cases:
(v.constructors
|> List.map (fun (constructor : polyVariantConstructor) ->
"#" ^ constructor.name
"#" ^ constructor.displayName
^
match constructor.args with
| [] -> ""
Expand Down
6 changes: 5 additions & 1 deletion analysis/src/SharedTypes.ml
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,11 @@ end = struct
{env with exported = structure.exported; pathRev; parent = Some env}
end

type polyVariantConstructor = {name: string; args: Types.type_expr list}
type polyVariantConstructor = {
name: string;
displayName: string;
args: Types.type_expr list;
}

type innerType = TypeExpr of Types.type_expr | ExtractedType of completionType
and completionType =
Expand Down
3 changes: 2 additions & 1 deletion analysis/src/TypeUtils.ml
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ let rec extractType ~env ~package (t : Types.type_expr) =
|> List.map (fun (label, field) ->
{
name = label;
displayName = Utils.printMaybeExoticIdent ~allowUident:true label;
args =
(* Multiple arguments are represented as a Ttuple, while a single argument is just the type expression itself. *)
(match field with
Expand Down Expand Up @@ -691,7 +692,7 @@ module Codegen = struct
(match c.args with
| [] -> None
| _ -> Some (any ()))
c.name))
c.displayName))
| Toption (_, innerType) ->
let extractedType =
match innerType with
Expand Down
17 changes: 17 additions & 0 deletions analysis/src/Utils.ml
Original file line number Diff line number Diff line change
Expand Up @@ -243,3 +243,20 @@ let rec flattenAnyNamespaceInPath path =
(* Namespaces are in reverse order, so "URL-RescriptBun" where RescriptBun is the namespace. *)
(parts |> List.rev) @ flattenAnyNamespaceInPath tail
else head :: flattenAnyNamespaceInPath tail

let printMaybeExoticIdent ?(allowUident = false) txt =
let len = String.length txt in

let rec loop i =
if i == len then txt
else if i == 0 then
match String.unsafe_get txt i with
| 'A' .. 'Z' when allowUident -> loop (i + 1)
| 'a' .. 'z' | '_' -> loop (i + 1)
| _ -> "\"" ^ txt ^ "\""
else
match String.unsafe_get txt i with
| 'A' .. 'Z' | 'a' .. 'z' | '0' .. '9' | '\'' | '_' -> loop (i + 1)
| _ -> "\"" ^ txt ^ "\""
in
loop 0
9 changes: 9 additions & 0 deletions analysis/tests/src/CompletionExpressions.res
Original file line number Diff line number Diff line change
Expand Up @@ -262,3 +262,12 @@ let arr = ["hello"]

// arr->Belt.Array.map()
// ^com

type exoticPolyvariant = [#"some exotic"]

let takesExotic = (e: exoticPolyvariant) => {
ignore(e)
}

// takesExotic()
// ^com
2 changes: 1 addition & 1 deletion analysis/tests/src/ExhaustiveSwitch.res
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
type someVariant = One | Two | Three(option<bool>)
type somePolyVariant = [#one | #two | #three(option<bool>)]
type somePolyVariant = [#one | #two | #three(option<bool>) | #"exotic ident"]

let withSomeVariant = One
let withSomePoly: somePolyVariant = #one
Expand Down
19 changes: 19 additions & 0 deletions analysis/tests/src/expected/CompletionExpressions.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1113,3 +1113,22 @@ Path Belt.Array.map
"insertTextFormat": 2
}]

Complete src/CompletionExpressions.res 271:15
posCursor:[271:15] posNoWhite:[271:14] Found expr:[271:3->271:16]
Pexp_apply ...[271:3->271:14] (...[271:15->271:16])
Completable: Cexpression CArgument Value[takesExotic]($0)
Package opens Pervasives.JsxModules.place holder
Resolved opens 1 pervasives
ContextPath CArgument Value[takesExotic]($0)
ContextPath Value[takesExotic]
Path takesExotic
[{
"label": "#\"some exotic\"",
"kind": 4,
"tags": [],
"detail": "#\"some exotic\"\n\n[#\"some exotic\"]",
"documentation": null,
"insertText": "#\"some exotic\"",
"insertTextFormat": 2
}]

2 changes: 1 addition & 1 deletion analysis/tests/src/expected/ExhaustiveSwitch.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ Path withSomePol
"detail": "insert exhaustive switch for value",
"documentation": null,
"filterText": "withSomePoly",
"insertText": "withSomePoly {\n | #one => ${1:failwith(\"todo\")}\n | #three(_) => ${2:failwith(\"todo\")}\n | #two => ${3:failwith(\"todo\")}\n }",
"insertText": "withSomePoly {\n | #one => ${1:failwith(\"todo\")}\n | #three(_) => ${2:failwith(\"todo\")}\n | #two => ${3:failwith(\"todo\")}\n | #\"exotic ident\" => ${4:failwith(\"todo\")}\n }",
"insertTextFormat": 2
}]

Expand Down