From 53edb6048cb610ff6949e4fda0c7dcce0ba45e9c Mon Sep 17 00:00:00 2001 From: sdh Date: Mon, 11 Jun 2018 14:22:12 -0700 Subject: [PATCH] Support basic ES6 class syntax in typed scope creation So far, we only support the class and extends keyword and an optional constructor method. All other methods are ignored and will probably cause an error if left untranspiled (since TypedScopeCreator will not annotate them with types). This should unblock work on class visibility. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=200111986 --- .../jscomp/FunctionTypeBuilder.java | 180 ++++---- .../google/javascript/jscomp/NodeUtil.java | 3 +- .../google/javascript/jscomp/TypeCheck.java | 185 +++++--- .../javascript/jscomp/TypeInference.java | 12 + .../javascript/jscomp/TypedScopeCreator.java | 233 +++++++++- .../jscomp/testing/TypeSubject.java | 5 + .../javascript/rhino/jstype/FunctionType.java | 2 +- .../rhino/jstype/InstanceObjectType.java | 20 +- .../rhino/jstype/JSTypeRegistry.java | 2 +- .../jscomp/TypeCheckNoTranspileTest.java | 407 ++++++++++++++++++ .../javascript/jscomp/TypeCheckTest.java | 2 +- .../jscomp/TypedScopeCreatorTest.java | 116 ++++- 12 files changed, 993 insertions(+), 174 deletions(-) diff --git a/src/com/google/javascript/jscomp/FunctionTypeBuilder.java b/src/com/google/javascript/jscomp/FunctionTypeBuilder.java index 97d3a616772..20b4fe887c6 100644 --- a/src/com/google/javascript/jscomp/FunctionTypeBuilder.java +++ b/src/com/google/javascript/jscomp/FunctionTypeBuilder.java @@ -80,6 +80,7 @@ final class FunctionTypeBuilder { private List extendedInterfaces = null; private ObjectType baseType = null; private JSType thisType = null; + private boolean isClass = false; private boolean isConstructor = false; private boolean makesStructs = false; private boolean makesDicts = false; @@ -349,11 +350,17 @@ FunctionTypeBuilder inferReturnType( return this; } + FunctionTypeBuilder usingClassSyntax() { + this.isClass = true; + return this; + } + /** * Infer the role of the function (whether it's a constructor or interface) * and what it inherits from in JSDocInfo. */ - FunctionTypeBuilder inferInheritance(@Nullable JSDocInfo info) { + FunctionTypeBuilder inferInheritance( + @Nullable JSDocInfo info, @Nullable ObjectType baseType) { if (info != null) { isConstructor = info.isConstructor(); isInterface = info.isInterface(); @@ -361,98 +368,114 @@ FunctionTypeBuilder inferInheritance(@Nullable JSDocInfo info) { isAbstract = info.isAbstract(); makesStructs = info.makesStructs(); makesDicts = info.makesDicts(); + } + if (isClass) { + // If a CLASS literal has not been explicitly declared an interface, it's a constructor. + // If it's not expicitly @dict or @unrestricted then it's @struct. + isConstructor = !isInterface; + makesStructs = info == null || (!makesDicts && !info.makesUnrestricted()); + } + + if (makesStructs && !(isConstructor || isInterface)) { + reportWarning(CONSTRUCTOR_REQUIRED, "@struct", formatFnName()); + } else if (makesDicts && !isConstructor) { + reportWarning(CONSTRUCTOR_REQUIRED, "@dict", formatFnName()); + } - if (makesStructs && !(isConstructor || isInterface)) { - reportWarning(CONSTRUCTOR_REQUIRED, "@struct", formatFnName()); - } else if (makesDicts && !isConstructor) { - reportWarning(CONSTRUCTOR_REQUIRED, "@dict", formatFnName()); + // TODO(b/74253232): maybeGetNativeTypesOfBuiltin should also handle cases where a local type + // declaration shadows a templatized native type. + ImmutableList nativeClassTemplateTypeNames = + typeRegistry.maybeGetTemplateTypesOfBuiltin(fnName); + ImmutableList infoTemplateTypeNames = + info != null ? info.getTemplateTypeNames() : ImmutableList.of(); + // TODO(b/73386087): Make infoTemplateTypeNames.size() == nativeClassTemplateTypeName.size() a + // Preconditions check. It currently fails for "var symbol" in the externs. + if (nativeClassTemplateTypeNames != null + && infoTemplateTypeNames.size() == nativeClassTemplateTypeNames.size()) { + classTemplateTypeNames = nativeClassTemplateTypeNames; + typeRegistry.setTemplateTypeNames(classTemplateTypeNames); + } else if (!infoTemplateTypeNames.isEmpty() && (isConstructor || isInterface)) { + // Otherwise, create new template type for + // the template values of the constructor/interface + // Class template types, which can be used in the scope of a constructor + // definition. + ImmutableList.Builder builder = ImmutableList.builder(); + for (String typeParameter : infoTemplateTypeNames) { + builder.add(typeRegistry.createTemplateType(typeParameter)); } + classTemplateTypeNames = builder.build(); + typeRegistry.setTemplateTypeNames(classTemplateTypeNames); + } - // TODO(b/74253232): maybeGetNativeTypesOfBuiltin should also handle cases where a local type - // declaration shadows a templatized native type. - ImmutableList nativeClassTemplateTypeNames = - typeRegistry.maybeGetTemplateTypesOfBuiltin(fnName); - ImmutableList infoTemplateTypeNames = info.getTemplateTypeNames(); - // TODO(b/73386087): Make infoTemplateTypeNames.size() == nativeClassTemplateTypeName.size() a - // Preconditions check. It currently fails for "var symbol" in the externs. - if (nativeClassTemplateTypeNames != null - && infoTemplateTypeNames.size() == nativeClassTemplateTypeNames.size()) { - classTemplateTypeNames = nativeClassTemplateTypeNames; - typeRegistry.setTemplateTypeNames(classTemplateTypeNames); + // base type + if (info != null && info.hasBaseType()) { + if (isConstructor) { + ObjectType infoBaseType = + info.getBaseType().evaluate(scope, typeRegistry).toMaybeObjectType(); + // TODO(sdh): ensure that JSDoc's baseType and AST's baseType are compatible if both are set + baseType = infoBaseType; } else { - // Otherwise, create new template type for - // the template values of the constructor/interface - // Class template types, which can be used in the scope of a constructor - // definition. - ImmutableList typeParameters = info.getTemplateTypeNames(); - if (!typeParameters.isEmpty() && (isConstructor || isInterface)) { - ImmutableList.Builder builder = ImmutableList.builder(); - for (String typeParameter : typeParameters) { - builder.add(typeRegistry.createTemplateType(typeParameter)); - } - classTemplateTypeNames = builder.build(); - typeRegistry.setTemplateTypeNames(classTemplateTypeNames); - } + reportWarning(EXTENDS_WITHOUT_TYPEDEF, formatFnName()); } - - // base type - if (info.hasBaseType()) { - if (isConstructor) { - JSType maybeBaseType = - info.getBaseType().evaluate(scope, typeRegistry); - if (maybeBaseType != null - && maybeBaseType.setValidator(new ExtendedTypeValidator())) { - baseType = (ObjectType) maybeBaseType; - } - } else { - reportWarning(EXTENDS_WITHOUT_TYPEDEF, formatFnName()); - } + } + if (baseType != null && isConstructor) { + if (baseType.setValidator(new ExtendedTypeValidator())) { + this.baseType = baseType; } + } - // Implemented interfaces (for constructors only). - if (info.getImplementedInterfaceCount() > 0) { - if (isConstructor) { - implementedInterfaces = new ArrayList<>(); - Set baseInterfaces = new HashSet<>(); - for (JSTypeExpression t : info.getImplementedInterfaces()) { - JSType maybeInterType = t.evaluate(scope, typeRegistry); - - if (maybeInterType != null && - maybeInterType.setValidator(new ImplementedTypeValidator())) { - // Disallow implementing the same base (not templatized) interface - // type more than once. - JSType baseInterface = maybeInterType; - if (baseInterface.toMaybeTemplatizedType() != null) { - baseInterface = - baseInterface.toMaybeTemplatizedType().getReferencedType(); - } - if (!baseInterfaces.add(baseInterface)) { - reportWarning(SAME_INTERFACE_MULTIPLE_IMPLEMENTS, baseInterface.toString()); - } - - implementedInterfaces.add((ObjectType) maybeInterType); + // Implemented interfaces (for constructors only). + if (info != null && info.getImplementedInterfaceCount() > 0) { + if (isConstructor) { + implementedInterfaces = new ArrayList<>(); + Set baseInterfaces = new HashSet<>(); + for (JSTypeExpression t : info.getImplementedInterfaces()) { + JSType maybeInterType = t.evaluate(scope, typeRegistry); + + if (maybeInterType != null && + maybeInterType.setValidator(new ImplementedTypeValidator())) { + // Disallow implementing the same base (not templatized) interface + // type more than once. + JSType baseInterface = maybeInterType; + if (baseInterface.toMaybeTemplatizedType() != null) { + baseInterface = + baseInterface.toMaybeTemplatizedType().getReferencedType(); + } + if (!baseInterfaces.add(baseInterface)) { + reportWarning(SAME_INTERFACE_MULTIPLE_IMPLEMENTS, baseInterface.toString()); } + + implementedInterfaces.add((ObjectType) maybeInterType); } - } else if (isInterface) { - reportWarning( - TypeCheck.CONFLICTING_IMPLEMENTED_TYPE, formatFnName()); - } else { - reportWarning(CONSTRUCTOR_REQUIRED, "@implements", formatFnName()); } + } else if (isInterface) { + reportWarning( + TypeCheck.CONFLICTING_IMPLEMENTED_TYPE, formatFnName()); + } else { + reportWarning(CONSTRUCTOR_REQUIRED, "@implements", formatFnName()); } + } - // extended interfaces (for interfaces only) - // We've already emitted a warning if this is not an interface. - if (isInterface) { - extendedInterfaces = new ArrayList<>(); + // extended interfaces (for interfaces only) + // We've already emitted a warning if this is not an interface. + if (isInterface) { + extendedInterfaces = new ArrayList<>(); + if (info != null) { for (JSTypeExpression t : info.getExtendedInterfaces()) { JSType maybeInterfaceType = t.evaluate(scope, typeRegistry); if (maybeInterfaceType != null && maybeInterfaceType.setValidator(new ExtendedTypeValidator())) { extendedInterfaces.add((ObjectType) maybeInterfaceType); } + // de-dupe baseType (from extends keyword) if it's also in @extends jsdoc. + if (baseType != null && maybeInterfaceType.isSubtypeOf(baseType)) { + baseType = null; + } } } + if (baseType != null && baseType.setValidator(new ExtendedTypeValidator())) { + extendedInterfaces.add(baseType); + } } return this; @@ -512,8 +535,7 @@ FunctionTypeBuilder inferParameterTypes(JSDocInfo info) { * Infer the parameter types from the list of argument names and * the doc info. */ - FunctionTypeBuilder inferParameterTypes(@Nullable Node argsParent, - @Nullable JSDocInfo info) { + FunctionTypeBuilder inferParameterTypes(@Nullable Node argsParent, @Nullable JSDocInfo info) { if (argsParent == null) { if (info == null) { return this; @@ -592,6 +614,11 @@ FunctionTypeBuilder inferParameterTypes(@Nullable Node argsParent, return this; } + FunctionTypeBuilder inferImplicitConstructorParameters(Node parametersNode) { + this.parametersNode = parametersNode; + return this; + } + /** * @return Whether the given param is an optional param. */ @@ -624,8 +651,7 @@ private boolean isVarArgsParameter( /** * Infer the template type from the doc info. */ - FunctionTypeBuilder inferTemplateTypeName( - @Nullable JSDocInfo info, JSType ownerType) { + FunctionTypeBuilder inferTemplateTypeName(@Nullable JSDocInfo info, @Nullable JSType ownerType) { // NOTE: these template type names may override a list // of inherited ones from an overridden function. if (info != null) { diff --git a/src/com/google/javascript/jscomp/NodeUtil.java b/src/com/google/javascript/jscomp/NodeUtil.java index 14f367d0031..2956cbd5f62 100644 --- a/src/com/google/javascript/jscomp/NodeUtil.java +++ b/src/com/google/javascript/jscomp/NodeUtil.java @@ -5221,8 +5221,7 @@ public static Node getBestJSDocInfoNode(Node n) { /** Find the l-value that the given r-value is being assigned to. */ public static Node getBestLValue(Node n) { Node parent = n.getParent(); - boolean isFunctionDeclaration = isFunctionDeclaration(n); - if (isFunctionDeclaration) { + if (isFunctionDeclaration(n) || isClassDeclaration(n)) { return n.getFirstChild(); } else if (parent.isName()) { return parent; diff --git a/src/com/google/javascript/jscomp/TypeCheck.java b/src/com/google/javascript/jscomp/TypeCheck.java index 55fb5906ece..c995824a45d 100644 --- a/src/com/google/javascript/jscomp/TypeCheck.java +++ b/src/com/google/javascript/jscomp/TypeCheck.java @@ -844,6 +844,10 @@ public void visit(NodeTraversal t, Node n, Node parent) { visitFunction(t, n); break; + case CLASS: + visitClass(t, n); + break; + // These nodes have no interesting type behavior. // These nodes require data flow analysis. case PARAM_LIST: @@ -871,6 +875,7 @@ public void visit(NodeTraversal t, Node n, Node parent) { case FOR: case TEMPLATELIT_SUB: case REST: + case CLASS_MEMBERS: typeable = false; break; @@ -1924,91 +1929,133 @@ private void checkInterfaceConflictProperties(NodeTraversal t, Node n, */ private void visitFunction(NodeTraversal t, Node n) { FunctionType functionType = JSType.toMaybeFunctionType(n.getJSType()); - String functionPrivateName = n.getFirstChild().getString(); if (functionType.isConstructor()) { - FunctionType baseConstructor = functionType.getSuperClassConstructor(); - if (!Objects.equals(baseConstructor, getNativeType(OBJECT_FUNCTION_TYPE)) + checkConstructor(t, n, functionType); + } else if (functionType.isInterface()) { + checkInterface(t, n, functionType); + } else if (n.isGeneratorFunction()) { + // A generator function must return a Generator or supertype of Generator + JSType returnType = functionType.getReturnType(); + validator.expectGeneratorSupertype( + t, n, returnType, "A generator function must return a (supertype of) Generator"); + } + } + + /** Visits a CLASS node. */ + private void visitClass(NodeTraversal t, Node n) { + FunctionType functionType = JSType.toMaybeFunctionType(n.getJSType()); + Node extendsClause = n.getSecondChild(); + if (!extendsClause.isEmpty()) { + // Ensure that the `extends` clause is actually a constructor or interface. If it is, but + // it's the wrong one then checkConstructor or checkInterface will warn. + JSType superType = extendsClause.getJSType(); + if (!(superType.isConstructor() || superType.isInterface())) { + compiler.report( + t.makeError( + n, + CONFLICTING_EXTENDED_TYPE, + functionType.isConstructor() ? "constructor" : "interface", + getBestFunctionName(n))); + } + } + if (functionType.isConstructor()) { + checkConstructor(t, n, functionType); + } else if (functionType.isInterface()) { + checkInterface(t, n, functionType); + } else { + throw new IllegalStateException( + "CLASS node's type must be either constructor or interface: " + functionType); + } + } + + /** Checks a constructor, which may be either an ES5-style FUNCTION node, or a CLASS node. */ + private void checkConstructor(NodeTraversal t, Node n, FunctionType functionType) { + FunctionType baseConstructor = functionType.getSuperClassConstructor(); + if (!Objects.equals(baseConstructor, getNativeType(OBJECT_FUNCTION_TYPE)) + && baseConstructor != null + && baseConstructor.isInterface()) { + // Warn if a class extends an interface. + compiler.report( + t.makeError(n, CONFLICTING_EXTENDED_TYPE, "constructor", getBestFunctionName(n))); + } else { + if (n.isFunction() && baseConstructor != null - && baseConstructor.isInterface()) { + && baseConstructor.getSource() != null + && baseConstructor.getSource().getBooleanProp(Node.IS_ES6_CLASS) + && !functionType.getSource().getBooleanProp(Node.IS_ES6_CLASS)) { + // Warn if an ES5 class extends an ES6 class. compiler.report( - t.makeError(n, CONFLICTING_EXTENDED_TYPE, - "constructor", functionPrivateName)); - } else { - if (baseConstructor != null - && baseConstructor.getSource() != null - && baseConstructor.getSource().getBooleanProp(Node.IS_ES6_CLASS) - && !functionType.getSource().getBooleanProp(Node.IS_ES6_CLASS)) { - compiler.report( - t.makeError( - n, - ES5_CLASS_EXTENDING_ES6_CLASS, - functionType.getDisplayName(), - baseConstructor.getDisplayName())); - } + t.makeError( + n, + ES5_CLASS_EXTENDING_ES6_CLASS, + functionType.getDisplayName(), + baseConstructor.getDisplayName())); + } - // All interfaces are properly implemented by a class - for (JSType baseInterface : functionType.getImplementedInterfaces()) { - boolean badImplementedType = false; - ObjectType baseInterfaceObj = ObjectType.cast(baseInterface); - if (baseInterfaceObj != null) { - FunctionType interfaceConstructor = + // Warn if any interface property is missing or incorrect + for (JSType baseInterface : functionType.getImplementedInterfaces()) { + boolean badImplementedType = false; + ObjectType baseInterfaceObj = ObjectType.cast(baseInterface); + if (baseInterfaceObj != null) { + FunctionType interfaceConstructor = baseInterfaceObj.getConstructor(); - if (interfaceConstructor != null && !interfaceConstructor.isInterface()) { - badImplementedType = true; - } - } else { + if (interfaceConstructor != null && !interfaceConstructor.isInterface()) { badImplementedType = true; } - if (badImplementedType) { - report(t, n, BAD_IMPLEMENTED_TYPE, functionPrivateName); - } + } else { + badImplementedType = true; } - // check properties - validator.expectAllInterfaceProperties(t, n, functionType); - if (!functionType.isAbstract()) { - validator.expectAbstractMethodsImplemented(n, functionType); + if (badImplementedType) { + report(t, n, BAD_IMPLEMENTED_TYPE, getBestFunctionName(n)); } } - } else if (functionType.isInterface()) { - // Interface must extend only interfaces - for (ObjectType extInterface : functionType.getExtendedInterfaces()) { - if (extInterface.getConstructor() != null - && !extInterface.getConstructor().isInterface()) { - compiler.report( - t.makeError(n, CONFLICTING_EXTENDED_TYPE, - "interface", functionPrivateName)); - } + // check properties + validator.expectAllInterfaceProperties(t, n, functionType); + if (!functionType.isAbstract()) { + validator.expectAbstractMethodsImplemented(n, functionType); } + } + } - // Check whether the extended interfaces have any conflicts - if (functionType.getExtendedInterfacesCount() > 1) { - // Only check when extending more than one interfaces - HashMap properties = new HashMap<>(); - LinkedHashMap currentProperties = new LinkedHashMap<>(); - for (ObjectType interfaceType : functionType.getExtendedInterfaces()) { - currentProperties.clear(); - checkInterfaceConflictProperties(t, n, functionPrivateName, - properties, currentProperties, interfaceType); - properties.putAll(currentProperties); - } + /** Checks an interface, which may be either an ES5-style FUNCTION node, or a CLASS node. */ + private void checkInterface(NodeTraversal t, Node n, FunctionType functionType) { + // Interface must extend only interfaces + for (ObjectType extInterface : functionType.getExtendedInterfaces()) { + if (extInterface.getConstructor() != null && !extInterface.getConstructor().isInterface()) { + compiler.report( + t.makeError(n, CONFLICTING_EXTENDED_TYPE, "interface", getBestFunctionName(n))); } + } - List loopPath = functionType.checkExtendsLoop(); - if (loopPath != null) { - String strPath = ""; - for (int i = 0; i < loopPath.size() - 1; i++) { - strPath += loopPath.get(i).getDisplayName() + " -> "; - } - strPath += Iterables.getLast(loopPath).getDisplayName(); - compiler.report(t.makeError(n, INTERFACE_EXTENDS_LOOP, - loopPath.get(0).getDisplayName(), strPath)); + // Check whether the extended interfaces have any conflicts + if (functionType.getExtendedInterfacesCount() > 1) { + // Only check when extending more than one interfaces + HashMap properties = new HashMap<>(); + LinkedHashMap currentProperties = new LinkedHashMap<>(); + for (ObjectType interfaceType : functionType.getExtendedInterfaces()) { + currentProperties.clear(); + checkInterfaceConflictProperties(t, n, getBestFunctionName(n), + properties, currentProperties, interfaceType); + properties.putAll(currentProperties); } - } else if (n.isGeneratorFunction()) { - // A generator function must return a Generator or supertype of Generator - JSType returnType = functionType.getReturnType(); - validator.expectGeneratorSupertype( - t, n, returnType, "A generator function must return a (supertype of) Generator"); } + + List loopPath = functionType.checkExtendsLoop(); + if (loopPath != null) { + String strPath = ""; + for (int i = 0; i < loopPath.size() - 1; i++) { + strPath += loopPath.get(i).getDisplayName() + " -> "; + } + strPath += Iterables.getLast(loopPath).getDisplayName(); + compiler.report(t.makeError(n, INTERFACE_EXTENDS_LOOP, + loopPath.get(0).getDisplayName(), strPath)); + } + } + + private String getBestFunctionName(Node n) { + checkState(n.isClass() || n.isFunction()); + String name = NodeUtil.getBestLValueName(NodeUtil.getBestLValue(n)); + return name != null ? name : ""; } /** diff --git a/src/com/google/javascript/jscomp/TypeInference.java b/src/com/google/javascript/jscomp/TypeInference.java index 303d16067bc..1ca1f6ca087 100644 --- a/src/com/google/javascript/jscomp/TypeInference.java +++ b/src/com/google/javascript/jscomp/TypeInference.java @@ -363,6 +363,10 @@ private FlowScope traverse(Node n, FlowScope scope) { scope = traverseGetProp(n, scope); break; + case CLASS: + scope = traverseClass(n, scope); + break; + case AND: scope = traverseAnd(n, scope).getJoinedFlowScope(); break; @@ -931,6 +935,14 @@ private FlowScope traverseName(Node n, FlowScope scope) { return scope; } + private FlowScope traverseClass(Node n, FlowScope scope) { + // The name already has a type applied (from TypedScopeCreator) if it's non-empty, and the + // members are traversed in the class scope (and in their own function scopes). But the extends + // clause and computed property keys are in the outer scope and must be traversed here. + scope = traverse(n.getSecondChild(), scope); + return scope; + } + /** Traverse each element of the array. */ private FlowScope traverseArrayLiteral(Node n, FlowScope scope) { scope = traverseChildren(n, scope); diff --git a/src/com/google/javascript/jscomp/TypedScopeCreator.java b/src/com/google/javascript/jscomp/TypedScopeCreator.java index 8c94177c5e8..81393b23352 100644 --- a/src/com/google/javascript/jscomp/TypedScopeCreator.java +++ b/src/com/google/javascript/jscomp/TypedScopeCreator.java @@ -311,6 +311,8 @@ private TypedScope createScopeInternal(Node root, TypedScope typedParent) { } if (root.isFunction()) { scopeBuilder = new FunctionScopeBuilder(newScope); + } else if (root.isClass()) { + scopeBuilder = new ClassScopeBuilder(newScope); } else { scopeBuilder = new NormalScopeBuilder(newScope); } @@ -551,22 +553,49 @@ public final boolean shouldTraverse(NodeTraversal t, Node n, Node parent) { sourceName = NodeUtil.getSourceName(n); } visitPreorder(t, n, parent); - return parent == null || !NodeUtil.createsScope(n); + return shouldDescend(n, parent); } @Override public final void visit(NodeTraversal t, Node n, Node parent) { inputId = t.getInputId(); - attachLiteralTypes(n); - visitPostorder(t, n, parent); - if (parent == null) { + if (parent != null) { + attachLiteralTypes(n); + visitPostorder(t, n, parent); + if (deferredActions.containsKey(n)) { // streams are expensive, only make if needed + deferredActions.removeAll(n).stream().forEach(Runnable::run); + } + } else if (!deferredActions.isEmpty()) { // Run *all* remaining deferred actions, in case any were missed. deferredActions.values().stream().forEach(Runnable::run); - } else { - deferredActions.removeAll(n).stream().forEach(Runnable::run); } } + /** Whether or not to descend into a given node. */ + boolean shouldDescend(Node n, Node parent) { + if (parent == null) { + return true; + } else if (parent.isClass()) { + // We need some special handling for classes, since we need to fully traverse the 'extends' + // node in the outer scope before (post-order) visiting the class node. But node traversals + // lump the 'extends' node into the inner scope, causing problems. + // If we're in the class scope we want to skip the extends node (since it was already + // visited in the outer scope). If we're in the outer scope, we only want to descend into + // the extends node and not into any others. This ensures all children are still visited + // exactly once. + // TODO(sdh): Rework NodeTraversal to visit the extends node in the outer scope, and rework + // this class to leverage enterScope and exitScope and call different template methods + // depending on whether we're in our own scope or a child scope. + boolean isClassScope = parent == currentScope.getRootNode(); + boolean isExtendsNode = n == parent.getSecondChild(); + return isClassScope != isExtendsNode; + } else if (n.isClass()) { + // We need to descend into classes from the outer scope in order to see the extends node. + return true; + } + return !NodeUtil.createsScope(n); + } + /** Called by shouldTraverse on nodes after ensuring the inputId is set. */ void visitPreorder(NodeTraversal t, Node n, Node parent) {} @@ -616,6 +645,15 @@ void attachLiteralTypes(Node n) { } break; + case CLASS: + // NOTE(sdh): We can't handle function nodes here because they need special behavior to + // deal with hoisting. But since classes aren't hoisted, and may need to be handled in + // such places as default method initializers (i.e. in a FunctionScope) or class extends + // clauses (technically part of the ClassScope, but visited instead by the NormalScope), + // they can be handled consistently in all scopes. + defineClassLiteral(n); + break; + // NOTE(johnlenz): If we ever support Array tuples, // we will need to handle them here as we do object literals // above. @@ -795,6 +833,37 @@ void defineVar(Node n) { } } + /** + * Defines a class literal. Handles any of the following cases: + *
    + *
  • Class declarations: class Foo { ... } + *
  • Class assignments: foo.Bar = class { ... } + *
  • Bleeding names: foo.Bar = class Baz { ... } + *
  • Properties: {foo: class { ... }} + *
  • Callbacks: foo(class { ... }) + *
+ */ + void defineClassLiteral(Node n) { + assertDefinitionNode(n, Token.CLASS); + + // Determine the name and JSDocInfo and l-value for the class. + // Any of these may be null. + Node lValue = NodeUtil.getBestLValue(n); + JSDocInfo info = NodeUtil.getBestJSDocInfo(n); + String className = NodeUtil.getBestLValueName(lValue); + + // Create the type and assign it on the CLASS node. + FunctionType classType = createClassTypeFromNodes(n, className, info, lValue); + setDeferredType(n, classType); + + // Declare this symbol in the current scope iff it's a class + // declaration. Otherwise, the declaration will happen in other + // code paths. + if (NodeUtil.isClassDeclaration(n)) { + defineSlot(n.getFirstChild(), classType); + } + } + /** * Defines a function literal. */ @@ -856,13 +925,112 @@ private boolean shouldUseFunctionLiteralType( return isLValueRootedInGlobalScope(lValue) || !type.isReturnTypeInferred(); } + /** + * Creates a new class type from the given class literal. + * This function does not need to worry about stubs and aliases because + * they are handled by createFunctionTypeFromNodes instead. + */ + private FunctionType createClassTypeFromNodes( + Node n, + @Nullable String name, + @Nullable JSDocInfo info, + @Nullable Node lvalueNode) { + + FunctionTypeBuilder builder = + new FunctionTypeBuilder(name, compiler, n, currentScope) + .usingClassSyntax() + .setContents(new AstFunctionContents(n)) + .setDeclarationScope(lvalueNode != null ? getLValueRootScope(lvalueNode) : null) + .inferTemplateTypeName(info, null); + + Node extendsClause = n.getSecondChild(); + Node classMembers = n.getLastChild(); + + // Look at the extends clause and/or JSDoc info to find a super class. Use generics from the + // JSDoc to supplement the extends type when available. + ObjectType baseType = findSuperClassFromNodes(extendsClause); + builder.inferInheritance(info, baseType); + + // Look for an explicit constructor. + Node constructor = NodeUtil.getFirstPropMatchingKey(classMembers, "constructor"); + if (constructor != null) { + // Note: constructor should have the following structure: + // MEMBER_FUNCTION_DEF [jsdoc_info] + // FUNCTION + // NAME + // PARAM_LIST ... + // BLOCK ... + // NodeUtil.getFirstPropMatchingKey returns the FUNCTION node. + JSDocInfo ctorInfo = NodeUtil.getBestJSDocInfo(constructor); + builder.inferParameterTypes(constructor.getSecondChild(), ctorInfo); + } else if (extendsClause.isEmpty()) { + // No explicit constructor and no superclass: constructor is no-args. + builder.inferImplicitConstructorParameters(new Node(Token.PARAM_LIST)); + } else { + // No explicit constructor, but we have a superclass. If we know it's type, then copy its + // constructor arguments. If not, make the constructor arguments unknown. + // TODO(sdh): consider allowing attaching constructor @param annotations somewhere else? + FunctionType extendsCtor = baseType != null ? baseType.getConstructor() : null; + if (extendsCtor != null) { + // Known superclass: copy the parameters node. + builder.inferImplicitConstructorParameters(extendsCtor.getParametersNode().cloneTree()); + } else { + // Unresolveable extends clause: suppress typechecking. + builder.inferImplicitConstructorParameters( + typeRegistry.createParametersWithVarArgs( + typeRegistry.getNativeType(JSTypeNative.UNKNOWN_TYPE))); + } + } + + // TODO(sdh): Handle template parameters. The constructor should store all parameters, + // while the instance type should only have the class-level parameters? + + return builder.buildAndRegister(); + } + + /** + * Look at the {@code extends} clause to find the instance type being extended. + * Returns {@code null} if there is no such clause, and unknown if the type cannot + * be determined. + */ + @Nullable + private ObjectType findSuperClassFromNodes(Node extendsNode) { + if (extendsNode.isEmpty()) { + // No extends clause: return null. + return null; + } + JSType ctorType = extendsNode.getJSType(); + if (ctorType == null) { + if (extendsNode.isQualifiedName()) { + // Look up qualified names in the scope (types won't be set on the AST until inference). + TypedVar var = currentScope.getVar(extendsNode.getQualifiedName()); + if (var != null) { + ctorType = var.getType(); + } + } else if (extendsNode.isCall()) { + // TODO(sdh): Do some limited type inference to get the return type for a mixin? + // The difficulty is that mixin functions are likely to be generic, so we need to be at + // least a little sophisticated here. + } + } + if (ctorType != null && (ctorType.isConstructor() || ctorType.isInterface())) { + return ctorType.toMaybeFunctionType().getInstanceType(); + } + // We couldn't determine the type, so for TypedScope creation purposes we will treat it as if + // there were no extends clause. TypeCheck will give a more precise error later. + return null; + } + /** * Creates a new function type, based on the given nodes. * * This handles two cases that are semantically very different, but * are not mutually exclusive: - * - A function literal that needs a type attached to it. - * - An assignment expression with function-type info in the JsDoc. + * - A function literal that needs a type attached to it (called from + * defineClassLiteral with a non-null FUNCTION node for rValue). + * - An assignment expression with function-type info in the JsDoc + * (called from getDeclaredType on a stub (rValue == null) or alias + * (rValue is a qualified name). * * All parameters are optional, and we will do the best we can to create * a function type. @@ -883,6 +1051,7 @@ private FunctionType createFunctionTypeFromNodes( @Nullable String name, @Nullable JSDocInfo info, @Nullable Node lvalueNode) { + // Check for an alias. if (rValue != null && rValue.isQualifiedName() && lvalueNode != null) { TypedVar var = currentScope.getVar(rValue.getQualifiedName()); if (var != null && var.getType() != null && var.getType().isFunctionType()) { @@ -898,6 +1067,7 @@ private FunctionType createFunctionTypeFromNodes( } } + // No alias: look for an explicit @type in JSDocInfo. if (info != null && info.hasType()) { JSType type = info.getType().evaluate(currentScope, typeRegistry); @@ -910,10 +1080,12 @@ private FunctionType createFunctionTypeFromNodes( } } + // No alias or explicit @type, so look for a function literal, or @param/@return. Node errorRoot = rValue == null ? lvalueNode : rValue; boolean isFnLiteral = rValue != null && rValue.isFunction(); Node fnRoot = isFnLiteral ? rValue : null; Node parametersNode = isFnLiteral ? rValue.getSecondChild() : null; + // Find the type of any overridden function. Node ownerNode = NodeUtil.getBestLValueOwner(lvalueNode); String ownerName = NodeUtil.getBestLValueName(ownerNode); @@ -952,12 +1124,10 @@ private FunctionType createFunctionTypeFromNodes( .setDeclarationScope(lvalueNode != null ? getLValueRootScope(lvalueNode) : null) .inferFromOverriddenFunction(overriddenType, parametersNode) .inferTemplateTypeName(info, prototypeOwner) - .inferInheritance(info); + .inferInheritance(info, null); if (info == null || !info.hasReturnType()) { - /** - * when there is no {@code @return} annotation, look for inline return type declaration - */ + // when there is no @return annotation, look for inline return type declaration if (rValue != null && rValue.isFunction() && rValue.getFirstChild() != null) { JSDocInfo nameDocInfo = rValue.getFirstChild().getJSDocInfo(); builder.inferReturnType(nameDocInfo, true); @@ -1137,6 +1307,7 @@ void defineSlot(Node n, JSType type, boolean inferred) { if (n.isName()) { checkArgument( parent.isFunction() + || parent.isClass() || NodeUtil.isNameDeclaration(parent) || parent.isParamList() || (parent.isRest() && parent.getParent().isParamList()) @@ -1989,11 +2160,11 @@ void visitPreorder(NodeTraversal t, Node n, Node parent) { // This isn't a big deal in most cases since a NamedType will be created and resolved later, // but if a NamedType is used for a superclass, we lose a lot of valuable checking. Recursing // into child blocks immediately prevents this from being a problem. - if (parent != null && NodeUtil.createsBlockScope(n)) { + if (parent != null && NodeUtil.createsBlockScope(n) && !n.isClass()) { createScope(n, currentScope); } - // All other functions are handled when we see the actual function node. + // All other functions (and classes, etc) are handled when we see the actual function node. if (n.isFunction() && !NodeUtil.isHoistedFunctionDeclaration(n)) { defineFunctionLiteral(n); } @@ -2146,6 +2317,40 @@ void declareArguments() { } // end declareArguments } // end FunctionScopeBuilder + /** + * Scope builder subclass for class scopes, which only contain a bleeding class name. Methods + * are handled by FunctionScopeBuilder and NormalScopeBuilder for the bodies. + */ + private final class ClassScopeBuilder extends AbstractScopeBuilder { + + ClassScopeBuilder(TypedScope scope) { + super(scope); + } + + @Override + void visitPreorder(NodeTraversal t, Node n, Node parent) { + // These are not descended into, so must be done preorder + if (n.isFunction()) { + if (parent.getString().equals("constructor")) { + // Constructor has already been analyzed, so pull that here. + n.setJSType(currentScope.getRootNode().getJSType()); + } + // TODO(sdh): handle non-constructor methods. + } + } + + @Override + void visitPostorder(NodeTraversal t, Node n, Node parent) { + if (n.isName() + && parent == currentScope.getRootNode() + && NodeUtil.isClassExpression(parent)) { + // Declare bleeding class name in scope. Pull the type off the AST. + checkState(!n.getString().isEmpty()); // anonymous classes have EMPTY nodes, not NAME + defineSlot(n, parent.getJSType(), false); + } + } + } // end ClassScopeBuilder + /** * Does a first-order function analysis that just looks at simple things like what variables are * escaped, and whether 'this' is used. diff --git a/src/com/google/javascript/jscomp/testing/TypeSubject.java b/src/com/google/javascript/jscomp/testing/TypeSubject.java index 044aa4efa58..51767371fae 100644 --- a/src/com/google/javascript/jscomp/testing/TypeSubject.java +++ b/src/com/google/javascript/jscomp/testing/TypeSubject.java @@ -103,6 +103,11 @@ public void isObjectTypeWithoutProperty(String propName) { "Type " + actualAsString() + " should not have property " + propName, actualPropType); } + public void isSubtypeOf(JSType superType) { + String message = "Type " + actualAsString() + " should be a subtype of " + superType; + assertTrue(message, actual().isSubtypeOf(superType)); + } + public void toStringIsEqualTo(String typeString) { assertEquals(typeString, actual().toString()); } diff --git a/src/com/google/javascript/rhino/jstype/FunctionType.java b/src/com/google/javascript/rhino/jstype/FunctionType.java index 9825f023492..7c087b6af2c 100644 --- a/src/com/google/javascript/rhino/jstype/FunctionType.java +++ b/src/com/google/javascript/rhino/jstype/FunctionType.java @@ -160,7 +160,7 @@ private enum PropAccess { templateTypeMap); setPrettyPrint(true); - checkArgument(source == null || Token.FUNCTION == source.getToken()); + checkArgument(source == null || source.isFunction() || source.isClass()); checkNotNull(arrowType); this.source = source; this.kind = kind; diff --git a/src/com/google/javascript/rhino/jstype/InstanceObjectType.java b/src/com/google/javascript/rhino/jstype/InstanceObjectType.java index 0e4a2d17139..7422f4ad3a2 100644 --- a/src/com/google/javascript/rhino/jstype/InstanceObjectType.java +++ b/src/com/google/javascript/rhino/jstype/InstanceObjectType.java @@ -94,12 +94,20 @@ boolean defineProperty(String name, JSType type, boolean inferred, @Override StringBuilder appendTo(StringBuilder sb, boolean forAnnotations) { - return constructor.hasReferenceName() - ? sb.append( - forAnnotations - ? constructor.getNormalizedReferenceName() - : constructor.getReferenceName()) - : super.appendTo(sb, forAnnotations); + if (!constructor.hasReferenceName()) { + return super.appendTo(sb, forAnnotations); + } else if (forAnnotations) { + return sb.append(constructor.getNormalizedReferenceName()); + } + String name = constructor.getReferenceName(); + if (name.isEmpty()) { + Node n = constructor.getSource(); + return sb.append(""); + } + return sb.append(name); } @Override diff --git a/src/com/google/javascript/rhino/jstype/JSTypeRegistry.java b/src/com/google/javascript/rhino/jstype/JSTypeRegistry.java index 4f5b649cb0f..352d87b571e 100644 --- a/src/com/google/javascript/rhino/jstype/JSTypeRegistry.java +++ b/src/com/google/javascript/rhino/jstype/JSTypeRegistry.java @@ -1739,7 +1739,7 @@ public FunctionType createConstructorType( JSType returnType, ImmutableList templateKeys, boolean isAbstract) { - checkArgument(source == null || source.isFunction()); + checkArgument(source == null || source.isFunction() || source.isClass()); return new FunctionBuilder(this) .forConstructor() .withName(name) diff --git a/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java b/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java index 20405cb7d89..560304d945a 100644 --- a/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java +++ b/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java @@ -1764,4 +1764,411 @@ public void testITemplateArray1() { "found : string", "required: number")); } + + public void testClassDeclaration() { + testTypes( + lines( + "class Foo {}", // + "var /** !Foo */ foo = new Foo();")); + } + + public void testClassDeclarationMismatch() { + testTypes( + lines( + "class Foo {}", // + "class Bar {}", + "var /** !Foo */ foo = new Bar();"), + lines( + "initializing variable", // + "found : Bar", + "required: Foo")); + } + + public void testClassGenerics() { + testTypes( + lines( + "/** @template T */", // + "class Foo {}", + "var /** !Foo */ x = new Foo();", + "var /** !Foo */ y = x;"), + lines( + "initializing variable", // + "found : Foo", + "required: Foo")); + } + + public void testClassTooManyTypeParameters() { + // TODO(sdh): This should give a warning about too many type parameters. + testTypes( + lines( + "class Foo {}", // + "var /** !Foo */ x = new Foo();", + "var /** !Foo */ y = x;")); + } + + public void testClassDeclarationWithExtends() { + testTypes( + lines( + "class Foo {}", // + "class Bar extends Foo {}", + "var /** !Foo */ foo = new Bar();")); + } + + public void testClassDeclarationWithExtendsMismatch() { + testTypes( + lines( + "class Foo {}", // + "class Bar extends Foo {}", + "var /** !Bar */ foo = new Foo();"), + lines( + "initializing variable", // + "found : Foo", + "required: Bar")); + } + + public void testClassDeclarationWithTransitiveExtends() { + testTypes( + lines( + "class Foo {}", // + "class Bar extends Foo {}", + "class Baz extends Bar {}", + "var /** !Foo */ foo = new Baz();")); + } + + public void testClassDeclarationWithAnonymousExtends() { + testTypes( + lines( + "class Foo {}", // + "class Bar extends class extends Foo {} {}", + "var /** !Foo */ foo = new Bar();")); + } + + public void testClassDeclarationInlineConstructorParameters() { + testTypes( + lines( + "class Foo {", // + " constructor(/** number */ arg) {}", + "}", + "new Foo(42);")); + } + + public void testClassDeclarationConstructorParametersMismatch() { + testTypes( + lines( + "class Foo {", + " constructor(/** number */ arg) {}", + "}", + "new Foo('xyz');"), + lines( + "actual parameter 1 of Foo does not match formal parameter", + "found : string", + "required: number")); + } + + public void testClassDeclarationTraditionalConstructorParameters() { + testTypes( + lines( + "class Foo {", // + " /** @param {number} arg */", + " constructor(arg) {}", + "}", + "new Foo(42);")); + } + + public void testClassDeclarationTraditionalConstructorParametersMismatch() { + testTypes( + lines( + "class Foo {", + " /** @param {number} arg */", + " constructor(arg) {}", + "}", + "new Foo('xyz');"), + lines( + "actual parameter 1 of Foo does not match formal parameter", + "found : string", + "required: number")); + } + + public void testClassDeclarationInheritedConstructorParameters() { + testTypes( + lines( + "class Foo {", + " constructor(/** number */ arg) {}", + "}", + "class Bar extends Foo {}", + "new Bar(42);")); + } + + public void testClassDeclarationInheritedConstructorParametersMismatch() { + testTypes( + lines( + "class Foo {", + " constructor(/** number */ arg) {}", + "}", + "class Bar extends Foo {}", + "new Bar('xyz');"), + lines( + "actual parameter 1 of Bar does not match formal parameter", + "found : string", + "required: number")); + } + + public void testClassPassedAsParameter() { + testTypes( + lines( + "class Foo {}", + "function foo(/** function(new: Foo) */ arg) {}", + "foo(class extends Foo {});")); + } + + public void testClassPassedAsParameterClassMismatch() { + testTypes( + lines( + "class Foo {}", + "function foo(/** function(new: Foo) */ arg) {}", + "foo(class {});"), + lines( + "actual parameter 1 of foo does not match formal parameter", + "found : function(new:): undefined", + "required: function(new:Foo): ?")); + } + + public void testClassPassedAsParameterConstructorParamsMismatch() { + testTypes( + lines( + "class Foo {", + " constructor(/** string */ arg) {}", + "}", + "function foo(/** function(new: Foo, number) */ arg) {}", + "foo(Foo);"), + lines( + "actual parameter 1 of foo does not match formal parameter", + "found : function(new:Foo, string): undefined", + "required: function(new:Foo, number): ?")); + } + + public void testClassExpression() { + testTypes( + lines( + "var Foo = class Bar {}", // + "var /** !Foo */ foo = new Foo();")); + } + + public void testClassExpressionDoesNotDefineTypeNameInOuterScope() { + testTypes( + lines( + "var Foo = class Bar {}", // + "var /** !Bar */ foo = new Foo();"), + "Bad type annotation. Unknown type Bar"); + } + + public void testClassExpressionDoesNotDefineConstructorReferenceInOuterScope() { + // Test that Bar is not defined in the outer scope, which makes it unknown (the error is + // generated by VarCheck, which is not run here). If it were defined in the outer scope then + // we'd get a type error assigning it to null. + testTypes( + lines( + "var Foo = class Bar {}", // + "var /** null */ foo = new Bar();")); + } + + public void testClassExpressionAsStaticClassProeprty() { + testTypes( + lines( + "class Foo {}", // + "Foo.Bar = class extends Foo {}", + "var /** !Foo */ foo = new Foo.Bar();")); + } + + public void testClassSyntaxClassExtendsInterface() { + testTypes( + lines( + "/** @interface */", // + "class Bar {}", + "class Foo extends Bar {}"), + "Foo cannot extend this type; constructors can only extend constructors"); + } + + public void testClassSyntaxClassExtendsNonClass() { + testTypes( + "class Foo extends 42 {}", + "Foo cannot extend this type; constructors can only extend constructors"); + } + + public void testClassSyntaxInterfaceExtendsClass() { + testTypes( + lines( + "class Bar {}", // + "/** @interface */", + "class Foo extends Bar {}"), + "Foo cannot extend this type; interfaces can only extend interfaces"); + } + + public void testClassSyntaxInterfaceExtendsInterface() { + testTypes( + lines( + "/** @interface */", // + "class Bar {}", + "/** @interface */", + "class Foo extends Bar {}", + "var /** !Foo */ foo;", + "var /** !Bar */ bar = foo;")); + } + + public void testClassSyntaxInterfaceExtendsInterfaceMismatch() { + testTypes( + lines( + "/** @interface */", // + "class Bar {}", + "/** @interface */", + "class Foo extends Bar {}", + "var /** !Bar */ bar;", + "var /** !Foo */ foo = bar;"), + lines( + "initializing variable", // + "found : Bar", + "required: Foo")); + } + + public void testClassSyntaxRecord() { + // TODO(sdh): Add a matching property. + testTypes( + lines( + "/** @record */", // + "class Rec {}", + "var /** !Rec */ rec = {};")); + } + + public void testClassSyntaxRecordMismatch() { + // TODO(sdh): Should be an error. + testTypes( + lines( + "/** @record */", // + "class Rec {", + " constructor() {", + " /** @type {number} */", + " this.foo;", + " }", + "}", + "var /** !Rec */ rec = {foo: string};")); + } + + public void testClassJSDocExtendsInconsistentWithExtendsClause() { + // TODO(sdh): Should be an error. + testTypes( + lines( + "class Bar {}", // + "class Baz {}", + "/** @extends {Bar} */", + "class Foo extends Baz {}")); + } + + public void testClassJSDocExtendsWithMissingExtendsClause() { + // TODO(sdh): Should be an error, but we may need to clean up the codebase first. + testTypes( + lines( + "class Bar {}", // + "/** @extends {Bar} */", + "class Foo {}")); + } + + public void testClassMissingSuperCall() { + // TODO(sdh): Should be an error to access 'this' before super (but maybe not in TypeCheck). + testTypes( + lines( + "class Bar {}", // + "class Foo extends Bar {", + " constructor() {", + " this.x = 42;", + " }", + "}")); + } + + public void testClassDeclarationWithExtendsOnlyInJSDoc() { + // TODO(sdh): Should be an error, but we may need to clean up the codebase first. + testTypes( + lines( + "class Foo {}", // + "/** @extends {Foo} */", + "class Bar {}", + "var /** !Foo */ foo = new Bar();")); + } + + public void testClassConstructorTypeParametersNotIncludedOnClass() { + // TODO(sdh): This should give a warning about too many type parameters. + testTypes( + lines( + "/** @template T */", + "class Foo {", + " /** @template U */", + " constructor() {}", + "}", + "var /** !Foo */ x = new Foo();", + "var /** !Foo */ y = x;")); + } + + public void testClassConstructorTypeParametersChecked() { + // TODO(sdh): This should *not* give an error. + testTypes( + lines( + "/** @template T */", + "class Foo {", + " /** @template U */", + " constructor(/** U */ arg1, /** function(U): T */ arg2) {}", + "}", + "/** @param {string} arg", + " @return {number} */", + "function f(arg) {}", + "var /** !Foo */ foo = new Foo('x', f);"), + new String[] { + "Bad type annotation. Unknown type U", + "Bad type annotation. Unknown type U", + }); + } + + public void testClassConstructorTypeParametersWithClassTypeMismatch() { + // TODO(sdh): This should only fail with the type mismatch error. + testTypes( + lines( + "/** @template T */", + "class Foo {", + " /** @template U */", + " constructor(/** U */ arg1, /** function(U): T */ arg2) {}", + "}", + "/** @param {string} arg", + " @return {number} */", + "function f(arg) {}", + "var /** !Foo */ foo = new Foo('x', f);"), + new String[] { + "Bad type annotation. Unknown type U", + "Bad type annotation. Unknown type U", + lines( + "initializing variable", // + "found : Foo", + "required: Foo"), + }); + } + + public void testClassConstructorTypeParametersWithParameterTypeMismatch() { + // TODO(sdh): This should fail with the correct error. + testTypes( + lines( + "/** @template T */", + "class Foo {", + " /** @template U */", + " constructor(/** U */ arg1, /** function(U): T */ arg2) {}", + "}", + "/** @param {string} arg", + " @return {number} */", + "function f(arg) {}", + "var foo = new Foo(42, f);"), + new String[] { + "Bad type annotation. Unknown type U", + "Bad type annotation. Unknown type U", + }); + // lines( + // "actual parameter 2 of Foo does not match formal parameter", + // "found : function(string): ?", + // "required: function(number): ?")); + } } diff --git a/test/com/google/javascript/jscomp/TypeCheckTest.java b/test/com/google/javascript/jscomp/TypeCheckTest.java index 33c123354d2..8bc2e5a5054 100644 --- a/test/com/google/javascript/jscomp/TypeCheckTest.java +++ b/test/com/google/javascript/jscomp/TypeCheckTest.java @@ -20071,7 +20071,7 @@ public void testAnnotatedObjectLiteralInBlocklessArrow2() { "() => f({/** @constructor */ g: function() {}});"), lines( "actual parameter 1 of f does not match formal parameter", - "found : {g: function(new:): undefined}", + "found : {g: function(new:): undefined}", "required: {g: function(): string}", "missing : []", "mismatch: [g]")); diff --git a/test/com/google/javascript/jscomp/TypedScopeCreatorTest.java b/test/com/google/javascript/jscomp/TypedScopeCreatorTest.java index fc5e5d30131..43fb262092f 100644 --- a/test/com/google/javascript/jscomp/TypedScopeCreatorTest.java +++ b/test/com/google/javascript/jscomp/TypedScopeCreatorTest.java @@ -31,6 +31,7 @@ import static com.google.javascript.rhino.jstype.JSTypeNative.UNKNOWN_TYPE; import com.google.common.base.Predicate; +import com.google.common.collect.ImmutableList; import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback; import com.google.javascript.rhino.InputId; import com.google.javascript.rhino.Node; @@ -1336,6 +1337,115 @@ public void testConstructorNode() { assertEquals("function(new:goog.Foo): undefined", ctor.toString()); } + public void testClassDeclaration() { + testSame("class Foo {}"); + FunctionType foo = (FunctionType) (findNameType("Foo", globalScope)); + assertTrue(foo.isConstructor()); + assertScope(globalScope).declares("Foo").withTypeThat().isEqualTo(foo); + } + + public void testClassDeclarationWithConstructor() { + testSame( + lines( + "class Foo {", // + " /** @param {number} arg */", + " constructor(arg) {}", + "}")); + FunctionType foo = (FunctionType) (findNameType("Foo", globalScope)); + assertTrue(foo.isConstructor()); + List params = ImmutableList.copyOf(foo.getParameterTypes()); + assertThat(params).hasSize(1); + assertType(params.get(0)).isNumber(); + } + + public void testClassDeclarationWithExtends() { + testSame( + lines( + "class Bar {}", // + "class Foo extends Bar {}")); + FunctionType bar = (FunctionType) (findNameType("Bar", globalScope)); + FunctionType foo = (FunctionType) (findNameType("Foo", globalScope)); + assertType(foo.getInstanceType()).isSubtypeOf(bar.getInstanceType()); + assertScope(globalScope).declares("Bar").withTypeThat().isEqualTo(bar); + assertScope(globalScope).declares("Foo").withTypeThat().isEqualTo(foo); + } + + public void testClassDeclarationWithNestedExtends() { + testSame( + lines( + "class Bar {}", // + "class Foo extends class extends class extends Bar {} {} {}")); + FunctionType bar = (FunctionType) (findNameType("Bar", globalScope)); + FunctionType foo = (FunctionType) (findNameType("Foo", globalScope)); + assertType(foo.getInstanceType()).isSubtypeOf(bar.getInstanceType()); + } + + public void testClassDeclarationWithInheritedConstructor() { + testSame( + lines( + "class Bar {", + " constructor(/** string */ arg) {}", + "}", + "class Foo extends Bar {}")); + FunctionType foo = (FunctionType) (findNameType("Foo", globalScope)); + List params = ImmutableList.copyOf(foo.getParameterTypes()); + assertThat(params).hasSize(1); + assertType(params.get(0)).isString(); + } + + public void testClassDeclarationWithOverriddenConstructor() { + testSame( + lines( + "class Bar {", + " constructor(/** string */ arg) {}", + "}", + "class Foo extends Bar {", + " constructor(/** number */ arg) {}", + "}")); + FunctionType foo = (FunctionType) (findNameType("Foo", globalScope)); + List params = ImmutableList.copyOf(foo.getParameterTypes()); + assertThat(params).hasSize(1); + assertType(params.get(0)).isNumber(); + } + + public void testClassDeclarationWithNestedExtendsAndInheritedConstructor() { + testSame( + lines( + "class Bar {", + " constructor(/** string */ arg) {}", + "}", + "class Foo extends class extends class extends Bar {} {} {}")); + FunctionType foo = (FunctionType) (findNameType("Foo", globalScope)); + List params = ImmutableList.copyOf(foo.getParameterTypes()); + assertThat(params).hasSize(1); + assertType(params.get(0)).isString(); + } + + public void testClassExpressionDeclaration() { + testSame("var Foo = class Bar {}"); + FunctionType foo = (FunctionType) (findNameType("Foo", globalScope)); + assertTrue(foo.isConstructor()); + FunctionType bar = (FunctionType) (findNameType("Bar", globalScope)); + assertEquals(foo, bar); + } + + public void testClassExpressionBleedingNameScope() { + testSame( + lines( + "var Foo = class Bar {", + " constructor() {", + " CTOR:;", + " }", + "}")); + FunctionType foo = (FunctionType) (findNameType("Foo", globalScope)); + TypedScope ctorBlockScope = getLabeledStatement("CTOR").enclosingScope; + TypedScope ctorScope = ctorBlockScope.getParentScope(); + TypedScope classScope = ctorScope.getParentScope(); + assertScope(globalScope).declares("Foo").withTypeThat().isEqualTo(foo); + assertScope(classScope).declares("Bar").directly().withTypeThat().isEqualTo(foo); + assertScope(globalScope).doesNotDeclare("Bar"); + } + public void testForLoopIntegration() { testSame("var y = 3; for (var x = true; x; y = x) {}"); @@ -2470,15 +2580,15 @@ private static Node createEmptyRoot() { return root; } - private JSType findNameType(final String name, TypedScope scope) { + private JSType findNameType(String name, TypedScope scope) { return findTypeOnMatchedNode(n -> n.matchesQualifiedName(name), scope); } - private String findNameTypeStr(final String name, TypedScope scope) { + private String findNameTypeStr(String name, TypedScope scope) { return findNameType(name, scope).toString(); } - private JSType findTokenType(final Token type, TypedScope scope) { + private JSType findTokenType(Token type, TypedScope scope) { return findTypeOnMatchedNode(n -> type == n.getToken(), scope); }