Skip to content

Commit

Permalink
Support async functions in CheckMissingReturn
Browse files Browse the repository at this point in the history
For async functions, if the declared type in JSDoc is Promise<T> or IThenable<T>, we will use T as the declared return type. Otherwise we treat the declared return type as unknown.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=202339849
  • Loading branch information
lauraharker authored and brad4d committed Jun 28, 2018
1 parent 3edafa4 commit 0e46738
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 2 deletions.
16 changes: 14 additions & 2 deletions src/com/google/javascript/jscomp/CheckMissingReturn.java
Expand Up @@ -16,6 +16,7 @@


package com.google.javascript.jscomp; package com.google.javascript.jscomp;



import com.google.common.base.Predicate; import com.google.common.base.Predicate;
import com.google.javascript.jscomp.ControlFlowGraph.Branch; import com.google.javascript.jscomp.ControlFlowGraph.Branch;
import com.google.javascript.jscomp.NodeTraversal.ScopedCallback; import com.google.javascript.jscomp.NodeTraversal.ScopedCallback;
Expand All @@ -25,6 +26,7 @@
import com.google.javascript.rhino.jstype.JSType; import com.google.javascript.rhino.jstype.JSType;
import com.google.javascript.rhino.jstype.JSTypeNative; import com.google.javascript.rhino.jstype.JSTypeNative;
import com.google.javascript.rhino.jstype.TernaryValue; import com.google.javascript.rhino.jstype.TernaryValue;
import javax.annotation.Nullable;


/** /**
* Checks functions for missing return statements. Return statements are only * Checks functions for missing return statements. Return statements are only
Expand Down Expand Up @@ -91,7 +93,7 @@ public boolean apply(DiGraphEdge<Node, ControlFlowGraph.Branch> input) {
@Override @Override
public void enterScope(NodeTraversal t) { public void enterScope(NodeTraversal t) {
Node n = t.getScopeRoot(); Node n = t.getScopeRoot();
JSType returnType = explicitReturnExpected(n); JSType returnType = getExplicitReturnTypeIfExpected(n);


if (returnType == null) { if (returnType == null) {
// No return value is expected, so nothing to check. // No return value is expected, so nothing to check.
Expand Down Expand Up @@ -172,7 +174,12 @@ public void visit(NodeTraversal t, Node n, Node parent) {
* *
* @return If a return type is expected, returns it. Otherwise, returns null. * @return If a return type is expected, returns it. Otherwise, returns null.
*/ */
private JSType explicitReturnExpected(Node scopeRoot) { @Nullable
private JSType getExplicitReturnTypeIfExpected(Node scopeRoot) {
if (!scopeRoot.isFunction()) {
// Nothing to do in a global/module/block scope.
return null;
}
FunctionType scopeType = JSType.toMaybeFunctionType(scopeRoot.getJSType()); FunctionType scopeType = JSType.toMaybeFunctionType(scopeRoot.getJSType());


if (scopeType == null) { if (scopeType == null) {
Expand All @@ -193,6 +200,11 @@ private JSType explicitReturnExpected(Node scopeRoot) {
return null; return null;
} }


if (scopeRoot.isAsyncFunction()) {
// Unwrap the declared return type (e.g. "!Promise<number>" becomes "number")
returnType = Promises.getTemplateTypeOfThenable(compiler.getTypeRegistry(), returnType);
}

if (!isVoidOrUnknown(returnType)) { if (!isVoidOrUnknown(returnType)) {
return returnType; return returnType;
} }
Expand Down
25 changes: 25 additions & 0 deletions test/com/google/javascript/jscomp/CheckMissingReturnTest.java
Expand Up @@ -332,4 +332,29 @@ public void testGeneratorFunctionDoesntWarn() {
"/** @return {!Object} */", // Return type more vague than Generator is also OK "/** @return {!Object} */", // Return type more vague than Generator is also OK
"function *gen() {}")); "function *gen() {}"));
} }

public void testAsyncFunction_noJSDoc() {
setAcceptedLanguage(LanguageMode.ECMASCRIPT_2017);
// Note: we add the alert because CheckMissingReturn never warns on functions with empty bodies.
testNoWarning("async function foo() { alert(1); }");
}

public void testAsyncFunction_returnsUndefined() {
setAcceptedLanguage(LanguageMode.ECMASCRIPT_2017);
testNoWarning("/** @return {!Promise<undefined>} */ async function foo() { alert(1); }");
}

public void testAsyncFunction_returnsUnknown() {
setAcceptedLanguage(LanguageMode.ECMASCRIPT_2017);
testNoWarning("/** @return {!Promise<?>} */ async function foo() { alert(1); }");
}

public void testAsyncFunction_returnsNumber() {
setAcceptedLanguage(LanguageMode.ECMASCRIPT_2017);
testNoWarning("/** @return {!Promise<number>} */ async function foo() { return 1; }");

testWarning(
"/** @return {!Promise<string>} */ async function foo() { alert(1); }",
CheckMissingReturn.MISSING_RETURN_STATEMENT);
}
} }

0 comments on commit 0e46738

Please sign in to comment.