diff --git a/src/com/google/javascript/jscomp/FunctionTypeBuilder.java b/src/com/google/javascript/jscomp/FunctionTypeBuilder.java index 1ce8e66f110..51182fd47b6 100644 --- a/src/com/google/javascript/jscomp/FunctionTypeBuilder.java +++ b/src/com/google/javascript/jscomp/FunctionTypeBuilder.java @@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Strings.nullToEmpty; import static com.google.javascript.jscomp.TypeCheck.BAD_IMPLEMENTED_TYPE; import static com.google.javascript.rhino.jstype.JSTypeNative.FUNCTION_FUNCTION_TYPE; @@ -529,8 +530,8 @@ FunctionTypeBuilder inferParameterTypes(JSDocInfo info) { } /** Infer the parameter types from the list of parameter names and the JSDoc info. */ - FunctionTypeBuilder inferParameterTypes(@Nullable Node argsParent, @Nullable JSDocInfo info) { - if (argsParent == null) { + FunctionTypeBuilder inferParameterTypes(@Nullable Node paramsParent, @Nullable JSDocInfo info) { + if (paramsParent == null) { if (info == null) { return this; } else { @@ -549,30 +550,41 @@ FunctionTypeBuilder inferParameterTypes(@Nullable Node argsParent, @Nullable JSD Set allJsDocParams = (info == null) ? new HashSet<>() : new HashSet<>(info.getParameterNames()); boolean isVarArgs = false; - for (Node arg : argsParent.children()) { + int paramIndex = 0; + for (Node param : paramsParent.children()) { boolean isOptionalParam = false; - if (arg.isDefaultValue()) { + if (param.isDefaultValue()) { // The first child is the actual positional parameter - arg = checkNotNull(arg.getFirstChild(), arg); + param = checkNotNull(param.getFirstChild(), param); } - if (arg.isRest()) { + if (param.isRest()) { isVarArgs = true; - arg = arg.getOnlyChild(); + param = param.getOnlyChild(); } else { - isVarArgs = isVarArgsParameter(arg, info); - isOptionalParam = isOptionalParameter(arg, info); + isVarArgs = isVarArgsParameter(param, info); + isOptionalParam = isOptionalParameter(param, info); } - String argumentName = arg.getString(); - allJsDocParams.remove(argumentName); + String paramName = null; + if (param.isName()) { + paramName = param.getString(); + } else { + checkState(param.isDestructuringPattern()); + // Right now, the only way to match a JSDoc param to a destructuring parameter is through + // ordering the JSDoc parameters. So the third formal parameter will correspond to the + // third JSDoc parameter. + if (info != null) { + paramName = info.getParameterNameAt(paramIndex); + } + } + allJsDocParams.remove(paramName); // type from JSDocInfo JSType parameterType = null; - if (info != null && info.hasParameterType(argumentName)) { - parameterType = - info.getParameterType(argumentName).evaluate(templateScope, typeRegistry); - } else if (arg.getJSDocInfo() != null && arg.getJSDocInfo().hasType()) { - JSTypeExpression parameterTypeExpression = arg.getJSDocInfo().getType(); + if (info != null && info.hasParameterType(paramName)) { + parameterType = info.getParameterType(paramName).evaluate(templateScope, typeRegistry); + } else if (param.getJSDocInfo() != null && param.getJSDocInfo().hasType()) { + JSTypeExpression parameterTypeExpression = param.getJSDocInfo().getType(); parameterType = parameterTypeExpression.evaluate(templateScope, typeRegistry); isOptionalParam = parameterTypeExpression.isOptionalArg(); isVarArgs = parameterTypeExpression.isVarArgs(); @@ -593,8 +605,8 @@ FunctionTypeBuilder inferParameterTypes(@Nullable Node argsParent, @Nullable JSD if (oldParameterType != null) { oldParameterType = oldParameterType.getNext(); } + paramIndex++; } - // Copy over any old parameters that aren't in the param list. if (!isVarArgs) { while (oldParameterType != null && !isVarArgs) { @@ -669,6 +681,9 @@ private void setConstructorTemplateTypeNames(List templates, @Null * @return Whether the given param is an optional param. */ private boolean isOptionalParameter(Node param, @Nullable JSDocInfo info) { + if (param.isDestructuringPattern()) { + return false; + } if (codingConvention.isOptionalParameter(param)) { return true; } @@ -684,6 +699,9 @@ private boolean isOptionalParameter(Node param, @Nullable JSDocInfo info) { */ private boolean isVarArgsParameter( Node param, @Nullable JSDocInfo info) { + if (param.isDestructuringPattern()) { + return false; + } if (codingConvention.isVarArgsParameter(param)) { return true; } diff --git a/src/com/google/javascript/jscomp/TypeInference.java b/src/com/google/javascript/jscomp/TypeInference.java index 7039d9b401f..befccab9b45 100644 --- a/src/com/google/javascript/jscomp/TypeInference.java +++ b/src/com/google/javascript/jscomp/TypeInference.java @@ -167,7 +167,10 @@ private void inferParameters(TypedScope functionScope) { // set astParameter = restParamName astParameter = astParameter.getOnlyChild(); } - checkState(astParameter.isName(), astParameter); + if (!astParameter.isName()) { + // TODO(lharker): support destructuring parameters + continue; + } TypedVar var = functionScope.getVar(astParameter.getString()); checkNotNull(var); if (var.isTypeInferred() && var.getType() == unknownType) { diff --git a/src/com/google/javascript/jscomp/TypedScopeCreator.java b/src/com/google/javascript/jscomp/TypedScopeCreator.java index 981c2252cc6..ab6aed9c783 100644 --- a/src/com/google/javascript/jscomp/TypedScopeCreator.java +++ b/src/com/google/javascript/jscomp/TypedScopeCreator.java @@ -2423,6 +2423,11 @@ void declareArguments() { if (astParameter.isDefaultValue()) { astParameter = astParameter.getFirstChild(); } + + if (!astParameter.isName()) { + // TODO(lharker): support destructuring ast parameters + continue; + } JSType paramType = jsDocParameter == null ? unknownType : jsDocParameter.getJSType(); boolean inferred = paramType == null || paramType.equals(unknownType); if (isRestParameter) { diff --git a/test/com/google/javascript/jscomp/TypedScopeCreatorTest.java b/test/com/google/javascript/jscomp/TypedScopeCreatorTest.java index be37d4c8d9e..04644f88771 100644 --- a/test/com/google/javascript/jscomp/TypedScopeCreatorTest.java +++ b/test/com/google/javascript/jscomp/TypedScopeCreatorTest.java @@ -408,6 +408,146 @@ public void testDefaultParameterFullJSDoc_setToUndefined() { .toStringIsEqualTo("(string|undefined)"); } + public void testDestructuringParameterWithNoJSDoc() { + testSame("function f([x, y], {z}) {}"); + + TypedVar fVar = checkNotNull(globalScope.getVar("f")); + assertType(fVar.getType()).toStringIsEqualTo("function(?, ?): undefined"); + assertFalse(fVar.isTypeInferred()); + } + + public void testArrayPatternParameterWithFullJSDoc() { + testSame( + lines( + "/**", + " * @param {string} x", + " * @param {!Iterable} arr", + " */", + "function f(x, [y]) {}")); + + TypedVar fVar = checkNotNull(globalScope.getVar("f")); + assertType(fVar.getType()).toStringIsEqualTo("function(string, Iterable): undefined"); + assertFalse(fVar.isTypeInferred()); + + TypedVar xVar = checkNotNull(lastFunctionScope.getVar("x")); + assertType(xVar.getType()).toStringIsEqualTo("string"); + assertFalse(xVar.isTypeInferred()); + + // TODO(b/77597706): declare y in the function scope + TypedVar yVar = lastFunctionScope.getVar("y"); + assertNull(yVar); + // assertType(yVar.getType()).toStringIsEqualTo("number"); + // assertFalse(yVar.isTypeInferred()); + } + + public void testArrayPatternParameterWithRestWithFullJSDoc() { + testSame("/** @param {!Iterable} arr */ function f([x, ...y]) {}"); + + TypedVar fVar = checkNotNull(globalScope.getVar("f")); + assertType(fVar.getType()).toStringIsEqualTo("function(Iterable): undefined"); + assertFalse(fVar.isTypeInferred()); + } + + public void testObjectPatternParameterWithFullJSDoc() { + testSame("/** @param {{a: string, b: number}} arr */ function f({a, b}) {}"); + + TypedVar fVar = checkNotNull(globalScope.getVar("f")); + assertType(fVar.getType()).toStringIsEqualTo("function({a: string, b: number}): undefined"); + assertFalse(fVar.isTypeInferred()); + } + + public void testObjectPatternParameterWithUnknownPropertyWithFullJSDoc() { + testSame("/** @param {{a: string}} arr */ function f({a, b}) {}"); + + TypedVar fVar = checkNotNull(globalScope.getVar("f")); + assertType(fVar.getType()).toStringIsEqualTo("function({a: string}): undefined"); + assertFalse(fVar.isTypeInferred()); + } + + public void testNestedPatternParameterWithFullJSDoc() { + testSame( + lines( + "/**", + " * @param {string} x", + " * @param {{a: !Iterable}} obj", + " * @param {!Iterable<{z: null}>} arr", + " */", + "function f(x, {a: [y]}, [{z}]) {}")); + + TypedVar fVar = checkNotNull(globalScope.getVar("f")); + assertType(fVar.getType()) + .toStringIsEqualTo( + "function(string, {a: Iterable}, Iterable<{z: null}>): undefined"); + assertFalse(fVar.isTypeInferred()); + + TypedVar xVar = checkNotNull(lastFunctionScope.getVar("x")); + assertType(xVar.getType()).toStringIsEqualTo("string"); + assertFalse(xVar.isTypeInferred()); + } + + public void testObjectPatternParameterWithComputedPropertyWithFullJSDoc() { + // TODO(lharker): re-enable type info validation. it's currently failing on the computed + // property because TypeInference doesn't traverse the destructuring pattern. + disableTypeInfoValidation(); + testSame( + lines( + "/**", + " * @param {string} x", + " * @param {!Object} arr", + " */", + "function f(x, {['foobar' + 3]: a}) {}")); + + TypedVar xVar = checkNotNull(lastFunctionScope.getVar("x")); + assertType(xVar.getType()).toStringIsEqualTo("string"); + assertFalse(xVar.isTypeInferred()); + } + + public void testOutOfOrderJSDocForDestructuringParameter() { + // Even though regular JSDoc parameters can be out of order, there's currently no way for + // putting arbitrary orders on destructuring parameters. + testWarning( + lines( + "/**", + " * @param {!Iterable} arr", + " * @param {string} x", + " */", + "function f(x, [y, z]) {}"), + FunctionTypeBuilder.INEXISTENT_PARAM); + + TypedVar fVar = checkNotNull(globalScope.getVar("f")); + // TODO(b/77597706): it would make more sense for this to be function(string, ?) instead + assertType(fVar.getType()).toStringIsEqualTo("function(string, string): undefined"); + assertFalse(fVar.isTypeInferred()); + } + + public void testObjectPatternParameterWithInlineJSDoc() { + testSame("function f({/** number */ x}) {}"); + + // TODO(lharker): infer that f takes {x: number} + TypedVar fVar = checkNotNull(globalScope.getVar("f")); + assertType(fVar.getType()).toStringIsEqualTo("function(?): undefined"); + assertFalse(fVar.isTypeInferred()); + } + + public void testArrayPatternParameterWithInlineJSDoc() { + testSame("function f([/** number */ x]) {}"); + + // TODO(lharker): either forbid this case or infer that f takes an !Iterable + TypedVar fVar = checkNotNull(globalScope.getVar("f")); + assertType(fVar.getType()).toStringIsEqualTo("function(?): undefined"); + assertFalse(fVar.isTypeInferred()); + } + + public void testArrayPatternParametersWithDifferingInlineJSDoc() { + testSame("function f([/** number */ x, /** string */ y]) {}"); + + // TODO(lharker): forbid this case, as there's not a good way to type the function without + // having tuple types. + TypedVar fVar = checkNotNull(globalScope.getVar("f")); + assertType(fVar.getType()).toStringIsEqualTo("function(?): undefined"); + assertFalse(fVar.isTypeInferred()); + } + public void testStubProperty() { testSame("function Foo() {}; Foo.bar;"); ObjectType foo = (ObjectType) globalScope.getVar("Foo").getType(); @@ -3032,3 +3172,4 @@ private ObjectType getNativeObjectType(JSTypeNative type) { return (ObjectType) registry.getNativeType(type); } } +