Skip to content

Commit

Permalink
Preserve type information in Es6RewriteGenerators
Browse files Browse the repository at this point in the history 
Tested through the AstValidator's type info validation feature and through checking the actual type on a subset of the unit tests in Es6RewriteGenerators.

This moves typechecking in Es6RewriteGenerators before transpilation, and also adds an option to inject the required polyfills (base.js and es6/generator_engine.js) before typechecking in the unit tests.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=191312444
  • Loading branch information
@lauraharker
lauraharker committed Apr 3, 2018
1 parent 5b8de41 commit b904af7
Show file tree
Hide file tree
Showing 4 changed files with 421 additions and 55 deletions.
191 changes: 148 additions & 43 deletions src/com/google/javascript/jscomp/Es6RewriteGenerators.java
View file
Expand Up @@ -17,8 +17,10 @@

import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
import static com.google.javascript.jscomp.Es6ToEs3Util.withType;

import com.google.common.collect.Iterables;
import com.google.javascript.jscomp.AbstractCompiler.MostRecentTypechecker;
import com.google.javascript.jscomp.ControlFlowGraph.Branch;
import com.google.javascript.jscomp.graph.DiGraph.DiGraphEdge;
import com.google.javascript.jscomp.parsing.parser.FeatureSet;
Expand All @@ -28,6 +30,11 @@
import com.google.javascript.rhino.JSDocInfoBuilder;
import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.Token;
import com.google.javascript.rhino.jstype.FunctionType;
import com.google.javascript.rhino.jstype.JSType;
import com.google.javascript.rhino.jstype.JSTypeNative;
import com.google.javascript.rhino.jstype.JSTypeRegistry;
import com.google.javascript.rhino.jstype.ObjectType;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.HashMap;
Expand Down Expand Up @@ -77,9 +84,38 @@ final class Es6RewriteGenerators implements HotSwapCompilerPass {

private final AbstractCompiler compiler;

private final JSTypeRegistry registry;

private final boolean shouldAddTypes;

private final JSType unknownType;
private final JSType numberType;
private final JSType booleanType;
private final JSType nullType;
private final JSType nullableStringType;

Es6RewriteGenerators(AbstractCompiler compiler) {
checkNotNull(compiler);
this.compiler = compiler;
registry = compiler.getTypeRegistry();

if (compiler.getMostRecentTypechecker() == MostRecentTypechecker.OTI) {
// typechecking has run, so we must preserve and propagate type information
shouldAddTypes = true;
unknownType = registry.getNativeType(JSTypeNative.UNKNOWN_TYPE);
numberType = registry.getNativeType(JSTypeNative.NUMBER_TYPE);
booleanType = registry.getNativeType(JSTypeNative.BOOLEAN_TYPE);
nullType = registry.getNativeType(JSTypeNative.NULL_TYPE);
nullableStringType =
registry.createNullableType(registry.getNativeType(JSTypeNative.STRING_TYPE));
} else {
shouldAddTypes = false;
unknownType = null;
numberType = null;
booleanType = null;
nullType = null;
nullableStringType = null;
}
}

@Override
Expand Down Expand Up @@ -206,10 +242,35 @@ private class SingleGeneratorFunctionTranspiler {
/** The body of a replacement function. */
Node newGeneratorBody;

/** The original inferred return type of the Generator */
JSType originalGenReturnType;

SingleGeneratorFunctionTranspiler(Node genFunc, int genaratorNestingLevel) {
this.generatorNestingLevel = genaratorNestingLevel;
this.originalGeneratorBody = genFunc.getLastChild();
this.context = new TranspilationContext();
ObjectType contextType = null;
if (shouldAddTypes) {
// Find the yield type of the generator.
// e.g. given @return {!Generator<number>}, we want this.yieldType to be number.
JSType yieldType = unknownType;
if (genFunc.getJSType() != null && genFunc.getJSType().isFunctionType()) {
FunctionType fnType = genFunc.getJSType().toMaybeFunctionType();
this.originalGenReturnType = fnType.getReturnType();
yieldType = TypeCheck.getTemplateTypeOfGenerator(registry, this.originalGenReturnType);
}

JSType globalContextType = registry.getGlobalType("$jscomp.generator.Context");
if (globalContextType == null) {
// We don't have the es6/generator polyfill, which can happen in tests using a
// NonInjectingCompiler or if someone sets --inject_libraries=false. Don't crash, just
// back off on giving some type information.
contextType = registry.getNativeObjectType(JSTypeNative.OBJECT_TYPE);
} else {
contextType =
registry.createTemplatizedType(globalContextType.toMaybeObjectType(), yieldType);
}
}
this.context = new TranspilationContext(contextType);
}

public void transpile() {
Expand All @@ -233,8 +294,7 @@ public void transpile() {

// switch ($jscomp$generator$context.nextAddress) {
// }
Node switchNode =
IR.switchNode(IR.getprop(context.getJsContextNameNode(genFunc), "nextAddress"));
Node switchNode = IR.switchNode(context.getContextField(genFunc, "nextAddress"));

// Prepare a "program" function:
// function ($jscomp$generator$context) {
Expand All @@ -250,9 +310,8 @@ public void transpile() {
// once, which messes up its understanding of the types assigned to variables
// within it.
IR.doNode( // TODO(skill): Remove do-while loop when this pass moves after
// type checking or when OTI is fixed to handle this correctly.
IR.block(switchNode),
IR.number(0))));
// type checking or when OTI is fixed to handle this correctly.
IR.block(switchNode), withType(IR.number(0), numberType))));

// Propagate all suppressions from original generator function to a new "program" function.
JSDocInfoBuilder jsDocBuilder = new JSDocInfoBuilder(false);
Expand All @@ -269,13 +328,23 @@ public void transpile() {

// Replace original generator function body with:
// return $jscomp.generator.createGenerator(<origGenerator>, <program function>);
Node createGenerator =
IR.getprop(
withType(
IR.getprop(withType(IR.name("$jscomp"), unknownType), "generator"), unknownType),
"createGenerator");
newGeneratorBody =
IR.block(
IR.returnNode(
IR.call(
IR.getprop(IR.name("$jscomp"), "generator", "createGenerator"),
genFuncName.cloneNode(),
program)).useSourceInfoFromForTree(originalGeneratorBody));
IR.returnNode(
withType(
IR.call(createGenerator, genFuncName.cloneNode(), program),
this.originalGenReturnType))
.useSourceInfoFromForTree(originalGeneratorBody));

if (shouldAddTypes) {
createGenerator.setJSType(registry.createFunctionType(originalGenReturnType));
program.setJSType(registry.createFunctionType(unknownType));
}

// Newly introduced functions have to be reported immediately.
compiler.reportChangeToChangeScope(program);
Expand Down Expand Up @@ -612,7 +681,7 @@ void transpileIf(Node n, @Nullable TranspilationContext.Case breakCase) {

// Only "else" block is unmarked, swap "if" and "else" blocks and negate the condition.
if (ifBlock.isGeneratorMarker() && !elseBlock.isGeneratorMarker()) {
condition = IR.not(condition).useSourceInfoFrom(condition);
condition = withType(IR.not(condition), booleanType).useSourceInfoFrom(condition);
Node tmpNode = ifBlock;
ifBlock = elseBlock;
elseBlock = tmpNode;
Expand Down Expand Up @@ -689,8 +758,12 @@ void transpileFor(
condition = prepareNodeForWrite(maybeDecomposeExpression(condition.detach()));
context.writeGeneratedNode(
IR.ifNode(
IR.not(condition).useSourceInfoFrom(condition),
context.createJumpToBlock(endCase, /** allowEmbedding=*/ true, n))
withType(IR.not(condition), booleanType).useSourceInfoFrom(condition),
context.createJumpToBlock(
endCase,
/** allowEmbedding= */
true,
n))
.useSourceInfoFrom(n));
}

Expand Down Expand Up @@ -753,24 +826,33 @@ void transpileForIn(
.useSourceInfoFrom(target);
// "$context.forIn(x)"
forIn.addChildToFront(context.callContextMethod(target, "forIn", detachedCond));
ObjectType propertyIteratorType =
shouldAddTypes ? forIn.getFirstChild().getJSType().toMaybeObjectType() : null;
forIn.setJSType(propertyIteratorType);
// "var ..., $for$in = $context.forIn(expr)"
init.addChildToBack(forIn);

// "(i = $for$in.getNext()) != null"
Node forInGetNext =
withType(
IR.getprop(
forIn.cloneNode(), IR.string("getNext").useSourceInfoFrom(detachedCond)),
shouldAddTypes ? propertyIteratorType.getPropertyType("getNext") : null)
.useSourceInfoFrom(detachedCond);

Node forCond =
IR.ne(
IR.assign(
target,
IR.call(
IR.getprop(
forIn.cloneNode(),
IR.string("getNext").useSourceInfoFrom(detachedCond))
.useSourceInfoFrom(detachedCond))
.useSourceInfoFrom(detachedCond))
.useSourceInfoFrom(detachedCond),
IR.nullNode().useSourceInfoFrom(forIn))
.useSourceInfoFrom(detachedCond);
IR.assign(
withType(target, nullableStringType),
withType(IR.call(forInGetNext), nullableStringType)
.useSourceInfoFrom(detachedCond))
.useSourceInfoFrom(detachedCond),
withType(IR.nullNode(), nullType).useSourceInfoFrom(forIn))
.useSourceInfoFrom(detachedCond);
forCond.setGeneratorMarker(target.isGeneratorMarker());
// annotate types
forCond.setJSType(booleanType);
forCond.getFirstChild().setJSType(nullableStringType);

// Prepare "for" statement.
// "for (var i, $for$in = $context.forIn(expr); (i = $for$in.getNext()) != null; ) {}"
Expand All @@ -797,8 +879,12 @@ void transpileWhile(
Node body = n.removeFirstChild();
context.writeGeneratedNode(
IR.ifNode(
IR.not(condition).useSourceInfoFrom(condition),
context.createJumpToBlock(endCase, /** allowEmbedding=*/ true, n))
withType(IR.not(condition), booleanType).useSourceInfoFrom(condition),
context.createJumpToBlock(
endCase,
/** allowEmbedding= */
true,
n))
.useSourceInfoFrom(n));

// Transpile "while" body
Expand Down Expand Up @@ -1046,12 +1132,16 @@ private class TranspilationContext {
boolean thisReferenceFound;
boolean argumentsReferenceFound;

TranspilationContext() {
/** The JSType for this context. May be null. */
final ObjectType contextType;

TranspilationContext(ObjectType contextType) {
programEndCase = new Case();
checkState(programEndCase.id == 0);
currentCase = new Case();
checkState(currentCase.id == 1);
allCases.add(currentCase);
this.contextType = contextType;
}

/**
Expand Down Expand Up @@ -1241,7 +1331,8 @@ Case maybeCreateCase(@Nullable Case other) {

/** Returns the name node of context parameter passed to the program. */
Node getJsContextNameNode(Node sourceNode) {
return getScopedName(GENERATOR_CONTEXT).useSourceInfoFrom(sourceNode);
return withType(
getScopedName(GENERATOR_CONTEXT).useSourceInfoFrom(sourceNode), this.contextType);
}

/** Returns unique name in the current context. */
Expand All @@ -1251,15 +1342,25 @@ Node getScopedName(String name) {

/** Creates node that access a specified field of the current context. */
Node getContextField(Node sourceNode, String fieldName) {
return IR.getprop(
getJsContextNameNode(sourceNode),
IR.string(fieldName).useSourceInfoFrom(sourceNode))
return withType(
IR.getprop(
getJsContextNameNode(sourceNode),
IR.string(fieldName).useSourceInfoFrom(sourceNode)),
shouldAddTypes ? this.contextType.getPropertyType(fieldName) : null)
.useSourceInfoFrom(sourceNode);
}

/** Creates node that make a call to a context function. */
Node callContextMethod(Node sourceNode, String methodName, Node... args) {
return IR.call(getContextField(sourceNode, methodName), args).useSourceInfoFrom(sourceNode);
Node contextField = getContextField(sourceNode, methodName);
Node callNode = IR.call(contextField, args).useSourceInfoFrom(sourceNode);
if (shouldAddTypes) {
callNode.setJSType(
contextField.getJSType().isFunctionType()
? contextField.getJSType().toMaybeFunctionType().getReturnType()
: unknownType);
}
return callNode;
}

/** Creates node that make a call to a context function. */
Expand Down Expand Up @@ -1479,12 +1580,14 @@ void enterCatchBlock(@Nullable Case finallyCase, Node exceptionName) {
args.add(nextCatchCase.getNumber(exceptionName));
}

Node enterCatchBlockCall =
callContextMethod(exceptionName, "enterCatchBlock", args.toArray(new Node[0]));
exceptionName.setJSType(enterCatchBlockCall.getJSType());
writeGeneratedNode(
IR.exprResult(
IR.assign(
exceptionName,
callContextMethod(
exceptionName, "enterCatchBlock", args.toArray(new Node[0])))
withType(
IR.assign(exceptionName, enterCatchBlockCall),
enterCatchBlockCall.getJSType())
.useSourceInfoFrom(exceptionName))
.useSourceInfoFrom(exceptionName));
}
Expand All @@ -1510,7 +1613,7 @@ void enterFinallyBlock(
if (nextCatchCase != null || nextFinallyCase != null) {
args.add(
nextCatchCase == null
? IR.number(0).useSourceInfoFrom(sourceNode)
? withType(IR.number(0), numberType).useSourceInfoFrom(sourceNode)
: nextCatchCase.getNumber(sourceNode));
if (nextFinallyCase != null) {
args.add(nextFinallyCase.getNumber(sourceNode));
Expand All @@ -1519,11 +1622,11 @@ void enterFinallyBlock(
} else {
args.add(
nextCatchCase == null
? IR.number(0).useSourceInfoFrom(sourceNode)
? withType(IR.number(0), numberType).useSourceInfoFrom(sourceNode)
: nextCatchCase.getNumber(sourceNode));
args.add(
nextFinallyCase == null
? IR.number(0).useSourceInfoFrom(sourceNode)
? withType(IR.number(0), numberType).useSourceInfoFrom(sourceNode)
: nextFinallyCase.getNumber(sourceNode));
args.add(IR.number(nestedFinallyBlockCount).useSourceInfoFrom(sourceNode));
}
Expand Down Expand Up @@ -1649,7 +1752,8 @@ private class Case {
}

Node createCaseNode() {
return IR.caseNode(IR.number(id).useSourceInfoFrom(caseBlock), caseBlock)
return IR.caseNode(
withType(IR.number(id), numberType).useSourceInfoFrom(caseBlock), caseBlock)
.useSourceInfoFrom(caseBlock);
}

Expand All @@ -1658,7 +1762,7 @@ Node getNumber(Node sourceNode) {
if (jumpTo != null) {
return jumpTo.getNumber(sourceNode);
}
Node node = IR.number(id).useSourceInfoFrom(sourceNode);
Node node = withType(IR.number(id).useSourceInfoFrom(sourceNode), numberType);
references.add(node);
return node;
}
Expand Down Expand Up @@ -1904,7 +2008,8 @@ void visitVar(Node varStatement) {
commaExpression =
commaExpression == null
? assignment
: IR.comma(commaExpression, assignment).useSourceInfoFrom(assignment);
: withType(IR.comma(commaExpression, assignment), assignment.getJSType())
.useSourceInfoFrom(assignment);
}
varStatement.replaceWith(IR.exprResult(commaExpression));
}
Expand Down
10 changes: 7 additions & 3 deletions src/com/google/javascript/jscomp/TypeCheck.java
View file
Expand Up @@ -2161,13 +2161,17 @@ private void visitYield(NodeTraversal t, Node n) {
"Yielded type does not match declared return type.");
}

private JSType getTemplateTypeOfGenerator(JSType generator) {
return getTemplateTypeOfGenerator(typeRegistry, generator);
}

/**
* Returns the given type's resolved template type corresponding to the corresponding to the
* Generator, Iterable or Iterator template key if possible.
*
* If the given type is not an Iterator or Iterable, returns the unknown type..
* <p>If the given type is not an Iterator or Iterable, returns the unknown type..
*/
private JSType getTemplateTypeOfGenerator(JSType generator) {
static JSType getTemplateTypeOfGenerator(JSTypeRegistry typeRegistry, JSType generator) {
ObjectType dereferencedType = generator.dereference();
if (dereferencedType != null) {
TemplateTypeMap templateTypeMap = dereferencedType.getTemplateTypeMap();
Expand All @@ -2183,7 +2187,7 @@ private JSType getTemplateTypeOfGenerator(JSType generator) {
return templateTypeMap.getResolvedTemplateType(typeRegistry.getIteratorTemplate());
}
}
return getNativeType(UNKNOWN_TYPE);
return typeRegistry.getNativeType(UNKNOWN_TYPE);
}

/**
Expand Down

0 comments on commit b904af7

Please sign in to comment.