Skip to content

Commit

Permalink
[NTI] Keep the declared type of a VAR even if the initializer has a m…
Browse files Browse the repository at this point in the history 
…ore precise type.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=160156195
  • Loading branch information
@dimvar @brad4d
dimvar authored and brad4d committed Jun 26, 2017
1 parent 7db7728 commit 5a170e4
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 51 deletions.
66 changes: 36 additions & 30 deletions src/com/google/javascript/jscomp/NewTypeInference.java
View file
Expand Up @@ -16,6 +16,7 @@

package com.google.javascript.jscomp;

import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
Expand Down Expand Up @@ -915,7 +916,7 @@ private void analyzeFunctionBwd(
inEnv = outEnv;
} else {
JSType declRetType = this.currentScope.getDeclaredFunctionType().getReturnType();
declRetType = declRetType == null ? UNKNOWN : declRetType;
declRetType = firstNonNull(declRetType, UNKNOWN);
inEnv = analyzeExprBwd(retExp, outEnv, declRetType).env;
}
break;
Expand Down Expand Up @@ -1083,9 +1084,8 @@ n, RETURN_NONDECLARED_TYPE, errorMsgWithTypeDiff(declRetType, actualRetType)),
if (NodeUtil.isTypedefDecl(n)) {
break;
}
for (Node nameNode = n.getFirstChild(); nameNode != null;
nameNode = nameNode.getNext()) {
outEnv = processVarDeclaration(nameNode, outEnv);
for (Node nameNode : n.children()) {
outEnv = processVarDeclFwd(nameNode, outEnv);
}
break;
case SWITCH:
Expand Down Expand Up @@ -1369,8 +1369,11 @@ private static boolean hasPathWithNoReturn(ControlFlowGraph<Node> cfg) {
return false;
}

/** Processes a single variable declaration in a VAR statement. */
private TypeEnv processVarDeclaration(Node nameNode, TypeEnv inEnv) {
/**
* This method processes a single variable declaration in a VAR statement, in the forward
* phase of the analysis.
*/
private TypeEnv processVarDeclFwd(Node nameNode, TypeEnv inEnv) {
String varName = nameNode.getString();
JSType declType = this.currentScope.getDeclaredTypeOf(varName);

Expand All @@ -1387,31 +1390,35 @@ private TypeEnv processVarDeclaration(Node nameNode, TypeEnv inEnv) {
maybeSetTypeI(nameNode, declType);
return envPutType(inEnv, varName, declType);
}

TypeEnv outEnv = inEnv;
JSType rhsType;
if (rhs == null) {
rhsType = UNDEFINED;
} else {
EnvTypePair pair = analyzeExprFwd(rhs, inEnv, declType != null ? declType : UNKNOWN);
JSType rhsType = null;
if (rhs != null) {
EnvTypePair pair = analyzeExprFwd(rhs, inEnv, firstNonNull(declType, UNKNOWN));
outEnv = pair.env;
rhsType = pair.type;
if (declType != null) {
if (rhsType.isSubtypeOf(declType)) {
registerImplicitUses(rhs, rhsType, declType);
} else {
registerMismatchAndWarn(
JSError.make(rhs, MISTYPED_ASSIGN_RHS, errorMsgWithTypeDiff(declType, rhsType)),
rhsType, declType);
}
}
}
if (declType != null && rhs != null) {
if (!rhsType.isSubtypeOf(declType)) {
registerMismatchAndWarn(
JSError.make(rhs, MISTYPED_ASSIGN_RHS, errorMsgWithTypeDiff(declType, rhsType)),
rhsType, declType);
// Don't flow the wrong initialization
rhsType = declType;
} else {
registerImplicitUses(rhs, rhsType, declType);
// We do this to preserve type attributes like declaredness of
// a property
rhsType = declType.specialize(rhsType);
JSType varType = rhsType;
if (rhs == null) {
varType = UNDEFINED;
} else if (declType != null) {
// If the declared type comes from a @const that was inferred during GTI, don't use here.
JSDocInfo jsdoc = NodeUtil.getBestJSDocInfo(nameNode);
if (jsdoc != null && (!jsdoc.hasConstAnnotation() || jsdoc.hasType())) {
varType = declType;
}
}
maybeSetTypeI(nameNode, rhsType);
return envPutType(outEnv, varName, rhsType);
maybeSetTypeI(nameNode, varType);
return envPutType(outEnv, varName, varType);
}

private EnvTypePair analyzeExprFwd(Node expr, TypeEnv inEnv) {
Expand Down Expand Up @@ -3282,7 +3289,7 @@ private boolean mayWarnAboutBadIObjectIndex(Node n, JSType iobjectType,
// you want to pass around the result type and it has to be non-null.
private JSType getIndexedTypeOrUnknown(JSType t) {
JSType tmp = t.getIndexedType();
return tmp == null ? UNKNOWN : tmp;
return firstNonNull(tmp, UNKNOWN);
}

private EnvTypePair analyzePropAccessFwd(Node receiver, String pname,
Expand Down Expand Up @@ -3350,7 +3357,7 @@ private EnvTypePair analyzePropAccessFwd(Node receiver, String pname,
if (this.convention.isSuperClassReference(pname)) {
if (ft != null && ft.isUniqueConstructor()) {
JSType result = ft.getSuperPrototype();
pair.type = result != null ? result : UNDEFINED;
pair.type = firstNonNull(result, UNDEFINED);
return pair;
}
}
Expand Down Expand Up @@ -3727,7 +3734,7 @@ private EnvTypePair analyzeNameBwd(
// unintuitive warnings.
// It's a trade-off.
JSType declType = this.currentScope.getDeclaredTypeOf(varName);
preciseType = declType == null ? requiredType : declType;
preciseType = firstNonNull(declType, requiredType);
}
return EnvTypePair.addBinding(outEnv, varName, preciseType);
}
Expand Down Expand Up @@ -4453,8 +4460,7 @@ private LValueResultBwd analyzeLValueBwd(Node expr, TypeEnv outEnv,
String name = expr.getQualifiedName();
JSType declType = this.currentScope.getDeclaredTypeOf(name);
if (doSlicing) {
pair.env = envPutType(pair.env, name,
declType != null ? declType : UNKNOWN);
pair.env = envPutType(pair.env, name, firstNonNull(declType, UNKNOWN));
}
return new LValueResultBwd(pair.env, pair.type,
pair.type.hasNonScalar() ? new QualifiedName(name) : null);
Expand Down
6 changes: 0 additions & 6 deletions test/com/google/javascript/jscomp/DisambiguatePropertiesTest.java
View file
Expand Up @@ -399,13 +399,7 @@ public void testUnionType_1() {
+ "B.a = 0;\n"
+ "/** @constructor */ function Baz() {}\n"
+ "Baz.prototype.a = 0;\n";

this.mode = TypeInferenceMode.OTI_ONLY;
testSets(js, "{a=[[Bar.prototype, Foo.prototype], [Baz.prototype]]}");

// NTI knows that B has type Bar
this.mode = TypeInferenceMode.NTI_ONLY;
testSets(js, "{a=[[Bar.prototype], [Baz.prototype], [Foo.prototype]]}");
}

public void testUnionType_2() {
Expand Down
35 changes: 20 additions & 15 deletions test/com/google/javascript/jscomp/NewTypeInferenceTest.java
View file
Expand Up @@ -2433,10 +2433,21 @@ public void testSimpleObjectLiterals() {
NewTypeInference.INVALID_ARGUMENT_TYPE);
}

public void testInferPreciseTypeWithDeclaredUnknown() {
public void testDeclaredVersusInferredTypeForVar() {
typeCheck("var /** ? */ x = 'str'; x - 123;");

typeCheck(
"var /** ? */ x = 'str'; x - 123;",
"var /** ? */ x; x - 123;",
NewTypeInference.INVALID_OPERAND_TYPE);

typeCheck(LINE_JOINER.join(
"function f(/** number|undefined */ x) {",
" if (x !== undefined) {",
" /** @const */",
" var c = x;",
" return c - 5;",
" }",
"}"));
}

public void testSimpleLooseObjects() {
Expand Down Expand Up @@ -7502,9 +7513,9 @@ public void testGenericClassInstantiation() {
"function Foo() {}",
"/** @param {T} x */",
"Foo.prototype.method = function(x) {};",
"var /** @type {Foo<string>} */ foo = null;",
"var /** @type {?Foo<string>} */ foo = null;",
"foo.method('asdf');"),
NewTypeInference.PROPERTY_ACCESS_ON_NONOBJECT);
NewTypeInference.NULLABLE_DEREFERENCE);
}

public void testLooserCheckingForInferredProperties() {
Expand Down Expand Up @@ -8272,7 +8283,7 @@ public void testSpecializedInstanceofCantGoToBottom() {

public void testDeclaredGenericArrayTypes() {
typeCheck(LINE_JOINER.join(
"/** @type {Array<string>} */",
"/** @type {!Array<string>} */",
"var arr = ['str'];",
"arr[0]++;"),
NewTypeInference.INVALID_OPERAND_TYPE);
Expand Down Expand Up @@ -8308,13 +8319,7 @@ public void testDeclaredGenericArrayTypes() {

// We warn here even though the declared type of the lvalue includes null.
typeCheck(LINE_JOINER.join(
"/** @type {Array<number>} */",
"var arr = [1, 2, 3];",
"arr[0] = 'str';"),
NewTypeInference.MISTYPED_ASSIGN_RHS);

typeCheck(LINE_JOINER.join(
"function f(/** Array<number> */ arr) {",
"function f(/** ?Array<number> */ arr) {",
" arr[0] = 'str';",
"}"),
NewTypeInference.NULLABLE_DEREFERENCE,
Expand All @@ -8337,7 +8342,7 @@ public void testDeclaredGenericArrayTypes() {
"arr[0] = new Super;"));

typeCheck(LINE_JOINER.join(
"/** @type {Array<number>} */ var arr = [];",
"/** @type {!Array<number>} */ var arr = [];",
"arr[0] = 'str';"),
NewTypeInference.MISTYPED_ASSIGN_RHS);

Expand Down Expand Up @@ -16879,7 +16884,7 @@ public void testIObjectBracketAccesses() {
"}"));

typeCheck(LINE_JOINER.join(
"/** @type {!Array<number>|number} */",
"/** @type {!Array<number>} */",
"var x = [1,2,3];",
"x[0] = 'asdf';"),
NewTypeInference.MISTYPED_ASSIGN_RHS);
Expand Down Expand Up @@ -18279,7 +18284,7 @@ public void testRandomPropDefsDontShadowConstDefinition() {
" * @extends {Bar}",
" */",
"function Foo() {};",
"/** @type {Foo} */",
"/** @type {!Foo} */",
"var foo = new Foo();",
"/** @type {number} */",
"foo.CONST = 0;"),
Expand Down

0 comments on commit 5a170e4

Please sign in to comment.