diff --git a/src/com/google/javascript/jscomp/TypedScopeCreator.java b/src/com/google/javascript/jscomp/TypedScopeCreator.java index ccb9d868ae9..f52d2ecb0e6 100644 --- a/src/com/google/javascript/jscomp/TypedScopeCreator.java +++ b/src/com/google/javascript/jscomp/TypedScopeCreator.java @@ -224,6 +224,21 @@ void resolve() { } } + /** Stores the type and qualified name for a destructuring rvalue, which has no AST node */ + private static class RValueInfo { + @Nullable final JSType type; + @Nullable final String qualifiedName; + + private RValueInfo(JSType type, String qualifiedName) { + this.type = type; + this.qualifiedName = qualifiedName; + } + + private static RValueInfo empty() { + return new RValueInfo(null, null); + } + } + TypedScopeCreator(AbstractCompiler compiler) { this(compiler, compiler.getCodingConvention()); } @@ -860,15 +875,24 @@ void defineVarChild(JSDocInfo declarationInfo, Node child, TypedScope scope) { defineDestructuringPatternInVarDeclaration( pattern, scope, - // Note that value will be null if we are in an enhanced for loop - // for (const {x, y} of data) { - () -> value != null ? getDeclaredRValueType(/* lValue= */ null, value) : unknownType); + () -> + // Note that value will be null if we are in an enhanced for loop + // for (const {x, y} of data) { + value != null + ? new RValueInfo( + getDeclaredRValueType(/* lValue= */ null, value), value.getQualifiedName()) + : new RValueInfo(unknownType, /* qualifiedName= */ null)); } } - @Nullable - private JSType inferTypeForDestructuredTarget( - DestructuredTarget target, Supplier patternTypeSupplier) { + /** + * Returns information about the qualified name and type of the target, if it exists. + * + *

Never returns null, but will return an RValueInfo with null `type` and `qualifiedName` + * slots. + */ + private RValueInfo inferTypeForDestructuredTarget( + DestructuredTarget target, Supplier patternTypeSupplier) { // Currently we only do type inference for string key nodes in object patterns here, to // handle aliasing types. e.g // const {Foo} = ns; @@ -878,29 +902,30 @@ private JSType inferTypeForDestructuredTarget( // and only return a non-null type here if we are accessing a declared property on a known // type. if (!target.hasStringKey() || target.hasDefaultValue()) { - return null; + return RValueInfo.empty(); } - JSType patternType = patternTypeSupplier.get(); - if (patternType == null || patternType.isUnknownType() || !patternType.isObjectType()) { - // TypeCheck should emit an error later if this is not an object type. - return null; - } - ObjectType patternObjectType = patternType.toMaybeObjectType(); + RValueInfo rvalue = patternTypeSupplier.get(); + JSType patternType = rvalue.type; String propertyName = target.getStringKey().getString(); - if (patternObjectType.hasProperty(propertyName) - && !patternObjectType.isPropertyTypeInferred(propertyName)) { - return patternObjectType.getPropertyType(propertyName); + String qualifiedName = + rvalue.qualifiedName != null ? rvalue.qualifiedName + "." + propertyName : null; + if (patternType == null || patternType.isUnknownType()) { + return new RValueInfo(null, qualifiedName); } - return null; + if (patternType.hasProperty(propertyName)) { + JSType type = patternType.findPropertyType(propertyName); + return new RValueInfo(type, qualifiedName); + } + return new RValueInfo(null, qualifiedName); } void defineDestructuringPatternInVarDeclaration( - Node pattern, TypedScope scope, Supplier patternTypeSupplier) { + Node pattern, TypedScope scope, Supplier patternTypeSupplier) { for (DestructuredTarget target : DestructuredTarget.createAllNonEmptyTargetsInPattern( - typeRegistry, patternTypeSupplier, pattern)) { + typeRegistry, () -> patternTypeSupplier.get().type, pattern)) { - Supplier typeSupplier = + Supplier typeSupplier = () -> inferTypeForDestructuredTarget(target, patternTypeSupplier); if (target.getNode().isDestructuringPattern()) { @@ -1809,7 +1834,7 @@ JSType getDeclaredType( JSDocInfo info, Node lValue, @Nullable Node rValue, - @Nullable Supplier declaredRValueTypeSupplier) { + @Nullable Supplier declaredRValueTypeSupplier) { if (info != null && info.hasType()) { return getDeclaredTypeInAnnotation(lValue, info); } else if (rValue != null @@ -1835,16 +1860,17 @@ && shouldUseFunctionLiteralType( if (NodeUtil.isConstantDeclaration(compiler.getCodingConvention(), info, lValue)) { if (rValue != null) { JSType rValueType = getDeclaredRValueType(lValue, rValue); - maybeDeclareAliasType(lValue, rValue, rValueType); + maybeDeclareAliasType(lValue, rValue.getQualifiedName(), rValueType); if (rValueType != null) { return rValueType; } } else if (declaredRValueTypeSupplier != null) { - JSType rValueType = declaredRValueTypeSupplier.get(); - // TODO(b/77597706): make this work for destructuring? - // maybeDeclareAliasType(lValue, rValue, rValueType); - if (rValueType != null) { - return rValueType; + RValueInfo rvalueInfo = declaredRValueTypeSupplier.get(); + if (rvalueInfo != null) { + maybeDeclareAliasType(lValue, rvalueInfo.qualifiedName, rvalueInfo.type); + if (rvalueInfo.type != null) { + return rvalueInfo.type; + } } } } @@ -1858,16 +1884,17 @@ && shouldUseFunctionLiteralType( } /** - * For a const alias, like `const alias = other.name`, this may declare `alias` - * as a type name, depending on what other.name is defined to be. + * For a const alias, like `const alias = other.name`, this may declare `alias` as a type name, + * depending on what other.name is defined to be. + * + * @param rvalueName the rvalue's qualified name if it exists, null otherwise */ - private void maybeDeclareAliasType(Node lValue, Node rValue, JSType rValueType) { + private void maybeDeclareAliasType(Node lValue, String rvalueName, JSType rValueType) { // NOTE: this allows some strange patterns such allowing instance properties // to be aliases of constructors, and then creating a local alias of that to be // used as a type name. Consider restricting this. - // TODO(b/77597706): handle destructuring here - if (!lValue.isQualifiedName() || !rValue.isQualifiedName()) { + if (!lValue.isQualifiedName() || (rvalueName == null)) { return; } // Treat @const-annotated aliases like @constructor/@interface if RHS has instance type @@ -1879,7 +1906,7 @@ private void maybeDeclareAliasType(Node lValue, Node rValue, JSType rValueType) currentScope, lValue.getQualifiedName(), functionType.getInstanceType()); } else { // Also infer a type name for aliased @typedef - JSType rhsNamedType = typeRegistry.getType(currentScope, rValue.getQualifiedName()); + JSType rhsNamedType = typeRegistry.getType(currentScope, rvalueName); if (rhsNamedType != null) { typeRegistry.declareType(currentScope, lValue.getQualifiedName(), rhsNamedType); } @@ -2737,12 +2764,21 @@ private void declareDestructuringParameter( if (target.getNode().isDestructuringPattern()) { declareDestructuringParameter(isInferred, target.getNode(), inferredType); } else { - checkState( - target.getNode().isName(), - "Expected all parameters to be names, got %s", - target.getNode()); - - declareSingleParameterName(isInferred, target.getNode(), inferredType); + Node paramName = target.getNode(); + checkState(paramName.isName(), "Expected all parameters to be names, got %s", paramName); + + if ((inferredType == null || inferredType.isUnknownType()) + && paramName.getJSDocInfo() != null + && paramName.getJSDocInfo().hasType()) { + // see if the parameter has its own inline JSDoc + // TODO(b/112651122): this should happen inside FunctionTypeBuilder, so that we can + // check that calls to the function match the inline JSDoc. + inferredType = + typeRegistry.evaluateTypeExpression( + paramName.getJSDocInfo().getType(), currentScope); + isInferred = false; + } + declareSingleParameterName(isInferred, paramName, inferredType); } } } diff --git a/test/com/google/javascript/jscomp/TypedScopeCreatorTest.java b/test/com/google/javascript/jscomp/TypedScopeCreatorTest.java index cb013f72433..bbf4ef6cfad 100644 --- a/test/com/google/javascript/jscomp/TypedScopeCreatorTest.java +++ b/test/com/google/javascript/jscomp/TypedScopeCreatorTest.java @@ -276,6 +276,26 @@ public void testConstDeclarationObjectPatternInfersType_forAliasedConstructor() assertFalse(fooInstanceVar.isTypeInferred()); } + @Test + public void testConstDeclarationObjectPatternInfersType_forAliasedTypedef() { + testSame( + lines( + "const ns = {};", + "/** @typedef {string} */ ns.Foo;", + "const {Foo} = ns;", + "let /** !Foo */ f = 'bar';")); + + TypedVar fooVar = checkNotNull(globalScope.getVar("Foo")); + assertType(fooVar.getType()).toStringIsEqualTo("None"); + + JSType fooType = registry.getGlobalType("Foo"); + assertType(fooType).isEqualTo(getNativeType(JSTypeNative.STRING_TYPE)); + + TypedVar fooInstanceVar = checkNotNull(globalScope.getVar("f")); + assertType(fooInstanceVar.getType()).toStringIsEqualTo("string"); + assertFalse(fooInstanceVar.isTypeInferred()); + } + @Test public void testConstDeclarationObjectPatternInfersTypeAsDeclared() { testSame( @@ -941,31 +961,106 @@ public void testOutOfOrderJSDocForDestructuringParameter() { public void testObjectPatternParameterWithInlineJSDoc() { testSame("function f({/** number */ x}) {}"); - // TODO(lharker): infer that f takes {x: number} + // TODO(b/112651122): infer that f takes {x: number} TypedVar fVar = checkNotNull(globalScope.getVar("f")); assertType(fVar.getType()).toStringIsEqualTo("function(?): undefined"); assertFalse(fVar.isTypeInferred()); + + TypedVar xVar = checkNotNull(lastFunctionScope.getVar("x")); + assertType(xVar.getType()).toStringIsEqualTo("number"); + assertFalse(xVar.isTypeInferred()); } @Test public void testArrayPatternParameterWithInlineJSDoc() { testSame("function f([/** number */ x]) {}"); - // TODO(lharker): either forbid this case or infer that f takes an !Iterable + // TODO(b/112651122): 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()); + assertFalse(fVar.isTypeInferred()); + + TypedVar xVar = checkNotNull(lastFunctionScope.getVar("x")); + assertType(xVar.getType()).toStringIsEqualTo("number"); + assertFalse(xVar.isTypeInferred()); } @Test 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 + // TODO(b/112651122): 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()); + + TypedVar xVar = checkNotNull(lastFunctionScope.getVar("x")); + assertType(xVar.getType()).toStringIsEqualTo("number"); + assertFalse(xVar.isTypeInferred()); + + TypedVar yVar = checkNotNull(lastFunctionScope.getVar("y")); + assertType(yVar.getType()).toStringIsEqualTo("string"); + assertFalse(yVar.isTypeInferred()); + } + + @Test + public void testObjectPatternCanAliasUnionTypeProperty() { + // tests that we can get properties off the union type `{objA|objB}` even though it's not + // represented as an ObjectType. (this used to crash in TypedScopeCreator) + testSame( + lines( + "const ns = {}; ", + "", + "/** @typedef {{propA: number}} */", + "let objA;", + "/** @typedef {{propB: string}} */", + "let objB;", + "", + "/** @type {objA|objB} */", + "ns.ctor;", + "", + "const {propA, propB} = ns.ctor;")); + + TypedVar propAVar = checkNotNull(globalScope.getVar("propA")); + assertType(propAVar.getType()).isEqualTo(getNativeType(JSTypeNative.NUMBER_TYPE)); + assertFalse(propAVar.isTypeInferred()); + + TypedVar propBVar = checkNotNull(globalScope.getVar("propB")); + assertType(propBVar.getType()).isEqualTo(getNativeType(JSTypeNative.STRING_TYPE)); + assertFalse(propBVar.isTypeInferred()); + } + + @Test + public void testConstNestedObjectPatternInArrayPattern() { + // regression test for a TypedScopeCreator crash + disableTypeInfoValidation(); + testSame( + lines( + "const /** !Array<{b: string}> */ a = [{b: 'bbb'}];", // + "const [{b}] = a;")); + + TypedVar b = checkNotNull(globalScope.getVar("b")); + assertType(b.getType()).isUnknown(); + assertTrue(b.isTypeInferred()); // b is inferred but not treated as declared + } + + @Test + public void testConstNestedObjectPatternWithComputedPropertyIsUnknown() { + // regression test for a TypedScopeCreator crash + disableTypeInfoValidation(); // fails on a['foo'].b = function() {}; + testSame( + lines( + "const a = {};", + "/** @const */ a['foo'] = {};", + "/** @const */ a['foo'].b = function() {};", + "", + "const {['foo']: {b}} = a;")); + + TypedVar b = checkNotNull(globalScope.getVar("b")); + assertType(b.getType()).isUnknown(); + assertTrue(b.isTypeInferred()); } @Test