From f641b9a91f1e2cf90c01b28cdba495c78224ecd7 Mon Sep 17 00:00:00 2001 From: mrolig Date: Fri, 2 Jun 2017 10:10:48 -0700 Subject: [PATCH] Add extra information to help understand type mismatch errors for record types. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=157844020 --- .../javascript/jscomp/TypeValidator.java | 69 +++++++- .../javascript/jscomp/TypeCheckTest.java | 162 ++++++++++++------ .../javascript/jscomp/TypeValidatorTest.java | 12 +- 3 files changed, 182 insertions(+), 61 deletions(-) diff --git a/src/com/google/javascript/jscomp/TypeValidator.java b/src/com/google/javascript/jscomp/TypeValidator.java index 670f96726bc..c00627c00f5 100644 --- a/src/com/google/javascript/jscomp/TypeValidator.java +++ b/src/com/google/javascript/jscomp/TypeValidator.java @@ -28,6 +28,7 @@ import static com.google.javascript.rhino.jstype.JSTypeNative.UNKNOWN_TYPE; import static com.google.javascript.rhino.jstype.JSTypeNative.VOID_TYPE; +import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import com.google.javascript.jscomp.parsing.parser.util.format.SimpleFormat; import com.google.javascript.rhino.JSDocInfo; @@ -48,6 +49,8 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; +import java.util.TreeSet; import javax.annotation.Nullable; /** @@ -83,6 +86,13 @@ class TypeValidator implements Serializable { "found : {1}\n" + "required: {2}"; + private static final String FOUND_REQUIRED_MISSING = + "{0}\n" + + "found : {1}\n" + + "required: {2}\n" + + "missing : [{3}]\n" + + "mismatch: [{4}]"; + static final DiagnosticType INVALID_CAST = DiagnosticType.warning("JSC_INVALID_CAST", "invalid cast - must be a subtype or supertype\n" + @@ -777,7 +787,7 @@ void expectAbstractMethodsImplemented(Node n, FunctionType ctorType) { } /** Report a type mismatch */ - private void mismatch(NodeTraversal t, Node n, String msg, JSType found, JSType required) { + private void mismatch(NodeTraversal unusedT, Node n, String msg, JSType found, JSType required) { mismatch(n, msg, found, required); } @@ -787,26 +797,67 @@ private void mismatch(NodeTraversal t, Node n, String msg, JSType found, JSTypeN private void mismatch(Node n, String msg, JSType found, JSType required) { if (!found.isSubtype(required, this.subtypingMode)) { - JSError err = JSError.make( - n, TYPE_MISMATCH_WARNING, formatFoundRequired(msg, found, required)); + Set missing = null; + Set mismatch = null; + if (required.isStructuralType()) { + missing = new TreeSet<>(); + mismatch = new TreeSet<>(); + ObjectType requiredObject = required.toMaybeObjectType(); + ObjectType foundObject = found.toMaybeObjectType(); + if (requiredObject != null && foundObject != null) { + for (String property : requiredObject.getPropertyNames()) { + JSType propRequired = requiredObject.getPropertyType(property); + boolean hasProperty = foundObject.hasProperty(property); + if (!propRequired.isExplicitlyVoidable() || hasProperty) { + if (hasProperty) { + if (!foundObject.getPropertyType(property).isSubtype(propRequired, subtypingMode)) { + mismatch.add(property); + } + } else { + missing.add(property); + } + } + } + } + } + JSError err = + JSError.make( + n, + TYPE_MISMATCH_WARNING, + formatFoundRequired(msg, found, required, missing, mismatch)); TypeMismatch.registerMismatch( this.mismatches, this.implicitInterfaceUses, found, required, err); report(err); } } - /** - * Formats a found/required error message. - */ - private static String formatFoundRequired(String description, JSType found, - JSType required) { + /** Formats a found/required error message. */ + private static String formatFoundRequired( + String description, + JSType found, + JSType required, + Set missing, + Set mismatch) { String foundStr = found.toString(); String requiredStr = required.toString(); if (foundStr.equals(requiredStr)) { foundStr = found.toAnnotationString(); requiredStr = required.toAnnotationString(); } - return MessageFormat.format(FOUND_REQUIRED, description, foundStr, requiredStr); + String missingStr = ""; + String mismatchStr = ""; + if (missing != null && !missing.isEmpty()) { + missingStr = Joiner.on(",").join(missing); + } + if (mismatch != null && !mismatch.isEmpty()) { + mismatchStr = Joiner.on(",").join(mismatch); + } + if (missingStr.length() > 0 || mismatchStr.length() > 0) { + return MessageFormat.format( + FOUND_REQUIRED_MISSING, description, foundStr, requiredStr, missingStr, mismatchStr); + } else { + return MessageFormat.format(FOUND_REQUIRED, description, foundStr, requiredStr); + } } /** diff --git a/test/com/google/javascript/jscomp/TypeCheckTest.java b/test/com/google/javascript/jscomp/TypeCheckTest.java index 466c0972433..147dfe215dc 100644 --- a/test/com/google/javascript/jscomp/TypeCheckTest.java +++ b/test/com/google/javascript/jscomp/TypeCheckTest.java @@ -251,20 +251,29 @@ public void testTypeCheck24() throws Exception { public void testTypeCheck25() throws Exception { - testTypes("function foo(/** {a: number} */ obj) {};" - + "foo({b: 'abc'});", - "actual parameter 1 of foo does not match formal parameter\n" + - "found : {a: (number|undefined), b: string}\n" + - "required: {a: number}"); + testTypes( + LINE_JOINER.join( + "function foo(/** {a: number} */ obj) {};", + "foo({b: 'abc'});"), + LINE_JOINER.join( + "actual parameter 1 of foo does not match formal parameter", + "found : {a: (number|undefined), b: string}", + "required: {a: number}", + "missing : []", + "mismatch: [a]")); } public void testTypeCheck26() throws Exception { - testTypes("function foo(/** {a: number} */ obj) {};" - + "foo({a: 'abc'});", - "actual parameter 1 of foo does not match formal parameter\n" - + "found : {a: (number|string)}\n" - + "required: {a: number}"); - + testTypes( + LINE_JOINER.join( + "function foo(/** {a: number} */ obj) {};", + "foo({a: 'abc'});"), + LINE_JOINER.join( + "actual parameter 1 of foo does not match formal parameter", + "found : {a: (number|string)}", + "required: {a: number}", + "missing : []", + "mismatch: [a]")); } public void testTypeCheck27() throws Exception { @@ -13027,12 +13036,16 @@ public void testActiveXObject() throws Exception { public void testRecordType1() throws Exception { testTypes( - "/** @param {{prop: number}} x */" + - "function f(x) {}" + - "f({});", - "actual parameter 1 of f does not match formal parameter\n" + - "found : {prop: (number|undefined)}\n" + - "required: {prop: number}"); + LINE_JOINER.join( + "/** @param {{prop: number}} x */", + "function f(x) {}", + "f({});"), + LINE_JOINER.join( + "actual parameter 1 of f does not match formal parameter", + "found : {prop: (number|undefined)}", + "required: {prop: number}", + "missing : []", + "mismatch: [prop]")); } public void testRecordType2() throws Exception { @@ -13044,12 +13057,16 @@ public void testRecordType2() throws Exception { public void testRecordType3() throws Exception { testTypes( - "/** @param {{prop: number}} x */" + - "function f(x) {}" + - "f({prop: 'x'});", - "actual parameter 1 of f does not match formal parameter\n" + - "found : {prop: (number|string)}\n" + - "required: {prop: number}"); + LINE_JOINER.join( + "/** @param {{prop: number}} x */", + "function f(x) {}", + "f({prop: 'x'});"), + LINE_JOINER.join( + "actual parameter 1 of f does not match formal parameter", + "found : {prop: (number|string)}", + "required: {prop: number}", + "missing : []", + "mismatch: [prop]")); } public void testRecordType4() throws Exception { @@ -13063,12 +13080,18 @@ public void testRecordType4() throws Exception { "function g(x) {}" + "var x = {}; f(x); g(x);", ImmutableList.of( - "actual parameter 1 of f does not match formal parameter\n" + - "found : {prop: (number|string|undefined)}\n" + - "required: {prop: (number|undefined)}", - "actual parameter 1 of g does not match formal parameter\n" + - "found : {prop: (number|string|undefined)}\n" + - "required: {prop: (string|undefined)}")); + LINE_JOINER.join( + "actual parameter 1 of f does not match formal parameter", + "found : {prop: (number|string|undefined)}", + "required: {prop: (number|undefined)}", + "missing : []", + "mismatch: [prop]"), + LINE_JOINER.join( + "actual parameter 1 of g does not match formal parameter", + "found : {prop: (number|string|undefined)}", + "required: {prop: (string|undefined)}", + "missing : []", + "mismatch: [prop]"))); } public void testRecordType5() throws Exception { @@ -14715,7 +14738,9 @@ public void testTemplatizedStructuralMismatch1() throws Exception { LINE_JOINER.join( "actual parameter 1 of f does not match formal parameter", "found : Foo", - "required: WithPropT")); + "required: WithPropT", + "missing : []", + "mismatch: [prop]")); } public void testTemplatizedStructuralMismatch2() throws Exception { @@ -14731,7 +14756,9 @@ public void testTemplatizedStructuralMismatch2() throws Exception { LINE_JOINER.join( "actual parameter 1 of f does not match formal parameter", "found : Foo", - "required: WithPropT")); + "required: WithPropT", + "missing : []", + "mismatch: [prop]")); } public void testTemplatizedStructuralMismatch3() throws Exception { @@ -14753,7 +14780,9 @@ public void testTemplatizedStructuralMismatch3() throws Exception { LINE_JOINER.join( "actual parameter 1 of f does not match formal parameter", "found : Foo", - "required: WithPropT")); + "required: WithPropT", + "missing : []", + "mismatch: [prop]")); } public void testTemplatizedStructuralMismatch4() throws Exception { @@ -14776,7 +14805,9 @@ public void testTemplatizedStructuralMismatch4() throws Exception { LINE_JOINER.join( "actual parameter 1 of f does not match formal parameter", "found : Foo", - "required: WithProp")); + "required: WithProp", + "missing : []", + "mismatch: [prop]")); } public void testTemplatizedStructuralMismatchNotFound() throws Exception { @@ -15960,7 +15991,9 @@ public void testStructuralInterfaceMatching37() throws Exception { LINE_JOINER.join( "assignment", "found : {fun: function ((I7|null)): (C7|null), prop: {prop: (C7|null)}}", - "required: {fun: function ((C7|null)): (I7|null), prop: {prop: (I7|null)}}")); + "required: {fun: function ((C7|null)): (I7|null), prop: {prop: (I7|null)}}", + "missing : []", + "mismatch: [fun,prop]")); } /** @@ -16339,7 +16372,10 @@ public void testRecordWithUnknownProperty() throws Exception { LINE_JOINER.join( "initializing variable", "found : Foo", - "required: {str: string, unknown: ?}")); + "required: {str: string, unknown: ?}", + "missing : [unknown]", + "mismatch: []" + )); } public void testRecordWithOptionalUnknownProperty() throws Exception { @@ -16361,7 +16397,9 @@ public void testRecordWithTopProperty() throws Exception { LINE_JOINER.join( "initializing variable", "found : Foo", - "required: {str: string, top: *}")); + "required: {str: string, top: *}", + "missing : [top]", + "mismatch: []")); } public void testStructuralInterfaceWithOptionalProperty() throws Exception { @@ -16391,7 +16429,9 @@ public void testStructuralInterfaceWithUnknownProperty() throws Exception { LINE_JOINER.join( "initializing variable", "found : Foo", - "required: Rec")); + "required: Rec", + "missing : [unknown]", + "mismatch: []")); } public void testStructuralInterfaceWithOptionalUnknownProperty() throws Exception { @@ -16429,7 +16469,9 @@ public void testStructuralInterfaceWithTopProperty() throws Exception { LINE_JOINER.join( "initializing variable", "found : Foo", - "required: Rec")); + "required: Rec", + "missing : [top]", + "mismatch: []")); } public void testStructuralInterfaceCycleDoesntCrash() throws Exception { @@ -16478,7 +16520,9 @@ public void testStructuralInterfacesMatchOwnProperties2() throws Exception { LINE_JOINER.join( "initializing variable", "found : Foo", - "required: WithProp")); + "required: WithProp", + "missing : [prop]", + "mismatch: []")); } public void testStructuralInterfacesMatchOwnProperties3() throws Exception { @@ -16495,7 +16539,9 @@ public void testStructuralInterfacesMatchOwnProperties3() throws Exception { LINE_JOINER.join( "initializing variable", "found : Foo", - "required: WithProp")); + "required: WithProp", + "missing : []", + "mismatch: [prop]")); } @@ -16522,7 +16568,9 @@ public void testStructuralInterfacesMatchFunctionNamespace2() throws Exception { LINE_JOINER.join( "initializing variable", "found : function (): undefined", - "required: WithProp")); + "required: WithProp", + "missing : [prop]", + "mismatch: []")); } public void testStructuralInterfacesMatchFunctionNamespace3() throws Exception { @@ -16537,7 +16585,9 @@ public void testStructuralInterfacesMatchFunctionNamespace3() throws Exception { LINE_JOINER.join( "initializing variable", "found : function (): undefined", - "required: WithProp")); + "required: WithProp", + "missing : []", + "mismatch: [prop]")); } public void testRecursiveTemplatizedStructuralInterface() throws Exception { @@ -16626,7 +16676,9 @@ public void testCovarianceForRecordType4() throws Exception { LINE_JOINER.join( "assignment", "found : {prop: (C2|null)}", - "required: {prop: (C|null), prop2: (C|null)}")); + "required: {prop: (C|null), prop2: (C|null)}", + "missing : [prop2]", + "mismatch: []")); } public void testCovarianceForRecordType5() throws Exception { @@ -16645,7 +16697,9 @@ public void testCovarianceForRecordType5() throws Exception { LINE_JOINER.join( "assignment", "found : {prop: (C2|null)}", - "required: {prop: (C|null)}")); + "required: {prop: (C|null)}", + "missing : []", + "mismatch: [prop]")); } public void testCovarianceForRecordType6() throws Exception { @@ -16664,7 +16718,9 @@ public void testCovarianceForRecordType6() throws Exception { LINE_JOINER.join( "assignment", "found : {prop: (C|null)}", - "required: {prop: (C2|null)}")); + "required: {prop: (C2|null)}", + "missing : []", + "mismatch: [prop]")); } public void testCovarianceForRecordType7() throws Exception { @@ -16683,7 +16739,9 @@ public void testCovarianceForRecordType7() throws Exception { LINE_JOINER.join( "assignment", "found : {prop: (C2|null), prop2: (C|null)}", - "required: {prop: (C2|null), prop2: (C2|null)}")); + "required: {prop: (C2|null), prop2: (C2|null)}", + "missing : []", + "mismatch: [prop2]")); } public void testCovarianceForRecordType8() throws Exception { @@ -16743,7 +16801,9 @@ public void testCovarianceForRecordType10() throws Exception { LINE_JOINER.join( "assignment", "found : Foo", - "required: {x: Foo}")); + "required: {x: Foo}", + "missing : []", + "mismatch: [x]")); } public void testCovarianceForRecordType11() throws Exception { @@ -17311,7 +17371,9 @@ public void testCovarianceForRecordType30() throws Exception { LINE_JOINER.join( "assignment", "found : {prop1: (A|null|undefined)}", - "required: {prop1: (A|null)}")); + "required: {prop1: (A|null)}", + "missing : []", + "mismatch: [prop1]")); } public void testCovarianceForRecordType31() throws Exception { @@ -17333,7 +17395,9 @@ public void testCovarianceForRecordType31() throws Exception { LINE_JOINER.join( "assignment", "found : {prop1: (A|null|undefined)}", - "required: {prop1: (A|null)}")); + "required: {prop1: (A|null)}", + "missing : []", + "mismatch: [prop1]")); } public void testDuplicateVariableDefinition1() throws Exception { diff --git a/test/com/google/javascript/jscomp/TypeValidatorTest.java b/test/com/google/javascript/jscomp/TypeValidatorTest.java index b33315e225b..af47d833a03 100644 --- a/test/com/google/javascript/jscomp/TypeValidatorTest.java +++ b/test/com/google/javascript/jscomp/TypeValidatorTest.java @@ -127,7 +127,9 @@ public void testFunctionMismatchMediumLengthTypes() throws Exception { " c: string,", " d: string,", " e: string", - "}")); + "}", + "missing : []", + "mismatch: [e]")); } /** @@ -150,7 +152,9 @@ public void testFunctionMismatchLongTypes() throws Exception { "found : {a: string, b: string, c: string, d: string, e: string, f: string," + " g: string, h: string, i: string, j: string, k: (number|string)}", "required: {a: string, b: string, c: string, d: string, e: string, f: string," - + " g: string, h: string, i: string, j: string, k: string}")); + + " g: string, h: string, i: string, j: string, k: string}", + "missing : []", + "mismatch: [k]")); } /** @@ -176,7 +180,9 @@ public void testFunctionMismatchTypedef() throws Exception { "found : {a: string, b: string, c: string, d: string, e: string, f: string," + " g: string, h: string, i: string, j: string, k: (number|string)}", "required: {a: string, b: string, c: string, d: string, e: string, f: string," - + " g: string, h: string, i: string, j: string, k: string}")); + + " g: string, h: string, i: string, j: string, k: string}", + "missing : []", + "mismatch: [k]")); } public void testNullUndefined() {