Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Sep 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
20 changes: 2 additions & 18 deletions jscomp/gentype/Converter.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion jscomp/gentype/EmitJs.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion jscomp/gentype/EmitType.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion jscomp/gentype/GenTypeCommon.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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_
Expand Down
70 changes: 11 additions & 59 deletions jscomp/gentype/NamedArgs.ml
Original file line number Diff line number Diff line change
@@ -1,62 +1,14 @@
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, aType) ->
match lbl with
| Nolabel -> {aName = ""; aType}
| Label lbl -> {aName = lbl; aType}
| OptLabel lbl -> {aName = lbl; aType = Option aType})
in
match types with
| [{aType}] when aType = unitT ->
[] (* treat a single argument of type unit as no argument *)
| _ -> types
17 changes: 11 additions & 6 deletions jscomp/gentype/TranslateStructure.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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]) *)
Expand Down
7 changes: 1 addition & 6 deletions jscomp/gentype/TypeVars.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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)}
Expand Down Expand Up @@ -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_))
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand Down Expand Up @@ -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 = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand Down
12 changes: 12 additions & 0 deletions jscomp/gentype_tests/typescript-react-example/src/LabeledFun.bs.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -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:(undefined | number), c:number, _4:number, e:number, f:number) => number = LabeledFunBS.labelled;
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
@genType
let labelled = (a, ~b=3, ~c, d, ~e, ~f) => a + b + c + d + e + f
Original file line number Diff line number Diff line change
Expand Up @@ -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:(undefined | Promise<(undefined | string)>), _2:void) => boolean = TestPromiseBS.barx;
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Original file line number Diff line number Diff line change
@@ -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 };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down