LGTM
http://codereview.appspot.com/110096/diff/2030/3041 File src/com/google/caja/parser/quasiliteral/AlphaRenaming.java (right): http://codereview.appspot.com/110096/diff/2030/3041#newcode62 Line 62: * not to conflict with generated code names. Perhaps explain why this class takes an Expression and not something else? Can just cut & paste from your review comments to me? ;) http://codereview.appspot.com/110096/diff/2030/3041#newcode132 Line 132: Expression e, NameContext<String, ?> ctxt, Set<String> freeSynthetics, So you don't think it's a good idea to rename "ctxt" and "freeSynthetics" (whatever we end up calling them)? http://codereview.appspot.com/110096/diff/2030/3040 File src/com/google/caja/parser/quasiliteral/AlphaRenamingRewriter.java (right): http://codereview.appspot.com/110096/diff/2030/3040#newcode60 Line 60: matches="/* Reference */ @r", /* Expression */ ?? http://codereview.appspot.com/110096/diff/2030/3036 File tests/com/google/caja/parser/quasiliteral/AlphaRenamingTest.java (right): http://codereview.appspot.com/110096/diff/2030/3036#newcode63 Line 63: public final void testPropertNames() throws Exception { testPropert*y*Names http://codereview.appspot.com/110096/diff/2030/3036#newcode104 Line 104: ); Move closing paren 1 line up? http://codereview.appspot.com/110096/diff/2030/3036#newcode113 Line 113: + " var c = b;" Fwiw, the reason for this rewriting -- the addition of the function name as a "var" -- was not clear to me from reading the code. I suspect it has to do with trying to regularize the underlying semantics of function name scoping in JS ... but how precisely? By capturing the reference to the enclosing function even if that symbol is assigned to later on in the containing scope? http://codereview.appspot.com/110096/diff/2030/3036#newcode178 Line 178: + " function c() {" What is the rationale for not coming up with *yet* another variable here, so that it would be -- (function () { function b() { var c = b; function d() { var e = d; return e, a; } } return b; }) ? http://codereview.appspot.com/110096/diff/2030/3036#newcode233 Line 233: + " function c(e) { var d = c; return e * e; }" We should have a TODO for optimizing out the function name assignment "var" if that name is unused. It would be too messy to do now but maybe, in a future society, we would have a scope representation that would allow that to be done simply. http://codereview.appspot.com/110096
