Skip to content

Commit

Permalink
For types defined using CLASS, use the MEMBER_FUNCTION_DEF of "constr…
Browse files Browse the repository at this point in the history
…uctor", rather than the CLASS node, as the definition node of the type's "constructor" property.

This change makes it possible to lookup JSDoc attached to the CLASS and "constructor" separately. Such distinction is a prerequisite for separate handling for class/constructor access-controls.

Additionally, literal accesses to "constructor" properties (e.g. `Foo.prototype.constructor`, `foo.constructor`) will become subject to access controls following this change. Previously, they had been subject to the same controls as the class itself by dint of using the same JSDoc.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=209617028
  • Loading branch information
nreid260 authored and blickly committed Aug 27, 2018
1 parent 5706ef9 commit 67ca821
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 13 deletions.
3 changes: 2 additions & 1 deletion src/com/google/javascript/jscomp/TypedScopeCreator.java
Expand Up @@ -1093,7 +1093,7 @@ private FunctionType createClassTypeFromNodes(

FunctionType qmarkCtor = classType.forgetParameterAndReturnTypes();
ObjectType classPrototypeType = classType.getPrototypeProperty();
classPrototypeType.defineDeclaredProperty("constructor", qmarkCtor, classType.getSource());
classPrototypeType.defineDeclaredProperty("constructor", qmarkCtor, constructor);
}

return classType;
Expand Down Expand Up @@ -2748,6 +2748,7 @@ void visitPostorder(NodeTraversal t, Node n, Node parent) {
.allowLaterTypeInference(false)
.defineSlot();
} else if (n.isMemberFunctionDef() && !"constructor".equals(n.getString())) {
// Ignore "constructor" since it has special handling in `createClassTypeFromNodes()`.
defineMemberFunction(n);
} else if (n.isGetterDef() || n.isSetterDef()) {
defineGetterSetter(n);
Expand Down
91 changes: 79 additions & 12 deletions test/com/google/javascript/jscomp/TypedScopeCreatorTest.java
Expand Up @@ -28,6 +28,7 @@
import static com.google.javascript.rhino.jstype.JSTypeNative.OBJECT_TYPE;
import static com.google.javascript.rhino.jstype.JSTypeNative.STRING_TYPE;
import static com.google.javascript.rhino.jstype.JSTypeNative.UNKNOWN_TYPE;
import static com.google.javascript.rhino.testing.NodeSubject.assertNode;
import static com.google.javascript.rhino.testing.TypeSubject.assertType;

import com.google.common.base.Predicate;
Expand Down Expand Up @@ -2000,18 +2001,67 @@ public void testClassDeclaration() {
assertScope(globalScope).declares("Foo").withTypeThat().isEqualTo(foo);
}

public void testClassDeclarationWithoutConstructor() {
testSame("class Foo {}");

FunctionType fooClass = (FunctionType) findNameType("Foo", globalScope);
ObjectType fooProto = fooClass.getPrototype();

// Test class typing.
assertTrue(fooClass.isConstructor());

// Test constructor property.
assertThat(fooProto.hasOwnProperty("constructor")).isTrue();
assertNode(fooProto.getOwnPropertyDefSite("constructor")).isNull();
assertType(fooProto)
.withTypeOfProp("constructor")
.toStringIsEqualTo("function(new:Foo, ...?): ?");
}

public void testClassDeclarationWithConstructor() {
testSame(
lines(
"class Foo {", //
" /** @param {number} arg */",
" constructor(arg) {}",
" constructor(arg) {",
" CTOR_BODY:;",
" }",
"}"));
FunctionType foo = (FunctionType) (findNameType("Foo", globalScope));
assertTrue(foo.isConstructor());
List<JSType> params = ImmutableList.copyOf(foo.getParameterTypes());

FunctionType fooClass = (FunctionType) findNameType("Foo", globalScope);
ObjectType fooProto = fooClass.getPrototype();
List<JSType> params = ImmutableList.copyOf(fooClass.getParameterTypes());
Node ctorDef = getLabeledStatement("CTOR_BODY").statementNode.getAncestor(3);

// Test class typing.
assertTrue(fooClass.isConstructor());
assertThat(params).hasSize(1);
assertType(params.get(0)).isNumber();

// Test constructor property.
assertThat(fooProto.hasOwnProperty("constructor")).isTrue();
assertNode(fooProto.getOwnPropertyDefSite("constructor")).isSameAs(ctorDef);
assertType(fooProto)
.withTypeOfProp("constructor")
.toStringIsEqualTo("function(new:Foo, ...?): ?");
}

public void testInterfaceClassDeclarationWithConstructor() {
testSame(
lines(
"/** @interface */", //
"class Foo {",
" constructor(arg) {}",
"}"));

FunctionType fooClass = (FunctionType) findNameType("Foo", globalScope);
ObjectType fooProto = fooClass.getPrototype();

// Test class typing.
assertTrue(fooClass.isInterface());

// Test constructor property.
assertThat(fooProto.hasOwnProperty("constructor")).isFalse();
}

public void testClassDeclarationWithExtends() {
Expand Down Expand Up @@ -2043,10 +2093,22 @@ public void testClassDeclarationWithInheritedConstructor() {
" constructor(/** string */ arg) {}",
"}",
"class Foo extends Bar {}"));
FunctionType foo = (FunctionType) (findNameType("Foo", globalScope));
List<JSType> params = ImmutableList.copyOf(foo.getParameterTypes());

FunctionType fooClass = (FunctionType) findNameType("Foo", globalScope);
ObjectType fooProto = fooClass.getPrototype();
List<JSType> params = ImmutableList.copyOf(fooClass.getParameterTypes());

// Test class typing.
assertTrue(fooClass.isConstructor());
assertThat(params).hasSize(1);
assertType(params.get(0)).isString();

// Test constructor property.
assertThat(fooProto.hasOwnProperty("constructor")).isTrue();
assertNode(fooProto.getOwnPropertyDefSite("constructor")).isNull();
assertType(fooProto)
.withTypeOfProp("constructor")
.toStringIsEqualTo("function(new:Foo, ...?): ?");
}

public void testClassDeclarationWithOverriddenConstructor() {
Expand All @@ -2056,25 +2118,30 @@ public void testClassDeclarationWithOverriddenConstructor() {
" constructor(/** string */ arg) {}",
"}",
"class Foo extends Bar {",
" constructor(/** number */ arg) { super(''); }",
" constructor(/** number */ arg) { CTOR_BODY:super(''); }",
" static method() {}",
"}"));

FunctionType bar = (FunctionType) (findNameType("Bar", globalScope));
ObjectType barObject = bar.getInstanceType();
JSType barConstructorProperty = barObject.getPropertyType("constructor");

FunctionType foo = (FunctionType) (findNameType("Foo", globalScope));
ObjectType fooObject = foo.getInstanceType();
ObjectType fooProto = foo.getPrototype();
JSType fooConstructorProperty = fooObject.getPropertyType("constructor");
Node fooCtorDef = getLabeledStatement("CTOR_BODY").statementNode.getAncestor(3);

assertType(foo).withTypeOfProp("method").isNotUnknown();
assertType(foo).withTypeOfProp("method").isNotEmpty();

ObjectType barObject = bar.getInstanceType();
ObjectType fooObject = foo.getInstanceType();

List<JSType> params = ImmutableList.copyOf(foo.getParameterTypes());
assertThat(params).hasSize(1);
assertType(params.get(0)).isNumber();
JSType barConstructorProperty = barObject.getPropertyType("constructor");
assertType(barConstructorProperty).toStringIsEqualTo("function(new:Bar, ...?): ?");

JSType fooConstructorProperty = fooObject.getPropertyType("constructor");
assertType(fooConstructorProperty).toStringIsEqualTo("function(new:Foo, ...?): ?");
assertNode(fooProto.getOwnPropertyDefSite("constructor")).isSameAs(fooCtorDef);

assertType(fooConstructorProperty).isSubtypeOf(barConstructorProperty);
assertType(fooConstructorProperty).withTypeOfProp("method").isNotUnknown();
Expand Down

0 comments on commit 67ca821

Please sign in to comment.