Skip to content

Commit

Permalink
[NTI] When there is a type mismatch, show the part of the type causin…
Browse files Browse the repository at this point in the history
…g the mismatch when possible.

/////////////////////////////////////////////
Sample new messages:

Invalid type for parameter 1 of function f.
Expected : {a:number, b:string}
Found    : {a:number, b:number}
More details:
Incompatible types for property b.
Expected : string
Found    : number

Invalid type for parameter 1 of function f.
Expected : Foo
Found    : Bar|null
More details:
The found type is a union that includes an unexpected type: null

Invalid type for parameter 1 of function f.
Expected : function(number|string):?
Found    : function(number):undefined
More details:
The expected and found types are functions which have incompatible types for argument 1.
Expected a supertype of : number|string
but found               : number
/////////////////////////////////////////////
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=122292354
  • Loading branch information
dimvar authored and blickly committed May 13, 2016
1 parent 39c75dd commit 93d3403
Show file tree
Hide file tree
Showing 7 changed files with 551 additions and 65 deletions.
108 changes: 78 additions & 30 deletions src/com/google/javascript/jscomp/NewTypeInference.java
Expand Up @@ -31,6 +31,7 @@
import com.google.javascript.jscomp.newtypes.FunctionTypeBuilder; import com.google.javascript.jscomp.newtypes.FunctionTypeBuilder;
import com.google.javascript.jscomp.newtypes.JSType; import com.google.javascript.jscomp.newtypes.JSType;
import com.google.javascript.jscomp.newtypes.JSTypes; import com.google.javascript.jscomp.newtypes.JSTypes;
import com.google.javascript.jscomp.newtypes.MismatchInfo;
import com.google.javascript.jscomp.newtypes.QualifiedName; import com.google.javascript.jscomp.newtypes.QualifiedName;
import com.google.javascript.jscomp.newtypes.TypeEnv; import com.google.javascript.jscomp.newtypes.TypeEnv;
import com.google.javascript.jscomp.newtypes.UniqueNameGenerator; import com.google.javascript.jscomp.newtypes.UniqueNameGenerator;
Expand Down Expand Up @@ -69,33 +70,28 @@ final class NewTypeInference implements CompilerPass {
static final DiagnosticType MISTYPED_ASSIGN_RHS = DiagnosticType.warning( static final DiagnosticType MISTYPED_ASSIGN_RHS = DiagnosticType.warning(
"JSC_NTI_MISTYPED_ASSIGN_RHS", "JSC_NTI_MISTYPED_ASSIGN_RHS",
"The right side in the assignment is not a subtype of the left side.\n" "The right side in the assignment is not a subtype of the left side.\n"
+ "left side : {0}\n" + "{0}");
+ "right side : {1}\n");


static final DiagnosticType INVALID_OPERAND_TYPE = DiagnosticType.warning( static final DiagnosticType INVALID_OPERAND_TYPE = DiagnosticType.warning(
"JSC_NTI_INVALID_OPERAND_TYPE", "JSC_NTI_INVALID_OPERAND_TYPE",
"Invalid type(s) for operator {0}.\n" "Invalid type(s) for operator {0}.\n"
+ "expected : {1}\n" + "{1}");
+ "found : {2}\n");


static final DiagnosticType RETURN_NONDECLARED_TYPE = DiagnosticType.warning( static final DiagnosticType RETURN_NONDECLARED_TYPE = DiagnosticType.warning(
"JSC_NTI_RETURN_NONDECLARED_TYPE", "JSC_NTI_RETURN_NONDECLARED_TYPE",
"Returned type does not match declared return type.\n" + "Returned type does not match declared return type.\n"
"declared : {0}\n" + + "{0}");
"found : {1}\n");


static final DiagnosticType INVALID_INFERRED_RETURN_TYPE = static final DiagnosticType INVALID_INFERRED_RETURN_TYPE =
DiagnosticType.warning( DiagnosticType.warning(
"JSC_NTI_INVALID_INFERRED_RETURN_TYPE", "JSC_NTI_INVALID_INFERRED_RETURN_TYPE",
"Function called in context that expects incompatible type.\n" + "Function called in context that expects incompatible type.\n"
"expected : {0}\n" + + "{0}");
"found : {1}\n");


static final DiagnosticType INVALID_ARGUMENT_TYPE = DiagnosticType.warning( static final DiagnosticType INVALID_ARGUMENT_TYPE = DiagnosticType.warning(
"JSC_NTI_INVALID_ARGUMENT_TYPE", "JSC_NTI_INVALID_ARGUMENT_TYPE",
"Invalid type for parameter {0} of function {1}.\n" + "Invalid type for parameter {0} of function {1}.\n"
"expected : {2}\n" + + "{2}");
"found : {3}\n");


static final DiagnosticType CROSS_SCOPE_GOTCHA = DiagnosticType.warning( static final DiagnosticType CROSS_SCOPE_GOTCHA = DiagnosticType.warning(
"JSC_NTI_CROSS_SCOPE_GOTCHA", "JSC_NTI_CROSS_SCOPE_GOTCHA",
Expand Down Expand Up @@ -130,8 +126,7 @@ final class NewTypeInference implements CompilerPass {
DiagnosticType.warning( DiagnosticType.warning(
"JSC_NTI_INVALID_INDEX_TYPE", "JSC_NTI_INVALID_INDEX_TYPE",
"Invalid type for index.\n" "Invalid type for index.\n"
+ "expected : {0}\n" + "{0}");
+ "found : {1}\n");


static final DiagnosticType BOTTOM_INDEX_TYPE = static final DiagnosticType BOTTOM_INDEX_TYPE =
DiagnosticType.warning( DiagnosticType.warning(
Expand All @@ -142,7 +137,8 @@ final class NewTypeInference implements CompilerPass {
static final DiagnosticType INVALID_OBJLIT_PROPERTY_TYPE = static final DiagnosticType INVALID_OBJLIT_PROPERTY_TYPE =
DiagnosticType.warning( DiagnosticType.warning(
"JSC_NTI_INVALID_OBJLIT_PROPERTY_TYPE", "JSC_NTI_INVALID_OBJLIT_PROPERTY_TYPE",
"Object-literal property declared as {0} but has type {1}."); "Invalid type for object-literal property.\n"
+ "{0}");


static final DiagnosticType FORIN_EXPECTS_OBJECT = static final DiagnosticType FORIN_EXPECTS_OBJECT =
DiagnosticType.warning( DiagnosticType.warning(
Expand Down Expand Up @@ -182,8 +178,8 @@ final class NewTypeInference implements CompilerPass {
static final DiagnosticType INVALID_THIS_TYPE_IN_BIND = static final DiagnosticType INVALID_THIS_TYPE_IN_BIND =
DiagnosticType.warning( DiagnosticType.warning(
"JSC_NTI_INVALID_THIS_TYPE_IN_BIND", "JSC_NTI_INVALID_THIS_TYPE_IN_BIND",
"The first argument to bind has type {0} which is not a subtype of" "Invalid type for the first argument to bind.\n"
+ " {1}."); + "{0}");


static final DiagnosticType CANNOT_BIND_CTOR = static final DiagnosticType CANNOT_BIND_CTOR =
DiagnosticType.warning( DiagnosticType.warning(
Expand Down Expand Up @@ -245,7 +241,7 @@ final class NewTypeInference implements CompilerPass {
static final DiagnosticType NOT_CALLABLE = static final DiagnosticType NOT_CALLABLE =
DiagnosticType.warning( DiagnosticType.warning(
"JSC_NTI_NOT_FUNCTION_TYPE", "JSC_NTI_NOT_FUNCTION_TYPE",
"{0} expressions are not callable"); "Cannot call non-function type {0}");


static final DiagnosticType WRONG_ARGUMENT_COUNT = static final DiagnosticType WRONG_ARGUMENT_COUNT =
DiagnosticType.warning( DiagnosticType.warning(
Expand Down Expand Up @@ -854,7 +850,7 @@ private void analyzeFunctionFwd(
} }
if (!actualRetType.isSubtypeOf(declRetType)) { if (!actualRetType.isSubtypeOf(declRetType)) {
warnings.add(JSError.make(n, RETURN_NONDECLARED_TYPE, warnings.add(JSError.make(n, RETURN_NONDECLARED_TYPE,
declRetType.toString(), actualRetType.toString())); errorMsgWithTypeDiff(declRetType, actualRetType)));
} }
break; break;
} }
Expand Down Expand Up @@ -1154,7 +1150,7 @@ private TypeEnv processVarDeclaration(Node nameNode, TypeEnv inEnv) {
if (!rhsType.isSubtypeOf(declType)) { if (!rhsType.isSubtypeOf(declType)) {
warnings.add(JSError.make( warnings.add(JSError.make(
rhs, MISTYPED_ASSIGN_RHS, rhs, MISTYPED_ASSIGN_RHS,
declType.toString(), rhsType.toString())); errorMsgWithTypeDiff(declType, rhsType)));
// Don't flow the wrong initialization // Don't flow the wrong initialization
rhsType = declType; rhsType = declType;
} else { } else {
Expand Down Expand Up @@ -1926,7 +1922,7 @@ private EnvTypePair analyzeFunctionBindFwd(Node call, TypeEnv inEnv) {
env = pair.env; env = pair.env;
if (!pair.type.isSubtypeOf(reqThisType)) { if (!pair.type.isSubtypeOf(reqThisType)) {
warnings.add(JSError.make(call, INVALID_THIS_TYPE_IN_BIND, warnings.add(JSError.make(call, INVALID_THIS_TYPE_IN_BIND,
pair.type.toString(), reqThisType.toString())); errorMsgWithTypeDiff(reqThisType, pair.type)));
} }
} }


Expand Down Expand Up @@ -1967,7 +1963,7 @@ private TypeEnv analyzeCallNodeArgumentsFwd(Node call, Node firstArg,
String fnName = getReadableCalleeName(call.getFirstChild()); String fnName = getReadableCalleeName(call.getFirstChild());
warnings.add(JSError.make(arg, INVALID_ARGUMENT_TYPE, warnings.add(JSError.make(arg, INVALID_ARGUMENT_TYPE,
Integer.toString(i + 1), fnName, Integer.toString(i + 1), fnName,
formalType.toString(), pair.type.toString())); errorMsgWithTypeDiff(formalType, pair.type)));
argTypeForDeferredCheck = null; // No deferred check needed. argTypeForDeferredCheck = null; // No deferred check needed.
} }
argTypesForDeferredCheck.add(argTypeForDeferredCheck); argTypesForDeferredCheck.add(argTypeForDeferredCheck);
Expand Down Expand Up @@ -2452,7 +2448,7 @@ private EnvTypePair analyzeObjLitFwd(
if (!pair.type.isSubtypeOf(jsdocType)) { if (!pair.type.isSubtypeOf(jsdocType)) {
warnings.add(JSError.make( warnings.add(JSError.make(
prop, INVALID_OBJLIT_PROPERTY_TYPE, prop, INVALID_OBJLIT_PROPERTY_TYPE,
jsdocType.toString(), pair.type.toString())); errorMsgWithTypeDiff(jsdocType, pair.type)));
pair.type = jsdocType; pair.type = jsdocType;
} }
} }
Expand Down Expand Up @@ -2608,6 +2604,51 @@ private boolean tightenTypeAndDontWarn(
&& required.isNonLooseSubtypeOf(inferred); && required.isNonLooseSubtypeOf(inferred);
} }


private static String errorMsgWithTypeDiff(JSType expected, JSType found) {
MismatchInfo mismatch = JSType.whyNotSubtypeOf(found, expected);
if (mismatch == null) {
return "Expected : " + expected + "\n"
+ "Found : " + found + "\n";
}
StringBuilder builder = new StringBuilder(
"Expected : " + expected + "\n"
+ "Found : " + found + "\n"
+ "More details:\n");
if (mismatch.isPropMismatch()) {
builder.append(
"Incompatible types for property "
+ mismatch.getPropName() + ".\n"
+ "Expected : " + mismatch.getExpectedType() + "\n"
+ "Found : " + mismatch.getFoundType());
} else if (mismatch.isMissingProp()) {
builder.append(
"The found type is missing property "
+ mismatch.getPropName());
} else if (mismatch.wantedRequiredFoundOptional()) {
builder.append(
"In found type, property " + mismatch.getPropName()
+ " is optional but should be required.");
} else if (mismatch.isArgTypeMismatch()) {
builder.append(
"The expected and found types are functions which have"
+ " incompatible types for argument "
+ (mismatch.getArgIndex() + 1) + ".\n"
+ "Expected a supertype of : " + mismatch.getExpectedType() + "\n"
+ "but found : " + mismatch.getFoundType());
} else if (mismatch.isRetTypeMismatch()) {
builder.append(
"The expected and found types are functions which have"
+ " incompatible return types.\n"
+ "Expected a subtype of : " + mismatch.getExpectedType() + "\n"
+ "but found : " + mismatch.getFoundType());
} else if (mismatch.isUnionTypeMismatch()) {
builder.append(
"The found type is a union that includes an unexpected type: "
+ mismatch.getFoundType());
}
return builder.toString();
}

////////////////////////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////////////////////////
// These functions return true iff they produce a warning // These functions return true iff they produce a warning


Expand Down Expand Up @@ -2764,7 +2805,7 @@ private boolean mayWarnAboutBadIObjectIndex(Node n, JSType iobjectType,
if (!foundIndexType.isSubtypeOf(requiredIndexType)) { if (!foundIndexType.isSubtypeOf(requiredIndexType)) {
warnings.add(JSError.make( warnings.add(JSError.make(
n, NewTypeInference.INVALID_INDEX_TYPE, n, NewTypeInference.INVALID_INDEX_TYPE,
requiredIndexType.toString(), foundIndexType.toString())); errorMsgWithTypeDiff(requiredIndexType, foundIndexType)));
return true; return true;
} }
return false; return false;
Expand Down Expand Up @@ -3547,9 +3588,16 @@ private void warnInvalidOperand(
(expected instanceof String) || (expected instanceof JSType)); (expected instanceof String) || (expected instanceof JSType));
Preconditions.checkArgument( Preconditions.checkArgument(
(actual instanceof String) || (actual instanceof JSType)); (actual instanceof String) || (actual instanceof JSType));
warnings.add(JSError.make( if (expected instanceof JSType && actual instanceof JSType) {
expr, INVALID_OPERAND_TYPE, Token.name(operatorType), warnings.add(JSError.make(
expected.toString(), actual.toString())); expr, INVALID_OPERAND_TYPE, Token.name(operatorType),
errorMsgWithTypeDiff((JSType) expected, (JSType) actual)));
} else {
warnings.add(JSError.make(
expr, INVALID_OPERAND_TYPE, Token.name(operatorType),
"Expected : " + expected.toString() + "\n"
+ "Found : " + actual.toString() + "\n"));
}
} }


private static class EnvTypePair { private static class EnvTypePair {
Expand Down Expand Up @@ -4029,8 +4077,8 @@ private void runCheck(
!fnSummary.getReturnType().isSubtypeOf(this.expectedRetType)) { !fnSummary.getReturnType().isSubtypeOf(this.expectedRetType)) {
warnings.add(JSError.make( warnings.add(JSError.make(
this.callSite, INVALID_INFERRED_RETURN_TYPE, this.callSite, INVALID_INFERRED_RETURN_TYPE,
this.expectedRetType.toString(), errorMsgWithTypeDiff(
fnSummary.getReturnType().toString())); this.expectedRetType, fnSummary.getReturnType())));
} }
int i = 0; int i = 0;
Node argNode = callSite.getSecondChild(); Node argNode = callSite.getSecondChild();
Expand Down
30 changes: 23 additions & 7 deletions src/com/google/javascript/jscomp/newtypes/FunctionType.java
Expand Up @@ -415,11 +415,17 @@ private static FunctionType looseJoin(FunctionType f1, FunctionType f2) {
} }


public boolean isValidOverride(FunctionType other) { public boolean isValidOverride(FunctionType other) {
return isSubtypeOfHelper(other, false, SubtypeCache.create()); return isSubtypeOfHelper(other, false, SubtypeCache.create(), null);
} }


boolean isSubtypeOf(FunctionType other, SubtypeCache subSuperMap) { boolean isSubtypeOf(FunctionType other, SubtypeCache subSuperMap) {
return isSubtypeOfHelper(other, true, subSuperMap); return isSubtypeOfHelper(other, true, subSuperMap, null);
}

static void whyNotSubtypeOf(FunctionType f1, FunctionType f2,
SubtypeCache subSuperMap, MismatchInfo[] boxedInfo) {
Preconditions.checkArgument(boxedInfo.length == 1);
f1.isSubtypeOfHelper(f2, true, subSuperMap, boxedInfo);
} }


// When we write ...?, it has a special meaning, it is NOT a variable-arity // When we write ...?, it has a special meaning, it is NOT a variable-arity
Expand All @@ -431,8 +437,8 @@ private boolean acceptsAnyArguments() {
&& this.restFormals != null && this.restFormals.isUnknown(); && this.restFormals != null && this.restFormals.isUnknown();
} }


private boolean isSubtypeOfHelper( private boolean isSubtypeOfHelper(FunctionType other, boolean checkThisType,
FunctionType other, boolean checkThisType, SubtypeCache subSuperMap) { SubtypeCache subSuperMap, MismatchInfo[] boxedInfo) {
if (other.isTopFunction() || if (other.isTopFunction() ||
other.isQmarkFunction() || this.isQmarkFunction()) { other.isQmarkFunction() || this.isQmarkFunction()) {
return true; return true;
Expand All @@ -452,7 +458,7 @@ private boolean isSubtypeOfHelper(
// and the fix is not trivial, so for now we decided to not fix. // and the fix is not trivial, so for now we decided to not fix.
// See unit tests in NewTypeInferenceES5OrLowerTest#testGenericsSubtyping // See unit tests in NewTypeInferenceES5OrLowerTest#testGenericsSubtyping
return instantiateGenericsWithUnknown(this) return instantiateGenericsWithUnknown(this)
.isSubtypeOfHelper(other, checkThisType, subSuperMap); .isSubtypeOfHelper(other, checkThisType, subSuperMap, boxedInfo);
} }


if (!other.acceptsAnyArguments()) { if (!other.acceptsAnyArguments()) {
Expand All @@ -469,6 +475,10 @@ private boolean isSubtypeOfHelper(
if (thisFormal != null if (thisFormal != null
&& !thisFormal.isUnknown() && !otherFormal.isUnknown() && !thisFormal.isUnknown() && !otherFormal.isUnknown()
&& !otherFormal.isSubtypeOf(thisFormal, subSuperMap)) { && !otherFormal.isSubtypeOf(thisFormal, subSuperMap)) {
if (boxedInfo != null) {
boxedInfo[0] =
MismatchInfo.makeArgTypeMismatch(i, otherFormal, thisFormal);
}
return false; return false;
} }
} }
Expand Down Expand Up @@ -514,8 +524,14 @@ private boolean isSubtypeOfHelper(
} }


// covariance in the return type // covariance in the return type
return returnType.isUnknown() || other.returnType.isUnknown() boolean areRetTypesSubtypes = this.returnType.isUnknown()
|| returnType.isSubtypeOf(other.returnType, subSuperMap); || other.returnType.isUnknown()
|| this.returnType.isSubtypeOf(other.returnType, subSuperMap);
if (boxedInfo != null) {
boxedInfo[0] =
MismatchInfo.makeRetTypeMismatch(other.returnType, this.returnType);
}
return areRetTypesSubtypes;
} }


// Avoid using JSType#join if possible, to avoid creating new types // Avoid using JSType#join if possible, to avoid creating new types
Expand Down

0 comments on commit 93d3403

Please sign in to comment.