Skip to content

Commit

Permalink
[NTI] Use constructor types during const inference, but don't leak th…
Browse files Browse the repository at this point in the history 
…em to the

final result.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=144897236
  • Loading branch information
@dimvar @blickly
dimvar authored and blickly committed Jan 19, 2017
1 parent de36c96 commit ff29ddb
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 17 deletions.
61 changes: 44 additions & 17 deletions src/com/google/javascript/jscomp/GlobalTypeInfo.java
View file
Expand Up @@ -289,6 +289,10 @@ class GlobalTypeInfo implements CompilerPass, TypeIRegistry {
// Uses %, which is not allowed in identifiers, to avoid naming clashes
// with existing functions.
private static final String ANON_FUN_PREFIX = "%anon_fun";
// A property of this name is used as a marker during const inference,
// to avoid misuse of constructor types.
private static final QualifiedName CONST_INFERENCE_MARKER =
new QualifiedName("jscomp$infer$const$property");
private static final String WINDOW_INSTANCE = "window";
private static final String WINDOW_CLASS = "Window";
private DefaultNameGenerator funNameGen;
Expand Down Expand Up @@ -1995,7 +1999,7 @@ private FunctionType simpleInferFunctionType(Node n) {
if (n.isQualifiedName()) {
Declaration decl = currentScope.getDeclaration(QualifiedName.fromNode(n), false);
if (decl == null) {
JSType t = simpleInferExprType(n);
JSType t = simpleInferExprTypeRecur(n);
if (t != null) {
return t.getFunTypeIfSingletonObj();
}
Expand All @@ -2017,7 +2021,7 @@ private FunctionType simpleInferFunctionType(Node n) {
return decl.getTypeOfSimpleDecl().getFunTypeIfSingletonObj();
}
}
JSType t = simpleInferExprType(n);
JSType t = simpleInferExprTypeRecur(n);
return t == null ? null : t.getFunTypeIfSingletonObj();
}

Expand All @@ -2037,7 +2041,7 @@ private JSType simpleInferCallNewType(Node n) {
for (Node argNode = n.getSecondChild();
argNode != null;
argNode = argNode.getNext()) {
JSType t = simpleInferExprType(argNode);
JSType t = simpleInferExprTypeRecur(argNode);
if (t == null) {
return null;
}
Expand All @@ -2053,6 +2057,18 @@ private JSType simpleInferCallNewType(Node n) {
}

private JSType simpleInferExprType(Node n) {
JSType t = simpleInferExprTypeRecur(n);
// If the inferred type has the marker property, discard it.
// Note that when the marker is nested somewhere in the type, this heuristic breaks,
// and the marker leaks into the result.
// Hopefully this is rare in practice, but I'm not sure; try it out.
if (t == null || t.mayHaveProp(CONST_INFERENCE_MARKER)) {
return null;
}
return t;
}

private JSType simpleInferExprTypeRecur(Node n) {
switch (n.getToken()) {
case REGEXP:
return commonTypes.getRegexpType();
Expand All @@ -2063,12 +2079,12 @@ private JSType simpleInferExprType(Node n) {
return commonTypes.getArrayInstance();
}
Node child = n.getFirstChild();
JSType arrayType = simpleInferExprType(child);
JSType arrayType = simpleInferExprTypeRecur(child);
if (arrayType == null) {
return null;
}
while (null != (child = child.getNext())) {
if (!arrayType.equals(simpleInferExprType(child))) {
if (!arrayType.equals(simpleInferExprTypeRecur(child))) {
return null;
}
}
Expand All @@ -2086,7 +2102,7 @@ private JSType simpleInferExprType(Node n) {
case OBJECTLIT: {
JSType objLitType = commonTypes.getEmptyObjectLiteral();
for (Node prop : n.children()) {
JSType propType = simpleInferExprType(prop.getFirstChild());
JSType propType = simpleInferExprTypeRecur(prop.getFirstChild());
if (propType == null) {
return null;
}
Expand All @@ -2103,16 +2119,16 @@ private JSType simpleInferExprType(Node n) {
return simpleInferGetelemType(n);
case COMMA:
case ASSIGN:
return simpleInferExprType(n.getLastChild());
return simpleInferExprTypeRecur(n.getLastChild());
case CALL:
case NEW:
return simpleInferCallNewType(n);
case AND:
case OR:
return simpleInferAndOrType(n);
case HOOK: {
JSType lhs = simpleInferExprType(n.getSecondChild());
JSType rhs = simpleInferExprType(n.getLastChild());
JSType lhs = simpleInferExprTypeRecur(n.getSecondChild());
JSType rhs = simpleInferExprTypeRecur(n.getLastChild());
return lhs == null || rhs == null ? null : JSType.join(lhs, rhs);
}
default:
Expand Down Expand Up @@ -2164,7 +2180,7 @@ private JSType simpleInferPropAccessType(Node recv, String pname) {
}
}
}
JSType recvType = simpleInferExprType(recv);
JSType recvType = simpleInferExprTypeRecur(recv);
if (recvType != null && recvType.isScalar()) {
recvType = recvType.autobox();
}
Expand All @@ -2186,11 +2202,11 @@ private JSType simpleInferGetelemType(Node n) {
return propType;
}
}
JSType recvType = simpleInferExprType(recv);
JSType recvType = simpleInferExprTypeRecur(recv);
if (recvType != null) {
JSType indexType = recvType.getIndexType();
if (indexType != null) {
JSType propType = simpleInferExprType(propNode);
JSType propType = simpleInferExprTypeRecur(propNode);
if (propType != null && propType.isSubtypeOf(indexType)) {
return recvType.getIndexedType();
}
Expand All @@ -2203,9 +2219,20 @@ private JSType simpleInferDeclaration(Declaration decl) {
if (decl == null) {
return null;
}
// Namespaces (literals, enums, constructors) get populated during
// ProcessScope, so it's generally NOT safe to convert them to jstypes
// until after ProcessScope is done.
// Namespaces (literals, enums, constructors) get populated during ProcessScope,
// so it's generally NOT safe to convert them to jstypes until after ProcessScope is done.
// However, we've seen examples where it is useful to use the constructor type
// during inference, e.g., to get the type of the instance from it.
// We allow this use case but add a marker property to make sure that the constructor type
// itself doesn't leak into the result.
if (decl.getNominal() != null) {
FunctionType ctorFn = decl.getNominal().getConstructorFunction();
if (ctorFn == null) {
return null;
}
return commonTypes.fromFunctionType(ctorFn)
.withProperty(CONST_INFERENCE_MARKER, commonTypes.UNKNOWN);
}
if (decl.getNamespace() != null) {
return null;
}
Expand All @@ -2225,11 +2252,11 @@ private JSType simpleInferDeclaration(Declaration decl) {

private JSType simpleInferAndOrType(Node n) {
Preconditions.checkState(n.isOr() || n.isAnd());
JSType lhs = simpleInferExprType(n.getFirstChild());
JSType lhs = simpleInferExprTypeRecur(n.getFirstChild());
if (lhs == null) {
return null;
}
JSType rhs = simpleInferExprType(n.getSecondChild());
JSType rhs = simpleInferExprTypeRecur(n.getSecondChild());
if (rhs == null) {
return null;
}
Expand Down
61 changes: 61 additions & 0 deletions test/com/google/javascript/jscomp/NewTypeInferenceTest.java
View file
Expand Up @@ -18792,4 +18792,65 @@ public void testParameterizedObject() {
"}"),
NewTypeInference.MISTYPED_ASSIGN_RHS);
}

public void testUseConstructorTypeInConstInference() {
typeCheck(LINE_JOINER.join(
"/** @constructor */",
"function Foo() {}",
"/**",
" * @template T",
" * @param {function(new:T)} x",
" * @return {T}",
" */",
"function appContextGet(x) {",
" return new x;",
"}",
"/** @const */",
"var c = appContextGet(Foo);",
"function f() {",
" var /** null */ n = c;",
"}"),
NewTypeInference.MISTYPED_ASSIGN_RHS);

typeCheck(LINE_JOINER.join(
"/** @constructor */",
"function Foo() {}",
"/**",
" * @template T",
" * @param {T} x",
" * @return {T}",
" */",
"function appContextGet(x) {",
" return x;",
"}",
"/** @const */",
"var c = appContextGet(Foo);",
"function f() {",
" var /** null */ n = c;",
"}"),
GlobalTypeInfo.COULD_NOT_INFER_CONST_TYPE);

// Test to show that in convoluted cases the constructor type leaks
// into the const inference result even though it shouldn't.
// Correct behavior would be a COULD_NOT_INFER_CONST_TYPE warning.
// Currently, the const is inferred to a type that includes the marker property,
// which then is not present during NTI and we get a spurious warning.
typeCheck(LINE_JOINER.join(
"/** @constructor */",
"function Foo() {}",
"/**",
" * @template T",
" * @param {T} x",
" * @return {T}",
" */",
"function appContextGet(x) {",
" return x;",
"}",
"/** @const */",
"var c = 0 || { myprop: appContextGet(Foo) };",
"function f() {",
" return c;",
"}"),
NewTypeInference.MISTYPED_ASSIGN_RHS);
}
}

0 comments on commit ff29ddb

Please sign in to comment.