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.

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

Track scope properly when inferring values #869

Merged
merged 4 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 @@ -20,6 +20,7 @@

- Proper default for `"uncurried"` in V11 projects. https://github.com/rescript-lang/rescript-vscode/pull/867
- Treat `result` type as a proper built in type. https://github.com/rescript-lang/rescript-vscode/pull/860
- 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

#### :nail_care: Polish

Expand Down
42 changes: 20 additions & 22 deletions analysis/src/CompletionBackEnd.ml
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ let findAllCompletions ~(env : QueryEnv.t) ~prefix ~exact ~namesUsed
completionForExportedFields ~env ~prefix ~exact ~namesUsed
@ completionForExportedModules ~env ~prefix ~exact ~namesUsed

let processLocalValue name loc contextPath ~prefix ~exact ~env
let processLocalValue name loc contextPath scope ~prefix ~exact ~env
~(localTables : LocalTables.t) =
if Utils.checkName name ~prefix ~exact then
match Hashtbl.find_opt localTables.valueTable (name, Loc.start loc) with
Expand All @@ -279,14 +279,14 @@ let processLocalValue name loc contextPath ~prefix ~exact ~env
}
:: localTables.resultRev)
| None ->
Log.log
(Printf.sprintf "Completion Value Not Found %s loc:%s\n" name
(Loc.toString loc));
if !Cfg.debugFollowCtxPath then
Printf.printf "Completion Value Not Found %s loc:%s\n" name
(Loc.toString loc);
localTables.resultRev <-
Completion.create name ~env
~kind:
(match contextPath with
| Some contextPath -> FollowContextPath contextPath
| Some contextPath -> FollowContextPath (contextPath, scope)
| None ->
Value
(Ctype.newconstr
Expand Down Expand Up @@ -622,17 +622,17 @@ let completionsGetCompletionType ~full = function
| {Completion.kind = ExtractedType (typ, _); env} :: _ -> Some (typ, env)
| _ -> None

let rec completionsGetCompletionType2 ~debug ~full ~opens ~rawOpens ~pos ~scope
= function
let rec completionsGetCompletionType2 ~debug ~full ~opens ~rawOpens ~pos =
function
| {Completion.kind = Value typ; env} :: _
| {Completion.kind = ObjLabel typ; env} :: _
| {Completion.kind = Field ({typ}, _); env} :: _ ->
Some (TypeExpr typ, env)
| {Completion.kind = FollowContextPath ctxPath; env} :: _ ->
| {Completion.kind = FollowContextPath (ctxPath, scope); env} :: _ ->
ctxPath
|> getCompletionsForContextPath ~debug ~full ~env ~exact:true ~opens
~rawOpens ~pos ~scope
|> completionsGetCompletionType2 ~debug ~full ~opens ~rawOpens ~pos ~scope
|> completionsGetCompletionType2 ~debug ~full ~opens ~rawOpens ~pos
| {Completion.kind = Type typ; env} :: _ -> (
match TypeUtils.extractTypeFromResolvedType typ ~env ~full with
| None -> None
Expand All @@ -642,16 +642,16 @@ let rec completionsGetCompletionType2 ~debug ~full ~opens ~rawOpens ~pos ~scope
| _ -> None

and completionsGetTypeEnv2 ~debug (completions : Completion.t list) ~full ~opens
~rawOpens ~pos ~scope =
~rawOpens ~pos =
match completions with
| {Completion.kind = Value typ; env} :: _ -> Some (typ, env)
| {Completion.kind = ObjLabel typ; env} :: _ -> Some (typ, env)
| {Completion.kind = Field ({typ}, _); env} :: _ -> Some (typ, env)
| {Completion.kind = FollowContextPath ctxPath; env} :: _ ->
| {Completion.kind = FollowContextPath (ctxPath, scope); env} :: _ ->
ctxPath
|> getCompletionsForContextPath ~debug ~full ~opens ~rawOpens ~pos ~env
~exact:true ~scope
|> completionsGetTypeEnv2 ~debug ~full ~opens ~rawOpens ~pos ~scope
|> completionsGetTypeEnv2 ~debug ~full ~opens ~rawOpens ~pos
| _ -> None

and getCompletionsForContextPath ~debug ~full ~opens ~rawOpens ~pos ~env ~exact
Expand Down Expand Up @@ -754,7 +754,7 @@ and getCompletionsForContextPath ~debug ~full ~opens ~rawOpens ~pos ~env ~exact
cp
|> getCompletionsForContextPath ~debug ~full ~opens ~rawOpens ~pos ~env
~exact:true ~scope
|> completionsGetCompletionType2 ~debug ~full ~opens ~rawOpens ~pos ~scope
|> completionsGetCompletionType2 ~debug ~full ~opens ~rawOpens ~pos
with
| Some ((TypeExpr typ | ExtractedType (Tfunction {typ})), env) -> (
let rec reconstructFunctionType args tRet =
Expand Down Expand Up @@ -805,7 +805,6 @@ and getCompletionsForContextPath ~debug ~full ~opens ~rawOpens ~pos ~env ~exact
match
completionsForCtxPath
|> completionsGetCompletionType2 ~debug ~full ~opens ~rawOpens ~pos
~scope
with
| Some (TypeExpr typ, env) -> (
match typ |> TypeUtils.extractRecordType ~env ~package with
Expand Down Expand Up @@ -839,7 +838,7 @@ and getCompletionsForContextPath ~debug ~full ~opens ~rawOpens ~pos ~env ~exact
cp
|> getCompletionsForContextPath ~debug ~full ~opens ~rawOpens ~pos ~env
~exact:true ~scope
|> completionsGetTypeEnv2 ~debug ~full ~opens ~rawOpens ~pos ~scope
|> completionsGetTypeEnv2 ~debug ~full ~opens ~rawOpens ~pos
with
| Some (typ, env) -> (
match typ |> TypeUtils.extractObjectType ~env ~package with
Expand All @@ -866,7 +865,7 @@ and getCompletionsForContextPath ~debug ~full ~opens ~rawOpens ~pos ~env ~exact
cp
|> getCompletionsForContextPath ~debug ~full ~opens ~rawOpens ~pos ~env
~exact:true ~scope ~mode:Pipe
|> completionsGetTypeEnv2 ~debug ~full ~opens ~rawOpens ~pos ~scope
|> completionsGetTypeEnv2 ~debug ~full ~opens ~rawOpens ~pos
with
| None -> []
| Some (typ, envFromCompletionItem) -> (
Expand Down Expand Up @@ -1031,7 +1030,7 @@ and getCompletionsForContextPath ~debug ~full ~opens ~rawOpens ~pos ~env ~exact
path
|> getCompletionsForPath ~debug ~completionContext:Value ~exact:true
~package ~opens ~full ~pos ~env ~scope
|> completionsGetTypeEnv2 ~debug ~full ~opens ~rawOpens ~pos ~scope
|> completionsGetTypeEnv2 ~debug ~full ~opens ~rawOpens ~pos
in
let lowercaseComponent =
match pathToComponent with
Expand Down Expand Up @@ -1078,7 +1077,6 @@ and getCompletionsForContextPath ~debug ~full ~opens ~rawOpens ~pos ~env ~exact
|> getCompletionsForContextPath ~debug ~full ~opens ~rawOpens ~pos ~env
~exact:true ~scope
|> completionsGetCompletionType2 ~debug ~full ~opens ~rawOpens ~pos
~scope
with
| Some ((TypeExpr typ | ExtractedType (Tfunction {typ})), env) ->
(typ |> TypeUtils.getArgs ~full ~env, env)
Expand Down Expand Up @@ -1115,7 +1113,7 @@ and getCompletionsForContextPath ~debug ~full ~opens ~rawOpens ~pos ~env ~exact
rootCtxPath
|> getCompletionsForContextPath ~debug ~full ~opens ~rawOpens ~pos ~env
~exact:true ~scope
|> completionsGetCompletionType2 ~debug ~full ~opens ~rawOpens ~pos ~scope
|> completionsGetCompletionType2 ~debug ~full ~opens ~rawOpens ~pos
with
| Some (typ, env) -> (
match typ |> TypeUtils.resolveNestedPatternPath ~env ~full ~nested with
Expand Down Expand Up @@ -1528,7 +1526,7 @@ let rec processCompletable ~debug ~full ~scope ~env ~pos ~forHover completable =
path
|> getCompletionsForPath ~debug ~completionContext:Value ~exact:true
~package ~opens ~full ~pos ~env ~scope
|> completionsGetTypeEnv2 ~debug ~full ~opens ~rawOpens ~pos ~scope
|> completionsGetTypeEnv2 ~debug ~full ~opens ~rawOpens ~pos
in
match completable with
| Cnone -> []
Expand Down Expand Up @@ -1603,7 +1601,7 @@ let rec processCompletable ~debug ~full ~scope ~env ~pos ~forHover completable =
cp
|> getCompletionsForContextPath ~debug ~full ~opens ~rawOpens ~pos ~env
~exact:true ~scope
|> completionsGetTypeEnv2 ~debug ~full ~opens ~rawOpens ~pos ~scope
|> completionsGetTypeEnv2 ~debug ~full ~opens ~rawOpens ~pos
with
| Some (typ, _env) ->
if debug then
Expand Down Expand Up @@ -1639,7 +1637,7 @@ let rec processCompletable ~debug ~full ~scope ~env ~pos ~forHover completable =
contextPath
|> getCompletionsForContextPath ~debug ~full ~opens ~rawOpens ~pos ~env
~exact:true ~scope
|> completionsGetTypeEnv2 ~debug ~full ~opens ~rawOpens ~pos ~scope
|> completionsGetTypeEnv2 ~debug ~full ~opens ~rawOpens ~pos
with
| Some (typ, env) -> (
match
Expand Down
4 changes: 2 additions & 2 deletions analysis/src/Hover.ml
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ let getHoverViaCompletions ~debug ~path ~pos ~currentFile ~forHover
let opens = CompletionBackEnd.getOpens ~debug ~rawOpens ~package ~env in
match
CompletionBackEnd.completionsGetTypeEnv2 ~debug ~full ~rawOpens ~opens
~pos ~scope completions
~pos completions
with
| Some (typ, _env) ->
let typeString =
Expand All @@ -154,7 +154,7 @@ let getHoverViaCompletions ~debug ~path ~pos ~currentFile ~forHover
let opens = CompletionBackEnd.getOpens ~debug ~rawOpens ~package ~env in
match
CompletionBackEnd.completionsGetTypeEnv2 ~debug ~full ~rawOpens ~opens
~pos ~scope completions
~pos completions
with
| Some (typ, _env) ->
let typeString =
Expand Down
22 changes: 9 additions & 13 deletions analysis/src/Scope.ml
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
type item =
| Constructor of string * Location.t
| Field of string * Location.t
| Module of string * Location.t
| Open of string list
| Type of string * Location.t
| Value of string * Location.t * SharedTypes.Completable.contextPath option
type item = SharedTypes.ScopeTypes.item

type t = item list

open SharedTypes.ScopeTypes
Copy link
Collaborator

Choose a reason for hiding this comment

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

where is this used?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah in the pattern matching below.

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 exactly, I opened it just to have it function as it did before.


let itemToString item =
let str s = if s = "" then "\"\"" else s in
let list l = "[" ^ (l |> List.map str |> String.concat ", ") ^ "]" in
Expand All @@ -16,7 +12,7 @@ let itemToString item =
| Field (s, loc) -> "Field " ^ s ^ " " ^ Loc.toString loc
| Open sl -> "Open " ^ list sl
| Module (s, loc) -> "Module " ^ s ^ " " ^ Loc.toString loc
| Value (s, loc, _) -> "Value " ^ s ^ " " ^ Loc.toString loc
| Value (s, loc, _, _) -> "Value " ^ s ^ " " ^ Loc.toString loc
| Type (s, loc) -> "Type " ^ s ^ " " ^ Loc.toString loc
[@@live]

Expand All @@ -34,14 +30,14 @@ let addValue ~name ~loc ?contextPath x =
if showDebug then
Printf.printf "adding value '%s' with ctxPath: %s\n" name
(SharedTypes.Completable.contextPathToString contextPath));
Value (name, loc, contextPath) :: x
Value (name, loc, contextPath, x) :: x
let addType ~name ~loc x = Type (name, loc) :: x

let iterValuesBeforeFirstOpen f x =
let rec loop items =
match items with
| Value (s, loc, contextPath) :: rest ->
f s loc contextPath;
| Value (s, loc, contextPath, scope) :: rest ->
f s loc contextPath scope;
loop rest
| Open _ :: _ -> ()
| _ :: rest -> loop rest
Expand All @@ -52,8 +48,8 @@ let iterValuesBeforeFirstOpen f x =
let iterValuesAfterFirstOpen f x =
let rec loop foundOpen items =
match items with
| Value (s, loc, contextPath) :: rest ->
if foundOpen then f s loc contextPath;
| Value (s, loc, contextPath, scope) :: rest ->
if foundOpen then f s loc contextPath scope;
loop foundOpen rest
| Open _ :: rest -> loop true rest
| _ :: rest -> loop foundOpen rest
Expand Down
12 changes: 11 additions & 1 deletion analysis/src/SharedTypes.ml
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,16 @@ module Completable = struct
| ChtmlElement {prefix} -> "ChtmlElement <" ^ prefix
end

module ScopeTypes = struct
type item =
| Constructor of string * Location.t
| Field of string * Location.t
| Module of string * Location.t
| Open of string list
| Type of string * Location.t
| Value of string * Location.t * Completable.contextPath option * item list
end

module Completion = struct
type kind =
| Module of Module.t
Expand All @@ -746,7 +756,7 @@ module Completion = struct
| FileModule of string
| Snippet of string
| ExtractedType of completionType * [`Value | `Type]
| FollowContextPath of Completable.contextPath
| FollowContextPath of Completable.contextPath * ScopeTypes.item list

type t = {
name: string;
Expand Down
2 changes: 1 addition & 1 deletion analysis/src/Xform.ml
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ module ExhaustiveSwitch = struct
in
match
CompletionBackEnd.completionsGetCompletionType2 ~debug ~full
~rawOpens ~opens ~pos ~scope completions
~rawOpens ~opens ~pos completions
with
| Some (typ, _env) ->
let extractedType =
Expand Down
5 changes: 5 additions & 0 deletions analysis/tests/src/CompletionInferValues.res
Original file line number Diff line number Diff line change
Expand Up @@ -162,3 +162,8 @@ let fn3 = (~cb: sameFileRecord => unit) => {
// But pipe completion gets the wrong completion path. Should be `ReactDOM.Client.Root.t`, but ends up being `CompletionSupport2.ReactDOM.Client.Root.t`.
// let renderer = CompletionSupport2.makeRenderer(~prepare=() => "hello",~render=({support:{root}}) => {root->},())
// ^com

// Handles reusing the same name already in scope for bindings
let res = 1
// switch res { | res => res }
// ^hov
15 changes: 15 additions & 0 deletions analysis/tests/src/expected/CompletionInferValues.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -876,3 +876,18 @@ Path ReactDOM.Client.Root.
"documentation": null
}]

Hover src/CompletionInferValues.res 167:27
Nothing at that position. Now trying to use completion.
posCursor:[167:27] posNoWhite:[167:26] Found expr:[167:25->167:28]
Pexp_ident res:[167:25->167:28]
Completable: Cpath Value[res]
Package opens Pervasives.JsxModules.place holder
Resolved opens 1 pervasives
ContextPath Value[res]
Path res
Package opens Pervasives.JsxModules.place holder
Resolved opens 1 pervasives
ContextPath Value[res]
Path res
{"contents": {"kind": "markdown", "value": "```rescript\nint\n```"}}