From 07c1a3964d8874e6be5786fbdf7bb0d24c9b46d5 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Fri, 12 Jan 2024 22:16:37 +0100 Subject: [PATCH 1/2] fix bug with pipe completion not working inside of parenthesis in function call --- analysis/src/CompletionFrontEnd.ml | 115 +++++++++++++++--- analysis/tests/src/CompletionPipeChain.res | 6 + .../src/expected/CompletionPipeChain.res.txt | 63 ++++++++++ 3 files changed, 165 insertions(+), 19 deletions(-) diff --git a/analysis/src/CompletionFrontEnd.ml b/analysis/src/CompletionFrontEnd.ml index 8131e6b12..4f83dde1a 100644 --- a/analysis/src/CompletionFrontEnd.ml +++ b/analysis/src/CompletionFrontEnd.ml @@ -15,21 +15,31 @@ let findArgCompletables ~(args : arg list) ~endPos ~posBeforeCursor args [] in let unlabelledCount = ref (if isPipedExpr then 1 else 0) in + let someArgHadEmptyExprLoc = ref false in let rec loop args = match args with | {label = Some labelled; exp} :: rest -> if labelled.posStart <= posBeforeCursor && posBeforeCursor < labelled.posEnd - then Some (Completable.CnamedArg (contextPath, labelled.name, allNames)) - else if exp.pexp_loc |> Loc.hasPos ~pos:posBeforeCursor then - (* Completing in the assignment of labelled argument *) + then ( + if Debug.verbose () then + print_endline "[findArgCompletables] Completing named arg #2"; + Some (Completable.CnamedArg (contextPath, labelled.name, allNames))) + else if exp.pexp_loc |> Loc.hasPos ~pos:posBeforeCursor then ( + if Debug.verbose () then + print_endline + "[findArgCompletables] Completing in the assignment of labelled \ + argument"; match CompletionExpressions.traverseExpr exp ~exprPath:[] ~pos:posBeforeCursor ~firstCharBeforeCursorNoWhite with | None -> None | Some (prefix, nested) -> + if Debug.verbose () then + print_endline + "[findArgCompletables] Completing for labelled argument value"; Some (Cexpression { @@ -41,8 +51,10 @@ let findArgCompletables ~(args : arg list) ~endPos ~posBeforeCursor }; prefix; nested = List.rev nested; - }) - else if CompletionExpressions.isExprHole exp then + })) + else if CompletionExpressions.isExprHole exp then ( + if Debug.verbose () then + print_endline "[findArgCompletables] found exprhole"; Some (Cexpression { @@ -54,18 +66,35 @@ let findArgCompletables ~(args : arg list) ~endPos ~posBeforeCursor }; prefix = ""; nested = []; - }) + })) else loop rest | {label = None; exp} :: rest -> + if Debug.verbose () then + Printf.printf "[findArgCompletable] unlabelled arg expr is: %s \n" + (DumpAst.printExprItem ~pos:posBeforeCursor ~indentation:0 exp); + + (* Track whether there was an arg with an empty loc (indicates parser error)*) + if CursorPosition.locIsEmpty exp.pexp_loc ~pos:posBeforeCursor then + someArgHadEmptyExprLoc := true; + if Res_parsetree_viewer.isTemplateLiteral exp then None - else if exp.pexp_loc |> Loc.hasPos ~pos:posBeforeCursor then - (* Completing in an unlabelled argument *) + else if exp.pexp_loc |> Loc.hasPos ~pos:posBeforeCursor then ( + if Debug.verbose () then + print_endline + "[findArgCompletables] Completing in an unlabelled argument"; match CompletionExpressions.traverseExpr exp ~pos:posBeforeCursor ~firstCharBeforeCursorNoWhite ~exprPath:[] with - | None -> None + | None -> + if Debug.verbose () then + print_endline + "[findArgCompletables] found nothing when traversing expr"; + None | Some (prefix, nested) -> + if Debug.verbose () then + print_endline + "[findArgCompletables] completing for unlabelled argument #2"; Some (Cexpression { @@ -78,8 +107,10 @@ let findArgCompletables ~(args : arg list) ~endPos ~posBeforeCursor }; prefix; nested = List.rev nested; - }) - else if CompletionExpressions.isExprHole exp then + })) + else if CompletionExpressions.isExprHole exp then ( + if Debug.verbose () then + print_endline "[findArgCompletables] found an exprhole #2"; Some (Cexpression { @@ -92,15 +123,45 @@ let findArgCompletables ~(args : arg list) ~endPos ~posBeforeCursor }; prefix = ""; nested = []; - }) + })) else ( unlabelledCount := !unlabelledCount + 1; loop rest) | [] -> - if fnHasCursor then + let hadEmptyExpLoc = !someArgHadEmptyExprLoc in + if fnHasCursor then ( + if Debug.verbose () then + print_endline "[findArgCompletables] Function has cursor"; match charBeforeCursor with - | Some '~' -> Some (Completable.CnamedArg (contextPath, "", allNames)) + | Some '~' -> + if Debug.verbose () then + print_endline "[findArgCompletables] '~' is before cursor"; + Some (Completable.CnamedArg (contextPath, "", allNames)) + | _ when hadEmptyExpLoc -> + (* Special case: `Console.log(arr->)`, completing on the pipe. + This match branch happens when the fn call has the cursor and: + - there's no argument label or expr that has the cursor + - there's an argument expression with an empty loc (indicates parser error) + + In that case, it's safer to not complete for the unlabelled function + argument (which we do otherwise), and instead not complete and let the + completion engine move into the arguments one by one instead to check + for completions. + + This can be handled in a more robust way in a future refactor of the + completion engine logic. *) + if Debug.verbose () then + print_endline + "[findArgCompletables] skipping completion in fn call because \ + arg had empty loc"; + None | _ -> + if Debug.verbose () then + Printf.printf + "[findArgCompletables] Completing for unlabelled argument value \ + because nothing matched and is not labelled argument name \ + completion. isPipedExpr: %b\n" + isPipedExpr; Some (Cexpression { @@ -113,7 +174,7 @@ let findArgCompletables ~(args : arg list) ~endPos ~posBeforeCursor }; prefix = ""; nested = []; - }) + })) else None in match args with @@ -122,6 +183,8 @@ let findArgCompletables ~(args : arg list) ~endPos ~posBeforeCursor {label = None; exp = {pexp_desc = Pexp_construct ({txt = Lident "()"}, _)}}; ] when fnHasCursor -> + if Debug.verbose () then + print_endline "[findArgCompletables] Completing for unit argument"; Some (Completable.Cexpression { @@ -305,8 +368,16 @@ let completionWithParser1 ~currentFile ~debug ~offset ~path ~posCursor let setResultOpt x = if !result = None then match x with - | None -> () - | Some x -> result := Some (x, !scope) + | None -> + if Debug.verbose () then + print_endline + "[set_result] did not set new result because result already was set"; + () + | Some x -> + if Debug.verbose () then + Printf.printf "[set_result] set new result to %s\n" + (Completable.toString x); + result := Some (x, !scope) in let setResult x = setResultOpt (Some x) in let scopeValueDescription (vd : Parsetree.value_description) = @@ -965,13 +1036,13 @@ let completionWithParser1 ~currentFile ~debug ~offset ~path ~posCursor (_, {pexp_desc = Pexp_ident {txt = Longident.Lident id; loc}}); ] ) when loc |> Loc.hasPos ~pos:posBeforeCursor -> - (* Case foo->id *) + if Debug.verbose () then print_endline "[expr_iter] Case foo->id"; setPipeResult ~lhs ~id |> ignore | Pexp_apply ( {pexp_desc = Pexp_ident {txt = Lident ("|." | "|.u"); loc = opLoc}}, [(_, lhs); _] ) when Loc.end_ opLoc = posCursor -> - (* Case foo-> *) + if Debug.verbose () then print_endline "[expr_iter] Case foo->"; setPipeResult ~lhs ~id:"" |> ignore | Pexp_apply ( {pexp_desc = Pexp_ident {txt = Lident ("|." | "|.u")}}, @@ -983,6 +1054,8 @@ let completionWithParser1 ~currentFile ~debug ~offset ~path ~posCursor (Loc.end_ expr.pexp_loc = posCursor && charBeforeCursor = Some ')') -> ( (* Complete fn argument values and named args when the fn call is piped. E.g. someVar->someFn(). *) + if Debug.verbose () then + print_endline "[expr_iter] Complete fn arguments (piped)"; let args = extractExpApplyArgs ~args in let funCtxPath = exprToContextPath funExpr in let argCompletable = @@ -1014,6 +1087,8 @@ let completionWithParser1 ~currentFile ~debug ~offset ~path ~posCursor (Loc.end_ expr.pexp_loc = posCursor && charBeforeCursor = Some ')') -> ( (* Complete fn argument values and named args when the fn call is _not_ piped. E.g. someFn(). *) + if Debug.verbose () then + print_endline "[expr_iter] Complete fn arguments (not piped)"; let args = extractExpApplyArgs ~args in if debug then Printf.printf "Pexp_apply ...%s (%s)\n" @@ -1090,6 +1165,8 @@ let completionWithParser1 ~currentFile ~debug ~offset ~path ~posCursor match lbl with | Nolabel -> Some (ctxPath, currentUnlabelledCount + 1) | _ -> Some (ctxPath, currentUnlabelledCount)); + if Debug.verbose () then + print_endline "[expr_iter] Completing for argument value"; Some (Completable.CArgument { diff --git a/analysis/tests/src/CompletionPipeChain.res b/analysis/tests/src/CompletionPipeChain.res index b6b177173..82276baaf 100644 --- a/analysis/tests/src/CompletionPipeChain.res +++ b/analysis/tests/src/CompletionPipeChain.res @@ -92,3 +92,9 @@ let renderer = CompletionSupport2.makeRenderer( }, (), ) + +// Console.log(int->) +// ^com + +// Console.log(int->t) +// ^com diff --git a/analysis/tests/src/expected/CompletionPipeChain.res.txt b/analysis/tests/src/expected/CompletionPipeChain.res.txt index 52bfb0631..dd107c59f 100644 --- a/analysis/tests/src/expected/CompletionPipeChain.res.txt +++ b/analysis/tests/src/expected/CompletionPipeChain.res.txt @@ -490,3 +490,66 @@ Path ReactDOM.Client.Root.ren "documentation": null }] +Complete src/CompletionPipeChain.res 95:20 +posCursor:[95:20] posNoWhite:[95:19] Found expr:[95:3->95:21] +Pexp_apply ...[95:3->95:14] (...[95:15->0:-1]) +posCursor:[95:20] posNoWhite:[95:19] Found expr:[95:15->0:-1] +posCursor:[95:20] posNoWhite:[95:19] Found expr:[95:15->0:-1] +Completable: Cpath Value[int]-> +Package opens Pervasives.JsxModules.place holder +Resolved opens 1 pervasives +ContextPath Value[int]-> +ContextPath Value[int] +Path int +CPPipe env:CompletionPipeChain +CPPipe type path:Integer.t +CPPipe pathFromEnv:Integer found:true +Path Integer. +[{ + "label": "Integer.toInt", + "kind": 12, + "tags": [], + "detail": "t => int", + "documentation": null + }, { + "label": "Integer.increment", + "kind": 12, + "tags": [], + "detail": "(t, int) => t", + "documentation": null + }, { + "label": "Integer.decrement", + "kind": 12, + "tags": [], + "detail": "(t, int => int) => t", + "documentation": null + }, { + "label": "Integer.make", + "kind": 12, + "tags": [], + "detail": "int => t", + "documentation": null + }] + +Complete src/CompletionPipeChain.res 98:21 +posCursor:[98:21] posNoWhite:[98:20] Found expr:[98:3->98:22] +Pexp_apply ...[98:3->98:14] (...[98:15->98:21]) +posCursor:[98:21] posNoWhite:[98:20] Found expr:[98:15->98:21] +Completable: Cpath Value[int]->t +Package opens Pervasives.JsxModules.place holder +Resolved opens 1 pervasives +ContextPath Value[int]->t +ContextPath Value[int] +Path int +CPPipe env:CompletionPipeChain +CPPipe type path:Integer.t +CPPipe pathFromEnv:Integer found:true +Path Integer.t +[{ + "label": "Integer.toInt", + "kind": 12, + "tags": [], + "detail": "t => int", + "documentation": null + }] + From 1f1d9ebbb84d623a222ef37364a83bb51b713c4c Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Fri, 12 Jan 2024 22:20:31 +0100 Subject: [PATCH 2/2] function call --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a6d9949cf..7f7eafcb6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ - Fix issue with ambigious wraps in JSX prop values (`}`) - need to figure out if we're completing for a record body or if `{}` are just wraps for the type of `someProp`. In the case of ambiguity, completions for both scenarios are provided. https://github.com/rescript-lang/rescript-vscode/pull/894 - Many bugfixes around nested pattern and expression completion. https://github.com/rescript-lang/rescript-vscode/pull/892 +- Fix (very annoying) issue where empty pipe completion wouldn't work inside of a parenthesised function call: `Console.log(someArray->)` completing at the pipe. https://github.com/rescript-lang/rescript-vscode/pull/895 #### :nail_care: Polish