Skip to content

Commit

Permalink
Support destructuring and ES6 object literals in AmbiguateProperties.
Browse files Browse the repository at this point in the history 
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=223374956
  • Loading branch information
@lauraharker @tjgq
lauraharker authored and tjgq committed Nov 30, 2018
1 parent c3dbca0 commit 60d4194
Show file tree
Hide file tree
Showing 2 changed files with 199 additions and 5 deletions.
29 changes: 24 additions & 5 deletions src/com/google/javascript/jscomp/AmbiguateProperties.java
View file
Expand Up @@ -459,7 +459,8 @@ public void visit(NodeTraversal t, Node n, Node parent) {
return;

case OBJECTLIT:
processObjectLit(n);
case OBJECT_PATTERN:
processObjectLitOrPattern(n);
return;

case GETELEM:
Expand Down Expand Up @@ -531,7 +532,7 @@ private void processCall(Node call) {
}
}

private void processObjectLit(Node objectLit) {
private void processObjectLitOrPattern(Node objectLit) {
// Object.defineProperties literals are handled at the CALL node.
if (objectLit.getParent().isCall()
&& NodeUtil.isObjectDefinePropertiesDefinition(objectLit.getParent())) {
Expand All @@ -544,7 +545,16 @@ private void processObjectLit(Node objectLit) {
for (Node key = objectLit.getFirstChild(); key != null; key = key.getNext()) {
// We only want keys that were unquoted.
// Keys are STRING, GET, SET
if (key.isQuotedString()) {
if (key.isComputedProp()) {
if (key.getFirstChild().isString()) {
// Ensure that we never rename some other property in a way that could conflict with
// this computed prop. This is largely because we store quoted member fns as
// computed properties and want to be consistent with how other quoted properties
// invalidate property names
quotedNames.add(key.getFirstChild().getString());
}
// Never rename computed properties
} else if (key.isQuotedString()) {
// Ensure that we never rename some other property in a way
// that could conflict with this quoted key.
quotedNames.add(key.getString());
Expand All @@ -571,17 +581,26 @@ private void processClass(Node classNode) {
? classConstructorType.toMaybeFunctionType().getPrototype()
: compiler.getTypeRegistry().getNativeType(JSTypeNative.UNKNOWN_TYPE);
for (Node member : NodeUtil.getClassMembers(classNode).children()) {
if (member.isComputedProp() || member.isQuotedString()) {
// ignore ['foo']() {} or get 'foo'() {}
if (member.isQuotedString()) {
// ignore get 'foo'() {} and prevent property name collisions
// Note that only getters/setters are represented as quoted strings, not 'foo'() {}
// see https://github.com/google/closure-compiler/issues/3071
quotedNames.add(member.getString());
continue;
} else if (member.isComputedProp()) {
// ignore ['foo']() {}
// for simple cases, we also prevent renaming collisions
if (member.getFirstChild().isString()) {
quotedNames.add(member.getFirstChild().getString());
}
continue;
} else if ("constructor".equals(member.getString())) {
// don't rename `class C { constructor() {} }` !
// This only applies for ES6 classes, not generic properties called 'constructor', which
// is why it's handled in this method specifically.
continue;
}

JSType memberOwnerType = member.isStaticMember() ? classConstructorType : classPrototype;

// member could be a MEMBER_FUNCTION_DEF, GETTER_DEF, or SETTER_DEF
Expand Down
175 changes: 175 additions & 0 deletions test/com/google/javascript/jscomp/AmbiguatePropertiesTest.java
View file
Expand Up @@ -1276,6 +1276,13 @@ public void testQuotedMemberFnInClass_notAmbiguated() {
testSame("/** @dict */ class Foo { 'methodFoo'() {} }");
}

@Test
public void testQuotedMemberFnInClassReservesPropertyName() {
test(
"/** @unrestricted */ class Foo { 'a'() {} foo() {} }",
"/** @unrestricted */ class Foo { 'a'() {} b() {} }");
}

@Test
public void testSingleClass_withTwoMemberFns_notAmbiguated() {
test("class Foo { method1() {} method2() {} }", "class Foo { a() {} b() {} }");
Expand Down Expand Up @@ -1394,6 +1401,13 @@ public void testQuotedGetterInClass_isNotAmbiguated() {
testSame("/** @dict */ class Foo { get 'prop'() {} }");
}

@Test
public void testQuotedGetterInClassReservesPropertyName() {
test(
"/** @unrestricted */ class Foo { get 'a'() {} foo() {} }",
"/** @unrestricted */ class Foo { get 'a'() {} b() {} }");
}

@Test
public void testSetterInClass_isAmbiguated() {
test("class Foo { set prop(x) {} }", "class Foo { set a(x) {} }");
Expand All @@ -1419,6 +1433,24 @@ public void testComputedMemberFunctionInClass_notAmbiguated() {
testSame("/** @dict */ class Foo { ['method']() {}}");
}

@Test
public void testSimpleComputedMemberFnInClassReservesPropertyName() {
test(
"/** @unrestricted */ class Foo { ['a']() {} foo() {} }",
"/** @unrestricted */ class Foo { ['a']() {} b() {} }");
}

@Test
public void testComplexComputedMemberFnInClassDoesntReservePropertyName() {
// we don't try to evaluate 'a' + '' to see that it's identical to 'a', and so end up renaming
// 'foo' -> 'a'
// The property name invalidation is already just a heuristic, so just handle the very simple
// case of ['a']() {}
test(
"/** @unrestricted */ class Foo { ['a' + '']() {} foo() {} }",
"/** @unrestricted */ class Foo { ['a' + '']() {} a() {} }");
}

@Test
public void testEs6ClassConstructorMethod_notAmbiguated() {
testSame("class Foo { constructor() {} }");
Expand Down Expand Up @@ -1463,4 +1495,147 @@ public void testObjectSetPrototypeOfIsIgnored() {
// now trying to reference Foo.barMethod will not work, and will call barMethod instead.
// AmbiguateProperties currently ignores this case
}

@Test
public void testComputedPropertyInObjectLiteral_notAmbiguated() {
testSame("const obj = {['a']: 3, b: 4}");
}

@Test
public void testQuotedPropertyInObjectLiteralReservesPropertyName() {
test(
"const obj = {'a': 3}; class C { method() {}}",
// `method` is renamed to `b`, not `a`, to avoid colliding with the computed prop.
// This is just a heuristic; JSCompiler cannot statically determine all string
// property names.
"const obj = {'a': 3}; class C { b() {}}");
}

@Test
public void testQuotedMemberFnInObjectLiteralReservesPropertyName() {
test(
"const obj = {'a'() {}}; class C { method() {}}",
"const obj = {'a'() {}}; class C { b() {}}");
}

@Test
public void testComputedPropertyInObjectLiteralReservesPropertyName() {
test(
"const obj = {['a']: 3}; class C { method() {}}",
"const obj = {['a']: 3}; class C { b() {}}");
}

@Test
public void testMemberFnInObjectLiteralPreventsPropertyRenaming() {
// Don't rename the class member 'm' because the object literal type is invalidating, and
// also has a property 'm'
testSame("const obj = {m() {}}; class C {m() {}}");
}

@Test
public void testSimplePropInObjectLiteralPreventsPropertyRenaminge() {
// Don't rename the class member 'm' because the object literal type is invalidating, and
// also has a property 'm'
testSame("const obj = {m: 0}; class C {m() {}}");
}

@Test
public void testObjectPatternDeclarationWithStringKey_ambiguated() {
test(
lines(
"class Foo {", //
" method() {}",
"}",
"const {method} = new Foo();"),
lines(
"class Foo {", //
" a() {}",
"}",
"const {a: method} = new Foo();"));
}

@Test
public void testObjectPatternDeclarationWithStringKeWithDefault_ambiguated() {
test(
lines(
"class Foo {", //
" method() {}",
"}",
"const {method = () => 3} = new Foo();"),
lines(
"class Foo {", //
" a() {}",
"}",
"const {a: method = () => 3} = new Foo();"));
}

@Test
public void testNestedObjectPatternWithStringKey_ambiguated() {
test(
lines(
"class Foo {", //
" method() {}",
"}",
"const {foo: {method}} = {foo: new Foo()};"),
lines(
"class Foo {", //
" a() {}",
"}",
// note: we rename the 'method' access but not 'foo', because we don't try ambiguating
// properties on object literal types.
"const {foo: {a: method}} = {foo: new Foo()};"));
}

@Test
public void testObjectPatternParameterWithStringKey_ambiguated() {
test(
lines(
"class Foo {", //
" method() {}",
"}",
"/** @param {!Foo} foo */",
"function f({method}) {}"),
lines(
"class Foo {", //
" a() {}",
"}",
"/** @param {!Foo} foo */",
"function f({a: method}) {}"));
}

@Test
public void testObjectPatternQuotedStringKey_notAmbiguated() {
// this emits a warning for a computed property access on a struct, since property ambiguation
// will break this code.
ignoreWarnings(TypeValidator.ILLEGAL_PROPERTY_ACCESS);
test(
lines(
"class Foo {", //
" method() {}",
"}",
"const {'method': method} = new Foo();"),
lines(
"class Foo {", //
" a() {}",
"}",
"const {'method': method} = new Foo();"));
}

@Test
public void testObjectPatternComputedProperty_notAmbiguated() {
// this emits a warning for a computed property access on a struct, since property ambiguation
// will break this code.
ignoreWarnings(TypeValidator.ILLEGAL_PROPERTY_ACCESS);
test(
lines(
"class Foo {", //
" method() {}",
"}",
"const {['method']: method} = new Foo();"),
lines(
"class Foo {", //
" a() {}",
"}",
"const {['method']: method} = new Foo();"));
}
}

0 comments on commit 60d4194

Please sign in to comment.