diff --git a/src/com/google/javascript/jscomp/TypedScopeCreator.java b/src/com/google/javascript/jscomp/TypedScopeCreator.java index 7877bf88ffe..37efd27086d 100644 --- a/src/com/google/javascript/jscomp/TypedScopeCreator.java +++ b/src/com/google/javascript/jscomp/TypedScopeCreator.java @@ -63,7 +63,6 @@ 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; @@ -225,37 +224,19 @@ void resolve() { } } - /** - * 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. - */ + /** 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 Node qualifiedName; + @Nullable final String qualifiedName; - RValueInfo(JSType type, Node qualifiedName) { + private RValueInfo(JSType type, String qualifiedName) { this.type = type; this.qualifiedName = qualifiedName; } - static RValueInfo empty() { + private 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) { @@ -900,8 +881,7 @@ void defineVarChild(JSDocInfo declarationInfo, Node child, TypedScope scope) { // for (const {x, y} of data) { value != null ? new RValueInfo( - getDeclaredRValueType(/* lValue= */ null, value), - /* qualifiedName= */ value) + getDeclaredRValueType(/* lValue= */ null, value), value.getQualifiedName()) : new RValueInfo(unknownType, /* qualifiedName= */ null)); } } @@ -928,14 +908,16 @@ 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 rvalue.forProperty(null, propertyName); + return new RValueInfo(null, qualifiedName); } if (patternType.hasProperty(propertyName)) { JSType type = patternType.findPropertyType(propertyName); - return rvalue.forProperty(type, propertyName); + return new RValueInfo(type, qualifiedName); } - return rvalue.forProperty(null, propertyName); + return new RValueInfo(null, qualifiedName); } void defineDestructuringPatternInVarDeclaration( @@ -1890,7 +1872,7 @@ && 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; } @@ -1919,29 +1901,14 @@ && shouldUseFunctionLiteralType( * * @param rvalueName the rvalue's qualified name if it exists, null otherwise */ - private void maybeDeclareAliasType( - Node lValue, @Nullable Node rValue, @Nullable 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. - if (!lValue.isQualifiedName() || (rValue == null)) { + if (!lValue.isQualifiedName() || (rvalueName == 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() @@ -1949,27 +1916,15 @@ private void maybeDeclareAliasType( FunctionType functionType = rValueType.toMaybeFunctionType(); typeRegistry.declareType( currentScope, lValue.getQualifiedName(), functionType.getInstanceType()); - } else if (rValue.isQualifiedName()) { + } 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); } } } - /** 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. * @@ -2505,8 +2460,6 @@ 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); @@ -2518,14 +2471,6 @@ 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 dc4c4d85405..9438220ea3f 100644 --- a/src/com/google/javascript/rhino/Node.java +++ b/src/com/google/javascript/rhino/Node.java @@ -166,10 +166,8 @@ 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) { @@ -238,8 +236,6 @@ 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); } @@ -2545,17 +2541,6 @@ 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 79e737a66ad..385a1735b57 100644 --- a/src/com/google/javascript/rhino/jstype/NamedType.java +++ b/src/com/google/javascript/rhino/jstype/NamedType.java @@ -285,16 +285,45 @@ 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) { - handleUnresolvedType(reporter, /* ignoreForwardReferencedTypes= */ true); - return; + return null; } StaticTypedSlot slot = resolutionScope.getSlot(componentNames[0]); if (slot == null) { - handleUnresolvedType(reporter, /* ignoreForwardReferencedTypes= */ true); - return; + return null; } // If the first component has a type of 'Unknown', then any type @@ -302,51 +331,21 @@ private void resolveViaProperties(ErrorReporter reporter) { // noisy about it. JSType slotType = slot.getType(); if (slotType == null || slotType.isAllType() || slotType.isNoType()) { - handleUnresolvedType(reporter, /* ignoreForwardReferencedTypes= */ true); - return; + return null; } // resolving component by component for (int i = 1; i < componentNames.length; i++) { ObjectType parentObj = ObjectType.cast(slotType); if (parentObj == null) { - handleUnresolvedType(reporter, /* ignoreForwardReferencedTypes= */ true); - return; + return null; } if (componentNames[i].length() == 0) { - 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; - } + return null; } slotType = parentObj.getPropertyType(componentNames[i]); } - - // 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()); - } + return slotType; } private void setReferencedAndResolvedType( diff --git a/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java b/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java index fc14641d3e5..f57f417f240 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 testTypedefAliasValueTypeIsUndefined() { - // Aliasing a typedef (const Alias = SomeTypedef) should be interchangeable with the original. + public void testTypedefAlias() { + // Ensure that the type of a variable representing a typedef is "undefined" testTypes( lines( "/** @typedef {number} */", // preserve newlines @@ -729,139 +729,6 @@ public void testTypedefAliasValueTypeIsUndefined() { "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. @@ -5341,9 +5208,8 @@ 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;"), @@ -5358,23 +5224,19 @@ 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( - "initializing variable", // - "found : undefined", - "required: (number|string)")); + lines("Bad type annotation. Unknown type E")); } @Test public void testTypeNameAliasOnAliasedClassSideNamespace() { testTypes( lines( - "class Foo {}", + "class Foo {};", "/** @enum {number} */ Foo.E = {A: 1};", "class Bar extends Foo {};", "const B = Bar;", @@ -5386,36 +5248,4 @@ 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 7b2a208c002..a608aa06a5e 100644 --- a/test/com/google/javascript/jscomp/TypedScopeCreatorTest.java +++ b/test/com/google/javascript/jscomp/TypedScopeCreatorTest.java @@ -4020,7 +4020,9 @@ public void testAliasTypedefFromNamespace() { } @Test + @Ignore public void testAliasTypedefFromNamespaceAlias() { + // TODO(johnlenz): support typedef aliases. testSame( lines( "const Foo = {};",