Skip to content

Commit

Permalink
Fix bug in the old type checker where a stub method definition causes…
Browse files Browse the repository at this point in the history
… loss of type checking.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=184019882
  • Loading branch information
dimvar authored and lauraharker committed Feb 1, 2018
1 parent 4edb896 commit 20b4233
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 5 deletions.
32 changes: 27 additions & 5 deletions src/com/google/javascript/jscomp/TypedScopeCreator.java
Expand Up @@ -1806,6 +1806,22 @@ private ObjectType getObjectSlot(String slotName) {
return null;
}

/**
* When a class has a stub for a property, and the property exists on a super interface,
* use that type.
*/
private JSType getInheritedInterfacePropertyType(ObjectType obj, String propName) {
if (obj != null && obj.isPrototypeObject()) {
FunctionType f = obj.getOwnerFunction();
for (ObjectType i : f.getImplementedInterfaces()) {
if (i.hasProperty(propName)) {
return i.getPropertyType(propName);
}
}
}
return null;
}

/**
* Resolve any stub declarations to unknown types if we could not
* find types for them during traversal.
Expand All @@ -1826,17 +1842,19 @@ void resolveStubDeclarations() {
// If we see a stub property, make sure to register this property
// in the type registry.
ObjectType ownerType = getObjectSlot(ownerName);
defineSlot(n, parent, unknownType, true);
JSType inheritedType = getInheritedInterfacePropertyType(ownerType, propName);
JSType stubType = inheritedType == null ? unknownType : inheritedType;
defineSlot(n, parent, stubType, true);

if (ownerType != null &&
(isExtern || ownerType.isFunctionPrototypeType())) {
// If this is a stub for a prototype, just declare it
// as an unknown type. These are seen often in externs.
ownerType.defineInferredProperty(
propName, unknownType, n);
propName, stubType, n);
} else {
typeRegistry.registerPropertyOnType(
propName, ownerType == null ? unknownType : ownerType);
propName, ownerType == null ? stubType : ownerType);
}
}
}
Expand Down Expand Up @@ -2010,10 +2028,14 @@ void build() {

private ObjectType getThisTypeForCollectingProperties() {
Node rootNode = scope.getRootNode();
if (rootNode.isFromExterns()) return null;
if (rootNode.isFromExterns()) {
return null;
}

JSType type = rootNode.getJSType();
if (type == null || !type.isFunctionType()) return null;
if (type == null || !type.isFunctionType()) {
return null;
}

FunctionType fnType = type.toMaybeFunctionType();
JSType fnThisType = fnType.getTypeOfThis();
Expand Down
41 changes: 41 additions & 0 deletions test/com/google/javascript/jscomp/TypeCheckTest.java
Expand Up @@ -2920,6 +2920,47 @@ public void testStubFunctionDeclaration10() {
"function(number): number");
}


public void testStubMethodDeclarationDoesntBlockTypechecking_1() {
testTypes(
lines(
"/** @interface */",
"function Foo() {}",
"/** @return {number} */",
"Foo.prototype.method = function() {};",
"/**",
" * @constructor",
" * @implements {Foo}",
" */",
"function Bar() {}",
"Bar.prototype.method;",
"var /** null */ n = (new Bar).method();"),
lines(
"initializing variable",
"found : number",
"required: null"));
}

public void testStubMethodDeclarationDoesntBlockTypechecking_2() {
testTypes(
lines(
"/** @constructor */",
"function Foo() {}",
"/** @return {number} */",
"Foo.prototype.method = function() {};",
"/**",
" * @constructor",
" * @extends {Foo}",
" */",
"function Bar() {}",
"Bar.prototype.method;",
"var /** null */ n = (new Bar).method();"),
lines(
"initializing variable",
"found : number",
"required: null"));
}

public void testNestedFunctionInference1() {
String nestedAssignOfFooAndBar =
"/** @constructor */ function f() {};" +
Expand Down

0 comments on commit 20b4233

Please sign in to comment.