diff --git a/src/com/google/javascript/jscomp/NewTypeInference.java b/src/com/google/javascript/jscomp/NewTypeInference.java index 86cd6836205..17b856766bb 100644 --- a/src/com/google/javascript/jscomp/NewTypeInference.java +++ b/src/com/google/javascript/jscomp/NewTypeInference.java @@ -31,6 +31,7 @@ import com.google.javascript.jscomp.newtypes.FunctionTypeBuilder; import com.google.javascript.jscomp.newtypes.JSType; 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.TypeEnv; import com.google.javascript.jscomp.newtypes.UniqueNameGenerator; @@ -69,33 +70,28 @@ final class NewTypeInference implements CompilerPass { static final DiagnosticType MISTYPED_ASSIGN_RHS = DiagnosticType.warning( "JSC_NTI_MISTYPED_ASSIGN_RHS", "The right side in the assignment is not a subtype of the left side.\n" - + "left side : {0}\n" - + "right side : {1}\n"); + + "{0}"); static final DiagnosticType INVALID_OPERAND_TYPE = DiagnosticType.warning( "JSC_NTI_INVALID_OPERAND_TYPE", "Invalid type(s) for operator {0}.\n" - + "expected : {1}\n" - + "found : {2}\n"); + + "{1}"); static final DiagnosticType RETURN_NONDECLARED_TYPE = DiagnosticType.warning( "JSC_NTI_RETURN_NONDECLARED_TYPE", - "Returned type does not match declared return type.\n" + - "declared : {0}\n" + - "found : {1}\n"); + "Returned type does not match declared return type.\n" + + "{0}"); static final DiagnosticType INVALID_INFERRED_RETURN_TYPE = DiagnosticType.warning( "JSC_NTI_INVALID_INFERRED_RETURN_TYPE", - "Function called in context that expects incompatible type.\n" + - "expected : {0}\n" + - "found : {1}\n"); + "Function called in context that expects incompatible type.\n" + + "{0}"); static final DiagnosticType INVALID_ARGUMENT_TYPE = DiagnosticType.warning( "JSC_NTI_INVALID_ARGUMENT_TYPE", - "Invalid type for parameter {0} of function {1}.\n" + - "expected : {2}\n" + - "found : {3}\n"); + "Invalid type for parameter {0} of function {1}.\n" + + "{2}"); static final DiagnosticType CROSS_SCOPE_GOTCHA = DiagnosticType.warning( "JSC_NTI_CROSS_SCOPE_GOTCHA", @@ -130,8 +126,7 @@ final class NewTypeInference implements CompilerPass { DiagnosticType.warning( "JSC_NTI_INVALID_INDEX_TYPE", "Invalid type for index.\n" - + "expected : {0}\n" - + "found : {1}\n"); + + "{0}"); static final DiagnosticType BOTTOM_INDEX_TYPE = DiagnosticType.warning( @@ -142,7 +137,8 @@ final class NewTypeInference implements CompilerPass { static final DiagnosticType INVALID_OBJLIT_PROPERTY_TYPE = DiagnosticType.warning( "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 = DiagnosticType.warning( @@ -182,8 +178,8 @@ final class NewTypeInference implements CompilerPass { static final DiagnosticType INVALID_THIS_TYPE_IN_BIND = DiagnosticType.warning( "JSC_NTI_INVALID_THIS_TYPE_IN_BIND", - "The first argument to bind has type {0} which is not a subtype of" - + " {1}."); + "Invalid type for the first argument to bind.\n" + + "{0}"); static final DiagnosticType CANNOT_BIND_CTOR = DiagnosticType.warning( @@ -245,7 +241,7 @@ final class NewTypeInference implements CompilerPass { static final DiagnosticType NOT_CALLABLE = DiagnosticType.warning( "JSC_NTI_NOT_FUNCTION_TYPE", - "{0} expressions are not callable"); + "Cannot call non-function type {0}"); static final DiagnosticType WRONG_ARGUMENT_COUNT = DiagnosticType.warning( @@ -854,7 +850,7 @@ private void analyzeFunctionFwd( } if (!actualRetType.isSubtypeOf(declRetType)) { warnings.add(JSError.make(n, RETURN_NONDECLARED_TYPE, - declRetType.toString(), actualRetType.toString())); + errorMsgWithTypeDiff(declRetType, actualRetType))); } break; } @@ -1154,7 +1150,7 @@ private TypeEnv processVarDeclaration(Node nameNode, TypeEnv inEnv) { if (!rhsType.isSubtypeOf(declType)) { warnings.add(JSError.make( rhs, MISTYPED_ASSIGN_RHS, - declType.toString(), rhsType.toString())); + errorMsgWithTypeDiff(declType, rhsType))); // Don't flow the wrong initialization rhsType = declType; } else { @@ -1926,7 +1922,7 @@ private EnvTypePair analyzeFunctionBindFwd(Node call, TypeEnv inEnv) { env = pair.env; if (!pair.type.isSubtypeOf(reqThisType)) { warnings.add(JSError.make(call, INVALID_THIS_TYPE_IN_BIND, - pair.type.toString(), reqThisType.toString())); + errorMsgWithTypeDiff(reqThisType, pair.type))); } } @@ -1967,7 +1963,7 @@ private TypeEnv analyzeCallNodeArgumentsFwd(Node call, Node firstArg, String fnName = getReadableCalleeName(call.getFirstChild()); warnings.add(JSError.make(arg, INVALID_ARGUMENT_TYPE, Integer.toString(i + 1), fnName, - formalType.toString(), pair.type.toString())); + errorMsgWithTypeDiff(formalType, pair.type))); argTypeForDeferredCheck = null; // No deferred check needed. } argTypesForDeferredCheck.add(argTypeForDeferredCheck); @@ -2452,7 +2448,7 @@ private EnvTypePair analyzeObjLitFwd( if (!pair.type.isSubtypeOf(jsdocType)) { warnings.add(JSError.make( prop, INVALID_OBJLIT_PROPERTY_TYPE, - jsdocType.toString(), pair.type.toString())); + errorMsgWithTypeDiff(jsdocType, pair.type))); pair.type = jsdocType; } } @@ -2608,6 +2604,51 @@ private boolean tightenTypeAndDontWarn( && 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 @@ -2764,7 +2805,7 @@ private boolean mayWarnAboutBadIObjectIndex(Node n, JSType iobjectType, if (!foundIndexType.isSubtypeOf(requiredIndexType)) { warnings.add(JSError.make( n, NewTypeInference.INVALID_INDEX_TYPE, - requiredIndexType.toString(), foundIndexType.toString())); + errorMsgWithTypeDiff(requiredIndexType, foundIndexType))); return true; } return false; @@ -3547,9 +3588,16 @@ private void warnInvalidOperand( (expected instanceof String) || (expected instanceof JSType)); Preconditions.checkArgument( (actual instanceof String) || (actual instanceof JSType)); - warnings.add(JSError.make( - expr, INVALID_OPERAND_TYPE, Token.name(operatorType), - expected.toString(), actual.toString())); + if (expected instanceof JSType && actual instanceof JSType) { + warnings.add(JSError.make( + 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 { @@ -4029,8 +4077,8 @@ private void runCheck( !fnSummary.getReturnType().isSubtypeOf(this.expectedRetType)) { warnings.add(JSError.make( this.callSite, INVALID_INFERRED_RETURN_TYPE, - this.expectedRetType.toString(), - fnSummary.getReturnType().toString())); + errorMsgWithTypeDiff( + this.expectedRetType, fnSummary.getReturnType()))); } int i = 0; Node argNode = callSite.getSecondChild(); diff --git a/src/com/google/javascript/jscomp/newtypes/FunctionType.java b/src/com/google/javascript/jscomp/newtypes/FunctionType.java index f898e7c5ad6..febfac38b13 100644 --- a/src/com/google/javascript/jscomp/newtypes/FunctionType.java +++ b/src/com/google/javascript/jscomp/newtypes/FunctionType.java @@ -415,11 +415,17 @@ private static FunctionType looseJoin(FunctionType f1, FunctionType f2) { } public boolean isValidOverride(FunctionType other) { - return isSubtypeOfHelper(other, false, SubtypeCache.create()); + return isSubtypeOfHelper(other, false, SubtypeCache.create(), null); } 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 @@ -431,8 +437,8 @@ private boolean acceptsAnyArguments() { && this.restFormals != null && this.restFormals.isUnknown(); } - private boolean isSubtypeOfHelper( - FunctionType other, boolean checkThisType, SubtypeCache subSuperMap) { + private boolean isSubtypeOfHelper(FunctionType other, boolean checkThisType, + SubtypeCache subSuperMap, MismatchInfo[] boxedInfo) { if (other.isTopFunction() || other.isQmarkFunction() || this.isQmarkFunction()) { return true; @@ -452,7 +458,7 @@ private boolean isSubtypeOfHelper( // and the fix is not trivial, so for now we decided to not fix. // See unit tests in NewTypeInferenceES5OrLowerTest#testGenericsSubtyping return instantiateGenericsWithUnknown(this) - .isSubtypeOfHelper(other, checkThisType, subSuperMap); + .isSubtypeOfHelper(other, checkThisType, subSuperMap, boxedInfo); } if (!other.acceptsAnyArguments()) { @@ -469,6 +475,10 @@ private boolean isSubtypeOfHelper( if (thisFormal != null && !thisFormal.isUnknown() && !otherFormal.isUnknown() && !otherFormal.isSubtypeOf(thisFormal, subSuperMap)) { + if (boxedInfo != null) { + boxedInfo[0] = + MismatchInfo.makeArgTypeMismatch(i, otherFormal, thisFormal); + } return false; } } @@ -514,8 +524,14 @@ private boolean isSubtypeOfHelper( } // covariance in the return type - return returnType.isUnknown() || other.returnType.isUnknown() - || returnType.isSubtypeOf(other.returnType, subSuperMap); + boolean areRetTypesSubtypes = this.returnType.isUnknown() + || 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 diff --git a/src/com/google/javascript/jscomp/newtypes/JSType.java b/src/com/google/javascript/jscomp/newtypes/JSType.java index 31697e373db..e8405898112 100644 --- a/src/com/google/javascript/jscomp/newtypes/JSType.java +++ b/src/com/google/javascript/jscomp/newtypes/JSType.java @@ -166,7 +166,8 @@ private static JSType makeType(int mask, mask |= NON_SCALAR_MASK; } - if (objs.isEmpty() && typeVar == null && enums.isEmpty()) { + if (objs.isEmpty() && enums.isEmpty() + && typeVar == null && (mask & TYPEVAR_MASK) == 0) { return MaskType.make(mask); } if (!JSType.isInhabitable(objs)) { @@ -597,7 +598,7 @@ public JSType substituteGenerics(Map concreteTypes) { } JSType current = makeType( getMask() & ~TYPEVAR_MASK, builder.build(), null, getEnums()); - if ((getMask() & TYPEVAR_MASK) != 0) { + if (hasTypeVariable()) { current = JSType.join(current, concreteTypes.containsKey(getTypeVar()) ? concreteTypes.get(getTypeVar()) : fromTypeVar(getTypeVar())); } @@ -1037,7 +1038,7 @@ public JSType toBoolean() { } public boolean isNonLooseSubtypeOf(JSType other) { - return isSubtypeOfHelper(false, other, SubtypeCache.create()); + return isSubtypeOfHelper(false, other, SubtypeCache.create(), null); } @Override @@ -1045,20 +1046,40 @@ public boolean isSubtypeOf(TypeI other) { return isSubtypeOf(other, SubtypeCache.create()); } + public static MismatchInfo whyNotSubtypeOf(JSType t1, JSType t2) { + if (t1.isSingletonObj() && t2.isSingletonObj()) { + MismatchInfo[] boxedInfo = new MismatchInfo[1]; + ObjectType.whyNotSubtypeOf( + t1.getObjTypeIfSingletonObj(), + t2.getObjTypeIfSingletonObj(), + boxedInfo); + return boxedInfo[0]; + } + if (t1.isUnion()) { + MismatchInfo[] boxedInfo = new MismatchInfo[1]; + boolean areSubtypes = + t1.isSubtypeOfHelper(true, t2, SubtypeCache.create(), boxedInfo); + Preconditions.checkState(!areSubtypes); + return boxedInfo[0]; + } + return null; + } + boolean isSubtypeOf(TypeI other, SubtypeCache subSuperMap) { if (this == other) { return true; } JSType type2 = (JSType) other; if (isLoose() || type2.isLoose()) { - return autobox().isSubtypeOfHelper(true, type2.autobox(), subSuperMap); + return autobox().isSubtypeOfHelper(true, type2.autobox(), subSuperMap, null); } else { - return isSubtypeOfHelper(true, type2, subSuperMap); + return isSubtypeOfHelper(true, type2, subSuperMap, null); } } private boolean isSubtypeOfHelper( - boolean keepLoosenessOfThis, JSType other, SubtypeCache subSuperMap) { + boolean keepLoosenessOfThis, JSType other, SubtypeCache subSuperMap, + MismatchInfo[] boxedInfo) { if (isUnknown() || other.isUnknown() || other.isTop()) { return true; } @@ -1073,6 +1094,9 @@ private boolean isSubtypeOfHelper( } int mask = getMask() & ~ENUM_MASK; if ((mask | other.getMask()) != other.getMask()) { + if (boxedInfo != null && isUnion()) { + whyNotUnionSubtypes(this, other, boxedInfo); + } return false; } if (getTypeVar() != null && !getTypeVar().equals(other.getTypeVar())) { @@ -1083,8 +1107,36 @@ private boolean isSubtypeOfHelper( } // Because of optional properties, // x \le y \iff x \join y = y does not hold. - return ObjectType.isUnionSubtype( + boolean result = ObjectType.isUnionSubtype( keepLoosenessOfThis, getObjs(), other.getObjs(), subSuperMap); + if (boxedInfo != null) { + ObjectType.whyNotUnionSubtypes( + keepLoosenessOfThis, getObjs(), other.getObjs(), + subSuperMap, boxedInfo); + } + return result; + } + + private static void whyNotUnionSubtypes( + JSType found, JSType expected, MismatchInfo[] boxedInfo) { + if (NUMBER.isSubtypeOf(found) && !NUMBER.isSubtypeOf(expected)) { + boxedInfo[0] = MismatchInfo.makeUnionTypeMismatch(NUMBER); + } else if (STRING.isSubtypeOf(found) && !STRING.isSubtypeOf(expected)) { + boxedInfo[0] = MismatchInfo.makeUnionTypeMismatch(STRING); + } else if (BOOLEAN.isSubtypeOf(found) && !BOOLEAN.isSubtypeOf(expected)) { + boxedInfo[0] = MismatchInfo.makeUnionTypeMismatch(BOOLEAN); + } else if (NULL.isSubtypeOf(found) && !NULL.isSubtypeOf(expected)) { + boxedInfo[0] = MismatchInfo.makeUnionTypeMismatch(NULL); + } else if (UNDEFINED.isSubtypeOf(found) && !UNDEFINED.isSubtypeOf(expected)) { + boxedInfo[0] = MismatchInfo.makeUnionTypeMismatch(UNDEFINED); + } else if (found.hasTypeVariable() && !expected.hasTypeVariable()) { + boxedInfo[0] = MismatchInfo.makeUnionTypeMismatch( + fromTypeVar(found.getTypeVar())); + } else if ((found.getMask() & NON_SCALAR_MASK) != 0 + && (expected.getMask() & NON_SCALAR_MASK) == 0) { + boxedInfo[0] = MismatchInfo.makeUnionTypeMismatch(makeType( + NON_SCALAR_MASK, found.getObjs(), null, ImmutableSet.of())); + } } public JSType removeType(JSType other) { diff --git a/src/com/google/javascript/jscomp/newtypes/MismatchInfo.java b/src/com/google/javascript/jscomp/newtypes/MismatchInfo.java new file mode 100644 index 00000000000..0561af0864c --- /dev/null +++ b/src/com/google/javascript/jscomp/newtypes/MismatchInfo.java @@ -0,0 +1,136 @@ +/* + * Copyright 2016 The Closure Compiler Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.javascript.jscomp.newtypes; + +import com.google.common.base.Preconditions; + +/** + * When the expected and the found type don't match, this class contains + * information about the mismatch that allows us to pinpoint which parts of + * the types don't match. + */ +public class MismatchInfo { + private static enum Kind { + PROPERTY_TYPE_MISMATCH, + MISSING_PROPERTY, + WANTED_REQUIRED_PROP_FOUND_OPTIONAL, + ARGUMENT_TYPE_MISMATCH, + RETURN_TYPE_MISMATCH, + UNION_TYPE_MISMATCH + } + + private final Kind kind; + private String propName; + private int argIndex = -1; + private JSType expected; + private JSType found; + + private MismatchInfo(Kind kind) { + this.kind = kind; + } + + //// Factories + + static MismatchInfo makeUnionTypeMismatch(JSType found) { + MismatchInfo info = new MismatchInfo(Kind.UNION_TYPE_MISMATCH); + info.found = found; + return info; + } + + static MismatchInfo makePropTypeMismatch( + String propName, JSType expected, JSType found) { + MismatchInfo info = new MismatchInfo(Kind.PROPERTY_TYPE_MISMATCH); + info.propName = propName; + info.expected = expected; + info.found = found; + return info; + } + + static MismatchInfo makeMissingPropMismatch(String propName) { + MismatchInfo info = new MismatchInfo(Kind.MISSING_PROPERTY); + info.propName = propName; + return info; + } + + static MismatchInfo makeMaybeMissingPropMismatch(String propName) { + MismatchInfo info = + new MismatchInfo(Kind.WANTED_REQUIRED_PROP_FOUND_OPTIONAL); + info.propName = propName; + return info; + } + + static MismatchInfo makeArgTypeMismatch( + int argIndex, JSType expected, JSType found) { + MismatchInfo info = new MismatchInfo(Kind.ARGUMENT_TYPE_MISMATCH); + info.argIndex = argIndex; + info.expected = expected; + info.found = found; + return info; + } + + static MismatchInfo makeRetTypeMismatch(JSType expected, JSType found) { + MismatchInfo info = new MismatchInfo(Kind.RETURN_TYPE_MISMATCH); + info.expected = expected; + info.found = found; + return info; + } + + //// Getters + + public String getPropName() { + return Preconditions.checkNotNull(this.propName); + } + + public JSType getFoundType() { + return Preconditions.checkNotNull(this.found); + } + + public JSType getExpectedType() { + return Preconditions.checkNotNull(this.expected); + } + + public int getArgIndex() { + Preconditions.checkState(this.argIndex >= 0); + return this.argIndex; + } + + //// Predicates + + public boolean isPropMismatch() { + return this.kind == Kind.PROPERTY_TYPE_MISMATCH; + } + + public boolean isMissingProp() { + return this.kind == Kind.MISSING_PROPERTY; + } + + public boolean wantedRequiredFoundOptional() { + return this.kind == Kind.WANTED_REQUIRED_PROP_FOUND_OPTIONAL; + } + + public boolean isArgTypeMismatch() { + return this.kind == Kind.ARGUMENT_TYPE_MISMATCH; + } + + public boolean isRetTypeMismatch() { + return this.kind == Kind.RETURN_TYPE_MISMATCH; + } + + public boolean isUnionTypeMismatch() { + return this.kind == Kind.UNION_TYPE_MISMATCH; + } +} diff --git a/src/com/google/javascript/jscomp/newtypes/ObjectType.java b/src/com/google/javascript/jscomp/newtypes/ObjectType.java index fadc9094ed0..945a2104ed1 100644 --- a/src/com/google/javascript/jscomp/newtypes/ObjectType.java +++ b/src/com/google/javascript/jscomp/newtypes/ObjectType.java @@ -545,15 +545,35 @@ private static PersistentMap joinPropsLoosely( static boolean isUnionSubtype(boolean keepLoosenessOfThis, Set objs1, Set objs2, SubtypeCache subSuperMap) { + return isUnionSubtypeHelper( + keepLoosenessOfThis, objs1, objs2, subSuperMap, null); + } + + static void whyNotUnionSubtypes(boolean keepLoosenessOfThis, + Set objs1, Set objs2, SubtypeCache subSuperMap, + MismatchInfo[] boxedInfo) { + Preconditions.checkArgument(boxedInfo.length == 1); + boolean areSubtypes = isUnionSubtypeHelper( + keepLoosenessOfThis, objs1, objs2, subSuperMap, boxedInfo); + Preconditions.checkState(!areSubtypes); + } + + private static boolean isUnionSubtypeHelper(boolean keepLoosenessOfThis, + Set objs1, Set objs2, SubtypeCache subSuperMap, + MismatchInfo[] boxedInfo) { for (ObjectType obj1 : objs1) { boolean foundSupertype = false; for (ObjectType obj2 : objs2) { - if (obj1.isSubtypeOfHelper(keepLoosenessOfThis, obj2, subSuperMap)) { + if (obj1.isSubtypeOfHelper(keepLoosenessOfThis, obj2, subSuperMap, null)) { foundSupertype = true; break; } } if (!foundSupertype) { + if (boxedInfo != null) { + boxedInfo[0] = + MismatchInfo.makeUnionTypeMismatch(JSType.fromObjectType(obj1)); + } return false; } } @@ -561,7 +581,15 @@ static boolean isUnionSubtype(boolean keepLoosenessOfThis, } boolean isSubtypeOf(ObjectType obj2, SubtypeCache subSuperMap) { - return isSubtypeOfHelper(true, obj2, subSuperMap); + return isSubtypeOfHelper(true, obj2, subSuperMap, null); + } + + static void whyNotSubtypeOf( + ObjectType obj1, ObjectType obj2, MismatchInfo[] boxedInfo) { + Preconditions.checkArgument(boxedInfo.length == 1); + boolean areSubtypes = + obj1.isSubtypeOfHelper(true, obj2, SubtypeCache.create(), boxedInfo); + Preconditions.checkState(!areSubtypes); } /** @@ -571,7 +599,7 @@ boolean isSubtypeOf(ObjectType obj2, SubtypeCache subSuperMap) { * { } \le { p: num= } and also { p: num= } \le { }. */ private boolean isSubtypeOfHelper(boolean keepLoosenessOfThis, - ObjectType other, SubtypeCache subSuperMap) { + ObjectType other, SubtypeCache subSuperMap, MismatchInfo[] boxedInfo) { if (other == TOP_OBJECT) { return true; } @@ -608,7 +636,7 @@ private boolean isSubtypeOfHelper(boolean keepLoosenessOfThis, return false; } } - if (!arePropertiesSubtypes(other, otherPropNames, subSuperMap)) { + if (!arePropertiesSubtypes(other, otherPropNames, subSuperMap, boxedInfo)) { return false; } @@ -618,16 +646,21 @@ private boolean isSubtypeOfHelper(boolean keepLoosenessOfThis, // Can only be executed if we have declared types for callable objects. return false; } - return this.fn.isSubtypeOf(other.fn, subSuperMap); + boolean areFunsSubtypes = this.fn.isSubtypeOf(other.fn, subSuperMap); + if (boxedInfo != null) { + FunctionType.whyNotSubtypeOf(this.fn, other.fn, subSuperMap, boxedInfo); + } + return areFunsSubtypes; } private boolean arePropertiesSubtypes(ObjectType other, - Set otherPropNames, SubtypeCache subSuperMap) { + Set otherPropNames, SubtypeCache subSuperMap, + MismatchInfo[] boxedInfo) { for (String pname : otherPropNames) { QualifiedName qname = new QualifiedName(pname); if (!isPropertySubtype( - this.getLeftmostProp(qname), other.getLeftmostProp(qname), - subSuperMap)) { + pname, this.getLeftmostProp(qname), other.getLeftmostProp(qname), + subSuperMap, boxedInfo)) { return false; } } @@ -636,8 +669,8 @@ private boolean arePropertiesSubtypes(ObjectType other, if (!otherPropNames.contains(pname)) { QualifiedName qname = new QualifiedName(pname); if (!isPropertySubtype( - this.getLeftmostProp(qname), other.getLeftmostProp(qname), - subSuperMap)) { + pname, this.getLeftmostProp(qname), other.getLeftmostProp(qname), + subSuperMap, boxedInfo)) { return false; } } @@ -646,7 +679,14 @@ private boolean arePropertiesSubtypes(ObjectType other, return true; } - private static boolean isPropertySubtype( + private static boolean isPropertySubtype(String pname, Property prop1, + Property prop2, SubtypeCache subSuperMap, MismatchInfo[] boxedInfo) { + return boxedInfo != null + ? getPropMismatchInfo(pname, prop1, prop2, subSuperMap, boxedInfo) + : isPropertySubtypeHelper(prop1, prop2, subSuperMap); + } + + private static boolean isPropertySubtypeHelper( Property prop1, Property prop2, SubtypeCache subSuperMap) { if (prop2.isOptional()) { if (prop1 != null @@ -662,6 +702,33 @@ private static boolean isPropertySubtype( return true; } + // Like isPropertySubtypeHelper, but also provides mismatch information + private static boolean getPropMismatchInfo(String pname, Property prop1, + Property prop2, SubtypeCache subSuperMap, MismatchInfo[] boxedInfo) { + Preconditions.checkNotNull(pname); + if (prop2.isOptional()) { + if (prop1 != null + && !prop1.getType().isSubtypeOf(prop2.getType(), subSuperMap)) { + boxedInfo[0] = MismatchInfo.makePropTypeMismatch( + pname, prop2.getType(), prop1.getType()); + return false; + } + } else { + if (prop1 == null) { + boxedInfo[0] = MismatchInfo.makeMissingPropMismatch(pname); + return false; + } else if (prop1.isOptional()) { + boxedInfo[0] = MismatchInfo.makeMaybeMissingPropMismatch(pname); + return false; + } else if (!prop1.getType().isSubtypeOf(prop2.getType(), subSuperMap)) { + boxedInfo[0] = MismatchInfo.makePropTypeMismatch( + pname, prop2.getType(), prop1.getType()); + return false; + } + } + return true; + } + // We never infer properties as optional on loose objects, // and we don't warn about possibly inexistent properties. boolean isLooseSubtypeOf(ObjectType other, SubtypeCache subSuperMap) { diff --git a/test/com/google/javascript/jscomp/NewTypeInferenceES5OrLowerTest.java b/test/com/google/javascript/jscomp/NewTypeInferenceES5OrLowerTest.java index d63a3f74020..3cd3c919eeb 100644 --- a/test/com/google/javascript/jscomp/NewTypeInferenceES5OrLowerTest.java +++ b/test/com/google/javascript/jscomp/NewTypeInferenceES5OrLowerTest.java @@ -16482,4 +16482,129 @@ public void testReportUknownTypes() { " Foo.call(this);", "};")); } + + public void testPinpointTypeDiffWhenMismatch() { + typeCheckMessageContents(LINE_JOINER.join( + "function f(/** {a:number, b:string} */ x) {}", + "f({a: 123, b: 123});"), + NewTypeInference.INVALID_ARGUMENT_TYPE, + LINE_JOINER.join( + "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")); + + typeCheckMessageContents(LINE_JOINER.join( + "function f(/** {a:number, b:(string|undefined)} */ x) {}", + "f({a: 123, b: 123});"), + NewTypeInference.INVALID_ARGUMENT_TYPE, + LINE_JOINER.join( + "Invalid type for parameter 1 of function f.", + "Expected : {a:number, b:string|undefined=}", + "Found : {a:number, b:number}", + "More details:", + "Incompatible types for property b.", + "Expected : string|undefined", + "Found : number")); + + typeCheckMessageContents(LINE_JOINER.join( + "function f(/** {a:number, b:string} */ x) {}", + "f({a: 123});"), + NewTypeInference.INVALID_ARGUMENT_TYPE, + LINE_JOINER.join( + "Invalid type for parameter 1 of function f.", + "Expected : {a:number, b:string}", + "Found : {a:number}", + "More details:", + "The found type is missing property b")); + + typeCheckMessageContents(LINE_JOINER.join( + "function f(/** {a:number, b:string} */ x) {}", + "function g(/** {a:number, b:(string|undefined)} */ x) { f(x); }"), + NewTypeInference.INVALID_ARGUMENT_TYPE, + LINE_JOINER.join( + "Invalid type for parameter 1 of function f.", + "Expected : {a:number, b:string}", + "Found : {a:number, b:string|undefined=}", + "More details:", + "In found type, property b is optional but should be required.")); + + typeCheckMessageContents(LINE_JOINER.join( + "function f(/** function((number|string)) */ x) {}", + "function g(/** number */ x) {}", + "f(g);"), + NewTypeInference.INVALID_ARGUMENT_TYPE, + LINE_JOINER.join( + "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")); + + typeCheckMessageContents(LINE_JOINER.join( + "function f(/** function():number */ x) {}", + "/** @return {string} */ function g() { return 'asdf'; }", + "f(g)"), + NewTypeInference.INVALID_ARGUMENT_TYPE, + LINE_JOINER.join( + "Invalid type for parameter 1 of function f.", + "Expected : function():number", + "Found : function():string", + "More details:", + "The expected and found types are functions which have incompatible" + + " return types.", + "Expected a subtype of : number", + "but found : string")); + + typeCheckMessageContents(LINE_JOINER.join( + "/** @constructor */ function Foo() {}", + "/** @constructor */ function Bar() {}", + "function f(/** !Foo */ x) {}", + "function g(/** (!Foo|!Bar) */ x) {", + " f(x);", + "}"), + NewTypeInference.INVALID_ARGUMENT_TYPE, + LINE_JOINER.join( + "Invalid type for parameter 1 of function f.", + "Expected : Foo", + "Found : Bar|Foo", + "More details:", + "The found type is a union that includes an unexpected type: Bar")); + + typeCheckMessageContents(LINE_JOINER.join( + "/** @constructor */ function Foo() {}", + "/** @constructor @extends {Foo} */ function Bar() {}", + "function f(/** !Foo */ x) {}", + "function g(/** ?Bar */ x) {", + " f(x);", + "}"), + NewTypeInference.INVALID_ARGUMENT_TYPE, + LINE_JOINER.join( + "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")); + + typeCheckMessageContents(LINE_JOINER.join( + "function f(/** number */ x) {}", + "function g(/** number|string */ x) {", + " f(x);", + "}"), + NewTypeInference.INVALID_ARGUMENT_TYPE, + LINE_JOINER.join( + "Invalid type for parameter 1 of function f.", + "Expected : number", + "Found : number|string", + "More details:", + "The found type is a union that " + + "includes an unexpected type: string")); + } } diff --git a/test/com/google/javascript/jscomp/NewTypeInferenceTestBase.java b/test/com/google/javascript/jscomp/NewTypeInferenceTestBase.java index e2e4de84c61..b9b6b77c774 100644 --- a/test/com/google/javascript/jscomp/NewTypeInferenceTestBase.java +++ b/test/com/google/javascript/jscomp/NewTypeInferenceTestBase.java @@ -260,17 +260,19 @@ private final void typeCheck( parseAndTypeCheck(externs, js); JSError[] warnings = compiler.getWarnings(); JSError[] errors = compiler.getErrors(); - String errorMessage = - "Expected warning of type:\n" - + "================================================================\n" - + Arrays.toString(warningKinds) - + "================================================================\n" - + "but found:\n" - + "----------------------------------------------------------------\n" - + Arrays.toString(errors) + Arrays.toString(warnings) + "\n" - + "----------------------------------------------------------------\n"; + String errorMessage = LINE_JOINER.join( + "Expected warning of type:", + "================================================================", + Arrays.toString(warningKinds), + "================================================================", + "but found:", + "----------------------------------------------------------------", + Arrays.toString(errors) + Arrays.toString(warnings) + "", + "----------------------------------------------------------------"); assertEquals( - errorMessage + "Warning count", warningKinds.length, warnings.length + errors.length); + errorMessage + "Warning count", + warningKinds.length, + warnings.length + errors.length); for (JSError warning : warnings) { assertTrue( "Wrong warning type\n" + errorMessage, @@ -282,4 +284,44 @@ private final void typeCheck( Arrays.asList(warningKinds).contains(error.getType())); } } + + // Used only in the cases where we provide extra details in the error message. + // Don't use in other cases. + // It is deliberately less general; no custom externs and only a single + // warning per test. + protected final void typeCheckMessageContents( + String js, DiagnosticType warningKind, String warningMsg) { + parseAndTypeCheck(DEFAULT_EXTERNS, js); + JSError[] warnings = compiler.getWarnings(); + JSError[] errors = compiler.getErrors(); + assertEquals( + "Expected no errors, but found:\n" + Arrays.toString(errors), + 0, errors.length); + assertEquals( + "Expected one warning, but found:\n" + Arrays.toString(warnings), + 1, warnings.length); + JSError warning = warnings[0]; + assertEquals(LINE_JOINER.join( + "Wrong warning type", + "Expected warning of type:", + "================================================================", + warningKind.toString(), + "================================================================", + "but found:", + "----------------------------------------------------------------", + warning.toString(), + "----------------------------------------------------------------"), + warningKind, warning.getType()); + assertEquals(LINE_JOINER.join( + "Wrong warning message", + "Expected:", + "================================================================", + warningMsg, + "================================================================", + "but found:", + "----------------------------------------------------------------", + warning.description, + "----------------------------------------------------------------"), + warningMsg, warning.description); + } }