Skip to content

Commit

Permalink
Don't collapse properties on objects called with hasOwnProperty
Browse files Browse the repository at this point in the history
If we see a call like "a.b.hasOwnProperty(c);", treat it the same (in CollapseProperties) as a.b[c]. This fixes a bug where the compiler collapsed properties on objects that were only used through hasOwnProperty.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=184861876
  • Loading branch information
lauraharker authored and blickly committed Feb 7, 2018
1 parent c395501 commit c13cf48
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 0 deletions.
29 changes: 29 additions & 0 deletions src/com/google/javascript/jscomp/GlobalNamespace.java
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,13 @@ public void collect(JSModule module, Scope scope, Node n) {
}
name = n.getQualifiedName();
break;
case CALL:
if (isObjectHasOwnPropertyCall(n)) {
String qname = n.getFirstFirstChild().getQualifiedName();
Name globalName = getOrCreateName(qname, true);
globalName.usedHasOwnProperty = true;
}
return;
default:
return;
}
Expand Down Expand Up @@ -828,6 +835,23 @@ private boolean isClassDefiningCall(Node callNode) {
return className != null;
}

/** Detect calls of the form a.b.hasOwnProperty(c); that prevent property collapsing on a.b */
private boolean isObjectHasOwnPropertyCall(Node callNode) {
checkArgument(callNode.isCall(), callNode);
if (!callNode.hasTwoChildren()) {
return false;
}
Node fn = callNode.getFirstChild();
if (!fn.isGetProp()) {
return false;
}
Node callee = fn.getFirstChild();
Node method = fn.getSecondChild();
return method.isString()
&& "hasOwnProperty".equals(method.getString())
&& callee.isQualifiedName();
}

/**
* Determines whether the result of a hook (x?y:z) or boolean expression
* (x||y) or (x&&y) is assigned to a specific global name.
Expand Down Expand Up @@ -1019,6 +1043,7 @@ enum Type {
private boolean declaredType = false;
private boolean isDeclared = false;
private boolean isModuleProp = false;
private boolean usedHasOwnProperty = false;
int globalSets = 0;
int localSets = 0;
int localSetsWithNoCollapse = 0;
Expand Down Expand Up @@ -1297,6 +1322,10 @@ boolean canCollapseUnannotatedChildNames() {
return false;
}

if (usedHasOwnProperty) {
return false;
}

if (declaredType) {
return true;
}
Expand Down
40 changes: 40 additions & 0 deletions test/com/google/javascript/jscomp/CollapsePropertiesTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1172,6 +1172,46 @@ public void testBug1974371() {
+ "for (var key in Foo) {}");
}

public void testHasOwnProperty() {
testSame("var a = {b: 3}; if (a.hasOwnProperty(foo)) { alert('ok'); }");
testSame("var a = {b: 3}; if (a.hasOwnProperty(foo)) { alert('ok'); } a.b;");
}

public void testHasOwnPropertyOnNonGlobalName() {
testSame(lines(
"/** @constructor */",
"function A() {",
" this.foo = {a: 1, b: 1};",
"}",
"A.prototype.bar = function(prop) {",
" return this.foo.hasOwnProperty(prop);",
"}"));
testSame("var a = {'b': {'c': 1}}; a['b'].hasOwnProperty('c');");
testSame("var a = {b: 3}; if (Object.prototype.hasOwnProperty.call(a, 'b')) { alert('ok'); }");
}

public void testHasOwnPropertyNested() {
test(
"var a = {b: {c: 3}}; if (a.b.hasOwnProperty('c')) { alert('ok'); }",
"var a$b = {c: 3}; if (a$b.hasOwnProperty('c')) { alert('ok'); }");
test(
"var a = {b: {c: 3}}; if (a.b.hasOwnProperty('c')) { alert('ok'); } a.b.c;",
"var a$b = {c: 3}; if (a$b.hasOwnProperty('c')) { alert('ok'); } a$b.c;");
test("var a = {}; a.b = function(p) { log(a.b.c.hasOwnProperty(p)); }; a.b.c = {};",
"var a$b = function(p) { log(a$b$c.hasOwnProperty(p)); }; var a$b$c = {};");
}

public void testHasOwnPropertyMultiple() {
testSame("var a = {b: 3, c: 4, d: 5}; if (a.hasOwnProperty(prop)) { alert('ok'); }");
}

public void testObjectStaticMethodsPreventCollapsing() {
testSame("var a = {b: 3}; alert(Object.getOwnPropertyDescriptor(a, 'b'));");
testSame("var a = {b: 3}; alert(Object.getOwnPropertyDescriptors(a));");
testSame("var a = {b: 3}; alert(Object.getOwnPropertyNames(a));");
testSame("var a = {b: 3, [Symbol('c')]: 4}; alert(Object.getOwnPropertySymbols(a));");
}

private static final String COMMON_ENUM =
"/** @enum {Object} */ var Foo = {A: {c: 2}, B: {c: 3}};";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,12 @@ public void testObjLitAssignmentDepth4() {
"var a$b$c$d$e = 1; var a$b$c$d$f = 2; var g = null; use(a$b$c$d$e);");
}

public void testHasOwnProperty() {
test(
"var a = {'b': 1, 'c': 1}; var alias = a; alert(alias.hasOwnProperty('c'));",
"var a = {'b': 1, 'c': 1}; var alias = null; alert(a.hasOwnProperty('c'));");
}

public void testAliasCreatedForObjectDepth1_1() {
// An object's properties are not collapsed if the object is referenced
// in a such a way that an alias is created for it, if that alias is used.
Expand Down

0 comments on commit c13cf48

Please sign in to comment.