tweak logic some and break out functionality to own function

Author: zth

Diff for: analysis/src/CompletionBackEnd.ml

@@ -1342,45 +1342,13 @@ let rec completeTypedValue ~full ~prefix ~completionContext ~mode
       ]
     else []
   | Tfunction {env; typ; args} when prefix = "" && mode = Expression ->
-    let prettyPrintArgTyp ?currentIndex (argTyp : Types.type_expr) =
-      let indexText =
-        match currentIndex with
-        | None -> ""
-        | Some i -> string_of_int i
-      in
-      match argTyp |> TypeUtils.pathFromTypeExpr with
-      | None -> "v" ^ indexText
-      | Some p -> (
-        (* Pretty print a few common patterns. *)
-        match Path.head p |> Ident.name with
-        | "unit" -> "()"
-        | "ReactEvent" | "JsxEvent" -> "event"
-        | _ ->
-          let defaultVarName = "v" ^ indexText in
-          let txt =
-            match
-              TypeUtils.templateVarNameForTyp ~env ~package:full.package argTyp
-            with
-            | Some txt -> txt
-            | None -> (
-              match p |> Utils.expandPath |> List.rev |> Utils.lastElements with
-              | [] | ["t"] -> defaultVarName
-              | [someName; "t"] | [_; someName] | [someName] -> (
-                match someName with
-                | "string" | "int" | "float" | "array" | "option" | "bool" ->
-                  defaultVarName
-                | someName when String.length someName < 30 ->
-                  someName |> Utils.lowercaseFirstChar
-                | _ -> defaultVarName)
-              | _ -> defaultVarName)
-          in
-          txt)
-    in
     let mkFnArgs ~asSnippet =
       match args with
       | [(Nolabel, argTyp)] when TypeUtils.typeIsUnit argTyp -> "()"
       | [(Nolabel, argTyp)] ->
-        let varName = prettyPrintArgTyp argTyp in
+        let varName =
+          CompletionExpressions.prettyPrintFnTemplateArgName ~env ~full argTyp
+        in
         if asSnippet then "${1:" ^ varName ^ "}" else varName
       | _ ->
         let currentUnlabelledIndex = ref 0 in
@@ -1395,7 +1363,10 @@ let rec completeTypedValue ~full ~prefix ~completionContext ~mode
                    else (
                      currentUnlabelledIndex := !currentUnlabelledIndex + 1;
                      let num = !currentUnlabelledIndex in
-                     let varName = prettyPrintArgTyp typ ~currentIndex:num in
+                     let varName =
+                       CompletionExpressions.prettyPrintFnTemplateArgName
+                         ~currentIndex:num ~env ~full typ
+                     in
                      if asSnippet then
                        "${" ^ string_of_int num ^ ":" ^ varName ^ "}"
                      else varName))

Diff for: analysis/src/CompletionExpressions.ml

@@ -216,3 +216,39 @@ and traverseExprTupleItems tupleItems ~nextExprPath ~resultFromFoundItemNum ~pos
            if pos >= Loc.start e.Parsetree.pexp_loc then posNum := index);
     if !posNum > -1 then Some ("", resultFromFoundItemNum !posNum) else None
   | v, _ -> v
+
+let prettyPrintFnTemplateArgName ?currentIndex ~env ~full
+    (argTyp : Types.type_expr) =
+  let indexText =
+    match currentIndex with
+    | None -> ""
+    | Some i -> string_of_int i
+  in
+  let defaultVarName = "v" ^ indexText in
+  let argTyp, suffix, env =
+    TypeUtils.digToRelevantTemplateNameType ~env ~package:full.package argTyp
+  in
+  match TypeUtils.templateVarNameForTyp ~env ~package:full.package argTyp with
+  | Some txt -> txt
+  | None -> (
+    match argTyp |> TypeUtils.pathFromTypeExpr with
+    | None -> defaultVarName
+    | Some p -> (
+      match p |> Utils.expandPath |> List.rev |> Utils.lastElements with
+      | [] | ["t"] -> defaultVarName
+      | ["unit"] -> "()"
+      (* Special treatment for JsxEvent, since that's a common enough thing
+         used in event handlers. *)
+      | ["JsxEvent"; "synthetic"] -> "event"
+      | ["synthetic"] when env.file.moduleName = "JsxEvent" -> "event"
+      (* Ignore `t` types, and go for its module name instead. *)
+      | [someName; "t"] | [_; someName] | [someName] -> (
+        match someName with
+        | "string" | "int" | "float" | "array" | "option" | "bool" ->
+          defaultVarName
+        | someName when String.length someName < 30 ->
+          (* We cap how long the name can be, so we don't end up with super
+             long type names. *)
+          (someName |> Utils.lowercaseFirstChar) ^ suffix
+        | _ -> defaultVarName)
+      | _ -> defaultVarName))

Diff for: analysis/src/TypeUtils.ml

@@ -208,11 +208,27 @@ let rec templateVarNameForTyp ~env ~package (t : Types.type_expr) =
     templateVarNameForTyp ~env ~package t1
   | Tconstr (path, _, _) -> (
     match References.digConstructor ~env ~package path with
-    | Some (_env, {item = {attributes}}) -> (
-      ProcessAttributes.findTemplateVarNameAttribute attributes)
+    | Some (_env, {item = {attributes}}) ->
+      ProcessAttributes.findTemplateVarNameAttribute attributes
     | _ -> None)
   | _ -> None
 
+let rec digToRelevantTemplateNameType ~env ~package ?(suffix = "")
+    (t : Types.type_expr) =
+  match t.desc with
+  | Tlink t1 | Tsubst t1 | Tpoly (t1, []) ->
+    digToRelevantTemplateNameType ~suffix ~env ~package t1
+  | Tconstr (Path.Pident {name = "option"}, [t1], _) ->
+    digToRelevantTemplateNameType ~suffix ~env ~package t1
+  | Tconstr (Path.Pident {name = "array"}, [t1], _) ->
+    digToRelevantTemplateNameType ~suffix:"s" ~env ~package t1
+  | Tconstr (path, _, _) -> (
+    match References.digConstructor ~env ~package path with
+    | Some (env, {item = {decl = {type_manifest = Some typ}}}) ->
+      digToRelevantTemplateNameType ~suffix ~env ~package typ
+    | _ -> (t, suffix, env))
+  | _ -> (t, suffix, env)
+
 let rec resolveTypeForPipeCompletion ~env ~package ~lhsLoc ~full
     (t : Types.type_expr) =
   let builtin =

Diff for: analysis/src/Utils.ml

@@ -207,7 +207,7 @@ end
 
 let rec lastElements list =
   match list with
-  | ([_] | [_; _] | []) as res -> res
+  | ([_; _] | [_] | []) as res -> res
   | _ :: tl -> lastElements tl
 
 let lowercaseFirstChar s =

Diff for: analysis/tests/src/CompletionExpressions.res

@@ -227,6 +227,20 @@ let takesCb3 = cb => {
 // takesCb3()
 //          ^com
 
+let takesCb4 = cb => {
+  cb(Some({hi: true}))
+}
+
+// takesCb4()
+//          ^com
+
+let takesCb5 = cb => {
+  cb([Some({hi: true})])
+}
+
+// takesCb5()
+//          ^com
+
 module RecordSourceSelectorProxy = {
   @editor.templateVariableName("store")
   type t

Diff for: analysis/tests/src/expected/CompletionExpressions.res.txt

@@ -1005,9 +1005,45 @@ Path takesCb3
     "insertTextFormat": 2
   }]
 
-Complete src/CompletionExpressions.res 238:30
-posCursor:[238:30] posNoWhite:[238:29] Found expr:[238:3->238:31]
-Pexp_apply ...[238:3->238:20] (~updater238:22->238:29=...__ghost__[0:-1->0:-1])
+Complete src/CompletionExpressions.res 233:12
+posCursor:[233:12] posNoWhite:[233:11] Found expr:[233:3->233:13]
+Pexp_apply ...[233:3->233:11] (...[233:12->233:13])
+Completable: Cexpression CArgument Value[takesCb4]($0)
+ContextPath CArgument Value[takesCb4]($0)
+ContextPath Value[takesCb4]
+Path takesCb4
+[{
+    "label": "apiCallResult => {}",
+    "kind": 12,
+    "tags": [],
+    "detail": "option<apiCallResult> => 'a",
+    "documentation": null,
+    "sortText": "A",
+    "insertText": "${1:apiCallResult} => {$0}",
+    "insertTextFormat": 2
+  }]
+
+Complete src/CompletionExpressions.res 240:12
+posCursor:[240:12] posNoWhite:[240:11] Found expr:[240:3->240:13]
+Pexp_apply ...[240:3->240:11] (...[240:12->240:13])
+Completable: Cexpression CArgument Value[takesCb5]($0)
+ContextPath CArgument Value[takesCb5]($0)
+ContextPath Value[takesCb5]
+Path takesCb5
+[{
+    "label": "apiCallResults => {}",
+    "kind": 12,
+    "tags": [],
+    "detail": "array<option<apiCallResult>> => 'a",
+    "documentation": null,
+    "sortText": "A",
+    "insertText": "${1:apiCallResults} => {$0}",
+    "insertTextFormat": 2
+  }]
+
+Complete src/CompletionExpressions.res 252:30
+posCursor:[252:30] posNoWhite:[252:29] Found expr:[252:3->252:31]
+Pexp_apply ...[252:3->252:20] (~updater252:22->252:29=...__ghost__[0:-1->0:-1])
 Completable: Cexpression CArgument Value[commitLocalUpdate](~updater)
 ContextPath CArgument Value[commitLocalUpdate](~updater)
 ContextPath Value[commitLocalUpdate]