Gentype: fix issue with labelled args which should not be grouped.

Grouping labelled args is a leftover incompatible with zero cost conversion.

Fixes #6361

Author: cristianoc

CHANGELOG.md

@@ -12,6 +12,10 @@
 
 # 11.0.0-rc.4 (Unreleased)
 
+#### :bug: Bug Fix
+
+- Fix issue with GenType and labelled arguments https://github.com/rescript-lang/rescript-compiler/pull/6406
+
 #### :rocket: New Feature
 
 - Support renaming fields in inline records with `@as` attribute. [#6391](https://github.com/rescript-lang/rescript-compiler/pull/6391)

jscomp/gentype/Converter.ml

@@ -13,9 +13,6 @@ let typeGetInlined ~config ~lookupId ~typeNameIsInterface type0 =
       let argConverted = argTypes |> List.map (argTypeToGroupedArg ~visited) in
       let retNormalized = retType |> visit ~visited in
       Function {function_ with argTypes = argConverted; retType = retNormalized}
-    | GroupOfLabeledArgs _ ->
-      (* This case should only fire from withing a function *)
-      normalized_
     | Ident {builtin = true} -> normalized_
     | Ident {builtin = false; name; typeArgs} -> (
       if visited |> StringSet.mem name then (
@@ -77,21 +74,8 @@ let typeGetInlined ~config ~lookupId ~typeNameIsInterface type0 =
       in
       normalized
   and argTypeToGroupedArg ~visited {aName; aType} =
-    match aType with
-    | GroupOfLabeledArgs fields ->
-      let fieldsConverted =
-        fields
-        |> List.map (fun ({type_} as field) -> (field, type_ |> visit ~visited))
-      in
-      let tNormalized =
-        GroupOfLabeledArgs
-          (fieldsConverted
-          |> List.map (fun (field, t) -> {field with type_ = t}))
-      in
-      {aName; aType = tNormalized}
-    | _ ->
-      let tNormalized = aType |> visit ~visited in
-      {aName; aType = tNormalized}
+    let tNormalized = aType |> visit ~visited in
+    {aName; aType = tNormalized}
   in
   let normalized = type0 |> visit ~visited:StringSet.empty in
   if !Debug.converter then

jscomp/gentype/EmitJs.ml

@@ -549,7 +549,7 @@ let propagateAnnotationToSubTypes ~codeItems (typeMap : CodeItem.exportTypeMap)
       | Function {argTypes; retType} ->
         argTypes |> List.iter (fun {aType} -> visit aType);
         retType |> visit
-      | GroupOfLabeledArgs fields | Object (_, fields) ->
+      | Object (_, fields) ->
         fields |> List.iter (fun {type_} -> type_ |> visit)
       | Option t | Null t | Nullable t | Promise t -> t |> visit
       | Tuple innerTypes -> innerTypes |> List.iter visit

jscomp/gentype/EmitType.ml

@@ -105,7 +105,7 @@ let rec renderType ~(config : Config.t) ?(indent = None) ~typeNameIsInterface
   | Function {argTypes; retType; typeVars} ->
     renderFunType ~config ~indent ~inFunType ~typeNameIsInterface ~typeVars
       argTypes retType
-  | GroupOfLabeledArgs fields | Object (_, fields) ->
+  | Object (_, fields) ->
     let indent1 = fields |> Indent.heuristicFields ~indent in
     fields
     |> renderFields ~config ~indent:indent1 ~inFunType ~typeNameIsInterface

jscomp/gentype/GenTypeCommon.ml

@@ -69,7 +69,6 @@ type type_ =
   | Array of type_ * mutable_
   | Dict of type_
   | Function of function_
-  | GroupOfLabeledArgs of fields
   | Ident of ident
   | Null of type_
   | Nullable of type_

jscomp/gentype/NamedArgs.ml

@@ -1,62 +1,17 @@
 open GenTypeCommon
 
-type groupedArg = Group of fields | Arg of type_
-
-(**
- For convenient processing turns consecutive named arguments into a
- `NamedArgs` group, and individual non-named arguments into `Arg`s.
-*)
-let rec groupReversed ~revCurGroup ~revResult labeledTypes =
-  match (revCurGroup, labeledTypes) with
-  | [], (Nolabel, type_) :: tl ->
-    groupReversed ~revCurGroup:[] ~revResult:(Arg type_ :: revResult) tl
-  | _, (OptLabel name, type_) :: tl ->
-    (* Add it to the current group, not result. *)
-    groupReversed
-      ~revCurGroup:
-        ({
-           mutable_ = Immutable;
-           nameJS = name;
-           optional = Optional;
-           type_;
-           docString = DocString.empty;
-         }
-        :: revCurGroup)
-      ~revResult tl
-  | _, (Label name, type_) :: tl ->
-    groupReversed
-      ~revCurGroup:
-        ({
-           mutable_ = Immutable;
-           nameJS = name;
-           optional = Mandatory;
-           type_;
-           docString = DocString.empty;
-         }
-        :: revCurGroup)
-      ~revResult tl
-  | [], [] -> revResult
-  | _grpHd :: _grpTl, ([] as _tl) | _grpHd :: _grpTl, (Nolabel, _) :: _tl ->
-    (* Just form the group, and recurse ignoring the (None, t) in that case.
-       t will be handled in recursion. *)
-    groupReversed ~revCurGroup:[]
-      ~revResult:(Group revCurGroup :: revResult)
-      labeledTypes
-
-(** Special reverse that not only reverses the entire list but also the order of
-  items in the NamedArgs grouping. *)
-let rec reverse ?(soFar = []) lst =
-  match lst with
-  | [] -> soFar
-  | [Arg type_] when type_ = unitT && soFar = [] ->
-    (* treat a single argument of type unit as no argument *)
-    []
-  | Arg type_ :: tl -> reverse ~soFar:({aName = ""; aType = type_} :: soFar) tl
-  | Group fields :: tl ->
-    reverse
-      ~soFar:
-        ({aName = ""; aType = GroupOfLabeledArgs (List.rev fields)} :: soFar)
-      tl
-
 let group labeledTypes =
-  labeledTypes |> groupReversed ~revCurGroup:[] ~revResult:[] |> reverse
+  let types =
+    Ext_list.map labeledTypes (fun (lbl, type_) ->
+        let aName =
+          match lbl with
+          | Nolabel -> ""
+          | Label lbl | OptLabel lbl -> lbl
+        in
+
+        {aName; aType = type_})
+  in
+  match types with
+  | [{aType}] when aType = unitT ->
+    [] (* treat a single argument of type unit as no argument *)
+  | _ -> types

jscomp/gentype/TranslateStructure.ml

@@ -3,14 +3,19 @@ open GenTypeCommon
 let rec addAnnotationsToTypes_ ~config ~(expr : Typedtree.expression)
     (argTypes : argType list) =
   match (expr.exp_desc, expr.exp_type.desc, argTypes) with
-  | _, _, {aName; aType = GroupOfLabeledArgs fields} :: nextTypes ->
-    let fields1, nextTypes1 =
-      addAnnotationsToFields ~config expr fields nextTypes
-    in
-    {aName; aType = GroupOfLabeledArgs fields1} :: nextTypes1
-  | Texp_function {param; cases = [{c_rhs}]}, _, {aType} :: nextTypes ->
+  | Texp_function {arg_label; param; cases = [{c_rhs}]}, _, {aType} :: nextTypes
+    ->
     let nextTypes1 = nextTypes |> addAnnotationsToTypes_ ~config ~expr:c_rhs in
     let aName = Ident.name param in
+    let _ = Printtyped.implementation in
+    let aName =
+      if aName = "*opt*" then
+        match arg_label with
+        | Optional l ->
+          l
+        | _ -> "" (* should not happen *)
+      else aName
+    in
     {aName; aType} :: nextTypes1
   | Texp_construct ({txt = Lident "Function$"}, _, [funExpr]), _, _ ->
     (* let uncurried1: function$<_, _> = Function$(x => x |> string_of_int, [`Has_arity1]) *)

jscomp/gentype/TypeVars.ml

@@ -40,11 +40,6 @@ let rec substitute ~f type0 =
           |> List.map (fun {aName; aType = t} ->
                  {aName; aType = t |> substitute ~f});
       }
-  | GroupOfLabeledArgs fields ->
-    GroupOfLabeledArgs
-      (fields
-      |> List.map (fun field ->
-             {field with type_ = field.type_ |> substitute ~f}))
   | Ident {typeArgs = []} -> type0
   | Ident ({typeArgs} as ident) ->
     Ident {ident with typeArgs = typeArgs |> List.map (substitute ~f)}
@@ -80,7 +75,7 @@ let rec free_ type0 : StringSet.t =
     StringSet.diff
       ((argTypes |> freeOfList_) +++ (retType |> free_))
       (typeVars |> StringSet.of_list)
-  | GroupOfLabeledArgs fields | Object (_, fields) ->
+  | Object (_, fields) ->
     fields
     |> List.fold_left
          (fun s {type_} -> StringSet.union s (type_ |> free_))

jscomp/gentype_tests/typescript-react-example/package-lock.json

@@ -39,7 +39,7 @@ export const unnamed2: (param_0:number, param_1:number) => number = DocstringsBS
 
 export const unnamed2U: (param_0:number, param_1:number) => number = DocstringsBS.unnamed2U;
 
-export const grouped: (_1:{ readonly x: number; readonly y: number }, a:number, b:number, c:number, _5:{ readonly z: number }) => number = DocstringsBS.grouped;
+export const grouped: (x:number, y:number, a:number, b:number, c:number, z:number) => number = DocstringsBS.grouped;
 
 export const unitArgWithoutConversion: () => string = DocstringsBS.unitArgWithoutConversion;
 

jscomp/gentype_tests/typescript-react-example/src/Docstrings.gen.tsx

@@ -14,7 +14,7 @@ import type {TypedArray2_Uint8Array_t as Js_TypedArray2_Uint8Array_t} from '../s
 export type vehicle = { readonly name: string };
 
 // tslint:disable-next-line:interface-over-type-literal
-export type cb = (_1:{ readonly _to: vehicle }) => void;
+export type cb = (_to:vehicle) => void;
 
 // tslint:disable-next-line:interface-over-type-literal
 export type r = { readonly x: string };
@@ -71,11 +71,7 @@ export type NoProps_make_Props = {};
 
 export const NoProps_make: React.ComponentType<{}> = HooksBS.NoProps.make;
 
-export const functionWithRenamedArgs: (_1:{
-  readonly _to: vehicle; 
-  readonly _Type: vehicle; 
-  readonly cb: cb
-}) => string = HooksBS.functionWithRenamedArgs;
+export const functionWithRenamedArgs: (_to:vehicle, _Type:vehicle, cb:cb) => string = HooksBS.functionWithRenamedArgs;
 
 // tslint:disable-next-line:interface-over-type-literal
 export type WithRename_componentWithRenamedArgs_Props = {

jscomp/gentype_tests/typescript-react-example/src/Hooks.gen.tsx

@@ -23,10 +23,10 @@ export const makeRenamed: unknown = makeRenamedTypeChecked as React.ComponentTyp
 }>;
 
 // In case of type error, check the type of 'foo' in 'ImportHooks.res' and './hookExample'.
-export const fooTypeChecked: (_1:{ readonly person: person }) => string = fooNotChecked;
+export const fooTypeChecked: (person:person) => string = fooNotChecked;
 
 // Export 'foo' early to allow circular import from the '.bs.js' file.
-export const foo: unknown = fooTypeChecked as (_1:{ readonly person: person }) => string;
+export const foo: unknown = fooTypeChecked as (person:person) => string;
 
 // tslint:disable-next-line:interface-over-type-literal
 export type person = { readonly name: string; readonly age: number };

jscomp/gentype_tests/typescript-react-example/src/ImportHooks.gen.tsx

@@ -0,0 +1,9 @@
+/* TypeScript file generated from LabeledFun.res by genType. */
+/* eslint-disable import/first */
+
+
+// @ts-ignore: Implicit any on import
+import * as LabeledFunBS__Es6Import from './LabeledFun.bs';
+const LabeledFunBS: any = LabeledFunBS__Es6Import;
+
+export const labelled: (a:number, b:number, c:number, _4:number, e:number, f:number) => number = LabeledFunBS.labelled;

jscomp/gentype_tests/typescript-react-example/src/LabeledFun.bs.js

@@ -0,0 +1,2 @@
+@genType
+let labelled = (a, ~b=3, ~c, d, ~e, ~f) => a + b + c + d + e + f

jscomp/gentype_tests/typescript-react-example/src/LabeledFun.gen.tsx

@@ -17,4 +17,4 @@ export type toPayload = { readonly result: string };
 
 export const convert: (_1:Promise<fromPayload>) => Promise<toPayload> = TestPromiseBS.convert;
 
-export const barx: (_1:{ readonly x?: Promise<(undefined | string)> }, _2:void) => boolean = TestPromiseBS.barx;
+export const barx: (x:Promise<(undefined | string)>, _2:void) => boolean = TestPromiseBS.barx;

jscomp/gentype_tests/typescript-react-example/src/LabeledFun.res

@@ -46,4 +46,4 @@ export const sumU2: (n:number) => (_1:number) => void = UncurriedBS.sumU2;
 
 export const sumCurried: (n:number, _2:number) => void = UncurriedBS.sumCurried;
 
-export const sumLblCurried: (s:string, _2:{ readonly n: number; readonly m: number }) => void = UncurriedBS.sumLblCurried;
+export const sumLblCurried: (s:string, n:number, m:number) => void = UncurriedBS.sumLblCurried;

jscomp/gentype_tests/typescript-react-example/src/TestPromise.gen.tsx

@@ -1,8 +1,6 @@
 import * as React from "react";
 
-export const foo = (x: {
-  person: { readonly name: string; readonly age: number };
-}) => x.person.name;
+export const foo = (person: { readonly name: string; readonly age: number }) => person.name;
 
 type Props = {
   readonly person: { readonly name: string; readonly age: number };

jscomp/gentype_tests/typescript-react-example/src/Uncurried.gen.tsx

@@ -61,7 +61,7 @@ consoleLog(
 Uncurried.sumU(3, 4);
 Uncurried.sumU2(3)(4);
 Uncurried.sumCurried(3, 4);
-Uncurried.sumLblCurried("hello", { n: 3, m: 4 });
+Uncurried.sumLblCurried("hello", 3, 4);
 
 ReactDOM.render(
   <div>