From 1c15d0db456ce32f1b9b507aad97e5ee5c8285f7 Mon Sep 17 00:00:00 2001 From: Anthony Van de Gejuchte Date: Fri, 29 Jul 2016 03:18:21 +0200 Subject: [PATCH] Fix quoting of properties - Make AST_ConciseMethod child of AST_ObjectProperty. - Fix some typos. --- lib/ast.js | 39 ++++++------ lib/output.js | 57 +++++++++-------- lib/parse.js | 34 ++++++---- lib/transform.js | 3 + test/compress/harmony.js | 20 ++++++ test/compress/object.js | 131 ++++++++++++++++++++++++++++++++++++++- test/mocha/class.js | 31 +++++++++ 7 files changed, 257 insertions(+), 58 deletions(-) diff --git a/lib/ast.js b/lib/ast.js index 4a9e20e4..a3a894c5 100644 --- a/lib/ast.js +++ b/lib/ast.js @@ -448,8 +448,8 @@ var AST_ArrowParametersOrSeq = DEFNODE("ArrowParametersOrSeq", "expressions", { var AST_Lambda = DEFNODE("Lambda", "name argnames uses_arguments is_generator", { $documentation: "Base class for functions", $propdoc: { - is_generator: "is generatorFn or not", - name: "[AST_SymbolDeclaration?|AST_Node] the name of this function or computed expression", + is_generator: "[boolean] is generatorFn or not", + name: "[AST_SymbolDeclaration?] the name of this function", argnames: "[AST_SymbolFunarg|AST_Destructuring|AST_Expansion*] array of function arguments, destructurings, or expanding arguments", uses_arguments: "[boolean/S] tells whether this function accesses the arguments array" }, @@ -487,14 +487,6 @@ var AST_Arrow = DEFNODE("Arrow", null, { $documentation: "An ES6 Arrow function ((a) => b)" }, AST_Lambda); -var AST_ConciseMethod = DEFNODE("ConciseMethod", "is_generator static", { - $propdoc: { - is_generator: "is generatorFn or not", - static: "[boolean] whether this method is static (classes only)", - }, - $documentation: "An ES6 concise method inside an object or class" -}, AST_Lambda); - var AST_Defun = DEFNODE("Defun", null, { $documentation: "A function definition" }, AST_Lambda); @@ -996,11 +988,13 @@ var AST_Object = DEFNODE("Object", "properties", { var AST_ObjectProperty = DEFNODE("ObjectProperty", "key value", { $documentation: "Base class for literal object properties", $propdoc: { - key: "[string|AST_Node] the property name converted to a string for ObjectKeyVal. For setters and getters this is an arbitrary AST_Node.", - value: "[AST_Node] property value. For setters and getters this is an AST_Function." + key: "[string|AST_Node] the property name converted to a string for ObjectKeyVal. For setters, getters and computed property this is an arbitrary AST_Node", + value: "[AST_Node] property value. For setters and getters this is an AST_Function." }, _walk: function(visitor) { return visitor._visit(this, function(){ + if (this.key instanceof AST_Node) + this.key._walk(visitor); this.value._walk(visitor); }); } @@ -1016,28 +1010,33 @@ var AST_ObjectKeyVal = DEFNODE("ObjectKeyVal", "quote shorthand", { var AST_ObjectComputedKeyVal = DEFNODE("ObjectComputedKeyVal", null, { $documentation: "An object property whose key is computed. Like `[Symbol.iterator]: function...` or `[routes.homepage]: renderHomepage`", - _walk: function(visitor) { - return visitor._visit(this, function(){ - this.key._walk(visitor); - this.value._walk(visitor); - }); - } }, AST_ObjectProperty); -var AST_ObjectSetter = DEFNODE("ObjectSetter", "static", { +var AST_ObjectSetter = DEFNODE("ObjectSetter", "quote static", { $propdoc: { + quote: "[string|undefined] the original quote character, if any", static: "[boolean] whether this is a static setter (classes only)" }, $documentation: "An object setter property", }, AST_ObjectProperty); -var AST_ObjectGetter = DEFNODE("ObjectGetter", "static", { +var AST_ObjectGetter = DEFNODE("ObjectGetter", "quote static", { $propdoc: { + quote: "[string|undefined] the original quote character, if any", static: "[boolean] whether this is a static getter (classes only)" }, $documentation: "An object getter property", }, AST_ObjectProperty); +var AST_ConciseMethod = DEFNODE("ConciseMethod", "quote static is_generator", { + $propdoc: { + quote: "[string|undefined] the original quote character, if any", + static: "[boolean] whether this method is static (classes only)", + is_generator: "[boolean] is generatorFn or not", + }, + $documentation: "An ES6 concise method inside an object or class" +}, AST_ObjectProperty); + var AST_Class = DEFNODE("Class", "name extends properties", { $propdoc: { name: "[AST_SymbolClass|AST_SymbolDefClass?] optional class name.", diff --git a/lib/output.js b/lib/output.js index c0d3632a..d0238da8 100644 --- a/lib/output.js +++ b/lib/output.js @@ -966,16 +966,6 @@ function OutputStream(options) { } if (needs_parens) { output.print(")") } }); - DEFPRINT(AST_ConciseMethod, function(self, output){ - if (self.static) { - output.print("static"); - output.space(); - } - if (self.is_generator) { - output.print("*"); - } - self._do_print(output, true /* do not print "function" */); - }); /* -----[ exits ]----- */ AST_Exit.DEFMETHOD("_do_print", function(output, kind){ @@ -1430,17 +1420,7 @@ function OutputStream(options) { DEFPRINT(AST_NewTarget, function(self, output) { output.print("new.target"); }); - DEFPRINT(AST_ObjectKeyVal, function(self, output){ - var key = self.key; - var quote = self.quote; - var print_as_shorthand = self.shorthand && - self.value instanceof AST_Symbol && - self.key == self.value.print_to_string(); - - if (print_as_shorthand) { - output.print_name(key); - return; - } + AST_ObjectProperty.DEFMETHOD("print_property_name", function(key, quote, output) { if (output.option("quote_keys")) { output.print_string(key + ""); } else if ((typeof key == "number" @@ -1457,6 +1437,17 @@ function OutputStream(options) { } else { output.print_string(key, quote); } + }); + DEFPRINT(AST_ObjectKeyVal, function(self, output){ + var print_as_shorthand = self.shorthand && + self.value instanceof AST_Symbol && + self.key == self.value.print_to_string(); + + if (print_as_shorthand) { + output.print_name(self.key); + return; + } + self.print_property_name(self.key, self.quote, output); output.colon(); self.value.print(output); }); @@ -1468,11 +1459,7 @@ function OutputStream(options) { output.print(type); output.space(); if (self.key instanceof AST_SymbolMethod) { - if (typeof self.key.name === "string" && !is_identifier_string(self.key.name)) { - output.print_string(self.key.name); - } else { - self.key.print(output); - } + self.print_property_name(self.key.name, self.quote, output); } else { output.with_square(function() { self.key.print(output); @@ -1486,6 +1473,24 @@ function OutputStream(options) { DEFPRINT(AST_ObjectGetter, function(self, output){ self._print_getter_setter("get", self, output); }); + DEFPRINT(AST_ConciseMethod, function(self, output){ + if (self.static) { + output.print("static"); + output.space(); + } + if (self.is_generator) { + output.print("*"); + } + output.space(); + if (self.key instanceof AST_SymbolMethod) { + self.print_property_name(self.key.name, self.quote, output); + } else { + output.with_square(function() { + self.key.print(output); + }); + } + self.value._do_print(output, true); + }); DEFPRINT(AST_ObjectComputedKeyVal, function(self, output) { output.print("["); self.key.print(output); diff --git a/lib/parse.js b/lib/parse.js index 28c722fb..80691da4 100644 --- a/lib/parse.js +++ b/lib/parse.js @@ -1253,7 +1253,7 @@ function parse($TEXT, options) { }); }; - var function_ = function(ctor) { + var function_ = function(ctor, is_generator_property) { var start = S.token var in_statement = ctor === AST_Defun; @@ -1267,7 +1267,7 @@ function parse($TEXT, options) { unexpected(); var args = params_or_seq_().as_params(croak); - var body = _function_body(true, is_generator); + var body = _function_body(true, is_generator || is_generator_property); return new ctor({ start : args.start, end : body.end, @@ -1744,7 +1744,7 @@ function parse($TEXT, options) { start = S.token; var type = start.type; var name = as_property_name(); - if (type != "string" && type != "num" && !is("punc", ":")) { + if (!is("punc", ":")) { var concise = concise_method_or_getset(name, start); if (concise) { a.push(concise); @@ -1757,8 +1757,10 @@ function parse($TEXT, options) { if (type == "punc" && start.value == "[") { expect(":"); a.push(new AST_ObjectComputedKeyVal({ + start: start, key: name, - value: expression(false) + value: expression(false), + end: prev() })); continue; } @@ -1859,12 +1861,15 @@ function parse($TEXT, options) { } var is_static = false; var is_generator = false; + var property_token = start; if (is_class && name === "static" && !is("punc", "(")) { is_static = true; + property_token = S.token; name = as_property_name(); } if (name === null) { is_generator = true; + property_token = S.token; name = as_property_name(); if (name === null) { unexpected(); @@ -1872,23 +1877,28 @@ function parse($TEXT, options) { } if (is("punc", "(")) { name = get_ast(name, start); - return new AST_ConciseMethod({ - is_generator: is_generator, + var node = new AST_ConciseMethod({ start : start, static : is_static, - name : name, - argnames : params_or_seq_().as_params(croak), - body : _function_body(true, is_generator), + is_generator: is_generator, + key : name, + quote : name instanceof AST_SymbolMethod ? + property_token.quote : undefined, + value : function_(AST_Accessor, is_generator), end : prev() }); + return node; } + property_token = S.token; if (name == "get") { if (!is("punc") || is("punc", "[")) { - name = get_ast(as_property_name(), prev()); + name = get_ast(as_property_name(), start); return new AST_ObjectGetter({ start : start, static: is_static, key : name, + quote : name instanceof AST_SymbolMethod ? + property_token.quote : undefined, value : function_(AST_Accessor), end : prev() }); @@ -1896,11 +1906,13 @@ function parse($TEXT, options) { } else if (name == "set") { if (!is("punc") || is("punc", "[")) { - name = get_ast(as_property_name(), prev()); + name = get_ast(as_property_name(), start); return new AST_ObjectSetter({ start : start, static: is_static, key : name, + quote : name instanceof AST_SymbolMethod ? + property_token.quote : undefined, value : function_(AST_Accessor), end : prev() }); diff --git a/lib/transform.js b/lib/transform.js index c5df875b..8237c1e8 100644 --- a/lib/transform.js +++ b/lib/transform.js @@ -224,6 +224,9 @@ TreeTransformer.prototype = new TreeWalker; }); _(AST_ObjectProperty, function(self, tw){ + if (self.key instanceof AST_Node) { + self.key = self.key.transform(tw); + } self.value = self.value.transform(tw); }); diff --git a/test/compress/harmony.js b/test/compress/harmony.js index dd153413..122069dd 100644 --- a/test/compress/harmony.js +++ b/test/compress/harmony.js @@ -248,6 +248,26 @@ classes_can_have_computed_static: { } } +class_methods_and_getters_with_keep_quoted_props_enabled: { + beautify = { + quote_style: 3, + keep_quoted_props: true, + } + input: { + class clss { + a() {} + "b"() {} + get c() { return "c"} + get "d"() { return "d"} + set e(a) { doSomething(a); } + set 'f'(a) { doSomething(b); } + static g() {} + static "h"() {} + } + } + expect_exact: 'class clss{a(){}"b"(){}get c(){return"c"}get"d"(){return"d"}set e(a){doSomething(a)}set\'f\'(a){doSomething(b)}static g(){}static"h"(){}}' +} + new_target: { input: { new.target; diff --git a/test/compress/object.js b/test/compress/object.js index d13678a4..b493204a 100644 --- a/test/compress/object.js +++ b/test/compress/object.js @@ -112,6 +112,35 @@ computed_property_names: { expect_exact: 'obj({["x"+"x"]:6});' } +computed_property_names_evaluated_1: { + options = { + evaluate: true + } + input: { + obj({ + [1 + 1]: 2, + ["x" + "x"]: 6 + }); + } + expect_exact: 'obj({[2]:2,["xx"]:6});' +} + +computed_property_names_evaluated_2: { + options = { + evaluate: true + } + input: { + var foo = something(); + + var obj = { + [foo]() { + return "blah"; + } + } + } + expect_exact: 'var foo=something();var obj={[foo](){return"blah"}};' +} + shorthand_properties: { mangle = true; input: (function() { @@ -154,6 +183,9 @@ concise_methods_with_computed_property: { }, [1 + 2]() { return 3; + }, + ["1" + "4"]() { + return 14; } } } @@ -164,11 +196,74 @@ concise_methods_with_computed_property: { }, [3]() { return 3; + }, + ["14"]() { + return 14; } } } } +concise_methods_with_computed_property2: { + options = { + evaluate: true + } + input: { + var foo = { + [[1]](){ + return "success"; + } + }; + doSomething(foo[[1]]()); + } + expect_exact: { + 'var foo={[[1]](){return"success"}};doSomething(foo[[1]]());' + } +} + +concise_methods_with_various_property_names: { + input: { + var get = "bar"; + var a = { + bar() { + return this.get; + }, + 5() { + return "five"; + }, + 0xf55() { + return "f five five"; + }, + "five"() { + return 5; + }, + 0b1010(value) { + this._ten = value; + } + }; + } + expect: { + var get = "bar"; + var a = { + bar() { + return this.get; + }, + 5() { + return "five"; + }, + 0xf55() { + return "f five five"; + }, + "five"() { + return 5; + }, + 0b1010(value) { + this._ten = value; + } + }; + } +} + concise_methods_and_mangle_props: { mangle_props = { regex: /_/ @@ -218,7 +313,6 @@ concise_methods_and_keyword_names: { } } - getter_setter_with_computed_value: { input: { class C { @@ -339,3 +433,38 @@ property_with_unprintable_ascii_only: { } expect_exact: 'var foo={"\\0\\x01":"foo",get"\\0\\x01"(){return"bar"},set"\\0\\x01"(foo){save(foo)},*"\\0\\x01"(){return"foobar"}};class bar{get"\\0\\x01"(){return"bar"}set"\\0\\x01"(foo){save(foo)}*"\\0\\x01"(){return"foobar"}}' } + +property_with_unprintable_ascii_only_static: { + beautify = { + ascii_only: true + } + input: { + class foo { + static get "\x02\x03"() { + return "bar"; + } + static set "\x04\x05"(foo) { + save(foo); + } + } + } + expect_exact: 'class foo{static get"\\x02\\x03"(){return"bar"}static set"\\x04\\x05"(foo){save(foo)}}' +} + +methods_and_getters_with_keep_quoted_props_enabled: { + beautify = { + quote_style: 3, + keep_quoted_props: true, + } + input: { + var obj = { + a() {}, + "b"() {}, + get c() { return "c"}, + get "d"() { return "d"}, + set e(a) { doSomething(a); }, + set f(a) { doSomething(b); } + } + } + expect_exact: 'var obj={a(){},"b"(){},get c(){return"c"},get"d"(){return"d"},set e(a){doSomething(a)},set f(a){doSomething(b)}};' +} diff --git a/test/mocha/class.js b/test/mocha/class.js index 0df67e6a..6100afb9 100644 --- a/test/mocha/class.js +++ b/test/mocha/class.js @@ -23,4 +23,35 @@ describe("Class", function() { assert.throws(test(tests[i]), error); } }); + + it("Should return the correct token for class methods", function() { + var tests = [ + { + code: "class foo{static test(){}}", + token_value_start: "static", + token_value_end: "}" + }, + { + code: "class bar{*procedural(){}}", + token_value_start: "*", + token_value_end: "}" + }, + { + code: "class foobar{aMethod(){}}", + token_value_start: "aMethod", + token_value_end: "}" + }, + { + code: "class foobaz{get something(){}}", + token_value_start: "get", + token_value_end: "}" + } + ]; + + for (var i = 0; i < tests.length; i++) { + var ast = uglify.parse(tests[i].code); + assert.strictEqual(ast.body[0].properties[0].start.value, tests[i].token_value_start); + assert.strictEqual(ast.body[0].properties[0].end.value, tests[i].token_value_end); + } + }); });