From eb7ebfca64946f3dc7058e04502cd50b3dca2f42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Tsnobiladz=C3=A9?= Date: Tue, 20 Feb 2024 17:57:56 +0100 Subject: [PATCH 1/2] fix reexport of tagged template externals --- CHANGELOG.md | 4 ++++ jscomp/core/lam_compile_external_call.ml | 15 +++++++-------- jscomp/test/build.ninja | 3 +-- jscomp/test/tagged_template_test.js | 24 +++++++++++++++++++++++- jscomp/test/tagged_template_test.resi | 0 5 files changed, 35 insertions(+), 11 deletions(-) delete mode 100644 jscomp/test/tagged_template_test.resi diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b4bbb2fef..2e266fc1ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ #### :nail_care: Polish - No parens around tagged template literals. https://github.com/rescript-lang/rescript-compiler/pull/6639 +#### :bug: Bug Fix + +- Fix compiler crash when reexporting tagged template literal externals. https://github.com/rescript-lang/rescript-compiler/pull/6645 + # 11.1.0-rc.2 #### :rocket: New Feature diff --git a/jscomp/core/lam_compile_external_call.ml b/jscomp/core/lam_compile_external_call.ml index 617996b7e2..db03d15a2a 100644 --- a/jscomp/core/lam_compile_external_call.ml +++ b/jscomp/core/lam_compile_external_call.ml @@ -252,16 +252,15 @@ let translate_scoped_access scopes obj = let translate_ffi (cxt : Lam_compile_context.t) arg_types (ffi : External_ffi_types.external_spec) (args : J.expression list) = match ffi with - | Js_call { external_module_name; name; splice; scopes; tagged_template = true } -> + | Js_call { external_module_name; name; splice: _; scopes; tagged_template = true } -> let fn = translate_scoped_module_val external_module_name name scopes in (match args with - | [ stringArgs; valueArgs ] -> ( - match (stringArgs, valueArgs) with - | ({expression_desc = Array (strings, _); _}, {expression_desc = Array (values, _); _}) -> - E.tagged_template fn strings values - | _ -> assert false - ) - | _ -> assert false) + | [ {expression_desc = Array (strings, _); _}; {expression_desc = Array (values, _); _} ] -> + E.tagged_template fn strings values + | _ -> let args, eff, dynamic = assemble_args_has_splice arg_types args in + add_eff eff + (if dynamic then splice_apply fn args + else E.call ~info:{ arity = Full; call_info = Call_na } fn args)) | Js_call { external_module_name = module_name; name = fn; splice; scopes; tagged_template = false } -> let fn = translate_scoped_module_val module_name fn scopes in if splice then diff --git a/jscomp/test/build.ninja b/jscomp/test/build.ninja index f2a4916fd5..e21c488ac4 100644 --- a/jscomp/test/build.ninja +++ b/jscomp/test/build.ninja @@ -576,8 +576,7 @@ o test/submodule.cmi test/submodule.cmj : cc test/submodule.res | $bsc $stdlib r o test/submodule_call.cmi test/submodule_call.cmj : cc test/submodule_call.res | test/submodule.cmj $bsc $stdlib runtime o test/switch_case_test.cmi test/switch_case_test.cmj : cc test/switch_case_test.res | test/mt.cmj $bsc $stdlib runtime o test/switch_string.cmi test/switch_string.cmj : cc test/switch_string.res | $bsc $stdlib runtime -o test/tagged_template_test.cmj : cc_cmi test/tagged_template_test.res | test/mt.cmj test/tagged_template_test.cmi $bsc $stdlib runtime -o test/tagged_template_test.cmi : cc test/tagged_template_test.resi | $bsc $stdlib runtime +o test/tagged_template_test.cmi test/tagged_template_test.cmj : cc test/tagged_template_test.res | test/mt.cmj $bsc $stdlib runtime o test/tailcall_inline_test.cmi test/tailcall_inline_test.cmj : cc test/tailcall_inline_test.res | test/mt.cmj $bsc $stdlib runtime o test/template.cmi test/template.cmj : cc test/template.res | $bsc $stdlib runtime o test/test.cmi test/test.cmj : cc test/test.res | $bsc $stdlib runtime diff --git a/jscomp/test/tagged_template_test.js b/jscomp/test/tagged_template_test.js index c0a4687743..331ac706d9 100644 --- a/jscomp/test/tagged_template_test.js +++ b/jscomp/test/tagged_template_test.js @@ -3,9 +3,21 @@ var Mt = require("./mt.js"); var Caml_array = require("../../lib/js/caml_array.js"); +var Caml_splice_call = require("../../lib/js/caml_splice_call.js"); var Tagged_template_libJs = require("./tagged_template_lib.js"); -var query = Tagged_template_libJs.sql`SELECT * FROM ${"users"} WHERE id = ${"5"}`; +function sql(prim0, prim1) { + return Caml_splice_call.spliceApply(Tagged_template_libJs.sql, [ + prim0, + prim1 + ]); +} + +var table = "users"; + +var id = "5"; + +var query = Tagged_template_libJs.sql`SELECT * FROM ${table} WHERE id = ${id}`; var length = Tagged_template_libJs.length`hello ${10} what's the total length? Is it ${3}?`; @@ -73,4 +85,14 @@ Mt.from_pair_suites("tagged templates", { } }); +var extraLength = 10; + +exports.sql = sql; +exports.table = table; +exports.id = id; +exports.query = query; +exports.extraLength = extraLength; +exports.length = length; +exports.foo = foo; +exports.res = res; /* query Not a pure module */ diff --git a/jscomp/test/tagged_template_test.resi b/jscomp/test/tagged_template_test.resi deleted file mode 100644 index e69de29bb2..0000000000 From efe0b5e655b66305d5bc6b1091746b889f5c3e90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Tsnobiladz=C3=A9?= Date: Tue, 20 Feb 2024 18:52:01 +0100 Subject: [PATCH 2/2] Allow identifier with modules in tagged template literals (e.g. Pg.sql``) --- CHANGELOG.md | 1 + jscomp/syntax/src/res_core.ml | 19 ++++---- .../printer/other/expected/string.res.txt | 2 + jscomp/syntax/tests/printer/other/string.res | 4 +- jscomp/test/tagged_template_test.js | 43 +++++++++++++------ jscomp/test/tagged_template_test.res | 13 +++++- 6 files changed, 58 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e266fc1ba..7e96d0be2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ #### :nail_care: Polish - No parens around tagged template literals. https://github.com/rescript-lang/rescript-compiler/pull/6639 +- Allow identifier with modules in tagged template literals (e.g. Pg.sql`select * from ${table} where id = ${id}`). https://github.com/rescript-lang/rescript-compiler/pull/6645 #### :bug: Bug Fix diff --git a/jscomp/syntax/src/res_core.ml b/jscomp/syntax/src/res_core.ml index eb44638408..b88a9fd4cc 100644 --- a/jscomp/syntax/src/res_core.ml +++ b/jscomp/syntax/src/res_core.ml @@ -2071,8 +2071,7 @@ and parsePrimaryExpr ~operand ?(noCall = false) p = | Backtick when noCall = false && p.prevEndPos.pos_lnum == p.startPos.pos_lnum -> ( match expr.pexp_desc with - | Pexp_ident {txt = Longident.Lident ident} -> - parseTemplateExpr ~prefix:ident p + | Pexp_ident long_ident -> parseTemplateExpr ~prefix:long_ident p | _ -> Parser.err ~startPos:expr.pexp_loc.loc_start ~endPos:expr.pexp_loc.loc_end p @@ -2253,13 +2252,15 @@ and parseBinaryExpr ?(context = OrdinaryExpr) ?a p prec = (* | _ -> false *) (* ) *) -and parseTemplateExpr ?(prefix = "js") p = +and parseTemplateExpr ?prefix p = let partPrefix = (* we could stop treating js and j prefix as something special for json, we would first need to remove @as(json`true`) feature *) match prefix with - | "js" | "j" | "json" -> Some prefix - | _ -> None + | Some {txt = Longident.Lident (("js" | "j" | "json") as prefix); _} -> + Some prefix + | Some _ -> None + | None -> Some "js" in let startPos = p.Parser.startPos in @@ -2296,8 +2297,7 @@ and parseTemplateExpr ?(prefix = "js") p = let values = Ext_list.filter_map parts snd in let endPos = p.Parser.endPos in - let genTaggedTemplateCall () = - let lident = Longident.Lident prefix in + let genTaggedTemplateCall lident = let ident = Ast_helper.Exp.ident ~attrs:[] ~loc:Location.none (Location.mknoloc lident) @@ -2348,8 +2348,9 @@ and parseTemplateExpr ?(prefix = "js") p = in match prefix with - | "js" | "j" | "json" -> genInterpolatedString () - | _ -> genTaggedTemplateCall () + | Some {txt = Longident.Lident ("js" | "j" | "json"); _} | None -> + genInterpolatedString () + | Some {txt = lident} -> genTaggedTemplateCall lident (* Overparse: let f = a : int => a + 1, is it (a : int) => or (a): int => * Also overparse constraints: diff --git a/jscomp/syntax/tests/printer/other/expected/string.res.txt b/jscomp/syntax/tests/printer/other/expected/string.res.txt index a0b671b08a..7a79a966c1 100644 --- a/jscomp/syntax/tests/printer/other/expected/string.res.txt +++ b/jscomp/syntax/tests/printer/other/expected/string.res.txt @@ -17,3 +17,5 @@ let heart = "\u2665" let smile = "emoji: \u{1F600}" let taggedTemplate = sql`select * from ${table} where id = ${id}` + +let taggedTemplate = Pg.sql`select * from ${table} where id = ${id}` diff --git a/jscomp/syntax/tests/printer/other/string.res b/jscomp/syntax/tests/printer/other/string.res index 8b6e7b234c..ee6287e06a 100644 --- a/jscomp/syntax/tests/printer/other/string.res +++ b/jscomp/syntax/tests/printer/other/string.res @@ -16,4 +16,6 @@ let heart = "\u2665" let smile = "emoji: \u{1F600}" -let taggedTemplate = sql`select * from ${table} where id = ${id}` \ No newline at end of file +let taggedTemplate = sql`select * from ${table} where id = ${id}` + +let taggedTemplate = Pg.sql`select * from ${table} where id = ${id}` \ No newline at end of file diff --git a/jscomp/test/tagged_template_test.js b/jscomp/test/tagged_template_test.js index 331ac706d9..c66d9ceb08 100644 --- a/jscomp/test/tagged_template_test.js +++ b/jscomp/test/tagged_template_test.js @@ -13,10 +13,16 @@ function sql(prim0, prim1) { ]); } +var Pg = { + sql: sql +}; + var table = "users"; var id = "5"; +var queryWithModule = Tagged_template_libJs.sql`SELECT * FROM ${table} WHERE id = ${id}`; + var query = Tagged_template_libJs.sql`SELECT * FROM ${table} WHERE id = ${id}`; var length = Tagged_template_libJs.length`hello ${10} what's the total length? Is it ${3}?`; @@ -48,38 +54,50 @@ Mt.from_pair_suites("tagged templates", { ], tl: { hd: [ - "with externals, it should return the result of the function", + "with module scoped externals, it should also return a string with the correct interpolations", (function (param) { return { TAG: "Eq", - _0: length, - _1: 52 + _0: queryWithModule, + _1: "SELECT * FROM 'users' WHERE id = '5'" }; }) ], tl: { hd: [ - "with rescript function, it should return a string with the correct interpolations", + "with externals, it should return the result of the function", (function (param) { return { TAG: "Eq", - _0: res, - _1: "| 5 * 10 = 50 |" + _0: length, + _1: 52 }; }) ], tl: { hd: [ - "a template literal tagged with json should generate a regular string interpolation for now", + "with rescript function, it should return a string with the correct interpolations", (function (param) { return { TAG: "Eq", - _0: "some random " + "string", - _1: "some random string" + _0: res, + _1: "| 5 * 10 = 50 |" }; }) ], - tl: /* [] */0 + tl: { + hd: [ + "a template literal tagged with json should generate a regular string interpolation for now", + (function (param) { + return { + TAG: "Eq", + _0: "some random " + "string", + _1: "some random string" + }; + }) + ], + tl: /* [] */0 + } } } } @@ -87,12 +105,13 @@ Mt.from_pair_suites("tagged templates", { var extraLength = 10; -exports.sql = sql; +exports.Pg = Pg; exports.table = table; exports.id = id; +exports.queryWithModule = queryWithModule; exports.query = query; exports.extraLength = extraLength; exports.length = length; exports.foo = foo; exports.res = res; -/* query Not a pure module */ +/* queryWithModule Not a pure module */ diff --git a/jscomp/test/tagged_template_test.res b/jscomp/test/tagged_template_test.res index 72bd021790..a1e169a582 100644 --- a/jscomp/test/tagged_template_test.res +++ b/jscomp/test/tagged_template_test.res @@ -1,9 +1,14 @@ -@module("./tagged_template_lib.js") @taggedTemplate -external sql: (array, array) => string = "sql" +module Pg = { + @module("./tagged_template_lib.js") @taggedTemplate + external sql: (array, array) => string = "sql" +} let table = "users" let id = "5" +let queryWithModule = Pg.sql`SELECT * FROM ${table} WHERE id = ${id}` + +open Pg let query = sql`SELECT * FROM ${table} WHERE id = ${id}` @module("./tagged_template_lib.js") @taggedTemplate @@ -29,6 +34,10 @@ Mt.from_pair_suites( ( "with externals, it should return a string with the correct interpolations", () => Eq(query, "SELECT * FROM 'users' WHERE id = '5'"), + ), + ( + "with module scoped externals, it should also return a string with the correct interpolations", + () => Eq(queryWithModule, "SELECT * FROM 'users' WHERE id = '5'"), ), ( "with externals, it should return the result of the function",