Skip to content

Commit

Permalink
Add support to infer the type of Generator function when the type is …
Browse files Browse the repository at this point in the history 
…not declared in the JSDoc.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=164332989
  • Loading branch information
@EatingW @dimvar
EatingW authored and dimvar committed Aug 7, 2017
1 parent 9cb2739 commit 2165d3a
Show file tree
Hide file tree
Showing 2 changed files with 144 additions and 23 deletions.
49 changes: 36 additions & 13 deletions src/com/google/javascript/jscomp/NewTypeInference.java
View file
Expand Up @@ -439,6 +439,8 @@ void add(JSError warning) {
private JSTypes commonTypes;
// RETVAL_ID is used when we calculate the summary type of a function
private static final String RETVAL_ID = "%return";
// YIELDVAL_ID is used when we calculate the summary type of a generator function
private static final String YIELDVAL_ID = "%yield";
private static final String THIS_ID = "this";
@SuppressWarnings("ConstantField")
private final String ABSTRACT_METHOD_NAME;
Expand Down Expand Up @@ -1172,6 +1174,23 @@ private void createSummary(NTIScope fn) {
builder.addNominalType(declType.getNominalType());
builder.addReceiverType(declType.getReceiverType());
builder.addAbstract(declType.isAbstract());
addRetTypeAndWarn(fn, exitEnv, declType, builder);

JSType summary = commonTypes.fromFunctionType(builder.buildFunction());
println("Function summary for ", fn.getReadableName());
println("\t", summary);
summary = changeTypeIfFunctionNamespace(fn, summary);
summaries.put(fn, summary);
maybeSetTypeI(fnRoot, summary);
Node fnNameNode = NodeUtil.getNameNode(fnRoot);
if (fnNameNode != null) {
maybeSetTypeI(fnNameNode, summary);
}
}

private void addRetTypeAndWarn(
NTIScope fn, TypeEnv exitEnv, DeclaredFunctionType declType, FunctionTypeBuilder builder) {
Node fnRoot = fn.getRoot();
JSType declRetType = declType.getReturnType();
JSType actualRetType = checkNotNull(envGetType(exitEnv, RETVAL_ID));

Expand All @@ -1197,8 +1216,9 @@ && hasPathWithNoReturn(this.cfg)) {
}
}
} else if (fnRoot.isGeneratorFunction()) {
// No declared return type for Generator. Infer to be Generator<?>
builder.addRetType(this.commonTypes.getGeneratorInstance(UNKNOWN));
// No declared return type for Generator. Use inferred type.
JSType yieldType = envGetType(exitEnv, YIELDVAL_ID);
builder.addRetType(this.commonTypes.getGeneratorInstance(firstNonNull(yieldType, UNKNOWN)));
} else if (declType.getNominalType() == null) {
// If someone uses the result of a function that doesn't return, they get a warning.
builder.addRetType(firstNonBottom(actualRetType, TOP));
Expand All @@ -1207,16 +1227,6 @@ && hasPathWithNoReturn(this.cfg)) {
// constructors called without new who don't explicitly declare @return.
builder.addRetType(UNDEFINED);
}
JSType summary = commonTypes.fromFunctionType(builder.buildFunction());
println("Function summary for ", fn.getReadableName());
println("\t", summary);
summary = changeTypeIfFunctionNamespace(fn, summary);
summaries.put(fn, summary);
maybeSetTypeI(fnRoot, summary);
Node fnNameNode = NodeUtil.getNameNode(fnRoot);
if (fnNameNode != null) {
maybeSetTypeI(fnNameNode, summary);
}
}

private JSType changeTypeIfFunctionNamespace(NTIScope fnScope, JSType fnType) {
Expand Down Expand Up @@ -2639,7 +2649,7 @@ private EnvTypePair analyzeSuperFwd(Node expr, TypeEnv inEnv) {

private EnvTypePair analyzeYieldFwd(Node expr, TypeEnv inEnv) {
if (!expr.hasChildren()) {
return new EnvTypePair(inEnv, UNKNOWN);
return new EnvTypePair(envPutType(inEnv, YIELDVAL_ID, UNDEFINED), UNKNOWN);
}
EnvTypePair resultPair = analyzeExprFwd(expr.getFirstChild(), inEnv);

Expand Down Expand Up @@ -2688,6 +2698,19 @@ private EnvTypePair analyzeYieldFwd(Node expr, TypeEnv inEnv) {
JSError.make(
expr, YIELD_NONDECLARED_TYPE, errorMsgWithTypeDiff(yieldType, actualRetType)),
actualRetType, yieldType);
resultPair.type = UNKNOWN;
return resultPair;
}

if (yieldType.isBottom() || yieldType.isUnknown()) {
// Infer the instantiated yield type of the function if there is no declared type.
JSType oldType = envGetType(resultPair.env, YIELDVAL_ID);
if (oldType == null) {
resultPair.env = envPutType(resultPair.env, YIELDVAL_ID, actualRetType);
} else {
resultPair.env = envPutType(
resultPair.env, YIELDVAL_ID, JSType.join(oldType, actualRetType));
}
}
resultPair.type = UNKNOWN;
return resultPair;
Expand Down
118 changes: 108 additions & 10 deletions test/com/google/javascript/jscomp/NewTypeInferenceTest.java
View file
Expand Up @@ -21217,16 +21217,6 @@ public void testGenerator() {
" yield 1;",
"}"));

// Infers Generator<?> if type not specified in JSDoc
typeCheck(
LINE_JOINER.join(
"function* gen() {",
" yield 1;",
"}",
"var g = gen();",
"var /** string */ x = g.next().value;",
"var /** number */ y = g.next().value;"));

// If type specified, check type of expression in yield.
typeCheck(
LINE_JOINER.join(
Expand Down Expand Up @@ -21394,5 +21384,113 @@ public void testGenerator() {
" return (x = 1);",
"}"),
NewTypeInference.MISTYPED_ASSIGN_RHS);

//Infers type of generator
typeCheck(
LINE_JOINER.join(
"function* gen() {",
" yield 1;",
"}",
"var /** Array<number> */ g = gen();"),
NewTypeInference.MISTYPED_ASSIGN_RHS);

//Infers type of generator
typeCheck(
LINE_JOINER.join(
"function* gen() {",
" yield 1;",
"}",
"var /** Generator<string> */ g = gen();"),
NewTypeInference.MISTYPED_ASSIGN_RHS);

// No warning when type is different
typeCheck(
LINE_JOINER.join(
"function* gen(){",
" yield 1;",
" yield '';",
"}",
"var /** Generator<number|string> */ x = gen();"));

// No children of yield is just undefined
typeCheck(
LINE_JOINER.join(
"function* gen(){",
" yield 1;",
" yield;",
"}",
"var /** Generator<number> */ x = gen();"),
NewTypeInference.MISTYPED_ASSIGN_RHS);

// No children of yield is just undefined
typeCheck(
LINE_JOINER.join(
"function* gen(){",
" yield 1;",
" yield;",
"}",
"var /** Generator<undefined> */ x = gen();"));

// Works with autoboxing
typeCheck(
LINE_JOINER.join(
"var /** number */ x;",
"var /** !Number */ y",
"function* gen(){",
" yield x;",
" yield y;",
"}",
"var g = gen();",
"var /** Number|number */ n = g.next().value;"));

// Works when there might be variables passed into the method next, and infer Generator<?>
typeCheck(
LINE_JOINER.join(
"function* gen() {",
" var x = yield 1;",
" yield x;",
"}",
"var /** !Generator<?> */ g = gen();"));

// Reassigning yield expression to variable that has a declared type produces Generator<?>
typeCheck(
LINE_JOINER.join(
"var /** number */ x = 1;",
"function* gen(){",
" yield x;",
" x = yield 1;",
" yield x;",
"}",
"var /** !Generator<?> */ g = gen();"));

// Inferring yield type works on yield* as well
typeCheck(
LINE_JOINER.join(
"function* gen() {",
" yield* 'abc';",
"}",
"var /** !Generator<string> */ g = gen();"));

typeCheck(
LINE_JOINER.join(
"function* gen() {",
" yield* 'abc';",
" yield 1;",
"}",
"var /** !Generator<string|number> */ g = gen();"));

typeCheck(
LINE_JOINER.join(
"function* gen() {",
" yield* [1,2,3];",
" yield* 'abc';",
"}",
"var /** !Generator<number|string> */ g = gen();"));

// No function body
typeCheck(
LINE_JOINER.join(
"function* gen() {}",
"var /** !Generator<?> */ g = gen();"));
}
}

0 comments on commit 2165d3a

Please sign in to comment.