diff --git a/src/com/google/javascript/jscomp/AmbiguateProperties.java b/src/com/google/javascript/jscomp/AmbiguateProperties.java index 2c5037c414e..1e2663de9bb 100644 --- a/src/com/google/javascript/jscomp/AmbiguateProperties.java +++ b/src/com/google/javascript/jscomp/AmbiguateProperties.java @@ -457,60 +457,77 @@ public void visit(NodeTraversal t, Node n, Node parent) { } case Token.CALL: { Node target = n.getFirstChild(); - if (!target.isName()) { + if (!target.isQualifiedName()) { break; } - String renameFunctionName = target.getOriginalName(); - if (renameFunctionName == null) { - renameFunctionName = target.getString(); - } - if (renameFunctionName == null - || !compiler.getCodingConvention().isPropertyRenameFunction(renameFunctionName)) { - break; - } - - if (n.getChildCount() != 2 && n.getChildCount() != 3) { - compiler.report( - JSError.make( - n, - DisambiguateProperties.Warnings.INVALID_RENAME_FUNCTION, - renameFunctionName, - " Must be called with 1 or 2 arguments.")); - break; - } - - Node propName = n.getSecondChild(); - if (!propName.isString()) { - compiler.report( - JSError.make( - n, - DisambiguateProperties.Warnings.INVALID_RENAME_FUNCTION, - renameFunctionName, - " The first argument must be a string literal.")); - break; - } - - if (propName.getString().contains(".")) { - compiler.report( - JSError.make( - n, - DisambiguateProperties.Warnings.INVALID_RENAME_FUNCTION, - renameFunctionName, - " The first argument must not be a property path.")); - break; - } - - JSType jstype = getJSType(n.getChildAtIndex(2)); - - maybeMarkCandidate(propName, jstype); + String renameFunctionName = target.getOriginalQualifiedName(); + if (renameFunctionName != null + && compiler.getCodingConvention().isPropertyRenameFunction(renameFunctionName)) { + + if (n.getChildCount() != 2 && n.getChildCount() != 3) { + compiler.report( + JSError.make( + n, + DisambiguateProperties.Warnings.INVALID_RENAME_FUNCTION, + renameFunctionName, + " Must be called with 1 or 2 arguments.")); + break; + } + + Node propName = n.getSecondChild(); + if (!propName.isString()) { + compiler.report( + JSError.make( + n, + DisambiguateProperties.Warnings.INVALID_RENAME_FUNCTION, + renameFunctionName, + " The first argument must be a string literal.")); + break; + } + + if (propName.getString().contains(".")) { + compiler.report( + JSError.make( + n, + DisambiguateProperties.Warnings.INVALID_RENAME_FUNCTION, + renameFunctionName, + " The first argument must not be a property path.")); + break; + } + + JSType jstype = getJSType(n.getSecondChild()); + + maybeMarkCandidate(propName, jstype); + } else if (NodeUtil.isObjectDefinePropertiesDefinition(n)) { + Node typeObj = n.getSecondChild(); + JSType jstype = getJSType(typeObj); + Node objectLiteral = typeObj.getNext(); + + if (!objectLiteral.isObjectLit()) { + break; + } + + for (Node key : objectLiteral.children()) { + if (key.isQuotedString()) { + quotedNames.add(key.getString()); + } else { + maybeMarkCandidate(key, jstype); + } + } + } break; } case Token.OBJECTLIT: + // Object.defineProperties literals are handled at the CALL node. + if (n.getParent().isCall() + && NodeUtil.isObjectDefinePropertiesDefinition(n.getParent())) { + break; + } + // The children of an OBJECTLIT node are keys, where the values // are the children of the keys. - for (Node key = n.getFirstChild(); key != null; - key = key.getNext()) { + for (Node key = n.getFirstChild(); key != null; key = key.getNext()) { // We only want keys that were unquoted. // Keys are STRING, GET, SET if (!key.isQuotedString()) { diff --git a/src/com/google/javascript/jscomp/DisambiguateProperties.java b/src/com/google/javascript/jscomp/DisambiguateProperties.java index 0ffd61b16dd..496de037e5e 100644 --- a/src/com/google/javascript/jscomp/DisambiguateProperties.java +++ b/src/com/google/javascript/jscomp/DisambiguateProperties.java @@ -502,6 +502,11 @@ private void handleGetProp(NodeTraversal t, Node n) { } private void handleObjectLit(NodeTraversal t, Node n) { + // Object.defineProperties literals are handled at the CALL node. + if (n.getParent().isCall() && NodeUtil.isObjectDefinePropertiesDefinition(n.getParent())) { + return; + } + for (Node child = n.getFirstChild(); child != null; child = child.getNext()) { @@ -529,16 +534,21 @@ private void handleObjectLit(NodeTraversal t, Node n) { private void handleCall(NodeTraversal t, Node call) { Node target = call.getFirstChild(); - if (!target.isName()) { + if (!target.isQualifiedName()) { return; } - String renameFunctionName = target.getOriginalQualifiedName(); - if (renameFunctionName == null - || !compiler.getCodingConvention().isPropertyRenameFunction(renameFunctionName)) { - return; + String functionName = target.getOriginalQualifiedName(); + if (functionName != null + && compiler.getCodingConvention().isPropertyRenameFunction(functionName)) { + handlePropertyRenameFunctionCall(t, call, functionName); + } else if (NodeUtil.isObjectDefinePropertiesDefinition(call)) { + handleObjectDefineProperties(t, call); } + } + private void handlePropertyRenameFunctionCall( + NodeTraversal t, Node call, String renameFunctionName) { if (call.getChildCount() != 2 && call.getChildCount() != 3) { compiler.report( JSError.make( @@ -605,6 +615,25 @@ private void handleCall(NodeTraversal t, Node call) { } } + private void handleObjectDefineProperties(NodeTraversal t, Node call) { + Node typeObj = call.getSecondChild(); + JSType type = getType(typeObj); + Node objectLiteral = typeObj.getNext(); + if (!objectLiteral.isObjectLit()) { + return; + } + + for (Node key : objectLiteral.children()) { + if (key.isQuotedString()) { + continue; + } + + String propName = key.getString(); + Property prop = getProperty(propName); + prop.scheduleRenaming(key, processProperty(t, prop, type, null)); + } + } + private void printErrorLocations(List errors, JSType t) { if (!t.isObject() || t.isAllType()) { return; diff --git a/test/com/google/javascript/jscomp/AmbiguatePropertiesTest.java b/test/com/google/javascript/jscomp/AmbiguatePropertiesTest.java index faa37f18186..a4daf57a7b9 100644 --- a/test/com/google/javascript/jscomp/AmbiguatePropertiesTest.java +++ b/test/com/google/javascript/jscomp/AmbiguatePropertiesTest.java @@ -31,10 +31,12 @@ public final class AmbiguatePropertiesTest extends CompilerTestCase { private AmbiguateProperties lastPass; private static final String EXTERNS = - "Function.prototype.call=function(){};" + - "Function.prototype.inherits=function(){};" + - "prop.toString;" + - "var google = { gears: { factory: {}, workerPool: {} } };"; + "Function.prototype.call=function(){};" + + "Function.prototype.inherits=function(){};" + + "/** @const */ var Object = {};" + + "Object.defineProperties = function(typeRef, definitions) {};" + + "prop.toString;" + + "var google = { gears: { factory: {}, workerPool: {} } };"; public AmbiguatePropertiesTest() { super(EXTERNS); @@ -301,6 +303,82 @@ public void testQuotedPrototypeProperty() { "bar['getA']();"); } + public void testObjectDefineProperties() { + String js = + LINE_JOINER.join( + "/** @constructor */ var Bar = function(){};", + "Bar.prototype.bar = 0;", + "/** @struct @constructor */ var Foo = function() {", + " this.bar_ = 'bar';", + "};", + "/** @type {?} */ Foo.prototype.bar;", + "Object.defineProperties(Foo.prototype, {", + " bar: {", + " configurable: true,", + " enumerable: true,", + " /** @this {Foo} */ get: function() { return this.bar_;},", + " /** @this {Foo} */ set: function(value) { this.bar_ = value; }", + " }", + "});"); + + String result = + LINE_JOINER.join( + "/** @constructor */ var Bar = function(){};", + "Bar.prototype.a = 0;", + "/** @struct @constructor */ var Foo = function() {", + " this.b = 'bar';", + "};", + "/** @type {?} */ Foo.prototype.a;", + "Object.defineProperties(Foo.prototype, {", + " a: {", + " configurable: true,", + " enumerable: true,", + " /** @this {Foo} */ get: function() { return this.b;},", + " /** @this {Foo} */ set: function(value) { this.b = value; }", + " }", + "});"); + + test(js, result); + } + + public void testObjectDefinePropertiesQuoted() { + String js = + LINE_JOINER.join( + "/** @constructor */ var Bar = function(){};", + "Bar.prototype.bar = 0;", + "/** @struct @constructor */ var Foo = function() {", + " this.bar_ = 'bar';", + "};", + "/** @type {?} */ Foo.prototype['bar'];", + "Object.defineProperties(Foo.prototype, {", + " 'a': {", + " configurable: true,", + " enumerable: true,", + " /** @this {Foo} */ get: function() { return this.bar_;},", + " /** @this {Foo} */ set: function(value) { this.bar_ = value; }", + " }", + "});"); + + String result = + LINE_JOINER.join( + "/** @constructor */ var Bar = function(){};", + "Bar.prototype.b = 0;", + "/** @struct @constructor */ var Foo = function() {", + " this.b = 'bar';", + "};", + "/** @type {?} */ Foo.prototype['bar'];", + "Object.defineProperties(Foo.prototype, {", + " 'a': {", + " configurable: true,", + " enumerable: true,", + " /** @this {Foo} */ get: function() { return this.b;},", + " /** @this {Foo} */ set: function(value) { this.b = value; }", + " }", + "});"); + + test(js, result); + } + public void testOverlappingOriginalAndGeneratedNames() { test("/** @constructor */ function Bar(){};" + "Bar.prototype.b = function(){};" diff --git a/test/com/google/javascript/jscomp/DisambiguatePropertiesTest.java b/test/com/google/javascript/jscomp/DisambiguatePropertiesTest.java index e0c4bd26b8d..af91dc007cd 100644 --- a/test/com/google/javascript/jscomp/DisambiguatePropertiesTest.java +++ b/test/com/google/javascript/jscomp/DisambiguatePropertiesTest.java @@ -989,6 +989,88 @@ public void testObjectLiteralReflected() { testSets(js, result, "{foo=[[F.prototype], [G.prototype]]}"); } + public void testObjectLiteralDefineProperties() { + String externs = + LINE_JOINER.join( + "/** @const */ var Object = {};", + "Object.defineProperties = function(typeRef, definitions) {}", + "/** @constructor */ function FooBar() {}", + "/** @type {string} */ FooBar.prototype.bar_;", + "/** @type {string} */ FooBar.prototype.bar;"); + + String js = + LINE_JOINER.join( + "/** @struct @constructor */ var Foo = function() {", + " this.bar_ = 'bar';", + "};", + "/** @type {?} */ Foo.prototype.bar;", + "Object.defineProperties(Foo.prototype, {", + " bar: {", + " configurable: true,", + " enumerable: true,", + " /** @this {Foo} */ get: function() { return this.bar_;},", + " /** @this {Foo} */ set: function(value) { this.bar_ = value; }", + " }", + "});"); + + String result = + LINE_JOINER.join( + "/** @struct @constructor */ var Foo = function() {", + " this.Foo$bar_ = 'bar';", + "};", + "/** @type {?} */ Foo.prototype.Foo_prototype$bar;", + "Object.defineProperties(Foo.prototype, {", + " Foo_prototype$bar: {", + " configurable: true,", + " enumerable: true,", + " /** @this {Foo} */ get: function() { return this.Foo$bar_;},", + " /** @this {Foo} */ set: function(value) { this.Foo$bar_ = value; }", + " }", + "});"); + testSets(externs, js, result, "{bar=[[Foo.prototype]], bar_=[[Foo]]}"); + } + + public void testObjectLiteralDefinePropertiesQuoted() { + String externs = + LINE_JOINER.join( + "/** @const */ var Object = {};", + "Object.defineProperties = function(typeRef, definitions) {}", + "/** @constructor */ function FooBar() {}", + "/** @type {string} */ FooBar.prototype.bar_;", + "/** @type {string} */ FooBar.prototype.bar;"); + + String js = + LINE_JOINER.join( + "/** @struct @constructor */ var Foo = function() {", + " this.bar_ = 'bar';", + "};", + "/** @type {?} */ Foo.prototype['bar'];", + "Object.defineProperties(Foo.prototype, {", + " 'bar': {", + " configurable: true,", + " enumerable: true,", + " /** @this {Foo} */ get: function() { return this.bar_;},", + " /** @this {Foo} */ set: function(value) { this.bar_ = value; }", + " }", + "});"); + + String result = + LINE_JOINER.join( + "/** @struct @constructor */ var Foo = function() {", + " this.Foo$bar_ = 'bar';", + "};", + "/** @type {?} */ Foo.prototype['bar'];", + "Object.defineProperties(Foo.prototype, {", + " 'bar': {", + " configurable: true,", + " enumerable: true,", + " /** @this {Foo} */ get: function() { return this.Foo$bar_;},", + " /** @this {Foo} */ set: function(value) { this.Foo$bar_ = value; }", + " }", + "});"); + testSets(externs, js, result, "{bar_=[[Foo]]}"); + } + public void testObjectLiteralLends() { String js = "" + "var mixin = function(x) { return x; };"