Skip to content

Commit

Permalink
Fix inference of super() calls
Browse files Browse the repository at this point in the history
Previously we were inferring them based on the return type of the constructor, which is typically undefined.  But it actually returns the 'this' instance for the remainder of the constructor, and we should infer it using the new: slot of the function.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=206413799
  • Loading branch information
shicks authored and blickly committed Jul 30, 2018
1 parent a98a012 commit ba681ad
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 9 deletions.
5 changes: 4 additions & 1 deletion src/com/google/javascript/jscomp/AstValidator.java
Expand Up @@ -441,7 +441,10 @@ private void validateExpressionType(Node n) {
break;

case CALL:
validateCallType(n);
if (!n.getFirstChild().isSuper()) {
// TODO(sdh): need to validate super() using validateNewType() instead, if it existed
validateCallType(n);
}
break;

default:
Expand Down
10 changes: 8 additions & 2 deletions src/com/google/javascript/jscomp/TypeInference.java
Expand Up @@ -1178,7 +1178,10 @@ private FlowScope traverseFunctionInvocation(Node n, FlowScope scope) {

Node left = n.getFirstChild();
JSType functionType = getJSType(left).restrictByNotNullOrUndefined();
if (functionType.isFunctionType()) {
if (left.isSuper()) {
// TODO(sdh): This will probably return the super type; might want to return 'this' instead?
return traverseInstantiation(n, functionType, scope);
} else if (functionType.isFunctionType()) {
FunctionType fnType = functionType.toMaybeFunctionType();
n.setJSType(fnType.getReturnType());
backwardsInferenceFromCallSite(n, fnType, scope);
Expand Down Expand Up @@ -1693,9 +1696,12 @@ private boolean inferTemplatedTypesForCall(Node n, FunctionType fnType, FlowScop

private FlowScope traverseNew(Node n, FlowScope scope) {
scope = traverseChildren(n, scope);

Node constructor = n.getFirstChild();
JSType constructorType = constructor.getJSType();
return traverseInstantiation(n, constructorType, scope);
}

private FlowScope traverseInstantiation(Node n, JSType constructorType, FlowScope scope) {
JSType type = null;
if (constructorType != null) {
constructorType = constructorType.restrictByNotNullOrUndefined();
Expand Down
45 changes: 39 additions & 6 deletions test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java
Expand Up @@ -1501,7 +1501,10 @@ public void testComputedProp1() {
public void testComputedProp2a() {
// Computed properties do type inference within
testTypes(
lines("var n; var obj = {[n = 'foo']: i}; var /** number */ m = n;"),
lines(
"var n;", //
"var obj = {[n = 'foo']: i};",
"var /** number */ m = n;"),
lines(
"initializing variable", // preserve new line
"found : string",
Expand All @@ -1525,7 +1528,10 @@ public void testComputedProp2b() {
public void testComputedProp2c() {
// Computed properties do type inference within
testTypes(
lines("var n; var obj = {[foo]: n = 'bar'}; var /** number */ m = n;"),
lines(
"var n;", //
"var obj = {[foo]: n = 'bar'};",
"var /** number */ m = n;"),
lines(
"initializing variable", // preserve new line
"found : string",
Expand All @@ -1535,14 +1541,19 @@ public void testComputedProp2c() {
public void testComputedProp3() {
// Computed prop does not exist as obj prop
testTypes(
lines("var i = 1; var obj = { ['var' + i]: i }; var x = obj.var1"),
lines(
"var i = 1;", //
"var obj = { ['var' + i]: i };",
"var x = obj.var1"),
"Property var1 never defined on obj");
}

public void testComputedProp3b() {
// Computed prop does not exist as obj prop even when a simple string literal
testTypes(
lines("var obj = { ['static']: 1 }; var /** number */ x = obj.static"),
lines(
"var obj = { ['static']: 1 };", //
"var /** number */ x = obj.static"),
"Property static never defined on obj");
}

Expand Down Expand Up @@ -1857,7 +1868,10 @@ public void testTaggedTemplateLiteral_backInference() {
"function f(x, z) {}",
// infers that "this" is ITemplateArray inside the function literal
"f`${ function() { /** @type {string} */ var x = this } }`;"),
lines("initializing variable", "found : ITemplateArray", "required: string"));
lines(
"initializing variable", //
"found : ITemplateArray",
"required: string"));
}

public void testITemplateArray1() {
Expand Down Expand Up @@ -3406,6 +3420,22 @@ public void testClassExtendsCycleOnlyInAst() {
"class Foo extends Foo {}"));
}

public void testClassSuperCallResult() {
testTypes(
lines(
"class Bar {}",
"class Foo extends Bar {",
" constructor() {",
" var /** null */ x = super();",
" }",
"}"),
// TODO(sdh): This should probably infer Foo, rather than Bar?
lines(
"initializing variable", //
"found : Bar",
"required: null"));
}

public void testAsyncFunctionWithoutJSDoc() {
testTypes("async function f() { return 3; }");
}
Expand Down Expand Up @@ -3510,7 +3540,10 @@ public void testAsyncCanReturnIThenable1() {
"async function getAString() {",
" return 1;",
"}"),
lines("inconsistent return type", "found : number", "required: string"));
lines(
"inconsistent return type", //
"found : number",
"required: string"));
}

public void testAsyncReturnStatementIsResolved() {
Expand Down

0 comments on commit ba681ad

Please sign in to comment.