Skip to content
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

add broken test for pipe completion on aliased types #700

Merged
merged 4 commits into from
Jan 18, 2023
Merged

add broken test for pipe completion on aliased types #700

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2745e7f
add broken test for pipe completion on aliased types
zth Jan 15, 2023
10a3077
one step forward (and one backward) in resolving aliases in pipe comp…
zth Jan 17, 2023
7343fca
refactor pipe completion to handle builtins properly in new approach …
zth Jan 18, 2023
3c9d46f
changleog
zth Jan 18, 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 @@ -38,6 +38,7 @@

- Highlight `catch` like a keyword https://github.com/rescript-lang/rescript-vscode/pull/677
- Make signature help work in calls nested inside of other calls. https://github.com/rescript-lang/rescript-vscode/pull/687
- Fix pipe completion to work on aliased types. https://github.com/rescript-lang/rescript-vscode/pull/700

## v1.10.0

Expand Down
159 changes: 68 additions & 91 deletions analysis/src/CompletionBackEnd.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -719,89 +719,52 @@ let rec getCompletionsForContextPath ~full ~opens ~rawOpens ~allFiles ~pos ~env
~exact:true ~scope
|> completionsGetTypeEnv
with
| None -> []
| Some (typ, envFromCompletionItem) -> (
(* If the type we're completing on is a type parameter, we won't be able to do
completion unless we know what that type parameter is compiled as. This
attempts to look up the compiled type for that type parameter by looking
for compiled information at the loc of that expression. *)
let typ =
match typ with
| {Types.desc = Tvar _} -> (
match
TypeUtils.findReturnTypeOfFunctionAtLoc lhsLoc ~env ~full
~debug:false
with
| None -> typ
| Some typFromLoc -> typFromLoc)
| _ -> typ
in
let {
arrayModulePath;
optionModulePath;
stringModulePath;
intModulePath;
floatModulePath;
promiseModulePath;
listModulePath;
resultModulePath;
} =
package.builtInCompletionModules
in
let getBuiltinTypePath path =
match path with
| Path.Pident id when Ident.name id = "array" -> Some arrayModulePath
| Path.Pident id when Ident.name id = "option" -> Some optionModulePath
| Path.Pident id when Ident.name id = "string" -> Some stringModulePath
| Path.Pident id when Ident.name id = "int" -> Some intModulePath
| Path.Pident id when Ident.name id = "float" -> Some floatModulePath
| Path.Pident id when Ident.name id = "promise" ->
Some promiseModulePath
| Path.Pident id when Ident.name id = "list" -> Some listModulePath
| Path.Pident id when Ident.name id = "result" -> Some resultModulePath
| Path.Pident id when Ident.name id = "lazy_t" -> Some ["Lazy"]
| Path.Pident id when Ident.name id = "char" -> Some ["Char"]
| Pdot (Pident id, "result", _) when Ident.name id = "Pervasives" ->
Some resultModulePath
| _ -> None
in
let rec expandPath (path : Path.t) =
match path with
| Pident id -> [Ident.name id]
| Pdot (p, s, _) -> s :: expandPath p
| Papply _ -> []
in
let getTypePath typ =
match typ.Types.desc with
| Tconstr (path, _typeArgs, _)
| Tlink {desc = Tconstr (path, _typeArgs, _)}
| Tsubst {desc = Tconstr (path, _typeArgs, _)}
| Tpoly ({desc = Tconstr (path, _typeArgs, _)}, []) ->
Some path
| _ -> None
in
let rec removeRawOpen rawOpen modulePath =
match (rawOpen, modulePath) with
| [_], _ -> Some modulePath
| s :: inner, first :: restPath when s = first ->
removeRawOpen inner restPath
| _ -> None
in
let rec removeRawOpens rawOpens modulePath =
match rawOpens with
| rawOpen :: restOpens -> (
let newModulePath = removeRawOpens restOpens modulePath in
match removeRawOpen rawOpen newModulePath with
| None -> newModulePath
| Some mp -> mp)
| [] -> modulePath
let env, typ =
typ
|> TypeUtils.resolveTypeForPipeCompletion ~env ~package ~full ~lhsLoc
in
let completionPath =
match getTypePath typ with
| Some typePath -> (
match getBuiltinTypePath typePath with
| Some path -> Some path
| None -> (
match expandPath typePath with
match typ with
| Builtin (builtin, _) ->
let {
arrayModulePath;
optionModulePath;
stringModulePath;
intModulePath;
floatModulePath;
promiseModulePath;
listModulePath;
resultModulePath;
} =
package.builtInCompletionModules
in
Some
(match builtin with
| Array -> arrayModulePath
| Option -> optionModulePath
| String -> stringModulePath
| Int -> intModulePath
| Float -> floatModulePath
| Promise -> promiseModulePath
| List -> listModulePath
| Result -> resultModulePath
| Lazy -> ["Lazy"]
| Char -> ["Char"])
| TypExpr t -> (
let rec expandPath (path : Path.t) =
match path with
| Pident id -> [Ident.name id]
| Pdot (p, s, _) -> s :: expandPath p
| Papply _ -> []
in
match t.Types.desc with
| Tconstr (path, _typeArgs, _)
| Tlink {desc = Tconstr (path, _typeArgs, _)}
| Tsubst {desc = Tconstr (path, _typeArgs, _)}
| Tpoly ({desc = Tconstr (path, _typeArgs, _)}, []) -> (
match expandPath path with
| _ :: pathRev ->
(* type path is relative to the completion environment
express it from the root of the file *)
Expand All @@ -816,11 +779,27 @@ let rec getCompletionsForContextPath ~full ~opens ~rawOpens ~allFiles ~pos ~env
else envFromCompletionItem.file.moduleName :: pathFromEnv_
in
Some pathFromEnv
| _ -> None))
| None -> None
| _ -> None)
| _ -> None)
in
match completionPath with
| Some completionPath -> (
let rec removeRawOpen rawOpen modulePath =
match (rawOpen, modulePath) with
| [_], _ -> Some modulePath
| s :: inner, first :: restPath when s = first ->
removeRawOpen inner restPath
| _ -> None
in
let rec removeRawOpens rawOpens modulePath =
match rawOpens with
| rawOpen :: restOpens -> (
let newModulePath = removeRawOpens restOpens modulePath in
match removeRawOpen rawOpen newModulePath with
| None -> newModulePath
| Some mp -> mp)
| [] -> modulePath
in
let completionPathMinusOpens =
completionPath
|> removeRawOpens package.opens
Expand Down Expand Up @@ -848,17 +827,16 @@ let rec getCompletionsForContextPath ~full ~opens ~rawOpens ~allFiles ~pos ~env
(* We add React element functions to the completion if we're in a JSX context *)
let forJsxCompletion =
if inJsx then
match getTypePath typ with
| Some (Path.Pident id) when Ident.name id = "int" -> Some "int"
| Some (Path.Pident id) when Ident.name id = "float" -> Some "float"
| Some (Path.Pident id) when Ident.name id = "string" ->
Some "string"
| Some (Path.Pident id) when Ident.name id = "array" -> Some "array"
match typ with
| Builtin (Int, t) -> Some ("int", t)
| Builtin (Float, t) -> Some ("float", t)
| Builtin (String, t) -> Some ("string", t)
| Builtin (Array, t) -> Some ("array", t)
| _ -> None
else None
in
match forJsxCompletion with
| Some builtinNameToComplete
| Some (builtinNameToComplete, typ)
when Utils.checkName builtinNameToComplete ~prefix:funNamePrefix
~exact:false ->
[
Expand All @@ -874,8 +852,7 @@ let rec getCompletionsForContextPath ~full ~opens ~rawOpens ~allFiles ~pos ~env
]
@ completions
| _ -> completions)
| None -> [])
| None -> [])
| None -> []))
| CTuple ctxPaths ->
(* Turn a list of context paths into a list of type expressions. *)
let typeExrps =
Expand Down
93 changes: 85 additions & 8 deletions analysis/src/TypeUtils.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,91 @@ let rec extractType ~env ~package (t : Types.type_expr) =
Some (Tpolyvariant {env; constructors; typeExpr = t})
| _ -> None

let findReturnTypeOfFunctionAtLoc loc ~(env : QueryEnv.t) ~full ~debug =
match References.getLocItem ~full ~pos:(loc |> Loc.end_) ~debug with
| Some {locType = Typed (_, typExpr, _)} -> (
match extractFunctionType ~env ~package:full.package typExpr with
| args, tRet when args <> [] -> Some tRet
| _ -> None)
| _ -> None

type builtinType =
| Array
| Option
| String
| Int
| Float
| Promise
| List
| Result
| Lazy
| Char

type pipeCompletionType =
| Builtin of builtinType * Types.type_expr
| TypExpr of Types.type_expr

let getBuiltinFromTypePath path =
match path with
| Path.Pident id when Ident.name id = "array" -> Some Array
| Path.Pident id when Ident.name id = "option" -> Some Option
| Path.Pident id when Ident.name id = "string" -> Some String
| Path.Pident id when Ident.name id = "int" -> Some Int
| Path.Pident id when Ident.name id = "float" -> Some Float
| Path.Pident id when Ident.name id = "promise" -> Some Promise
| Path.Pident id when Ident.name id = "list" -> Some List
| Path.Pident id when Ident.name id = "result" -> Some Result
| Path.Pident id when Ident.name id = "lazy_t" -> Some Lazy
| Path.Pident id when Ident.name id = "char" -> Some Char
| Pdot (Pident id, "result", _) when Ident.name id = "Pervasives" ->
Some Result
| _ -> None

let rec resolveTypeForPipeCompletion ~env ~package ~lhsLoc ~full
(t : Types.type_expr) =
let builtin =
match t.desc with
| Tconstr (path, _typeArgs, _)
| Tlink {desc = Tconstr (path, _typeArgs, _)}
| Tsubst {desc = Tconstr (path, _typeArgs, _)}
| Tpoly ({desc = Tconstr (path, _typeArgs, _)}, []) ->
path |> getBuiltinFromTypePath
| _ -> None
in
match builtin with
| Some builtin -> (env, Builtin (builtin, t))
| None -> (
(* If the type we're completing on is a type parameter, we won't be able to
do completion unless we know what that type parameter is compiled as.
This attempts to look up the compiled type for that type parameter by
looking for compiled information at the loc of that expression. *)
let typFromLoc =
match t with
| {Types.desc = Tvar _} -> (
match findReturnTypeOfFunctionAtLoc lhsLoc ~env ~full ~debug:false with
| None -> None
| Some typFromLoc -> Some typFromLoc)
| _ -> None
in
match typFromLoc with
| Some typFromLoc ->
typFromLoc |> resolveTypeForPipeCompletion ~lhsLoc ~env ~package ~full
| None ->
let rec digToRelevantType ~env ~package (t : Types.type_expr) =
match t.desc with
| Tlink t1 | Tsubst t1 | Tpoly (t1, []) ->
digToRelevantType ~env ~package t1
(* Don't descend into types named "t". Type t is a convention in the ReScript ecosystem. *)
| Tconstr (path, _, _) when path |> Path.last = "t" -> (env, TypExpr t)
Copy link
Collaborator

@cristianoc cristianoc Jan 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

What happens in practice if this case is omitted?
I was thinking of the different cases. E.g. when it's an alias to array, whether one wants to complete for array. And when it's a record definition, then one wants to treat is as the source of truth.
Just thinking aloud here, about what the convention could look like.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I added this case because it broke 2 tests in Completion.res, and looking at them I felt like the old behavior was better.

Yeah, I agree. I don't feel like there's a clear and good answer here, but I do lean a bit towards the current behavior, as in type t usually means the main type, and if it's in a module there's often functions in the module to work on it. But I agree it's not 100%.

One idea could be to complete for the module with the type t, and the aliased value. Unsure if that'd be messy to implement, but it might be good behavior. What do you think?

Merging since it doesn't change current behavior.

| Tconstr (path, _, _) -> (
match References.digConstructor ~env ~package path with
| Some (env, {item = {decl = {type_manifest = Some typ}}}) ->
digToRelevantType ~env ~package typ
| _ -> (env, TypExpr t))
| _ -> (env, TypExpr t)
in
digToRelevantType ~env ~package t)

(** 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 : completionType) ~env ~package ~nested =
match nested with
Expand Down Expand Up @@ -211,14 +296,6 @@ let rec resolveNested (typ : completionType) ~env ~package ~nested =
TypeExpr typ |> resolveNested ~env ~package ~nested
| _ -> None)

let findReturnTypeOfFunctionAtLoc loc ~(env : QueryEnv.t) ~full ~debug =
match References.getLocItem ~full ~pos:(loc |> Loc.end_) ~debug with
| Some {locType = Typed (_, typExpr, _)} -> (
match extractFunctionType ~env ~package:full.package typExpr with
| args, tRet when args <> [] -> Some tRet
| _ -> None)
| _ -> None

let getArgs ~env (t : Types.type_expr) ~full =
let rec getArgsLoop ~env (t : Types.type_expr) ~full ~currentArgumentPosition
=
Expand Down
11 changes: 11 additions & 0 deletions analysis/tests/src/CompletionPipeChain.res
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,14 @@ let _ = [123]->Js.Array2.forEach(v => Js.log(v))
let _ = [123]->Belt.Array.reduce(0, (acc, curr) => acc + curr)
// ->t
// ^com

type aliasedType = CompletionSupport.Test.t

let aliased: aliasedType = {name: 123}
let notAliased: CompletionSupport.Test.t = {name: 123}

// aliased->
// ^com

// notAliased->
// ^com
46 changes: 46 additions & 0 deletions analysis/tests/src/expected/CompletionPipeChain.res.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,3 +256,49 @@ Completable: Cpath Value[Belt, Array, reduce](Nolabel, Nolabel, Nolabel)->t
"documentation": {"kind": "markdown", "value": "\n Converts a given `int` to a `float`.\n\n ```res example\n Js.log(Belt.Int.toFloat(1) === 1.0) /* true */\n ```\n"}
}]

Complete src/CompletionPipeChain.res 70:12
posCursor:[70:12] posNoWhite:[70:11] Found expr:[70:3->0:-1]
Completable: Cpath Value[aliased]->
[{
"label": "CompletionSupport.Test.add",
"kind": 12,
"tags": [],
"detail": "t => int",
"documentation": null
}, {
"label": "CompletionSupport.Test.addSelf",
"kind": 12,
"tags": [],
"detail": "t => t",
"documentation": null
}, {
"label": "CompletionSupport.Test.make",
"kind": 12,
"tags": [],
"detail": "int => t",
"documentation": null
}]

Complete src/CompletionPipeChain.res 73:15
posCursor:[73:15] posNoWhite:[73:14] Found expr:[73:3->0:-1]
Completable: Cpath Value[notAliased]->
[{
"label": "CompletionSupport.Test.add",
"kind": 12,
"tags": [],
"detail": "t => int",
"documentation": null
}, {
"label": "CompletionSupport.Test.addSelf",
"kind": 12,
"tags": [],
"detail": "t => t",
"documentation": null
}, {
"label": "CompletionSupport.Test.make",
"kind": 12,
"tags": [],
"detail": "int => t",
"documentation": null
}]