diff --git a/src/com/google/javascript/jscomp/TypedScopeCreator.java b/src/com/google/javascript/jscomp/TypedScopeCreator.java index 37efd27086d..7877bf88ffe 100644 --- a/src/com/google/javascript/jscomp/TypedScopeCreator.java +++ b/src/com/google/javascript/jscomp/TypedScopeCreator.java @@ -63,6 +63,7 @@ import com.google.javascript.jscomp.NodeTraversal.AbstractScopedCallback; import com.google.javascript.jscomp.NodeTraversal.AbstractShallowStatementCallback; import com.google.javascript.rhino.ErrorReporter; +import com.google.javascript.rhino.IR; import com.google.javascript.rhino.InputId; import com.google.javascript.rhino.JSDocInfo; import com.google.javascript.rhino.Node; @@ -224,19 +225,37 @@ void resolve() { } } - /** Stores the type and qualified name for a destructuring rvalue, which has no AST node */ + /** + * Stores the type and qualified name for a destructuring rvalue, which has no actual AST node. + * The {@code qualifiedName} is therefore a detached Node cloned out of the source tree with extra + * getprops added onto it. Cloning should generally be pretty cheap (since it's always just a + * qualified name). Nevertheless, we may pull out a QualifiedName class abstraction at some point + * in the future, which would allow avoiding the clone in the first place. Using a Node here is + * necessary to avoid duplicating logic for non-destructured qualified name aliasing. + */ private static class RValueInfo { @Nullable final JSType type; - @Nullable final String qualifiedName; + @Nullable final Node qualifiedName; - private RValueInfo(JSType type, String qualifiedName) { + RValueInfo(JSType type, Node qualifiedName) { this.type = type; this.qualifiedName = qualifiedName; } - private static RValueInfo empty() { + static RValueInfo empty() { return new RValueInfo(null, null); } + + /** Returns a new RValueInfo whose qualified name has an extra property. */ + RValueInfo forProperty(JSType type, String property) { + if (qualifiedName == null) { + return new RValueInfo(type, null); + } else if (qualifiedName.getParent() == null) { + return new RValueInfo(type, IR.getprop(qualifiedName, property)); + } + // qualified name is already in the tree, so we need to clone it first. + return new RValueInfo(type, IR.getprop(qualifiedName.cloneTree(), property)); + } } TypedScopeCreator(AbstractCompiler compiler) { @@ -881,7 +900,8 @@ void defineVarChild(JSDocInfo declarationInfo, Node child, TypedScope scope) { // for (const {x, y} of data) { value != null ? new RValueInfo( - getDeclaredRValueType(/* lValue= */ null, value), value.getQualifiedName()) + getDeclaredRValueType(/* lValue= */ null, value), + /* qualifiedName= */ value) : new RValueInfo(unknownType, /* qualifiedName= */ null)); } } @@ -908,16 +928,14 @@ private RValueInfo inferTypeForDestructuredTarget( RValueInfo rvalue = patternTypeSupplier.get(); JSType patternType = rvalue.type; String propertyName = target.getStringKey().getString(); - String qualifiedName = - rvalue.qualifiedName != null ? rvalue.qualifiedName + "." + propertyName : null; if (patternType == null || patternType.isUnknownType()) { - return new RValueInfo(null, qualifiedName); + return rvalue.forProperty(null, propertyName); } if (patternType.hasProperty(propertyName)) { JSType type = patternType.findPropertyType(propertyName); - return new RValueInfo(type, qualifiedName); + return rvalue.forProperty(type, propertyName); } - return new RValueInfo(null, qualifiedName); + return rvalue.forProperty(null, propertyName); } void defineDestructuringPatternInVarDeclaration( @@ -1872,7 +1890,7 @@ && shouldUseFunctionLiteralType( if (NodeUtil.isConstantDeclaration(compiler.getCodingConvention(), info, lValue)) { if (rValue != null) { JSType rValueType = getDeclaredRValueType(lValue, rValue); - maybeDeclareAliasType(lValue, rValue.getQualifiedName(), rValueType); + maybeDeclareAliasType(lValue, rValue, rValueType); if (rValueType != null) { return rValueType; } @@ -1901,14 +1919,29 @@ && shouldUseFunctionLiteralType( * * @param rvalueName the rvalue's qualified name if it exists, null otherwise */ - private void maybeDeclareAliasType(Node lValue, String rvalueName, JSType rValueType) { + private void maybeDeclareAliasType( + Node lValue, @Nullable Node rValue, @Nullable 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. - if (!lValue.isQualifiedName() || (rvalueName == null)) { + if (!lValue.isQualifiedName() || (rValue == null)) { return; } + + Node definitionNode = getDefinitionNode(rValue); + if (definitionNode != null) { + JSType typedefType = definitionNode.getTypedefTypeProp(); + if (typedefType != null) { + // Propagate typedef type to typedef aliases. + lValue.setTypedefTypeProp(typedefType); + String qName = lValue.getQualifiedName(); + typeRegistry.identifyNonNullableName(qName); + typeRegistry.declareType(currentScope, qName, typedefType); + return; + } + } + // Treat @const-annotated aliases like @constructor/@interface if RHS has instance type if (rValueType != null && rValueType.isFunctionType() @@ -1916,15 +1949,27 @@ private void maybeDeclareAliasType(Node lValue, String rvalueName, JSType rValue FunctionType functionType = rValueType.toMaybeFunctionType(); typeRegistry.declareType( currentScope, lValue.getQualifiedName(), functionType.getInstanceType()); - } else { + } else if (rValue.isQualifiedName()) { // Also infer a type name for aliased @typedef - JSType rhsNamedType = typeRegistry.getType(currentScope, rvalueName); + JSType rhsNamedType = typeRegistry.getType(currentScope, rValue.getQualifiedName()); if (rhsNamedType != null) { typeRegistry.declareType(currentScope, lValue.getQualifiedName(), rhsNamedType); } } } + /** Returns the AST node associated with the definition, if any. */ + private Node getDefinitionNode(Node qnameNode) { + if (!qnameNode.isGetProp()) { + TypedVar var = currentScope.getVar(qnameNode.getQualifiedName()); + return var != null ? var.getNameNode() : null; + } + ObjectType parent = ObjectType.cast(lookupQualifiedName(qnameNode.getFirstChild())); + return parent != null + ? parent.getPropertyDefSite(qnameNode.getLastChild().getString()) + : null; + } + /** * Check for common idioms of a typed R-value assigned to a const L-value. * @@ -2460,6 +2505,8 @@ void checkForTypedef(Node candidate, JSDocInfo info) { JSType realType = info.getTypedefType().evaluate(currentScope, typeRegistry); if (realType == null) { report(JSError.make(candidate, MALFORMED_TYPEDEF, typedef)); + } else { + candidate.setTypedefTypeProp(realType); } typeRegistry.overwriteDeclaredType(currentScope, typedef, realType); @@ -2471,6 +2518,14 @@ void checkForTypedef(Node candidate, JSDocInfo info) { .withType(getNativeType(NO_TYPE)) .allowLaterTypeInference(false) .defineSlot(); + Node definitionNode = getDefinitionNode(candidate.getFirstChild()); + if (definitionNode != null) { + ObjectType parent = ObjectType.cast(definitionNode.getJSType()); + if (parent != null) { + JSType type = getNativeType(NO_TYPE); + parent.defineDeclaredProperty(candidate.getLastChild().getString(), type, candidate); + } + } } } } // end AbstractScopeBuilder diff --git a/src/com/google/javascript/rhino/Node.java b/src/com/google/javascript/rhino/Node.java index 9438220ea3f..dc4c4d85405 100644 --- a/src/com/google/javascript/rhino/Node.java +++ b/src/com/google/javascript/rhino/Node.java @@ -166,8 +166,10 @@ public class Node implements Serializable { MODULE_EXPORT = 97, // Mark a property as a module export so that collase properties // can act on it. IS_SHORTHAND_PROPERTY = 98, // Indicates that a property {x:x} was originally parsed as {x}. - ES6_MODULE = 99; // Indicates that a SCRIPT node is or was an ES6 module. Remains set + ES6_MODULE = 99, // Indicates that a SCRIPT node is or was an ES6 module. Remains set // after the module is rewritten. + TYPEDEF_TYPE = 100; // Record the type associated with a @typedef to enable looking up typedef + // in the AST possible without saving the type scope. private static final String propToString(byte propType) { switch (propType) { @@ -236,6 +238,8 @@ private static final String propToString(byte propType) { return "is_shorthand_property"; case ES6_MODULE: return "es6_module"; + case TYPEDEF_TYPE: + return "typedef_type"; default: throw new IllegalStateException("unexpected prop id " + propType); } @@ -2541,6 +2545,17 @@ public final boolean isDeleted() { return getBooleanProp(DELETED); } + /** If this node represents a typedef declaration, the associated JSType */ + public final void setTypedefTypeProp(JSType type) { + putProp(TYPEDEF_TYPE, type); + } + + /** If this node represents a typedef declaration, the associated JSType */ + public final JSType getTypedefTypeProp() { + return (JSType) getProp(TYPEDEF_TYPE); + } + + /** @param unused Whether a parameter was function to be unused. Set by RemoveUnusedVars */ public final void setUnusedParameter(boolean unused) { putBooleanProp(IS_UNUSED_PARAMETER, unused); } diff --git a/src/com/google/javascript/rhino/jstype/NamedType.java b/src/com/google/javascript/rhino/jstype/NamedType.java index 385a1735b57..79e737a66ad 100644 --- a/src/com/google/javascript/rhino/jstype/NamedType.java +++ b/src/com/google/javascript/rhino/jstype/NamedType.java @@ -285,45 +285,16 @@ private boolean resolveViaRegistry(ErrorReporter reporter) { * as properties. The scope must have been fully parsed and a symbol table constructed. */ private void resolveViaProperties(ErrorReporter reporter) { - JSType value = lookupViaProperties(); - // last component of the chain - if (value != null && value.isFunctionType() && - (value.isConstructor() || value.isInterface())) { - FunctionType functionType = value.toMaybeFunctionType(); - setReferencedAndResolvedType(functionType.getInstanceType(), reporter); - } else if (value != null && value.isNoObjectType()) { - setReferencedAndResolvedType( - registry.getNativeObjectType( - JSTypeNative.NO_OBJECT_TYPE), reporter); - } else if (value instanceof EnumType) { - setReferencedAndResolvedType( - ((EnumType) value).getElementsType(), reporter); - } else { - // We've been running into issues where people forward-declare - // non-named types. (This is legitimate...our dependency management - // code doubles as our forward-declaration code.) - // - // So if the type does resolve to an actual value, but it's not named, - // then don't respect the forward declaration. - handleUnresolvedType(reporter, value == null || value.isUnknownType()); - } - } - - /** - * Resolves a type by looking up its first component in the scope, and - * subsequent components as properties. The scope must have been fully - * parsed and a symbol table constructed. - * @return The type of the symbol, or null if the type could not be found. - */ - private JSType lookupViaProperties() { String[] componentNames = reference.split("\\.", -1); if (componentNames[0].length() == 0) { - return null; + handleUnresolvedType(reporter, /* ignoreForwardReferencedTypes= */ true); + return; } StaticTypedSlot slot = resolutionScope.getSlot(componentNames[0]); if (slot == null) { - return null; + handleUnresolvedType(reporter, /* ignoreForwardReferencedTypes= */ true); + return; } // If the first component has a type of 'Unknown', then any type @@ -331,21 +302,51 @@ private JSType lookupViaProperties() { // noisy about it. JSType slotType = slot.getType(); if (slotType == null || slotType.isAllType() || slotType.isNoType()) { - return null; + handleUnresolvedType(reporter, /* ignoreForwardReferencedTypes= */ true); + return; } // resolving component by component for (int i = 1; i < componentNames.length; i++) { ObjectType parentObj = ObjectType.cast(slotType); if (parentObj == null) { - return null; + handleUnresolvedType(reporter, /* ignoreForwardReferencedTypes= */ true); + return; } if (componentNames[i].length() == 0) { - return null; + handleUnresolvedType(reporter, /* ignoreForwardReferencedTypes= */ true); + return; + } else if (i == componentNames.length - 1) { + // Look for a typedefTypeProp on the definition node of the last component. + Node def = parentObj.getPropertyDefSite(componentNames[i]); + JSType typedefType = def != null ? def.getTypedefTypeProp() : null; + if (typedefType != null) { + setReferencedAndResolvedType(typedefType, reporter); + return; + } } slotType = parentObj.getPropertyType(componentNames[i]); } - return slotType; + + // Translate "constructor" types to "instance" types. + if (slotType == null) { + handleUnresolvedType(reporter, /* ignoreForwardReferencedTypes= */ true); + } else if (slotType.isFunctionType() && (slotType.isConstructor() || slotType.isInterface())) { + setReferencedAndResolvedType(slotType.toMaybeFunctionType().getInstanceType(), reporter); + } else if (slotType.isNoObjectType()) { + setReferencedAndResolvedType( + registry.getNativeObjectType(JSTypeNative.NO_OBJECT_TYPE), reporter); + } else if (slotType instanceof EnumType) { + setReferencedAndResolvedType(((EnumType) slotType).getElementsType(), reporter); + } else { + // We've been running into issues where people forward-declare + // non-named types. (This is legitimate...our dependency management + // code doubles as our forward-declaration code.) + // + // So if the type does resolve to an actual value, but it's not named, + // then don't respect the forward declaration. + handleUnresolvedType(reporter, slotType.isUnknownType()); + } } private void setReferencedAndResolvedType( diff --git a/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java b/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java index f57f417f240..fc14641d3e5 100644 --- a/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java +++ b/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java @@ -713,8 +713,8 @@ public void testTypedefFieldInLoopGlobal() { } @Test - public void testTypedefAlias() { - // Ensure that the type of a variable representing a typedef is "undefined" + public void testTypedefAliasValueTypeIsUndefined() { + // Aliasing a typedef (const Alias = SomeTypedef) should be interchangeable with the original. testTypes( lines( "/** @typedef {number} */", // preserve newlines @@ -729,6 +729,139 @@ public void testTypedefAlias() { "to : string")); } + @Test + public void testTypedefAliasOfLocalTypedef() { + // Aliasing should work on local typedefs as well as global. + testTypes( + lines( + "function f() {", + " /** @typedef {number} */", + " var MyNumber;", + " /** @const */", + " var Alias = MyNumber;", + " /** @type {Alias} */", + " var x = 'x';", + "}"), + lines( + "initializing variable", // preserve newlines + "found : string", + "required: number")); + } + + @Test + public void testTypedefLocalQualifiedName() { + // Aliasing should work on local typedefs as well as global. + testTypes( + lines( + "function f() {", + " /** @const */", + " var ns = {};", + " /** @typedef {number} */", + " ns.MyNumber;", + " /** @type {ns.MyNumber} */", + " var x = 'x';", + "}"), + lines( + "initializing variable", // preserve newlines + "found : string", + "required: number")); + } + + @Test + public void testTypedefLocalQualifiedNameAlias() { + // Aliasing should work on local typedefs as well as global. + testTypes( + lines( + "function f() {", + " /** @typedef {number} */", + " var MyNumber;", + " /** @const */", + " var ns = {};", + " /** @const */", + " ns.MyNumber = MyNumber;", + " /** @type {ns.MyNumber} */", + " var x = 'x';", + "}"), + lines( + "initializing variable", // preserve newlines + "found : string", + "required: number")); + } + + @Test + public void testTypedefLocalAliasOfGlobalTypedef() { + // Should also work if the alias is local but the typedef is global. + testTypes( + lines( + "/** @typedef {number} */", + "var MyNumber;", + "function f() {", + " /** @const */ var Alias = MyNumber;", + " var /** Alias */ x = 'x';", + "}"), + lines( + "initializing variable", // preserve newlines + "found : string", + "required: number")); + } + + @Test + public void testTypedefOnAliasedNamespace() { + // Aliasing a namespace (const alias = ns) should carry over any typedefs on the namespace. + testTypes( + lines( + "const ns = {};", + "/** @const */ ns.bar = 'x';", + "/** @typedef {number} */", // preserve newlines + "ns.MyNumber;", + "const alias = ns;", + "/** @const */ alias.foo = 42", + "/** @type {alias.MyNumber} */ const x = 'str';", + ""), + // TODO(sdh): should be non-nullable number, but we get nullability wrong. + lines( + "initializing variable", // preserve newlines + "found : string", + "required: (null|number)")); + } + + @Test + public void testTypedefOnLocalAliasedNamespace() { + // Aliasing a namespace (const alias = ns) should carry over any typedefs on the namespace. + testTypes( + lines( + "function f() {", + " const ns = {};", + " /** @typedef {number} */", // preserve newlines + " ns.MyNumber;", + " const alias = ns;", + " /** @const */ alias.foo = 42", + " /** @type {alias.MyNumber} */ const x = 'str';", + "}"), + // TODO(sdh): should be non-nullable number, but we get nullability wrong. + lines( + "initializing variable", // preserve newlines + "found : string", + "required: (null|number)")); + } + + @Test + public void testTypedefOnClassSideInheritedSubtype() { + // Class-side inheritance should carry over any typedefs nested on the class. + testTypes( + lines( + "class Base {}", + "/** @typedef {number} */", // preserve newlines + "Base.MyNumber;", + "class Sub extends Base {}", + "/** @type {Sub.MyNumber} */ const x = 'str';", + ""), + lines( + "initializing variable", // preserve newlines + "found : string", + "required: (null|number)")); + } + @Test public void testGetTypedPercent() { // Make sure names declared with `const` and `let` are counted correctly for typed percentage. @@ -5208,8 +5341,9 @@ public void testMixinImplementingInterfaceAndUnknownTemplatedSuperclass() { public void testTypeNameAliasOnAliasedNamespace() { testTypes( lines( - "class Foo {};", - "/** @enum {number} */ Foo.E = {A: 1};", + "class Foo {}", + "/** @enum {number} */", + "Foo.E = {A: 1};", "const F = Foo;", "const E = F.E;", "/** @type {E} */ let e = undefined;"), @@ -5224,19 +5358,23 @@ public void testTypeNameAliasOnAliasedNamespace() { public void testTypedefNameAliasOnAliasedNamespace() { testTypes( lines( - "class Foo {};", - "/** @typedef {number|string} */ Foo.E;", + "class Foo {}", + "/** @typedef {number|string} */", + "Foo.E;", "const F = Foo;", "const E = F.E;", "/** @type {E} */ let e = undefined;"), - lines("Bad type annotation. Unknown type E")); + lines( + "initializing variable", // + "found : undefined", + "required: (number|string)")); } @Test public void testTypeNameAliasOnAliasedClassSideNamespace() { testTypes( lines( - "class Foo {};", + "class Foo {}", "/** @enum {number} */ Foo.E = {A: 1};", "class Bar extends Foo {};", "const B = Bar;", @@ -5248,4 +5386,36 @@ public void testTypeNameAliasOnAliasedClassSideNamespace() { // TODO(johnlenz): this should not be nullable "required: (Foo.E|null)")); } + + @Test + public void testTypedefInExtern() { + testTypesWithExtraExterns( + "/** @typedef {boolean} */ var ConstrainBoolean;", + "var /** ConstrainBoolean */ x = 42;", + lines( + "initializing variable", // + "found : number", + "required: boolean")); + } + + @Test + public void testDeeplyNestedAliases() { + testTypes( + lines( + "const ns = {};", + "/** @typedef {number} */", + "ns.MyNumber;", + "const alias = {};", + "/** @const */", + "alias.child = ns;", + "const outer = {};", + "/** @const */", + "outer.inner = alias;", + "const /** outer.inner.child.MyNumber */ x = '';"), + lines( + "initializing variable", + "found : string", + // TODO(sdh): this should not be nullable + "required: (null|number)")); + } } diff --git a/test/com/google/javascript/jscomp/TypedScopeCreatorTest.java b/test/com/google/javascript/jscomp/TypedScopeCreatorTest.java index a608aa06a5e..7b2a208c002 100644 --- a/test/com/google/javascript/jscomp/TypedScopeCreatorTest.java +++ b/test/com/google/javascript/jscomp/TypedScopeCreatorTest.java @@ -4020,9 +4020,7 @@ public void testAliasTypedefFromNamespace() { } @Test - @Ignore public void testAliasTypedefFromNamespaceAlias() { - // TODO(johnlenz): support typedef aliases. testSame( lines( "const Foo = {};",