From 4bf5e34db1bd549cf54c788b08f380b37ce08597 Mon Sep 17 00:00:00 2001 From: glennsl Date: Tue, 30 Jun 2020 15:06:39 +0200 Subject: [PATCH 1/7] add failing test --- test/Components.re | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/Components.re b/test/Components.re index 03d3f7d..57e8f8a 100644 --- a/test/Components.re +++ b/test/Components.re @@ -264,3 +264,8 @@ module LocallyAbstractType: { (empty, hooks); }; }; + +module TypeAnnotation = { + let%component make: (~key:Key.t=?, unit) => element(node) = + ((), hooks) => (empty, hooks); +}; From 901558b4f339764b0ac949b29cba828ed9abe5a1 Mon Sep 17 00:00:00 2001 From: glennsl Date: Tue, 30 Jun 2020 15:07:24 +0200 Subject: [PATCH 2/7] allow Pexp_constraint --- jsx/brisk_ppx.ml | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/jsx/brisk_ppx.ml b/jsx/brisk_ppx.ml index e37de7e..d4813d9 100644 --- a/jsx/brisk_ppx.ml +++ b/jsx/brisk_ppx.ml @@ -123,14 +123,21 @@ module Declaration_ppx = struct | `Native -> [%expr Brisk_reconciler.Expert.nativeComponent] | `Component -> [%expr Brisk_reconciler.Expert.component] in - [%expr - let [%p component_ident_pattern ~loc] = - [%e create_component_expr] - ~useDynamicKey:[%e Ast_builder.(ebool ~loc useDynamicKey)] - [%e component_name] - in - fun ?(key = Brisk_reconciler.Key.none) -> - [%e map_component_expression expr]] + let fun_expr expr = + [%expr + let [%p component_ident_pattern ~loc] = + [%e create_component_expr] + ~useDynamicKey:[%e Ast_builder.(ebool ~loc useDynamicKey)] + [%e component_name] + in + fun ?(key = Brisk_reconciler.Key.none) -> + [%e map_component_expression expr]] + in + match expr with + | {pexp_desc = Pexp_constraint (expr, core_type)} -> + {expr with pexp_desc = Pexp_constraint (fun_expr expr, core_type)} + | _ -> fun_expr expr + let declare_attribute ctx typ = let open Ppxlib.Attribute in From da87888df84ed090f2932ead2ede50c5a7860013 Mon Sep 17 00:00:00 2001 From: glennsl Date: Tue, 30 Jun 2020 15:10:21 +0200 Subject: [PATCH 3/7] add more failing test --- test/Components.re | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/Components.re b/test/Components.re index 57e8f8a..809cc89 100644 --- a/test/Components.re +++ b/test/Components.re @@ -265,7 +265,12 @@ module LocallyAbstractType: { }; }; -module TypeAnnotation = { - let%component make: (~key:Key.t=?, unit) => element(node) = +module Pexp_constraint = { + let%component make: (~key:Key.t=?, unit) => element(node) = + ((), hooks) => (empty, hooks); +}; + +module Ppat_constraint = { + let%component make: 'a. (~key:Key.t=?, unit) => element(node) = ((), hooks) => (empty, hooks); }; From bc173bbae579ec3831dc207b715eb783e486cb1f Mon Sep 17 00:00:00 2001 From: glennsl Date: Tue, 30 Jun 2020 16:24:11 +0200 Subject: [PATCH 4/7] allow Ppat_constraint --- jsx/brisk_ppx.ml | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/jsx/brisk_ppx.ml b/jsx/brisk_ppx.ml index d4813d9..b4e821c 100644 --- a/jsx/brisk_ppx.ml +++ b/jsx/brisk_ppx.ml @@ -223,17 +223,32 @@ module Declaration_ppx = struct Extension.Context.structure_item Ast_pattern.( pstr - ( pstr_value __ (value_binding ~pat:(ppat_var __) ~expr:__ ^:: nil) + ( pstr_value __ (value_binding ~pat:__ ~expr:__ ^:: nil) ^:: nil )) (fun ~loc ~path recursive pat expr -> + let pat, var_name = + let var_name pat = + match pat with + | {ppat_desc = Ppat_var var} -> var.txt + | _ -> failwith "function name expected" + in + let var_pat name = + ATH.Pat.var ~loc (Ast_builder.Default.Located.mk ~loc name) + in + match pat with + | {ppat_desc = Ppat_constraint (pat, core_type)} -> + let name = var_name pat in + {pat with ppat_desc = Ppat_constraint (var_pat name, core_type)}, + name + | _ -> let name = var_name pat in var_pat name, name + in let component_name = - ATH.Exp.constant ~loc (ATH.Const.string (path ^ "." ^ pat)) + ATH.Exp.constant ~loc (ATH.Const.string (path ^ "." ^ var_name)) in let transformed_expression = transform_component_expr ~useDynamicKey:false ~attribute ~component_name expr in - let pat = ATH.Pat.var ~loc (Ast_builder.Default.Located.mk ~loc pat) in match recursive with | Recursive -> [%stri let rec [%p pat] = [%e transformed_expression]] | Nonrecursive -> [%stri let [%p pat] = [%e transformed_expression]]) From e790304b32e6ddd4658f2c5a58e75a0fbffcd863 Mon Sep 17 00:00:00 2001 From: glennsl Date: Fri, 10 Jul 2020 18:35:50 +0200 Subject: [PATCH 5/7] formatting --- jsx/brisk_ppx.ml | 28 +++++++++++++++------------- test/Components.re | 4 ++-- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/jsx/brisk_ppx.ml b/jsx/brisk_ppx.ml index b4e821c..160cb88 100644 --- a/jsx/brisk_ppx.ml +++ b/jsx/brisk_ppx.ml @@ -134,11 +134,10 @@ module Declaration_ppx = struct [%e map_component_expression expr]] in match expr with - | {pexp_desc = Pexp_constraint (expr, core_type)} -> - {expr with pexp_desc = Pexp_constraint (fun_expr expr, core_type)} + | { pexp_desc = Pexp_constraint (expr, core_type) } -> + { expr with pexp_desc = Pexp_constraint (fun_expr expr, core_type) } | _ -> fun_expr expr - let declare_attribute ctx typ = let open Ppxlib.Attribute in declare (attribute_name typ) ctx @@ -222,28 +221,31 @@ module Declaration_ppx = struct Extension.declare (attribute_name attribute) Extension.Context.structure_item Ast_pattern.( - pstr - ( pstr_value __ (value_binding ~pat:__ ~expr:__ ^:: nil) - ^:: nil )) + pstr (pstr_value __ (value_binding ~pat:__ ~expr:__ ^:: nil) ^:: nil)) (fun ~loc ~path recursive pat expr -> let pat, var_name = let var_name pat = match pat with - | {ppat_desc = Ppat_var var} -> var.txt + | { ppat_desc = Ppat_var var } -> var.txt | _ -> failwith "function name expected" in let var_pat name = ATH.Pat.var ~loc (Ast_builder.Default.Located.mk ~loc name) in match pat with - | {ppat_desc = Ppat_constraint (pat, core_type)} -> - let name = var_name pat in - {pat with ppat_desc = Ppat_constraint (var_pat name, core_type)}, - name - | _ -> let name = var_name pat in var_pat name, name + | { ppat_desc = Ppat_constraint (pat, core_type) } -> + let name = var_name pat in + ( { + pat with + ppat_desc = Ppat_constraint (var_pat name, core_type); + }, + name ) + | _ -> + let name = var_name pat in + (var_pat name, name) in let component_name = - ATH.Exp.constant ~loc (ATH.Const.string (path ^ "." ^ var_name)) + ATH.Exp.constant ~loc (ATH.Const.string (path ^ "." ^ var_name)) in let transformed_expression = transform_component_expr ~useDynamicKey:false ~attribute diff --git a/test/Components.re b/test/Components.re index 809cc89..5a8532a 100644 --- a/test/Components.re +++ b/test/Components.re @@ -266,11 +266,11 @@ module LocallyAbstractType: { }; module Pexp_constraint = { - let%component make: (~key:Key.t=?, unit) => element(node) = + let%component make: (~key: Key.t=?, unit) => element(node) = ((), hooks) => (empty, hooks); }; module Ppat_constraint = { - let%component make: 'a. (~key:Key.t=?, unit) => element(node) = + let%component make: 'a. (~key: Key.t=?, unit) => element(node) = ((), hooks) => (empty, hooks); }; From 000a0ed10666fb0d691a5a784f97d5e0fcb1caca Mon Sep 17 00:00:00 2001 From: glennsl Date: Sun, 12 Jul 2020 13:45:32 +0200 Subject: [PATCH 6/7] uss ppxlib instead of AST directly --- jsx/brisk_ppx.ml | 48 +++++++++++++++++++++++------------------------- 1 file changed, 23 insertions(+), 25 deletions(-) diff --git a/jsx/brisk_ppx.ml b/jsx/brisk_ppx.ml index 160cb88..c3a21c4 100644 --- a/jsx/brisk_ppx.ml +++ b/jsx/brisk_ppx.ml @@ -1,6 +1,7 @@ module P = Ppxlib.Ast module ATH = Ppxlib.Ast_helper module Ast_builder = Ppxlib.Ast_builder.Default +module Ast_pattern = Ppxlib.Ast_pattern let component_ident ~loc = Ast_builder.(pexp_ident ~loc (Located.lident ~loc "brisk-component")) @@ -35,7 +36,7 @@ module JSX_ppx = struct ATH.Exp.apply ~loc ~attrs (component_ident ~loc) args let is_jsx = - let open Ppxlib.Ast_pattern in + let open Ast_pattern in let jsx_attr = attribute ~name:(string "JSX") ~payload:__ in fun attr -> parse jsx_attr Ppxlib.Location.none @@ -77,7 +78,7 @@ end module Declaration_ppx = struct let func_pattern = - Ppxlib.Ast_pattern.( + Ast_pattern.( alt ( pexp_fun __ __ __ __ |> map ~f:(fun f lbl opt_arg pat expr -> @@ -86,7 +87,7 @@ module Declaration_ppx = struct |> map ~f:(fun f ident expr -> f (`Newtype (ident, expr))) )) let match_ pattern ?on_error loc ast_node ~with_ = - Ppxlib.Ast_pattern.parse pattern ?on_error loc ast_node with_ + Ast_pattern.parse pattern ?on_error loc ast_node with_ let attribute_name = function | `Component -> "component" @@ -133,15 +134,17 @@ module Declaration_ppx = struct fun ?(key = Brisk_reconciler.Key.none) -> [%e map_component_expression expr]] in - match expr with - | { pexp_desc = Pexp_constraint (expr, core_type) } -> - { expr with pexp_desc = Pexp_constraint (fun_expr expr, core_type) } - | _ -> fun_expr expr + match_ + Ast_pattern.(pexp_constraint __ __) + loc expr + ~with_:(fun expr core_type -> + Ast_builder.(pexp_constraint ~loc (fun_expr expr) core_type)) + ~on_error:(fun () -> fun_expr expr) let declare_attribute ctx typ = let open Ppxlib.Attribute in declare (attribute_name typ) ctx - Ppxlib.Ast_pattern.( + Ast_pattern.( alt_option (single_expr_payload (pexp_ident (lident __'))) (pstr nil)) (function | Some { txt = "useDynamicKey" } -> true @@ -193,7 +196,7 @@ module Declaration_ppx = struct in let transform ~useDynamicKey attribute value_binding = let value_binding_loc = value_binding.P.pvb_loc in - Ppxlib.Ast_pattern.(parse (value_binding ~pat:(ppat_var __) ~expr:__)) + Ast_pattern.(parse (value_binding ~pat:(ppat_var __) ~expr:__)) value_binding_loc value_binding (fun var_pat expr -> let component_name = ATH.Exp.constant ~loc:expr.P.pexp_loc (ATH.Const.string var_pat) @@ -217,32 +220,27 @@ module Declaration_ppx = struct | None -> unmatched_value_binding ) let register attribute = - let open Ppxlib in - Extension.declare (attribute_name attribute) - Extension.Context.structure_item + Ppxlib.Extension.declare (attribute_name attribute) + Ppxlib.Extension.Context.structure_item Ast_pattern.( pstr (pstr_value __ (value_binding ~pat:__ ~expr:__ ^:: nil) ^:: nil)) (fun ~loc ~path recursive pat expr -> let pat, var_name = let var_name pat = - match pat with - | { ppat_desc = Ppat_var var } -> var.txt - | _ -> failwith "function name expected" + match_ Ast_pattern.(ppat_var __) loc pat ~with_:(fun name -> name) in let var_pat name = - ATH.Pat.var ~loc (Ast_builder.Default.Located.mk ~loc name) + ATH.Pat.var ~loc (Ast_builder.Located.mk ~loc name) in - match pat with - | { ppat_desc = Ppat_constraint (pat, core_type) } -> + match_ + Ast_pattern.(ppat_constraint __ __) + loc pat + ~with_:(fun pat core_type -> let name = var_name pat in - ( { - pat with - ppat_desc = Ppat_constraint (var_pat name, core_type); - }, - name ) - | _ -> + (Ast_builder.(ppat_constraint ~loc (var_pat name) core_type), name)) + ~on_error:(fun () -> let name = var_name pat in - (var_pat name, name) + (var_pat name, name)) in let component_name = ATH.Exp.constant ~loc (ATH.Const.string (path ^ "." ^ var_name)) From 29def122d28007457910cd08f0136d7274f69e6e Mon Sep 17 00:00:00 2001 From: glennsl Date: Sun, 12 Jul 2020 13:46:43 +0200 Subject: [PATCH 7/7] add explanatory comments to test cases --- test/Components.re | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/Components.re b/test/Components.re index 5a8532a..ff658d0 100644 --- a/test/Components.re +++ b/test/Components.re @@ -265,11 +265,13 @@ module LocallyAbstractType: { }; }; +// Test to make sure type annotations are accepted module Pexp_constraint = { let%component make: (~key: Key.t=?, unit) => element(node) = ((), hooks) => (empty, hooks); }; +// Test to make sure type annotations with universal quantifiers are accepted module Ppat_constraint = { let%component make: 'a. (~key: Key.t=?, unit) => element(node) = ((), hooks) => (empty, hooks);