Permalink
Browse files

Tutorial on how to write refactorings with pfff

Summary:
In the long term I want to make it easy to write refactoring
by having something like sgrep but in the mean time you have
to do things quite manually.

Task ID: #

Blame Rev:

Reviewers: ahupp

CC: lesha, schrockn

Test Plan:
./simple_refactoring ../tests/php/spatch/arg_assert.php
...
 function foo($s) {
-  ArgAssert::isString($s, "s");
-  ArgAssert::isArray($s,
-                     array(1, 2));
+  ArgAssert::isString($s);
+  ArgAssert::isArray($s);
 }

Revert Plan:

Tags:

- begin *PUBLIC* platform impact section -
Bugzilla: #
- end platform impact -

DiffCamp Revision: 173283
  • Loading branch information...
1 parent a1b001d commit c7b66cb5471e390a83fd0379754135224a1b34f0 pad committed Oct 22, 2010
Showing with 248 additions and 0 deletions.
  1. +1 −0 .gitignore
  2. +21 −0 demos/Makefile
  3. +217 −0 demos/simple_refactoring.ml
  4. +7 −0 tests/php/spatch/arg_assert.php
  5. +2 −0 tests/php/spatch/arg_assert2.php
View
@@ -237,3 +237,4 @@ external/ocamlbdb/libcamlbdb.a
/lang_lisp/parsing/lexer_lisp.ml
/lang_haskell/parsing/lexer_hs.ml
/pfff.top
+/demos/simple_refactoring
View
@@ -3,10 +3,29 @@
# This is to show how to build a simple Makefile from scratch
# to people who wants to use pfff in their own application.
+##############################################################################
+# Simple compilation
+##############################################################################
+
+# This is intentionnaly not using OCAMLC, SYSLIBS and so on to show a simple
+# way to compile a pfff "script"
+simple_refactoring: simple_refactoring.ml
+ ocamlc -g -o simple_refactoring \
+ -I ../commons -I ../lang_php/parsing \
+ str.cma unix.cma nums.cma bigarray.cma \
+ ../commons/commons.cma ../h_program-lang/lib.cma \
+ ../lang_php/parsing/lib.cma \
+ simple_refactoring.ml
+
+clean::
+ rm simple_refactoring
+
##############################################################################
# Variables
##############################################################################
+# Now we switch to a more traditional Makefile
+
include ../Makefile.config
ifeq ($(FEATURE_BDB),1)
ANALYZECMA=../external/ocamlpcre/lib/pcre.cma \
@@ -97,6 +116,8 @@ rec::
clean::
rm -f *.opt *.byte
+
+
##############################################################################
# Developer rules
##############################################################################
View
@@ -0,0 +1,217 @@
+open Common
+
+open Ast_php
+
+module Ast = Ast_php
+module V = Visitor_php
+
+(*****************************************************************************)
+(* Prelude *)
+(*****************************************************************************)
+
+(*
+ * This file contains the implementation of a simple refactoring where we
+ * remove the second argument in all calls to ArgAssert::isString() and
+ * ArgAssert::isArray(). The goal is to provide a tutorial on how
+ * to write PHP refactorings using the Pfff infrastructure.
+ *
+ * To compile this file do:
+ *
+ * $ ocamlc -g -o simple_refactoring \
+ * -I ../commons -I ../lang_php/parsing \
+ * str.cma unix.cma nums.cma bigarray.cma \
+ * ../commons/commons.cma ../h_program-lang/lib.cma \
+ * ../lang_php/parsing/lib.cma \
+ * simple_refactoring.ml
+ *
+ * To run do for instance:
+ *
+ * $ ./simple_refactoring ../tests/php/spatch/arg_assert.php
+ *
+ * which should output:
+ *
+ * processing: ../tests/php/spatch/arg_assert.php (0/1)
+ * --- ../tests/php/spatch/arg_assert.php 2010-10-22 13:09:09.0 -0700
+ * +++ /tmp/trans-17581-464603.php 2010-10-22 13:21:52.0 -0700
+ * @@ -1,7 +1,6 @@
+ * <?php
+ *
+ * function foo($s) {
+ * - ArgAssert::isString($s, "s");
+ * - ArgAssert::isArray($s,
+ * - array(1, 2));
+ * + ArgAssert::isString($s);
+ * + ArgAssert::isArray($s);
+ * }
+ *
+ * If you modify the 'apply_patch' variable below it will actually
+ * do the modification in place on the file. It is set to false by
+ * default for ease of prototyping.
+ *
+ *
+ * Futur work:
+ * At some point we want to have a DSL for expressing "semantic patches"
+ * like coccinelle and express it simply with:
+ *
+ * @@ @@
+ * - ArgAssert::isString(X, Y)
+ * + ArgAssert::isString(X)
+ *
+ * or
+ *
+ * @@ @@
+ * ArgAssert::isString(X
+ * - ,Y
+ * )
+ *
+ * but we are not there yet. In the mean time we have to program
+ * "manually" those refactoring using the OCaml internal API
+ * to manipulate the AST.
+ *
+ * Timing: 1 pomodoro
+ *
+ * Alternatives:
+ * - lex-pass I believe has a special command just for that.
+ *
+ * See also main_spatch.ml
+ *)
+
+(*****************************************************************************)
+(* Globals *)
+(*****************************************************************************)
+
+let apply_patch = ref false
+
+(*****************************************************************************)
+(* Algorithm *)
+(*****************************************************************************)
+
+let main files_or_dirs =
+ let files = Lib_parsing_php.find_php_files_of_dir_or_files files_or_dirs in
+
+ Flag_parsing_php.show_parsing_error := false;
+ Flag_parsing_php.verbose_lexing := false;
+
+ files +> Common.index_list_and_total +> List.iter (fun (file, i, total) ->
+ pr2 (spf "processing: %s (%d/%d)" file i total);
+
+ (* step1: parse the file *)
+ let (ast_and_tokens_list, _stat) = Parse_php.parse file in
+ let ast = Parse_php.program_of_program2 ast_and_tokens_list in
+
+ (* step2: visit the AST and annotate the relevant tokens in AST leaves *)
+ let hook = { V.default_visitor with
+
+ V.kexpr = (fun (k, _) expr ->
+ match expr with
+
+ (*
+ * At this point 'expr' is the AST of a PHP expression that we
+ * can match over a specific pattern. The pattern we want is
+ * a static method call to the 'isString' function of the
+ * 'ArgAssert' class. One could look at ast_php.ml and
+ * find the appropriate OCaml constructors for those
+ * AST elements. An alternative is to use pfff -dump_php_ml
+ * to get a first draft for the pattern. For instance
+ *
+ * $ ./pfff -dump_php_ml tests/php/spatch/arg_assert2.php
+ *
+ * will output the internal representation of the AST of
+ * arg_assert2.php, which is:
+ *
+ * [StmtList(
+ * [ExprStmt(
+ * (Lv(
+ * (StaticMethodCallSimple(Qualifier(Name(("ArgAssert", i_1)), i_2),
+ * Name(("isString", i_3)),
+ * (i_4,
+ * [Left(
+ * Arg(
+ * (Lv((Var(DName(("s", i_5)), Ref(NoScope)), tlval_6)), t_7)));
+ * Right(i_8); Left(Arg((Sc(C(String(("s", i_9)))), t_10)))],
+ * i_11)),
+ * tlval_12)),
+ * t_13), i_14)]); FinalDef(i_15)]
+ *
+ * One can then copy paste part of this AST directly into this
+ * file as an OCaml pattern.
+ *
+ * The i_<num> are matching the corresponding tokens in the AST.
+ * For instance i_1 above is the token containing the information
+ * on the "ArgAssert" in the file. This token contain information
+ * such as the line position of the token and also the
+ * 'transfo' annotation that we can modify to remove the token
+ * for instance. See Ast_php.ml
+ *
+ * The t_<num> are type information about the expression or lvalue
+ * that we don't currently use.
+ *)
+ | (Lv(
+ (StaticMethodCallSimple(
+ Qualifier(Name(("ArgAssert", i_9)), i_10),
+ Name(("isString", i_11)),
+ (i_left_paren,
+ [Left(
+ Arg(
+ (Lv((Var(DName((var_name, i_13)), _scope_info), tlval_14)),
+ t_15)));
+ Right(i_token_comma);
+ Left(Arg((Sc(C(String((a_string, i_token_string)))), t_18)))
+ ],
+ i_right_paren)),
+ tlval_20)),
+ t_21) ->
+ i_token_comma.Ast.transfo <- Ast.Remove;
+ i_token_string.Ast.transfo <- Ast.Remove;
+
+ (* similar pattern except we accept any kind of expression as
+ * the second argument, not just constant strings.
+ *)
+ | (Lv(
+ (StaticMethodCallSimple(
+ Qualifier(Name(("ArgAssert", i_9)), i_10),
+ Name(("isArray", i_11)),
+ (i_12,
+ [Left(
+ Arg(
+ (Lv((Var(DName((var_name, i_13)), _scope_info), tlval_14)),
+ t_15)));
+ Right(token_comma);
+ Left(Arg(an_expr))
+ ],
+ i_19)),
+ tlval_20)),
+ t_21) ->
+
+ (* let's get all the tokens composing the expression *)
+ let tokens_in_expression = Lib_parsing_php.ii_of_expr an_expr in
+
+ tokens_in_expression +> List.iter (fun tok ->
+ tok.Ast.transfo <- Ast.Remove;
+ );
+ token_comma.Ast.transfo <- Ast.Remove;
+ | _ -> k expr
+ );
+ }
+ in
+ (V.mk_visitor hook).V.vprogram ast;
+
+ (* step3: unparse the annotated AST and show the diff *)
+ let s = Unparse_php.string_of_program2_using_tokens ast_and_tokens_list in
+
+ let tmpfile = Common.new_temp_file "trans" ".php" in
+ Common.write_file ~file:tmpfile s;
+
+ let diff = Common.cmd_to_list (spf "diff -u %s %s" file tmpfile) in
+ diff |> List.iter pr;
+
+ if !apply_patch
+ then Common.write_file ~file:file s;
+ )
+
+
+(*****************************************************************************)
+(* Entry point *)
+(*****************************************************************************)
+let _ =
+ main (Array.to_list Sys.argv)
@@ -0,0 +1,7 @@
+<?php
+
+function foo($s) {
+ ArgAssert::isString($s, "s");
+ ArgAssert::isArray($s,
+ array(1, 2));
+}
@@ -0,0 +1,2 @@
+<?php
+ArgAssert::isString($s, "s");

0 comments on commit c7b66cb

Please sign in to comment.