Compiler: Fix renaming for object partterns and assignment targets

CHANGES.md

@@ -12,6 +12,8 @@
 
 ## Bug fixes
 
+* Compiler: fix variable renaming for property binding and assignment target
+
 # 5.7.1 (2024-03-05) - Lille
 
 ## Features/Changes

compiler/lib/javascript.ml

@@ -367,7 +367,7 @@ and block = statement_list
 and statement_list = (statement * location) list
 
 and variable_declaration =
-  | DeclIdent of binding_ident * initialiser option
+  | DeclIdent of ident * initialiser option
   | DeclPattern of binding_pattern * initialiser
 
 and variable_declaration_kind =
@@ -416,16 +416,16 @@ and for_binding = binding
 and binding_element = binding * initialiser option
 
 and binding =
-  | BindingIdent of binding_ident
+  | BindingIdent of ident
   | BindingPattern of binding_pattern
 
 and binding_pattern =
-  | ObjectBinding of (binding_property, binding_ident) list_with_rest
+  | ObjectBinding of (binding_property, ident) list_with_rest
   | ArrayBinding of (binding_element option, binding) list_with_rest
 
 and object_target_elt =
-  | TargetPropertyId of ident * initialiser option
-  | TargetProperty of property_name * expression
+  | TargetPropertyId of ident_prop * initialiser option
+  | TargetProperty of property_name * expression * initialiser option
   | TargetPropertySpread of expression
   | TargetPropertyMethod of property_name * method_
 
@@ -439,11 +439,11 @@ and assignment_target =
   | ObjectTarget of object_target_elt list
   | ArrayTarget of array_target_elt list
 
-and binding_ident = ident
+and ident_prop = Prop_and_ident of ident
 
 and binding_property =
   | Prop_binding of property_name * binding_element
-  | Prop_ident of binding_ident * initialiser option
+  | Prop_ident of ident_prop * initialiser option
 
 and function_body = statement_list
 
@@ -530,7 +530,7 @@ and bound_idents_of_pattern p =
   match p with
   | ObjectBinding { list; rest } -> (
       List.concat_map list ~f:(function
-          | Prop_ident (i, _) -> [ i ]
+          | Prop_ident (Prop_and_ident i, _) -> [ i ]
           | Prop_binding (_, e) -> bound_idents_of_element e)
       @
       match rest with
@@ -588,10 +588,17 @@ let rec assignment_target_of_expr' x =
       let list =
         List.map l ~f:(function
             | Property (PNI n, EVar (S { name = n'; _ } as id))
-              when Utf8_string.equal n n' -> TargetPropertyId (id, None)
-            | Property (n, e) -> TargetProperty (n, assignment_target_of_expr' e)
+              when Utf8_string.equal n n' -> TargetPropertyId (Prop_and_ident id, None)
+            | Property (n, e) ->
+                let e, i =
+                  match e with
+                  | EBin (Eq, e, i) -> e, Some (i, N)
+                  | _ -> e, None
+                in
+                TargetProperty (n, assignment_target_of_expr' e, i)
             | CoverInitializedName (_, i, (e, loc)) ->
-                TargetPropertyId (i, Some (assignment_target_of_expr' e, loc))
+                TargetPropertyId
+                  (Prop_and_ident i, Some (assignment_target_of_expr' e, loc))
             | PropertySpread e -> TargetPropertySpread (assignment_target_of_expr' e)
             | PropertyMethod (n, m) -> TargetPropertyMethod (n, m))
       in

compiler/lib/javascript.mli

@@ -285,7 +285,7 @@ and block = statement_list
 and statement_list = (statement * location) list
 
 and variable_declaration =
-  | DeclIdent of binding_ident * initialiser option
+  | DeclIdent of ident * initialiser option
   | DeclPattern of binding_pattern * initialiser
 
 and variable_declaration_kind =
@@ -334,16 +334,16 @@ and for_binding = binding
 and binding_element = binding * initialiser option
 
 and binding =
-  | BindingIdent of binding_ident
+  | BindingIdent of ident
   | BindingPattern of binding_pattern
 
 and binding_pattern =
-  | ObjectBinding of (binding_property, binding_ident) list_with_rest
+  | ObjectBinding of (binding_property, ident) list_with_rest
   | ArrayBinding of (binding_element option, binding) list_with_rest
 
 and object_target_elt =
-  | TargetPropertyId of ident * initialiser option
-  | TargetProperty of property_name * expression
+  | TargetPropertyId of ident_prop * initialiser option
+  | TargetProperty of property_name * expression * initialiser option
   | TargetPropertySpread of expression
   | TargetPropertyMethod of property_name * method_
 
@@ -357,11 +357,11 @@ and assignment_target =
   | ObjectTarget of object_target_elt list
   | ArrayTarget of array_target_elt list
 
-and binding_ident = ident
+and ident_prop = Prop_and_ident of ident
 
 and binding_property =
   | Prop_binding of property_name * binding_element
-  | Prop_ident of binding_ident * initialiser option
+  | Prop_ident of ident_prop * initialiser option
 
 and function_body = statement_list
 

compiler/lib/js_output.ml

@@ -740,20 +740,31 @@ struct
     | EAssignTarget t -> (
         let property f p =
           match p with
-          | TargetPropertyId (id, None) -> ident f id
-          | TargetPropertyId (id, Some (e, _)) ->
+          | TargetPropertyId (Prop_and_ident id, None) -> ident f id
+          | TargetPropertyId (Prop_and_ident id, Some (e, _)) ->
               ident f id;
               PP.space f;
               PP.string f "=";
               PP.space f;
               expression AssignementExpression f e
-          | TargetProperty (pn, e) ->
+          | TargetProperty (pn, e, None) ->
               PP.start_group f 0;
               property_name f pn;
               PP.string f ":";
               PP.space f;
               expression AssignementExpression f e;
               PP.end_group f
+          | TargetProperty (pn, e, Some (ini, _)) ->
+              PP.start_group f 0;
+              property_name f pn;
+              PP.string f ":";
+              PP.space f;
+              expression AssignementExpression f e;
+              PP.space f;
+              PP.string f "=";
+              PP.space f;
+              expression AssignementExpression f ini;
+              PP.end_group f
           | TargetPropertySpread e ->
               PP.string f "...";
               expression AssignementExpression f e
@@ -1112,8 +1123,8 @@ struct
         PP.string f ":";
         PP.space f;
         binding_element f e
-    | Prop_ident (i, None) -> ident f i
-    | Prop_ident (i, Some (e, loc)) ->
+    | Prop_ident (Prop_and_ident i, None) -> ident f i
+    | Prop_ident (Prop_and_ident i, Some (e, loc)) ->
         ident f i;
         PP.space f;
         PP.string f "=";

compiler/lib/js_parser.mly

@@ -503,7 +503,7 @@ object_binding_pattern:
     { ObjectBinding {list=l;rest= Some r} }
 
 binding_property:
-  | i=ident e=initializer_? { Prop_ident (i, e) }
+  | i=ident e=initializer_? { Prop_ident (Prop_and_ident i, e) }
   | pn=property_name ":" e=binding_element { Prop_binding (pn, e) }
 
 binding_property_rest:

compiler/lib/js_traverse.ml

@@ -52,6 +52,8 @@ class type mapper = object
     -> Javascript.for_binding
     -> Javascript.for_binding
 
+  method binding_property : Javascript.binding_property -> Javascript.binding_property
+
   method variable_declaration :
        Javascript.variable_declaration_kind
     -> Javascript.variable_declaration
@@ -295,10 +297,11 @@ class map : mapper =
               EAssignTarget
                 (ObjectTarget
                    (List.map l ~f:(function
-                       | TargetPropertyId (i, e) ->
-                           TargetPropertyId (m#ident i, m#initialiser_o e)
-                       | TargetProperty (i, e) ->
-                           TargetProperty (m#property_name i, m#expression e)
+                       | TargetPropertyId (Prop_and_ident i, e) ->
+                           TargetPropertyId (Prop_and_ident (m#ident i), m#initialiser_o e)
+                       | TargetProperty (n, e, i) ->
+                           TargetProperty
+                             (m#property_name n, m#expression e, m#initialiser_o i)
                        | TargetPropertyMethod (n, x) ->
                            TargetPropertyMethod (m#property_name n, m#method_ x)
                        | TargetPropertySpread e -> TargetPropertySpread (m#expression e))))
@@ -376,10 +379,11 @@ class map : mapper =
       | None -> None
       | Some (b, e) -> Some (m#binding b, m#initialiser_o e)
 
-    method private binding_property x =
+    method binding_property x =
       match x with
       | Prop_binding (i, e) -> Prop_binding (m#property_name i, m#binding_element e)
-      | Prop_ident (i, e) -> Prop_ident (m#ident i, m#initialiser_o e)
+      | Prop_ident (Prop_and_ident i, e) ->
+          Prop_ident (Prop_and_ident (m#ident i), m#initialiser_o e)
 
     method expression_o x =
       match x with
@@ -641,12 +645,13 @@ class iter : iterator =
                   | TargetElementSpread e -> m#expression e)
           | ObjectTarget l ->
               List.iter l ~f:(function
-                  | TargetPropertyId (i, e) ->
+                  | TargetPropertyId (Prop_and_ident i, e) ->
                       m#ident i;
                       m#initialiser_o e
-                  | TargetProperty (i, e) ->
-                      m#property_name i;
-                      m#expression e
+                  | TargetProperty (n, e, i) ->
+                      m#property_name n;
+                      m#expression e;
+                      m#initialiser_o i
                   | TargetPropertyMethod (n, x) ->
                       m#property_name n;
                       m#method_ x
@@ -736,7 +741,7 @@ class iter : iterator =
     method private binding_property x =
       match x with
       | Prop_binding ((_ : property_name), e) -> m#binding_element e
-      | Prop_ident (i, e) ->
+      | Prop_ident (Prop_and_ident i, e) ->
           m#ident i;
           m#initialiser_o e
 
@@ -1340,6 +1345,14 @@ class rename_variable ~esm =
         let m' = m#update_state Lexical_block [] p in
         m'#statements p
 
+    method binding_property x =
+      match x with
+      | Prop_ident (Prop_and_ident (S { name = Utf8 name' as name; _ } as ident), e)
+        when StringMap.mem name' subst ->
+          let x = Prop_binding (PNI name, (BindingIdent ident, e)) in
+          super#binding_property x
+      | x -> x
+
     method expression e =
       match e with
       | EFun (ident, (k, params, body, nid)) ->
@@ -1351,6 +1364,16 @@ class rename_variable ~esm =
       | EClass (Some id, cl_decl) ->
           let m' = m#update_state Lexical_block [ id ] [] in
           EClass (Some (m'#ident id), m'#class_decl cl_decl)
+      | EAssignTarget (ObjectTarget l) ->
+          let l =
+            List.map l ~f:(function
+                | TargetPropertyId
+                    (Prop_and_ident (S { name = Utf8 name' as name; _ } as ident), rhs)
+                  when StringMap.mem name' subst ->
+                    TargetProperty (PNI name, EVar ident, rhs)
+                | b -> b)
+          in
+          super#expression (EAssignTarget (ObjectTarget l))
       | _ -> super#expression e
 
     method statement s =

compiler/lib/js_traverse.mli

@@ -47,6 +47,8 @@ class type mapper = object
     -> Javascript.for_binding
     -> Javascript.for_binding
 
+  method binding_property : Javascript.binding_property -> Javascript.binding_property
+
   method variable_declaration :
        Javascript.variable_declaration_kind
     -> Javascript.variable_declaration

compiler/tests-compiler/minify.ml

@@ -417,3 +417,58 @@ test()
           4: a(a,b=c.b){return a+b}console.log(a(1))}test(); |}];
       print_endline (run_javascript js_min_file);
       [%expect {| 3 |}])
+
+let%expect_test _ =
+  with_temp_dir ~f:(fun () ->
+      let minify js_prog =
+        let js_file =
+          js_prog |> Filetype.js_text_of_string |> Filetype.write_js ~name:"test.js"
+        in
+        let js_min_file =
+          js_file |> jsoo_minify ~flags:[ "--enable"; "shortvar" ] ~pretty:false
+        in
+        print_file (Filetype.path_of_js_file js_min_file)
+      in
+      minify {|
+function f (x) {
+  let {toto} = x;
+  return toto;
+}
+|};
+      [%expect
+        {|
+        $ cat "test.min.js"
+          1: function
+          2: f(a){let{toto:b}=a;return b} |}];
+      minify
+        {|
+function g(x) {
+  var toto, test, tata; 
+  for( { toto : tata = test } in x ) {
+    console.log(tata);
+  }
+}
+|};
+      [%expect
+        {|
+        $ cat "test.min.js"
+          1: function
+          2: g(a){var
+          3: d,c,b;for({toto:b=c}in
+          4: a)console.log(b)} |}];
+      minify
+        {|
+function h(x) {
+  var toto;
+  for( { toto } in x ) {
+    console.log(toto);
+  }
+}
+|};
+      [%expect
+        {|
+        $ cat "test.min.js"
+          1: function
+          2: h(a){var
+          3: b;for({toto:b}in
+          4: a)console.log(b)} |}])