Fix issue with unlabelled arg code swallowing completions (#937)

* tighten check when completing for unlabelled argument

* changelog

Author: zth

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
 

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 => () | <com> }` 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"

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

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
+  }]
+