Spatch, handling of metavariables

Summary:
Add support for metavariables, just like in sgrep. Can now do

- foo(X,Y)
+ bar(Y,X)

There are still many todos but this could be useful as-is already I think.

Next step is to support semantic patches, not just syntactic patches,
by incopoprating semantic analysis feedback back into the
patch (e.g. some forms of typing).

Test Plan:
./spatch -test
Testing bar.spatch on bar.php expecting bar.exp
Testing bar.spatch on bar2.php expecting bar2.exp
Testing border.spatch on border.php expecting border.exp
Testing border_var.spatch on border_var.php expecting border_var.exp
Testing border_var.spatch on border_var2.php expecting border_var2.exp
Testing distr_plus.spatch on distr_plus.php expecting distr_plus.exp
Testing foo.spatch on foo.php expecting foo.exp

you can now write
$ cat tests/php/spatch/border_var.spatch
   <ui:section-header
-   border=X
   ></ui:section-header>

DiffCamp Revision: 179856
Reviewed By: ahupp
CC: ngao, mroch, epriestley, adonohue, beau, amenghra, ahupp
Revert Plan:
OK

demos/foo3.php

@@ -0,0 +1,9 @@
+<?php
+
+function foo($a, $b) {
+  echo $a, $b;
+}
+
+foo("hello world", bar(2));
+foo("hello world", "this is " . "a string");
+

demos/remove_second_arg.spatch

@@ -0,0 +1,5 @@
+//remove_second_arg_foo.spatch
+  foo(X
+-    ,Y
+     )
+

demos/remove_second_arg_alt.spatch

@@ -0,0 +1,2 @@
+- foo(X, Y)
++ foo(X)

lang_php/matcher/metavars_php.ml

@@ -33,3 +33,20 @@ type mvar = string
 type metavars_binding = (mvar, Ast_php.any) Common.assoc
 
 let empty_environment = []
+
+let metavar_regexp_string = 
+  "\\([A-Z]\\([0-9]?_[A-Z]*\\)?\\)"
+
+let metavar_regexp  = Str.regexp metavar_regexp_string
+
+(* 
+ * Hacks abusing existing PHP constructs to encode extra constructions.
+ * One day we will have a pattern_php_ast.ml that mimic mostly
+ * ast_php.ml and extend it.
+ *)
+let is_metavar_name s = 
+  s ==~ metavar_regexp
+
+(* todo: replace this hack by allowing X->method(...) in php grammar *)
+let is_metavar_variable_name s = 
+  s =~ "V\\(_[A-Z]*\\)?"

lang_php/matcher/metavars_php.mli

@@ -4,3 +4,9 @@ type mvar = string
 type metavars_binding = (mvar, Ast_php.any) Common.assoc
 
 val empty_environment: metavars_binding
+
+val is_metavar_variable_name: string -> bool
+val is_metavar_name: string -> bool
+
+val metavar_regexp: Str.regexp
+val metavar_regexp_string: string

lang_php/matcher/php_vs_php.ml

@@ -54,17 +54,6 @@ module MV = Metavars_php
 (* Helpers *)
 (*****************************************************************************)
 
-(* 
- * Hacks abusing existing PHP constructs to encode extra constructions.
- * One day we will have a pattern_php_ast.ml that mimic mostly
- * ast_php.ml and extend it.
- *)
-let is_metavar_name s = 
-  s =~ "[A-Z]\\([0-9]?_[A-Z]*\\)?"
-
-let is_metavar_variable_name s = 
-  s =~ "V\\(_[A-Z]*\\)?"
-
 (*****************************************************************************)
 (* Functor parameter combinators *)
 (*****************************************************************************)
@@ -624,20 +613,33 @@ and m_variablebis a b =
   (* pad: iso on $V metavar. Match any kind of PHP 'variable'.
    * Note that this includes function call. It's 'variable' in
    * the PHP original grammar sense, as usual.
-  *)
-  | A.Var(a1, a2),  b when is_metavar_variable_name (A.dname a1) ->
-      (* TODO add in environment ? *)
-      return (
-        a, b
+   *)
+  | A.Var((A.DName (dname, info_dname)), a2),
+    b when MV.is_metavar_variable_name dname ->
+
+      X.envf (dname, info_dname) (B.Lvalue (b, Ast_php.noTypeVar())) >>= 
+      (function
+      | ((dname, info_dname), B.Lvalue (b, _))  ->
+        return (
+          A.Var((A.DName (dname, info_dname)), a2), 
+          b
+        )
+      | _ -> raise Impossible
       )
 
   (* pad, iso on variable name *)
-  | A.Var(a1, a2), B.Var(b1, b2) when is_metavar_name (A.dname a1) ->
-      (* TODO add in environment ? *)
-      return (
-        a, b
-      )
+  | A.Var((A.DName (dname, info_dname)), a2), 
+    B.Var(b1, b2) when MV.is_metavar_name dname ->
 
+      X.envf (dname, info_dname) (B.Lvalue (b, Ast_php.noTypeVar())) >>= 
+      (function
+      | ((dname, info_dname), B.Lvalue (b, _))  ->
+        return (
+          A.Var((A.DName (dname, info_dname)), a2), 
+          b
+        )
+      | _ -> raise Impossible
+      )
 
   | A.Var(a1, a2), B.Var(b1, b2) ->
     m_dname a1 b1 >>= (fun (a1, b1) -> 
@@ -951,7 +953,7 @@ and m_exprbis a b =
   match a, b with
   (* special case, metavars !! *)
   | ((A.Sc (A.C (A.CName (A.Name (name,info_name))))), 
-    e2) when is_metavar_name name ->
+    e2) when MV.is_metavar_name name ->
       X.envf (name, info_name) (B.Expr (e2, Ast_php.noType())) >>= (function
       | ((name, info_name), B.Expr (e2, _))  ->
         return (
@@ -1385,7 +1387,7 @@ and m_xhp_attr_name a b =
 
 and m_xhp_attr_value a b = 
   match a, b with
-  | A.SgrepXhpAttrValueMvar (name, i_name), b when is_metavar_name name ->
+  | A.SgrepXhpAttrValueMvar (name, i_name), b when MV.is_metavar_name name ->
       X.envf (name, i_name) (B.XhpAttrValue b) >>= (function
       | ((name, i_name), B.XhpAttrValue b) ->
           return (

lang_php/matcher/transforming_php.ml

@@ -28,6 +28,10 @@ module B = Ast_php
  * against another PHP AST providing a kind of patch but at a 
  * syntactical level.
  * 
+ * See https://github.com/facebook/pfff/wiki/Spatch
+ * 
+ * To understand the logic behind this code it may help to first read
+ * this: http://coccinelle.lip6.fr/papers/eurosys08.pdf
  *)
 
 (*****************************************************************************)
@@ -127,7 +131,7 @@ module XMATCH = struct
          *)
         let a = Lib_parsing_php.abstract_position_info_expr a in
         let b = Lib_parsing_php.abstract_position_info_expr b in
-        a = b
+        a =*= b
 
     | A.XhpAttrValue _, A.XhpAttrValue _ ->
 
@@ -137,6 +141,7 @@ module XMATCH = struct
 
     | _, _ -> false
 
+  (* this is quite similar to the code in matching_php.ml *)
   let check_and_add_metavar_binding  (mvar, valu) = fun tin ->
     match Common.assoc_option mvar tin with
     | Some valu' ->
@@ -154,11 +159,76 @@ module XMATCH = struct
         (* first time the metavar is binded. Just add it to the environment *)
         Some (Common.insert_assoc (mvar, valu) tin)
 
-  let distribute_transfo transfo any = 
+
+
+  let subst_metavars env add =
+    let env = 
+      env +> List.map (fun (mvar, any) -> mvar, Unparse_php.string_of_any any)
+    in
+    match add with
+    | B.AddNewlineAndIdent -> 
+        B.AddNewlineAndIdent
+    | B.AddStr s ->
+        let s' = Common.global_replace_regexp MV.metavar_regexp_string
+         (fun matched ->
+          try List.assoc matched env
+          with Not_found -> 
+            failwith (spf "metavariable %s was not found in environment" 
+                         matched)
+         ) s
+        in
+        B.AddStr s'
+
+  (* when a transformation contains a '+' part, as in 
+   * - 2
+   * + bar(X)
+   * 
+   * then before applying the transformation we need first to
+   * substitute all metavariables by their actual binded value
+   * in the environment.
+   *)
+  let adjust_transfo_with_env env transfo = 
+     match transfo with
+     | B.NoTransfo 
+     | B.Remove -> transfo
+
+     | B.AddBefore add ->
+         B.AddBefore (subst_metavars env add)
+     | B.AddAfter add ->
+         B.AddAfter (subst_metavars env add)
+     | B.Replace add ->
+         B.Replace (subst_metavars env add)
+
+
+  (* 
+   * Sometimes a metavariable like X will match an expression made of
+   * multiple tokens  like  '1*2'. 
+   * This metavariable may have a transformation associated with it,
+   * like  '- X',  in which case we want to propagate the removal
+   * transformation to all the tokens in the matched expression.
+   * 
+   * In some cases the transformation may also contains a +, as in
+   *   - X
+   *   + 3
+   * in which case we can not just propagate the transformation
+   * to all the tokens. Indeed doing so would duplicate the '+ 3'
+   * on all the matched tokens. We need instead to distribute
+   * the removal transformation and associate the '+' transformation
+   * part only to the very last matched token by X (here '2').
+   *)
+
+  let distribute_transfo transfo any env = 
     let ii = Lib_parsing_php.ii_of_any any in
-    (* TODO, adjust the + to the right place *)
-    ii +> List.iter (fun tok -> 
-      tok.B.transfo <- transfo
+
+    (match transfo with
+    | B.NoTransfo -> ()
+    | B.Remove -> ii +> List.iter (fun tok -> tok.B.transfo <- B.Remove)
+    | B.Replace add ->
+        ii +> List.iter (fun tok -> tok.B.transfo <- B.Remove);
+        let any_ii = List.hd ii in
+        any_ii.B.transfo <- adjust_transfo_with_env env transfo;
+    | B.AddBefore add -> raise Todo
+    | B.AddAfter add -> raise Todo
     )
 
   let (envf: (Metavars_php.mvar Ast_php.wrap, Ast_php.any) matcher) =
@@ -168,14 +238,16 @@ module XMATCH = struct
         fail tin
     | Some new_binding ->
         (* TODO: distribute transfo mark *)
-        distribute_transfo imvar.A.transfo any;
-
+        distribute_transfo imvar.A.transfo any tin;
         return ((mvar, imvar), any) new_binding
 
+
+
   (* propagate the transformation info *)
-  let tokenf a b = 
-    b.B.transfo <- a.A.transfo;
-    return (a, b)
+  let tokenf a b = fun tin ->
+    let transfo = a.A.transfo in
+    b.B.transfo <- adjust_transfo_with_env tin transfo;   
+    return (a, b) tin
 
 end
 
@@ -191,9 +263,3 @@ type ('a, 'b) transformer = 'a -> 'b ->
 let transform_e_e pattern e   env = 
   ignore (MATCH.m_expr pattern e   env);
   ()
-
-(*
-let match_v_v pattern e = 
-  let env = MV.empty_environment in
-  MATCH.m_variable pattern e env +> extract_bindings
-*)

main_spatch.ml

@@ -443,7 +443,7 @@ let unittest_spatch () =
 (*---------------------------------------------------------------------------*)
 let spatch_extra_actions () = [
   (* see also demos/simple_refactoring.ml *)
-  "-simple_transfo", "<files_or_dirs>",
+  "-simple_transfo", " <files_or_dirs>",
   Common.mk_action_n_arg (simple_transfo);
 
   "-test", "",
@@ -469,7 +469,7 @@ let options () =
     "-verbose", Arg.Set verbose, 
     " ";
   ] ++
-  Flag_parsing_php.cmdline_flags_pp () ++
+  (* Flag_parsing_php.cmdline_flags_pp () ++ *)
   Common.options_of_actions action (all_actions()) ++
   Common.cmdline_flags_devel () ++
   Common.cmdline_flags_verbose () ++

tests/php/spatch/bar.exp

@@ -0,0 +1,3 @@
+<?php
+
+bar(2,1);

tests/php/spatch/bar.php

@@ -0,0 +1,3 @@
+<?php
+
+foo(1,2);

tests/php/spatch/bar.spatch

@@ -0,0 +1,2 @@
+-   foo(X,Y)
++   bar(Y,X)

tests/php/spatch/bar2.exp

@@ -0,0 +1,3 @@
+<?php
+
+bar(3,nested_call(1,2));

tests/php/spatch/bar2.php

@@ -0,0 +1,3 @@
+<?php
+
+foo(nested_call(1,2),3);

tests/php/spatch/distr_plus.exp

@@ -0,0 +1,3 @@
+<?php
+
+foo(2, 1+2, 3);

tests/php/spatch/distr_plus.php

@@ -0,0 +1,3 @@
+<?php
+
+foo(1 + 2, 3);

tests/php/spatch/distr_plus.spatch

@@ -0,0 +1,5 @@
+
+foo(
+-   X
++   2, X
+   , ...)