From 40da9f6fe4343354e89b70eb9dd8bb4dd31c4d85 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Fri, 1 Mar 2024 13:12:20 +0100 Subject: [PATCH 1/2] tighten check when completing for unlabelled argument --- analysis/src/CompletionFrontEnd.ml | 28 ++++++++++++------- .../tests/src/CompletionFunctionArguments.res | 8 ++++++ .../CompletionFunctionArguments.res.txt | 23 +++++++++++++++ 3 files changed, 49 insertions(+), 10 deletions(-) diff --git a/analysis/src/CompletionFrontEnd.ml b/analysis/src/CompletionFrontEnd.ml index 07cec8cae..26871bc14 100644 --- a/analysis/src/CompletionFrontEnd.ml +++ b/analysis/src/CompletionFrontEnd.ml @@ -155,7 +155,11 @@ let findArgCompletables ~(args : arg list) ~endPos ~posBeforeCursor "[findArgCompletables] skipping completion in fn call because \ arg had empty loc"; None - | _ -> + | _ + when firstCharBeforeCursorNoWhite = Some '(' + || firstCharBeforeCursorNoWhite = Some ',' -> + (* Checks to ensure that completing for empty unlabelled arg makes + sense by checking what's left of the cursor. *) if Debug.verbose () then Printf.printf "[findArgCompletables] Completing for unlabelled argument value \ @@ -174,7 +178,8 @@ let findArgCompletables ~(args : arg list) ~endPos ~posBeforeCursor }; prefix = ""; nested = []; - })) + }) + | _ -> None) else None in match args with @@ -549,15 +554,17 @@ let completionWithParser1 ~currentFile ~debug ~offset ~path ~posCursor let inJsxContext = ref false in (* Identifies expressions where we can do typed pattern or expr completion. *) let typedCompletionExpr (exp : Parsetree.expression) = + let debugTypedCompletionExpr = false in if exp.pexp_loc |> CursorPosition.locHasCursor ~pos:posBeforeCursor then ( - if Debug.verbose () then print_endline "[typedCompletionExpr] Has cursor"; + if Debug.verbose () && debugTypedCompletionExpr then + print_endline "[typedCompletionExpr] Has cursor"; match exp.pexp_desc with (* No cases means there's no `|` yet in the switch *) | Pexp_match (({pexp_desc = Pexp_ident _} as expr), []) -> - if Debug.verbose () then + if Debug.verbose () && debugTypedCompletionExpr then print_endline "[typedCompletionExpr] No cases, with ident"; if locHasCursor expr.pexp_loc then ( - if Debug.verbose () then + if Debug.verbose () && debugTypedCompletionExpr then print_endline "[typedCompletionExpr] No cases - has cursor"; (* We can do exhaustive switch completion if this is an ident we can complete from. *) @@ -566,7 +573,7 @@ let completionWithParser1 ~currentFile ~debug ~offset ~path ~posCursor | Some contextPath -> setResult (CexhaustiveSwitch {contextPath; exprLoc = exp.pexp_loc})) | Pexp_match (_expr, []) -> - if Debug.verbose () then + if Debug.verbose () && debugTypedCompletionExpr then print_endline "[typedCompletionExpr] No cases, rest"; () | Pexp_match @@ -594,15 +601,16 @@ let completionWithParser1 ~currentFile ~debug ~offset ~path ~posCursor patternMode = Default; })) | Pexp_match (exp, cases) -> ( - if Debug.verbose () then print_endline "[typedCompletionExpr] Has cases"; + if Debug.verbose () && debugTypedCompletionExpr then + print_endline "[typedCompletionExpr] Has cases"; (* If there's more than one case, or the case isn't a pattern hole, figure out if we're completing another broken parser case (`switch x { | true => () | }` for example). *) match exp |> exprToContextPath with | None -> - if Debug.verbose () then + if Debug.verbose () && debugTypedCompletionExpr then print_endline "[typedCompletionExpr] Has cases - no ctx path" | Some ctxPath -> ( - if Debug.verbose () then + if Debug.verbose () && debugTypedCompletionExpr then print_endline "[typedCompletionExpr] Has cases - has ctx path"; let hasCaseWithCursor = cases @@ -616,7 +624,7 @@ let completionWithParser1 ~currentFile ~debug ~offset ~path ~posCursor locIsEmpty case.Parsetree.pc_lhs.ppat_loc) |> Option.is_some in - if Debug.verbose () then + if Debug.verbose () && debugTypedCompletionExpr then Printf.printf "[typedCompletionExpr] Has cases - has ctx path - \ hasCaseWithEmptyLoc: %b, hasCaseWithCursor: %b\n" diff --git a/analysis/tests/src/CompletionFunctionArguments.res b/analysis/tests/src/CompletionFunctionArguments.res index bc7d9e698..5000ee49c 100644 --- a/analysis/tests/src/CompletionFunctionArguments.res +++ b/analysis/tests/src/CompletionFunctionArguments.res @@ -113,3 +113,11 @@ let _ = // ^com }} /> + +let fineModuleVal = { + FineModule.online: true, + somethingElse: "", +} + +// makeItem(~changefreq=Monthly, ~lastmod=fineModuleVal->, ~priority=Low) +// ^com diff --git a/analysis/tests/src/expected/CompletionFunctionArguments.res.txt b/analysis/tests/src/expected/CompletionFunctionArguments.res.txt index c71b4540a..cfc0eea85 100644 --- a/analysis/tests/src/expected/CompletionFunctionArguments.res.txt +++ b/analysis/tests/src/expected/CompletionFunctionArguments.res.txt @@ -418,3 +418,26 @@ Path ReactEvent.Mouse.a "documentation": null }] +Complete src/CompletionFunctionArguments.res 121:57 +posCursor:[121:57] posNoWhite:[121:56] Found expr:[121:3->121:73] +Pexp_apply ...[121:3->121:11] (~changefreq121:13->121:23=...[121:24->121:31], ~lastmod121:34->121:41=...[121:42->0:-1], ~priority121:60->121:68=...[121:69->121:72]) +posCursor:[121:57] posNoWhite:[121:56] Found expr:[121:42->0:-1] +posCursor:[121:57] posNoWhite:[121:56] Found expr:[121:42->0:-1] +Completable: Cpath Value[fineModuleVal]-> +Package opens Pervasives.JsxModules.place holder +Resolved opens 1 pervasives +ContextPath Value[fineModuleVal]-> +ContextPath Value[fineModuleVal] +Path fineModuleVal +CPPipe env:CompletionFunctionArguments +CPPipe type path:FineModule.t +CPPipe pathFromEnv:FineModule found:true +Path FineModule. +[{ + "label": "FineModule.setToFalse", + "kind": 12, + "tags": [], + "detail": "t => t", + "documentation": null + }] + From 7fc2c0af02e353fa34e14a8ed6cdf850f1d40628 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Fri, 1 Mar 2024 13:13:49 +0100 Subject: [PATCH 2/2] changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f45327ca..a32a86574 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ #### :bug: Bug Fix +- Fix issue with unlabelled arg code swallowing completions. https://github.com/rescript-lang/rescript-vscode/pull/937 + +#### :bug: Bug Fix + - Fix issue where completion inside of switch expression would not work in some cases. https://github.com/rescript-lang/rescript-vscode/pull/936 - Fix bug that made empty prop expressions in JSX not complete if in the middle of a JSX element. https://github.com/rescript-lang/rescript-vscode/pull/935