From cc198bd128856930cb472397c7e066abad095de8 Mon Sep 17 00:00:00 2001 From: lharker Date: Tue, 20 Mar 2018 14:50:30 -0700 Subject: [PATCH] Typecheck returns with children in generators. We still don't typecheck returns with no children: /** @return {!Generator} */ function *gen() { return; } but now raise an error on returns with children that don't typecheck, like this: /** @return {!Generator} */ function *gen() { return 'foobar'; } ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=189815295 --- .../google/javascript/jscomp/TypeCheck.java | 62 ++++++++++++------- .../jscomp/TypeCheckNoTranspileTest.java | 7 ++- 2 files changed, 43 insertions(+), 26 deletions(-) diff --git a/src/com/google/javascript/jscomp/TypeCheck.java b/src/com/google/javascript/jscomp/TypeCheck.java index 0592e950d2e..a756063eb76 100644 --- a/src/com/google/javascript/jscomp/TypeCheck.java +++ b/src/com/google/javascript/jscomp/TypeCheck.java @@ -2080,17 +2080,17 @@ private void visitParameterList(NodeTraversal t, Node call, */ private void visitReturn(NodeTraversal t, Node n) { Node enclosingFunction = t.getEnclosingFunction(); + if (enclosingFunction.isGeneratorFunction() && !n.hasChildren()) { + // Allow "return;" in a generator function, even if it's not the declared return type. + // e.g. Don't warn for a generator function with JSDoc "@return {!Generator}" and + // a "return;" in the fn body, even though "undefined" does not match "number". + return; + } + JSType jsType = getJSType(enclosingFunction); if (jsType.isFunctionType()) { FunctionType functionType = jsType.toMaybeFunctionType(); - if (enclosingFunction.isGeneratorFunction()) { - // Allow "return;" in a generator function, even if it's not the declared return type. - // e.g. Don't warn for a generator function with JSDoc "@return {!Generator}" and - // a "return;" in the fn body, even though "undefined" does not match "number". - // TODO(b/73966409): Typecheck "return [expr]" in generator fns, just not "return;" - return; - } JSType returnType = functionType.getReturnType(); @@ -2098,6 +2098,10 @@ private void visitReturn(NodeTraversal t, Node n) { // (it's a void function) if (returnType == null) { returnType = getNativeType(VOID_TYPE); + } else if (enclosingFunction.isGeneratorFunction()) { + // Unwrap the template variable from a generator function's declared return type. + // e.g. if returnType is "Generator", make it just "string". + returnType = getTemplateTypeOfGenerator(returnType); } // fetching the returned value's type @@ -2123,23 +2127,8 @@ private void visitYield(NodeTraversal t, Node n) { JSType declaredYieldType = getNativeType(UNKNOWN_TYPE); if (jsType.isFunctionType()) { FunctionType functionType = jsType.toMaybeFunctionType(); - ObjectType dereferencedReturnType = functionType.getReturnType().dereference(); - if (dereferencedReturnType != null) { - TemplateTypeMap templateTypeMap = dereferencedReturnType.getTemplateTypeMap(); - if (templateTypeMap.hasTemplateKey(typeRegistry.getIterableTemplate())) { - // Generator JSDoc says - // @return {!Iterable} - // or - // @return {!Generator} - declaredYieldType = - templateTypeMap.getResolvedTemplateType(typeRegistry.getIterableTemplate()); - } else if (templateTypeMap.hasTemplateKey(typeRegistry.getIteratorTemplate())) { - // Generator JSDoc says - // @return {!Iterator} - declaredYieldType = - templateTypeMap.getResolvedTemplateType(typeRegistry.getIteratorTemplate()); - } - } + JSType returnType = functionType.getReturnType(); + declaredYieldType = getTemplateTypeOfGenerator(returnType); } // fetching the yielded value's type @@ -2174,6 +2163,31 @@ private void visitYield(NodeTraversal t, Node n) { "Yielded type does not match declared return type."); } + /** + * Returns the given type's resolved template type corresponding to the corresponding to the + * Generator, Iterable or Iterator template key if possible. + * + * If the given type is not an Iterator or Iterable, returns the unknown type.. + */ + private JSType getTemplateTypeOfGenerator(JSType generator) { + ObjectType dereferencedType = generator.dereference(); + if (dereferencedType != null) { + TemplateTypeMap templateTypeMap = dereferencedType.getTemplateTypeMap(); + if (templateTypeMap.hasTemplateKey(typeRegistry.getIterableTemplate())) { + // Generator JSDoc says + // @return {!Iterable} + // or + // @return {!Generator} + return templateTypeMap.getResolvedTemplateType(typeRegistry.getIterableTemplate()); + } else if (templateTypeMap.hasTemplateKey(typeRegistry.getIteratorTemplate())) { + // Generator JSDoc says + // @return {!Iterator} + return templateTypeMap.getResolvedTemplateType(typeRegistry.getIteratorTemplate()); + } + } + return getNativeType(UNKNOWN_TYPE); + } + /** * This function unifies the type checking involved in the core binary * operators and the corresponding assignment operators. The representation diff --git a/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java b/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java index fb75e58ca96..613ad497a16 100644 --- a/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java +++ b/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java @@ -359,8 +359,11 @@ public void testGenerator_return1() { } public void testGenerator_return2() { - // TODO(b/73966409): Emit a type mismatch warning here. - testTypes("/** @return {!Generator} */ function *gen() { return 1; }"); + testTypes("/** @return {!Generator} */ function *gen() { return 1; }", + lines( + "inconsistent return type", + "found : number", + "required: string")); } public void testGenerator_return3() {