Skip to content

Remove unsafe "j" interpolation #6068

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

Merged
merged 17 commits into from
Mar 12, 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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ subset of the arguments, and return a curried type with the remaining ones https
Also, `(. int) => string => bool` is not equivalen to `(. int, string) => bool` anymore.
These are only breaking changes for unformatted code.
- Exponentiation operator `**` is now right-associative. `2. ** 3. ** 2.` now compile to `Math.pow(2, Math.pow(3, 2))` and not anymore `Math.pow(Math.pow(2, 3), 2)`. Parentheses can be used to change precedence.
- Remove unsafe ``` j`$(a)$(b)` ``` interpolation deprecated in compiler version 10 https://github.com/rescript-lang/rescript-compiler/pull/6068
- Remove deprecated module `Printexc`

#### :bug: Bug Fix

Expand Down Expand Up @@ -79,6 +81,7 @@ These are only breaking changes for unformatted code.

- Better error message for extension point https://github.com/rescript-lang/rescript-compiler/pull/6057
- Improve format check help https://github.com/rescript-lang/rescript-compiler/pull/6056
- Deprecate unsafe ``` j`$(a)$(b)` ``` interpolation: use string templates ``` `${a}${b}` ``` instead https://github.com/rescript-lang/rescript-compiler/pull/6067

# 10.1.3

Expand Down
9 changes: 9 additions & 0 deletions jscomp/build_tests/super_errors/expected/jinterp.res.expected
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@

We've found a bug for you!
/.../fixtures/jinterp.res:3:10-21

1 │
2 │ let a = 11
3 │ let b = j`number $(a)`

The unsafe j`$(a)$(b)` interpolation was removed, use string template `${a}${b}` instead.
3 changes: 3 additions & 0 deletions jscomp/build_tests/super_errors/fixtures/jinterp.res
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@

let a = 11
let b = j`number $(a)`
127 changes: 7 additions & 120 deletions jscomp/frontend/ast_utf8_string_interp.ml
Original file line number Diff line number Diff line change
Expand Up @@ -62,20 +62,6 @@ type cxt = {

type exn += Error of pos * pos * error

let pp_error fmt err =
Format.pp_print_string fmt
@@
match err with
| Invalid_code_point -> "Invalid code point"
| Unterminated_backslash -> "\\ ended unexpectedly"
| Invalid_escape_code c -> "Invalid escape code: " ^ String.make 1 c
| Invalid_hex_escape -> "Invalid \\x escape"
| Invalid_unicode_escape -> "Invalid \\u escape"
| Unterminated_variable -> "$ unterminated"
| Unmatched_paren -> "Unmatched paren"
| Invalid_syntax_of_var s ->
"`" ^ s ^ "' is not a valid syntax of interpolated identifer"

let valid_lead_identifier_char x =
match x with 'a' .. 'z' | '_' -> true | _ -> false

Expand All @@ -97,31 +83,6 @@ let valid_identifier s =
| ' ' | '\n' | '\t' -> true
| _ -> false *)

(**
FIXME: multiple line offset
if there is no line offset. Note {|{j||} border will never trigger a new line
*)
let update_position border ({ lnum; offset; byte_bol } : pos)
(pos : Lexing.position) =
if lnum = 0 then { pos with pos_cnum = pos.pos_cnum + border + offset }
(* When no newline, the column number is [border + offset] *)
else
{
pos with
pos_lnum = pos.pos_lnum + lnum;
pos_bol = pos.pos_cnum + border + byte_bol;
pos_cnum =
pos.pos_cnum + border + byte_bol + offset
(* when newline, the column number is [offset] *);
}

let update border (start : pos) (finish : pos) (loc : Location.t) : Location.t =
let start_pos = loc.loc_start in
{
loc with
loc_start = update_position border start start_pos;
loc_end = update_position border finish start_pos;
}

(** Note [Var] kind can not be mpty *)
let empty_segment { content } = Ext_string.is_empty content
Expand Down Expand Up @@ -308,21 +269,6 @@ let transform_test s =
check_and_transform 0 s 0 cxt;
List.rev cxt.segments

(** TODO: test empty var $() $ failure,
Allow identifers x.A.y *)

open Ast_helper

(** Longident.parse "Pervasives.^" *)
let concat_ident : Longident.t = Ldot (Lident "Pervasives", "^")
(* FIXME: remove deps on `Pervasives` *)

(* JS string concatMany *)
(* Ldot (Ldot (Lident "Js", "String2"), "concat") *)

(* Longident.parse "Js.String.make" *)
let to_string_ident : Longident.t = Ldot (Ldot (Lident "Js", "String2"), "make")

module Delim = struct
let parse_processed = function
| None -> Some External_arg_spec.DNone
Expand All @@ -332,102 +278,43 @@ module Delim = struct

type interpolation =
| Js (* string interpolation *)
| J (* old unsafe interpolation *)
| Unrecognized (* no interpolation: delimiter not recognized *)
let parse_unprocessed = function
let parse_unprocessed loc = function
| "js" -> Js
| "j" -> J
| "j" ->
Location.raise_errorf ~loc
"The unsafe j`$(a)$(b)` interpolation was removed, use string template `${a}${b}` instead."
| _ -> Unrecognized

let escaped_j_delimiter = "*j" (* not user level syntax allowed *)
let unescaped_j_delimiter = "j"
let unescaped_js_delimiter = "js"
let escaped = Some escaped_j_delimiter
end

let border = String.length "{j|"

let aux loc (segment : segment) ~to_string_ident : Parsetree.expression =
match segment with
| { start; finish; kind; content } -> (
match kind with
| String ->
let loc = update border start finish loc in
Ast_compatible.const_exp_string content ?delimiter:Delim.escaped ~loc
| Var (soffset, foffset) ->
let loc =
{
loc with
loc_start = update_position (soffset + border) start loc.loc_start;
loc_end = update_position (foffset + border) finish loc.loc_start;
}
in
Ast_compatible.apply_simple ~loc
(Exp.ident ~loc { loc; txt = to_string_ident })
[ Exp.ident ~loc { loc; txt = Lident content } ])

let concat_exp a_loc x ~(lhs : Parsetree.expression) : Parsetree.expression =
let loc = Bs_loc.merge a_loc lhs.pexp_loc in
Ast_compatible.apply_simple ~loc
(Exp.ident { txt = concat_ident; loc })
[ lhs; aux loc x ~to_string_ident:(Longident.Lident "__unsafe_cast") ]

(* Invariant: the [lhs] is always of type string *)
let rec handle_segments loc (rev_segments : segment list) =
match rev_segments with
| [] -> Ast_compatible.const_exp_string ~loc "" ?delimiter:Delim.escaped
| [ segment ] -> aux loc segment ~to_string_ident (* string literal *)
| { content = "" } :: rest -> handle_segments loc rest
| a :: rest -> concat_exp loc a ~lhs:(handle_segments loc rest)

let transform_interp loc s =
let s_len = String.length s in
let buf = Buffer.create (s_len * 2) in
try
let cxt : cxt =
{
segment_start = { lnum = 0; offset = 0; byte_bol = 0 };
buf;
s_len;
segments = [];
pos_lnum = 0;
byte_bol = 0;
pos_bol = 0;
}
in

check_and_transform 0 s 0 cxt;
handle_segments loc cxt.segments
with Error (start, pos, error) ->
Location.raise_errorf ~loc:(update border start pos loc) "%a" pp_error error

let transform_exp (e : Parsetree.expression) s delim : Parsetree.expression =
match Delim.parse_unprocessed delim with
match Delim.parse_unprocessed e.pexp_loc delim with
| Js ->
let js_str = Ast_utf8_string.transform e.pexp_loc s in
{
e with
pexp_desc = Pexp_constant (Pconst_string (js_str, Delim.escaped));
}
| J -> transform_interp e.pexp_loc s
| Unrecognized -> e


let transform_pat (p : Parsetree.pattern) s delim : Parsetree.pattern =
match Delim.parse_unprocessed delim with
match Delim.parse_unprocessed p.ppat_loc delim with
| Js ->
let js_str = Ast_utf8_string.transform p.ppat_loc s in
{
p with
ppat_desc = Ppat_constant (Pconst_string (js_str, Delim.escaped));
}
| J (* No j interpolation on patterns *)
| Unrecognized -> p

let is_unicode_string opt = Ext_string.equal opt Delim.escaped_j_delimiter

let is_unescaped s =
Ext_string.equal s Delim.unescaped_j_delimiter
|| Ext_string.equal s Delim.unescaped_js_delimiter
Ext_string.equal s Delim.unescaped_js_delimiter

let parse_processed_delim = Delim.parse_processed
76 changes: 0 additions & 76 deletions jscomp/ounit_tests/ounit_unicode_tests.ml
Original file line number Diff line number Diff line change
Expand Up @@ -243,80 +243,4 @@ let suites =
0,2,0,3,String,")"
]
end;
__LOC__ >:: begin fun _ ->
match Ast_utf8_string_interp.transform_test {j| $( ()) |j}
with
|exception Ast_utf8_string_interp.Error
({lnum = 0; offset = 1; byte_bol = 0},
{lnum = 0; offset = 6; byte_bol = 0}, Invalid_syntax_of_var " (")
-> OUnit.assert_bool __LOC__ true
| _ -> OUnit.assert_bool __LOC__ false
end
;
__LOC__ >:: begin fun _ ->
match Ast_utf8_string_interp.transform_test {|$()|}
with
| exception Ast_utf8_string_interp.Error ({lnum = 0; offset = 0; byte_bol = 0},
{lnum = 0; offset = 3; byte_bol = 0}, Invalid_syntax_of_var "")
-> OUnit.assert_bool __LOC__ true
| _ -> OUnit.assert_bool __LOC__ false
end
;
__LOC__ >:: begin fun _ ->
match Ast_utf8_string_interp.transform_test {|$ ()|}
with
| exception Ast_utf8_string_interp.Error
({lnum = 0; offset = 0; byte_bol = 0},
{lnum = 0; offset = 1; byte_bol = 0}, Invalid_syntax_of_var "")
-> OUnit.assert_bool __LOC__ true
| _ -> OUnit.assert_bool __LOC__ false
end ;
__LOC__ >:: begin fun _ ->
match Ast_utf8_string_interp.transform_test {|$()|} with
| exception Ast_utf8_string_interp.Error
({lnum = 0; offset = 0; byte_bol = 0},
{lnum = 0; offset = 3; byte_bol = 0}, Invalid_syntax_of_var "")
-> OUnit.assert_bool __LOC__ true
| _ -> OUnit.assert_bool __LOC__ false
end
;
__LOC__ >:: begin fun _ ->
match Ast_utf8_string_interp.transform_test {|$(hello world)|} with
| exception Ast_utf8_string_interp.Error
({lnum = 0; offset = 0; byte_bol = 0},
{lnum = 0; offset = 14; byte_bol = 0}, Invalid_syntax_of_var "hello world")
-> OUnit.assert_bool __LOC__ true
| _ -> OUnit.assert_bool __LOC__ false
end


;
__LOC__ >:: begin fun _ ->
match Ast_utf8_string_interp.transform_test {|$( hi*) |} with
| exception Ast_utf8_string_interp.Error
({lnum = 0; offset = 0; byte_bol = 0},
{lnum = 0; offset = 7; byte_bol = 0}, Invalid_syntax_of_var " hi*")
->
OUnit.assert_bool __LOC__ true
| _ -> OUnit.assert_bool __LOC__ false
end;
__LOC__ >:: begin fun _ ->
match Ast_utf8_string_interp.transform_test {|xx $|} with
| exception Ast_utf8_string_interp.Error
({lnum = 0; offset = 3; byte_bol = 0},
{lnum = 0; offset = 3; byte_bol = 0}, Unterminated_variable)
->
OUnit.assert_bool __LOC__ true
| _ -> OUnit.assert_bool __LOC__ false
end ;

__LOC__ >:: begin fun _ ->
match Ast_utf8_string_interp.transform_test {|$(world |}; with
| exception Ast_utf8_string_interp.Error
({lnum = 0; offset = 0; byte_bol = 0},
{lnum = 0; offset = 9; byte_bol = 0}, Unmatched_paren)
->
OUnit.assert_bool __LOC__ true
| _ -> OUnit.assert_bool __LOC__ false
end
]
Loading