Skip to content

Commit

Permalink
Start warning for type mismatches in inline parameter annotations in …
Browse files Browse the repository at this point in the history
…callbacks

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=231810660
  • Loading branch information
lauraharker authored and blickly committed Feb 1, 2019
1 parent bea483e commit 1836f5b
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 9 deletions.
25 changes: 19 additions & 6 deletions src/com/google/javascript/jscomp/TypeInference.java
Expand Up @@ -1636,8 +1636,14 @@ private void updateBind(Node n) {
} }


/** /**
* For functions with function parameters, type inference will set the type of a function literal * Performs a limited back-inference on function arguments based on the expected parameter types.
* argument from the function parameter type. *
* <p>Currently this only does back-inference in two cases: it infers the type of function literal
* arguments and adds inferred properties to inferred object-typed arguments.
*
* <p>For example: if someone calls `Promise<string>.prototype.then` with `(result) => ...` then
* we infer that the type of the arrow function is `function(string): ?`, and inside the arrow
* function body we know that `result` is a string.
*/ */
private void updateTypeOfArguments(Node n, FunctionType fnType) { private void updateTypeOfArguments(Node n, FunctionType fnType) {
checkState(NodeUtil.isInvocation(n), n); checkState(NodeUtil.isInvocation(n), n);
Expand Down Expand Up @@ -1690,16 +1696,23 @@ private void updateTypeOfArguments(Node n, FunctionType fnType) {
&& iArgumentType.isFunctionType()) { && iArgumentType.isFunctionType()) {
FunctionType argFnType = iArgumentType.toMaybeFunctionType(); FunctionType argFnType = iArgumentType.toMaybeFunctionType();
JSDocInfo argJsdoc = iArgument.getJSDocInfo(); JSDocInfo argJsdoc = iArgument.getJSDocInfo();
boolean declared = argJsdoc != null && argJsdoc.containsDeclaration(); // Treat the parameter & return types of the function as 'declared' if the function has
// JSDoc with type annotations, or a parameter has inline JSDoc.
// Note that this does not distinguish between cases where all parameters have JSDoc vs
// only one parameter has JSDoc.
boolean declared =
(argJsdoc != null && argJsdoc.containsDeclaration())
|| NodeUtil.functionHasInlineJsdocs(iArgument);
iArgument.setJSType(matchFunction(restrictedParameter, argFnType, declared)); iArgument.setJSType(matchFunction(restrictedParameter, argFnType, declared));
} }
} }
} }


/** /**
* Take the current function type, and try to match the expected function * Take the current function type, and try to match the expected function type. This is a form of
* type. This is a form of backwards-inference, like record-type constraint * backwards-inference, like record-type constraint matching.
* matching. *
* @param declared Whether the given function type is user-provided as opposed to inferred
*/ */
private FunctionType matchFunction( private FunctionType matchFunction(
FunctionType expectedType, FunctionType currentType, boolean declared) { FunctionType expectedType, FunctionType currentType, boolean declared) {
Expand Down
43 changes: 40 additions & 3 deletions test/com/google/javascript/jscomp/TypeCheckTest.java
Expand Up @@ -7715,13 +7715,50 @@ public void testInferredParam8() {
} }


@Test @Test
public void testInferredParam9() { public void testFunctionLiteralParamWithInlineJSDocNotInferred() {
// TODO(b/77731069) This should warn "parameter 1 of f does not match formal parameter" testTypes(
lines(
"/** @param {function(string)} x */",
"function f(x) {}",
"f(function(/** number */ x) {});"),
lines(
"actual parameter 1 of f does not match formal parameter",
"found : function(number): undefined",
"required: function(string): ?"));
}

@Test
public void testFunctionLiteralParamWithInlineJSDocNotInferredWithTwoParams() {
testTypes(
lines(
"/** @param {function(string, number)} x */",
"function f(x) {}",
"f((/** string */ str, num) => {",
// TODO(b/123583824): this should be a type mismatch warning.
// found : number
// expected: string
// but the JSDoc on `str` blocks inference of `num`.
" const /** string */ newStr = num;",
"});"));
}

@Test
public void testJSDocOnNoParensArrowFnParameterIsIgnored() {
// This case is to document a potential bit of confusion: what happens when writing
// inline-like JSDoc on a 'naked' arrow function parameter (no parentheses)
// The actual behavior is that the JSDoc does nothing: this is equivalent to writing
// /** number */ function(x) { ...
testTypes( testTypes(
lines( lines(
"/** @param {function(string)} callback */", "/** @param {function(string)} callback */",
"function foo(callback) {}", "function foo(callback) {}",
"foo(function(/** number */ x) {});")); // The `/** number */` JSDoc is attached to the entire arrow function, not the
// parameter `x`
"foo(/** number */ x => { var /** number */ y = x; });"),
lines(
"initializing variable",
"found : string", // type of "x" is inferred to be string
"required: number"));
} }


@Test @Test
Expand Down

0 comments on commit 1836f5b

Please sign in to comment.