Skip to content

Commit

Permalink
Add support for Object.defineProperties to type-based renaming passes
Browse files Browse the repository at this point in the history 
Merge pull request #1823 from ChadKillingsworth/closure-compiler
Closes #1823
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=123980939
  • Loading branch information
@ChadKillingsworth @blickly
ChadKillingsworth authored and blickly committed Jun 3, 2016
1 parent 3af18d3 commit 6598a00
Show file tree
Hide file tree
Showing 4 changed files with 261 additions and 55 deletions.
109 changes: 63 additions & 46 deletions src/com/google/javascript/jscomp/AmbiguateProperties.java
View file
Expand Up @@ -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()) {
Expand Down
39 changes: 34 additions & 5 deletions src/com/google/javascript/jscomp/DisambiguateProperties.java
View file
Expand Up @@ -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()) {
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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<String> errors, JSType t) {
if (!t.isObject() || t.isAllType()) {
return;
Expand Down
86 changes: 82 additions & 4 deletions test/com/google/javascript/jscomp/AmbiguatePropertiesTest.java
View file
Expand Up @@ -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);
Expand Down Expand Up @@ -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(){};"
Expand Down
82 changes: 82 additions & 0 deletions test/com/google/javascript/jscomp/DisambiguatePropertiesTest.java
View file
Expand Up @@ -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; };"
Expand Down

0 comments on commit 6598a00

Please sign in to comment.