Skip to content

Commit

Permalink
Lookup the superclass/subclass for goog.inherits by property
Browse files Browse the repository at this point in the history
This fixes some cases where the superClass_ property was not defined on the subclass

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=234858167
  • Loading branch information
lauraharker committed Feb 21, 2019
1 parent f7398ba commit 21024fa
Show file tree
Hide file tree
Showing 5 changed files with 133 additions and 46 deletions.
11 changes: 7 additions & 4 deletions src/com/google/javascript/jscomp/TypeCheck.java
Expand Up @@ -41,6 +41,7 @@
import com.google.javascript.rhino.JSDocInfo; import com.google.javascript.rhino.JSDocInfo;
import com.google.javascript.rhino.JSTypeExpression; import com.google.javascript.rhino.JSTypeExpression;
import com.google.javascript.rhino.Node; import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.QualifiedName;
import com.google.javascript.rhino.Token; import com.google.javascript.rhino.Token;
import com.google.javascript.rhino.jstype.EnumType; import com.google.javascript.rhino.jstype.EnumType;
import com.google.javascript.rhino.jstype.FunctionType; import com.google.javascript.rhino.jstype.FunctionType;
Expand Down Expand Up @@ -2362,10 +2363,12 @@ private void checkCallConventions(NodeTraversal t, Node n) {
compiler.getCodingConvention().getClassesDefinedByCall(n); compiler.getCodingConvention().getClassesDefinedByCall(n);
TypedScope scope = t.getTypedScope(); TypedScope scope = t.getTypedScope();
if (relationship != null) { if (relationship != null) {
ObjectType superClass = TypeValidator.getInstanceOfCtor( ObjectType superClass =
scope.getVar(relationship.superclassName)); TypeValidator.getInstanceOfCtor(
ObjectType subClass = TypeValidator.getInstanceOfCtor( scope.lookupQualifiedName(QualifiedName.of(relationship.superclassName)));
scope.getVar(relationship.subclassName)); ObjectType subClass =
TypeValidator.getInstanceOfCtor(
scope.lookupQualifiedName(QualifiedName.of(relationship.subclassName)));
if (relationship.type == SubclassType.INHERITS if (relationship.type == SubclassType.INHERITS
&& superClass != null && superClass != null
&& !superClass.isEmptyType() && !superClass.isEmptyType()
Expand Down
21 changes: 6 additions & 15 deletions src/com/google/javascript/jscomp/TypeValidator.java
Expand Up @@ -191,21 +191,12 @@ class TypeValidator implements Serializable {
typeRegistry.getNativeObjectType(JSTypeNative.ASYNC_ITERATOR_TYPE)); typeRegistry.getNativeObjectType(JSTypeNative.ASYNC_ITERATOR_TYPE));
} }


/** /** Utility function that attempts to get an instance type from a potential constructor type */
* Utility function for getting a function type from a var. static ObjectType getInstanceOfCtor(@Nullable JSType t) {
*/ if (t == null) {
static FunctionType getFunctionType(@Nullable TypedVar v) { return null;
JSType t = v == null ? null : v.getType(); }
ObjectType o = t == null ? null : t.dereference(); FunctionType ctor = JSType.toMaybeFunctionType(t.dereference());
return JSType.toMaybeFunctionType(o);
}

/**
* Utility function for getting an instance type from a var pointing
* to the constructor.
*/
static ObjectType getInstanceOfCtor(@Nullable TypedVar v) {
FunctionType ctor = getFunctionType(v);
if (ctor != null && ctor.isConstructor()) { if (ctor != null && ctor.isConstructor()) {
return ctor.getInstanceType(); return ctor.getInstanceType();
} }
Expand Down
26 changes: 26 additions & 0 deletions src/com/google/javascript/jscomp/TypedScope.java
Expand Up @@ -21,10 +21,12 @@
import com.google.common.collect.Iterables; import com.google.common.collect.Iterables;
import com.google.javascript.rhino.JSDocInfo; import com.google.javascript.rhino.JSDocInfo;
import com.google.javascript.rhino.Node; import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.QualifiedName;
import com.google.javascript.rhino.jstype.JSType; import com.google.javascript.rhino.jstype.JSType;
import com.google.javascript.rhino.jstype.ObjectType; import com.google.javascript.rhino.jstype.ObjectType;
import com.google.javascript.rhino.jstype.StaticTypedScope; import com.google.javascript.rhino.jstype.StaticTypedScope;
import com.google.javascript.rhino.jstype.StaticTypedSlot; import com.google.javascript.rhino.jstype.StaticTypedSlot;
import javax.annotation.Nullable;


/** /**
* TypedScope contains information about variables and their types. Scopes can be nested, a scope * TypedScope contains information about variables and their types. Scopes can be nested, a scope
Expand Down Expand Up @@ -188,4 +190,28 @@ public JSDocInfo getJsdocOfTypeDeclaration(String typeName) {
StaticTypedSlot slot = getSlot(typeName); StaticTypedSlot slot = getSlot(typeName);
return slot == null ? null : slot.getJSDocInfo(); return slot == null ? null : slot.getJSDocInfo();
} }

/**
* Looks up a given qualified name in the scope. if that fails, looks up the component properties
* off of any owner types that are in scope.
*
* <p>This is always more or equally expensive as calling getVar(String name), so should only be
* used when necessary.
*/
@Nullable
public JSType lookupQualifiedName(QualifiedName qname) {
TypedVar slot = getVar(qname.join());
if (slot != null && !slot.isTypeInferred()) {
JSType type = slot.getType();
if (type != null && !type.isUnknownType()) {
return type;
}
} else if (!qname.isSimple()) {
JSType type = lookupQualifiedName(qname.getOwner());
if (type != null) {
return type.findPropertyType(qname.getComponent());
}
}
return null;
}
} }
36 changes: 9 additions & 27 deletions src/com/google/javascript/jscomp/TypedScopeCreator.java
Expand Up @@ -1977,7 +1977,7 @@ private Node getDefinitionNode(QualifiedName qname) {
TypedVar var = currentScope.getVar(qname.getComponent()); TypedVar var = currentScope.getVar(qname.getComponent());
return var != null ? var.getNameNode() : null; return var != null ? var.getNameNode() : null;
} }
ObjectType parent = ObjectType.cast(lookupQualifiedName(qname.getOwner())); ObjectType parent = ObjectType.cast(currentScope.lookupQualifiedName(qname.getOwner()));
return parent != null ? parent.getPropertyDefSite(qname.getComponent()) : null; return parent != null ? parent.getPropertyDefSite(qname.getComponent()) : null;
} }


Expand Down Expand Up @@ -2007,7 +2007,7 @@ private JSType getDeclaredRValueType(@Nullable Node lValue, Node rValue) {


// If rValue is a name, try looking it up in the current scope. // If rValue is a name, try looking it up in the current scope.
if (rValue.isQualifiedName()) { if (rValue.isQualifiedName()) {
return lookupQualifiedName(rValue.getQualifiedNameObject()); return currentScope.lookupQualifiedName(rValue.getQualifiedNameObject());
} }


// Check for simple invariant operations, such as "!x" or "+x" or "''+x" // Check for simple invariant operations, such as "!x" or "+x" or "''+x"
Expand All @@ -2024,11 +2024,10 @@ private JSType getDeclaredRValueType(@Nullable Node lValue, Node rValue) {
} }


if (rValue.isNew() && rValue.getFirstChild().isQualifiedName()) { if (rValue.isNew() && rValue.getFirstChild().isQualifiedName()) {
JSType targetType = lookupQualifiedName(rValue.getFirstChild().getQualifiedNameObject()); JSType targetType =
currentScope.lookupQualifiedName(rValue.getFirstChild().getQualifiedNameObject());
if (targetType != null) { if (targetType != null) {
FunctionType fnType = targetType FunctionType fnType = targetType.restrictByNotNullOrUndefined().toMaybeFunctionType();
.restrictByNotNullOrUndefined()
.toMaybeFunctionType();
if (fnType != null && fnType.hasInstanceType()) { if (fnType != null && fnType.hasInstanceType()) {
return fnType.getInstanceType(); return fnType.getInstanceType();
} }
Expand Down Expand Up @@ -2058,25 +2057,6 @@ private JSType getDeclaredRValueType(@Nullable Node lValue, Node rValue) {
return null; return null;
} }


private JSType lookupQualifiedName(QualifiedName qname) {
TypedVar slot = currentScope.getVar(qname.join());
if (slot != null && !slot.isTypeInferred()) {
JSType type = slot.getType();
if (type != null && !type.isUnknownType()) {
return type;
}
} else if (!qname.isSimple()) {
JSType type = lookupQualifiedName(qname.getOwner());
// NOTE: The scope only contains declared types at this
// point so any property we find is a value type
// to look up properties on.
if (type != null) {
return type.findPropertyType(qname.getComponent());
}
}
return null;
}

/** /**
* Look for class-defining calls. * Look for class-defining calls.
* Because JS has no 'native' syntax for defining classes, * Because JS has no 'native' syntax for defining classes,
Expand All @@ -2087,9 +2067,11 @@ void checkForClassDefiningCalls(Node n) {
codingConvention.getClassesDefinedByCall(n); codingConvention.getClassesDefinedByCall(n);
if (relationship != null) { if (relationship != null) {
ObjectType superClass = ObjectType superClass =
TypeValidator.getInstanceOfCtor(currentScope.getVar(relationship.superclassName)); TypeValidator.getInstanceOfCtor(
currentScope.lookupQualifiedName(QualifiedName.of(relationship.superclassName)));
ObjectType subClass = ObjectType subClass =
TypeValidator.getInstanceOfCtor(currentScope.getVar(relationship.subclassName)); TypeValidator.getInstanceOfCtor(
currentScope.lookupQualifiedName(QualifiedName.of(relationship.subclassName)));
if (superClass != null && subClass != null) { if (superClass != null && subClass != null) {
// superCtor and subCtor might be structural constructors // superCtor and subCtor might be structural constructors
// (like {function(new:Object)}) so we need to resolve them back // (like {function(new:Object)}) so we need to resolve them back
Expand Down
85 changes: 85 additions & 0 deletions test/com/google/javascript/jscomp/TypeCheckTest.java
Expand Up @@ -5059,6 +5059,91 @@ public void testGoodExtends14() {
"Property foo never defined on OldType.prototype"); "Property foo never defined on OldType.prototype");
} }


@Test
public void testGoodExtends_withAliasOfSuperclass() {
testTypesWithCommonExterns(
CLOSURE_DEFS
+ lines(
"/** @constructor */ const OldType = function () {};",
"const OldTypeAlias = OldType;",
"",
"/**",
" * @constructor",
" * @extends {OldTypeAlias}",
" */",
"function NewType() {};",
// Verify that we recognize the inheritance even when goog.inherits references
// OldTypeAlias, not OldType.
"goog.inherits(NewType, OldTypeAlias);",
"NewType.prototype.method = function() {",
" NewType.superClass_.foo.call(this);",
"};"),
"Property foo never defined on OldType.prototype");
}

@Test
public void testBadExtends_withAliasOfSuperclass() {
testTypesWithCommonExterns(
CLOSURE_DEFS
+ lines(
"const ns = {};",
"/** @constructor */ ns.OldType = function () {};",
"const nsAlias = ns;",
"",
"/**",
" * @constructor",
" * // no @extends here, NewType should not have a goog.inherits",
" */",
"function NewType() {};",
// Verify that we recognize the inheritance even when goog.inherits references
// nsAlias.OldType, not ns.OldType.
"goog.inherits(NewType, ns.OldType);"),
"Missing @extends tag on type NewType");
}

@Test
public void testBadExtends_withNamespacedAliasOfSuperclass() {
testTypesWithCommonExterns(
CLOSURE_DEFS
+ lines(
"const ns = {};",
"/** @constructor */ ns.OldType = function () {};",
"const nsAlias = ns;",
"",
"/**",
" * @constructor",
" * // no @extends here, NewType should not have a goog.inhertis",
" */",
"function NewType() {};",
// Verify that we recognize the inheritance even when goog.inherits references
// nsAlias.OldType, not ns.OldType.
"goog.inherits(NewType, nsAlias.OldType);"),
"Missing @extends tag on type NewType");
}

@Test
public void testGoodExtends_withNamespacedAliasOfSuperclass() {
testTypesWithCommonExterns(
CLOSURE_DEFS
+ lines(
"const ns = {};",
"/** @constructor */ ns.OldType = function () {};",
"const nsAlias = ns;",
"",
"/**",
" * @constructor",
" * @extends {nsAlias.OldType}",
" */",
"function NewType() {};",
// Verify that we recognize the inheritance even when goog.inherits references
// nsAlias.OldType, not ns.OldType.
"goog.inherits(NewType, nsAlias.OldType);",
"NewType.prototype.method = function() {",
" NewType.superClass_.foo.call(this);",
"};"),
"Property foo never defined on ns.OldType.prototype");
}

@Test @Test
public void testGoodExtends15() { public void testGoodExtends15() {
testTypes( testTypes(
Expand Down

0 comments on commit 21024fa

Please sign in to comment.