Skip to content

Commit cbd50de

Browse files
hhugohhugo
authored
Compiler: Fix renaming for object partterns and assignment targets (#1588)
1 parent d2cf189 commit cbd50de

File tree

8 files changed

+135
-35
lines changed

8 files changed

+135
-35
lines changed

‎CHANGES.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212

1313
## Bug fixes
1414

15+
* Compiler: fix variable renaming for property binding and assignment target
16+
1517
# 5.7.1 (2024-03-05) - Lille
1618

1719
## Features/Changes

‎compiler/lib/javascript.ml‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ and block = statement_list
367367
and statement_list = (statement * location) list
368368

369369
and variable_declaration =
370-
| DeclIdent of binding_ident * initialiser option
370+
| DeclIdent of ident * initialiser option
371371
| DeclPattern of binding_pattern * initialiser
372372

373373
and variable_declaration_kind =
@@ -416,16 +416,16 @@ and for_binding = binding
416416
and binding_element = binding * initialiser option
417417

418418
and binding =
419-
| BindingIdent of binding_ident
419+
| BindingIdent of ident
420420
| BindingPattern of binding_pattern
421421

422422
and binding_pattern =
423-
| ObjectBinding of (binding_property, binding_ident) list_with_rest
423+
| ObjectBinding of (binding_property, ident) list_with_rest
424424
| ArrayBinding of (binding_element option, binding) list_with_rest
425425

426426
and object_target_elt =
427-
| TargetPropertyId of ident * initialiser option
428-
| TargetProperty of property_name * expression
427+
| TargetPropertyId of ident_prop * initialiser option
428+
| TargetProperty of property_name * expression * initialiser option
429429
| TargetPropertySpread of expression
430430
| TargetPropertyMethod of property_name * method_
431431

@@ -439,11 +439,11 @@ and assignment_target =
439439
| ObjectTarget of object_target_elt list
440440
| ArrayTarget of array_target_elt list
441441

442-
and binding_ident = ident
442+
and ident_prop = Prop_and_ident of ident
443443

444444
and binding_property =
445445
| Prop_binding of property_name * binding_element
446-
| Prop_ident of binding_ident * initialiser option
446+
| Prop_ident of ident_prop * initialiser option
447447

448448
and function_body = statement_list
449449

@@ -530,7 +530,7 @@ and bound_idents_of_pattern p =
530530
match p with
531531
| ObjectBinding { list; rest } -> (
532532
List.concat_map list ~f:(function
533-
| Prop_ident (i, _) -> [ i ]
533+
| Prop_ident (Prop_and_ident i, _) -> [ i ]
534534
| Prop_binding (_, e) -> bound_idents_of_element e)
535535
@
536536
match rest with
@@ -588,10 +588,17 @@ let rec assignment_target_of_expr' x =
588588
let list =
589589
List.map l ~f:(function
590590
| Property (PNI n, EVar (S { name = n'; _ } as id))
591-
when Utf8_string.equal n n' -> TargetPropertyId (id, None)
592-
| Property (n, e) -> TargetProperty (n, assignment_target_of_expr' e)
591+
when Utf8_string.equal n n' -> TargetPropertyId (Prop_and_ident id, None)
592+
| Property (n, e) ->
593+
let e, i =
594+
match e with
595+
| EBin (Eq, e, i) -> e, Some (i, N)
596+
| _ -> e, None
597+
in
598+
TargetProperty (n, assignment_target_of_expr' e, i)
593599
| CoverInitializedName (_, i, (e, loc)) ->
594-
TargetPropertyId (i, Some (assignment_target_of_expr' e, loc))
600+
TargetPropertyId
601+
(Prop_and_ident i, Some (assignment_target_of_expr' e, loc))
595602
| PropertySpread e -> TargetPropertySpread (assignment_target_of_expr' e)
596603
| PropertyMethod (n, m) -> TargetPropertyMethod (n, m))
597604
in

‎compiler/lib/javascript.mli‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ and block = statement_list
285285
and statement_list = (statement * location) list
286286

287287
and variable_declaration =
288-
| DeclIdent of binding_ident * initialiser option
288+
| DeclIdent of ident * initialiser option
289289
| DeclPattern of binding_pattern * initialiser
290290

291291
and variable_declaration_kind =
@@ -334,16 +334,16 @@ and for_binding = binding
334334
and binding_element = binding * initialiser option
335335

336336
and binding =
337-
| BindingIdent of binding_ident
337+
| BindingIdent of ident
338338
| BindingPattern of binding_pattern
339339

340340
and binding_pattern =
341-
| ObjectBinding of (binding_property, binding_ident) list_with_rest
341+
| ObjectBinding of (binding_property, ident) list_with_rest
342342
| ArrayBinding of (binding_element option, binding) list_with_rest
343343

344344
and object_target_elt =
345-
| TargetPropertyId of ident * initialiser option
346-
| TargetProperty of property_name * expression
345+
| TargetPropertyId of ident_prop * initialiser option
346+
| TargetProperty of property_name * expression * initialiser option
347347
| TargetPropertySpread of expression
348348
| TargetPropertyMethod of property_name * method_
349349

@@ -357,11 +357,11 @@ and assignment_target =
357357
| ObjectTarget of object_target_elt list
358358
| ArrayTarget of array_target_elt list
359359

360-
and binding_ident = ident
360+
and ident_prop = Prop_and_ident of ident
361361

362362
and binding_property =
363363
| Prop_binding of property_name * binding_element
364-
| Prop_ident of binding_ident * initialiser option
364+
| Prop_ident of ident_prop * initialiser option
365365

366366
and function_body = statement_list
367367

‎compiler/lib/js_output.ml‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -740,20 +740,31 @@ struct
740740
| EAssignTarget t -> (
741741
let property f p =
742742
match p with
743-
| TargetPropertyId (id, None) -> ident f id
744-
| TargetPropertyId (id, Some (e, _)) ->
743+
| TargetPropertyId (Prop_and_ident id, None) -> ident f id
744+
| TargetPropertyId (Prop_and_ident id, Some (e, _)) ->
745745
ident f id;
746746
PP.space f;
747747
PP.string f "=";
748748
PP.space f;
749749
expression AssignementExpression f e
750-
| TargetProperty (pn, e) ->
750+
| TargetProperty (pn, e, None) ->
751751
PP.start_group f 0;
752752
property_name f pn;
753753
PP.string f ":";
754754
PP.space f;
755755
expression AssignementExpression f e;
756756
PP.end_group f
757+
| TargetProperty (pn, e, Some (ini, _)) ->
758+
PP.start_group f 0;
759+
property_name f pn;
760+
PP.string f ":";
761+
PP.space f;
762+
expression AssignementExpression f e;
763+
PP.space f;
764+
PP.string f "=";
765+
PP.space f;
766+
expression AssignementExpression f ini;
767+
PP.end_group f
757768
| TargetPropertySpread e ->
758769
PP.string f "...";
759770
expression AssignementExpression f e
@@ -1112,8 +1123,8 @@ struct
11121123
PP.string f ":";
11131124
PP.space f;
11141125
binding_element f e
1115-
| Prop_ident (i, None) -> ident f i
1116-
| Prop_ident (i, Some (e, loc)) ->
1126+
| Prop_ident (Prop_and_ident i, None) -> ident f i
1127+
| Prop_ident (Prop_and_ident i, Some (e, loc)) ->
11171128
ident f i;
11181129
PP.space f;
11191130
PP.string f "=";

‎compiler/lib/js_parser.mly‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,7 @@ object_binding_pattern:
503503
{ ObjectBinding {list=l;rest= Some r} }
504504

505505
binding_property:
506-
| i=ident e=initializer_? { Prop_ident (i, e) }
506+
| i=ident e=initializer_? { Prop_ident (Prop_and_ident i, e) }
507507
| pn=property_name ":" e=binding_element { Prop_binding (pn, e) }
508508

509509
binding_property_rest:

‎compiler/lib/js_traverse.ml‎

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ class type mapper = object
5252
-> Javascript.for_binding
5353
-> Javascript.for_binding
5454

55+
method binding_property : Javascript.binding_property -> Javascript.binding_property
56+
5557
method variable_declaration :
5658
Javascript.variable_declaration_kind
5759
-> Javascript.variable_declaration
@@ -295,10 +297,11 @@ class map : mapper =
295297
EAssignTarget
296298
(ObjectTarget
297299
(List.map l ~f:(function
298-
| TargetPropertyId (i, e) ->
299-
TargetPropertyId (m#ident i, m#initialiser_o e)
300-
| TargetProperty (i, e) ->
301-
TargetProperty (m#property_name i, m#expression e)
300+
| TargetPropertyId (Prop_and_ident i, e) ->
301+
TargetPropertyId (Prop_and_ident (m#ident i), m#initialiser_o e)
302+
| TargetProperty (n, e, i) ->
303+
TargetProperty
304+
(m#property_name n, m#expression e, m#initialiser_o i)
302305
| TargetPropertyMethod (n, x) ->
303306
TargetPropertyMethod (m#property_name n, m#method_ x)
304307
| TargetPropertySpread e -> TargetPropertySpread (m#expression e))))
@@ -376,10 +379,11 @@ class map : mapper =
376379
| None -> None
377380
| Some (b, e) -> Some (m#binding b, m#initialiser_o e)
378381

379-
method private binding_property x =
382+
method binding_property x =
380383
match x with
381384
| Prop_binding (i, e) -> Prop_binding (m#property_name i, m#binding_element e)
382-
| Prop_ident (i, e) -> Prop_ident (m#ident i, m#initialiser_o e)
385+
| Prop_ident (Prop_and_ident i, e) ->
386+
Prop_ident (Prop_and_ident (m#ident i), m#initialiser_o e)
383387

384388
method expression_o x =
385389
match x with
@@ -641,12 +645,13 @@ class iter : iterator =
641645
| TargetElementSpread e -> m#expression e)
642646
| ObjectTarget l ->
643647
List.iter l ~f:(function
644-
| TargetPropertyId (i, e) ->
648+
| TargetPropertyId (Prop_and_ident i, e) ->
645649
m#ident i;
646650
m#initialiser_o e
647-
| TargetProperty (i, e) ->
648-
m#property_name i;
649-
m#expression e
651+
| TargetProperty (n, e, i) ->
652+
m#property_name n;
653+
m#expression e;
654+
m#initialiser_o i
650655
| TargetPropertyMethod (n, x) ->
651656
m#property_name n;
652657
m#method_ x
@@ -736,7 +741,7 @@ class iter : iterator =
736741
method private binding_property x =
737742
match x with
738743
| Prop_binding ((_ : property_name), e) -> m#binding_element e
739-
| Prop_ident (i, e) ->
744+
| Prop_ident (Prop_and_ident i, e) ->
740745
m#ident i;
741746
m#initialiser_o e
742747

@@ -1340,6 +1345,14 @@ class rename_variable ~esm =
13401345
let m' = m#update_state Lexical_block [] p in
13411346
m'#statements p
13421347

1348+
method binding_property x =
1349+
match x with
1350+
| Prop_ident (Prop_and_ident (S { name = Utf8 name' as name; _ } as ident), e)
1351+
when StringMap.mem name' subst ->
1352+
let x = Prop_binding (PNI name, (BindingIdent ident, e)) in
1353+
super#binding_property x
1354+
| x -> x
1355+
13431356
method expression e =
13441357
match e with
13451358
| EFun (ident, (k, params, body, nid)) ->
@@ -1351,6 +1364,16 @@ class rename_variable ~esm =
13511364
| EClass (Some id, cl_decl) ->
13521365
let m' = m#update_state Lexical_block [ id ] [] in
13531366
EClass (Some (m'#ident id), m'#class_decl cl_decl)
1367+
| EAssignTarget (ObjectTarget l) ->
1368+
let l =
1369+
List.map l ~f:(function
1370+
| TargetPropertyId
1371+
(Prop_and_ident (S { name = Utf8 name' as name; _ } as ident), rhs)
1372+
when StringMap.mem name' subst ->
1373+
TargetProperty (PNI name, EVar ident, rhs)
1374+
| b -> b)
1375+
in
1376+
super#expression (EAssignTarget (ObjectTarget l))
13541377
| _ -> super#expression e
13551378

13561379
method statement s =

‎compiler/lib/js_traverse.mli‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ class type mapper = object
4747
-> Javascript.for_binding
4848
-> Javascript.for_binding
4949

50+
method binding_property : Javascript.binding_property -> Javascript.binding_property
51+
5052
method variable_declaration :
5153
Javascript.variable_declaration_kind
5254
-> Javascript.variable_declaration

‎compiler/tests-compiler/minify.ml‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,3 +417,58 @@ test()
417417
4: a(a,b=c.b){return a+b}console.log(a(1))}test(); |}];
418418
print_endline (run_javascript js_min_file);
419419
[%expect {| 3 |}])
420+
421+
let%expect_test _ =
422+
with_temp_dir ~f:(fun () ->
423+
let minify js_prog =
424+
let js_file =
425+
js_prog |> Filetype.js_text_of_string |> Filetype.write_js ~name:"test.js"
426+
in
427+
let js_min_file =
428+
js_file |> jsoo_minify ~flags:[ "--enable"; "shortvar" ] ~pretty:false
429+
in
430+
print_file (Filetype.path_of_js_file js_min_file)
431+
in
432+
minify {|
433+
function f (x) {
434+
let {toto} = x;
435+
return toto;
436+
}
437+
|};
438+
[%expect
439+
{|
440+
$ cat "test.min.js"
441+
1: function
442+
2: f(a){let{toto:b}=a;return b} |}];
443+
minify
444+
{|
445+
function g(x) {
446+
var toto, test, tata;
447+
for( { toto : tata = test } in x ) {
448+
console.log(tata);
449+
}
450+
}
451+
|};
452+
[%expect
453+
{|
454+
$ cat "test.min.js"
455+
1: function
456+
2: g(a){var
457+
3: d,c,b;for({toto:b=c}in
458+
4: a)console.log(b)} |}];
459+
minify
460+
{|
461+
function h(x) {
462+
var toto;
463+
for( { toto } in x ) {
464+
console.log(toto);
465+
}
466+
}
467+
|};
468+
[%expect
469+
{|
470+
$ cat "test.min.js"
471+
1: function
472+
2: h(a){var
473+
3: b;for({toto:b}in
474+
4: a)console.log(b)} |}])

0 commit comments

Comments
 (0)