Skip to content

Commit

Permalink
Fix crash when writing class Foo extends Foo {}
Browse files Browse the repository at this point in the history
This is obviously an error, but the compiler shouldn't crash on it.  In fixing this, I also cleaned up some variable names in `TypeValidator#expectExtends` since it was difficult to tell what the difference between "superCtor" and "declaredSuper" were.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=206108306
  • Loading branch information
shicks authored and brad4d committed Jul 26, 2018
1 parent f76817a commit 3a84b43
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 11 deletions.
34 changes: 23 additions & 11 deletions src/com/google/javascript/jscomp/TypeValidator.java
Expand Up @@ -690,35 +690,47 @@ void expectSuperType(NodeTraversal t, Node n, ObjectType superObject,


/** /**
* Expect that an ES6 class's extends clause is actually a supertype of the given class. * Expect that an ES6 class's extends clause is actually a supertype of the given class.
* Compares the registered supertype, which is taken from the JSDoc if present, otherwise
* from the AST, with the type in the extends node of the AST.
* *
* @param n The node where warnings should point to. * @param n The node where warnings should point to.
* @param subCtor The sub constructor type. * @param subCtor The sub constructor type.
* @param superCtor The expected super constructor. * @param astSuperCtor The expected super constructor from the extends node in the AST.
*/ */
void expectExtends(Node n, FunctionType subCtor, FunctionType superCtor) { void expectExtends(Node n, FunctionType subCtor, FunctionType astSuperCtor) {
if (superCtor == null || (!superCtor.isConstructor() && !superCtor.isInterface())) { if (astSuperCtor == null || (!astSuperCtor.isConstructor() && !astSuperCtor.isInterface())) {
// toMaybeFunctionType failed, or we've got a loose type. Let it go for now. // toMaybeFunctionType failed, or we've got a loose type. Let it go for now.
return; return;
} }
if (superCtor.isConstructor() != subCtor.isConstructor()) { if (astSuperCtor.isConstructor() != subCtor.isConstructor()) {
// Don't bother looking if one is a constructor and the other is an interface. // Don't bother looking if one is a constructor and the other is an interface.
// We'll report an error elsewhere. // We'll report an error elsewhere.
return; return;
} }
ObjectType superInstance = superCtor.getInstanceType(); ObjectType astSuperInstance = astSuperCtor.getInstanceType();
if (subCtor.isConstructor()) { if (subCtor.isConstructor()) {
// There should be exactly one superclass, and it needs to have this constructor. // There should be exactly one superclass, and it needs to have this constructor.
ObjectType declaredSuper = subCtor.getSuperClassConstructor().getInstanceType(); // Note: if the registered supertype (from the @extends jsdoc) was unresolved,
if (!superInstance.isEquivalentTo(declaredSuper)) { // then getSuperClassConstructor will be null - make sure not to crash.
mismatch(n, "mismatch in declaration of superclass type", superInstance, declaredSuper); FunctionType registeredSuperCtor = subCtor.getSuperClassConstructor();
if (registeredSuperCtor != null) {
ObjectType registeredSuperInstance = registeredSuperCtor.getInstanceType();
if (!astSuperInstance.isEquivalentTo(registeredSuperInstance)) {
mismatch(
n,
"mismatch in declaration of superclass type",
astSuperInstance,
registeredSuperInstance);
}
} }
} else if (subCtor.isInterface()) { } else if (subCtor.isInterface()) {
// Find an equivalent constructor in the superinterfaces. There may have been multiple // Find an equivalent constructor in the superinterfaces. There may have been multiple
// super-interfaces marked, but we can't know which was intended so just give the error // super-interfaces marked, but we can't know which was intended so just give the error
// on the first one. // on the first one.
if (!subCtor.explicitlyImplOrExtInterface(superCtor)) { if (!subCtor.explicitlyImplOrExtInterface(astSuperCtor)) {
ObjectType extended = subCtor.getExtendedInterfaces().get(0); ObjectType registeredExtended = subCtor.getExtendedInterfaces().get(0);
mismatch(n, "mismatch in declaration of superclass type", superInstance, extended); mismatch(
n, "mismatch in declaration of superclass type", astSuperInstance, registeredExtended);
} }
} }
} }
Expand Down
38 changes: 38 additions & 0 deletions test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java
Expand Up @@ -3368,6 +3368,44 @@ public void testClassExtendsForwardReferencedClass() {
"var /** !ns.Base */ x = new Sub();")); "var /** !ns.Base */ x = new Sub();"));
} }


public void testClassExtendsItself() {
testTypes(
"class Foo extends Foo {}",
new String[] {
"Could not resolve type in @extends tag of Foo",
"Parse error. Cycle detected in inheritance chain of type Foo",
});
}

public void testClassExtendsCycle() {
testTypes(
lines(
"class Foo extends Bar {}",
"class Bar extends Foo {}"),
"Parse error. Cycle detected in inheritance chain of type Bar");
}

public void testClassExtendsCycleOnlyInJsdoc() {
testTypes(
lines(
"class Bar {}",
"/** @extends {Foo} */",
"class Foo extends Bar {}"),
new String[] {
"Parse error. Cycle detected in inheritance chain of type Foo",
"Could not resolve type in @extends tag of Foo",
});
}

public void testClassExtendsCycleOnlyInAst() {
// TODO(sdh): This should give an error.
testTypes(
lines(
"class Bar {}",
"/** @extends {Bar} */",
"class Foo extends Foo {}"));
}

public void testAsyncFunctionWithoutJSDoc() { public void testAsyncFunctionWithoutJSDoc() {
testTypes("async function f() { return 3; }"); testTypes("async function f() { return 3; }");
} }
Expand Down

0 comments on commit 3a84b43

Please sign in to comment.