Skip to content

Commit

Permalink
Don't warn for missing super() in a constructor with an explicit return
Browse files Browse the repository at this point in the history
It's ok at runtime as long as the constructor returns an object, e.g.

class C {}
class D extends C {
  constructor() {
    return {foo: 3};
  }
}
console.log(new D()); // Object {foo: 3}

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=202696612
  • Loading branch information
lauraharker committed Jul 2, 2018
1 parent 7fa1896 commit efb87d0
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 2 deletions.
10 changes: 8 additions & 2 deletions src/com/google/javascript/jscomp/CheckSuper.java
Expand Up @@ -82,7 +82,7 @@ private boolean visitClass(NodeTraversal t, Node n) {
return true;
}

FindSuper finder = new FindSuper();
FindSuperOrReturn finder = new FindSuperOrReturn();
NodeTraversal.traverse(compiler, NodeUtil.getFunctionBody(constructor), finder);

if (!finder.found) {
Expand Down Expand Up @@ -119,7 +119,7 @@ private void visitSuper(NodeTraversal t, Node n, Node parent) {
}
}

private static final class FindSuper implements Callback {
private static final class FindSuperOrReturn implements Callback {
boolean found = false;

@Override
Expand All @@ -138,6 +138,12 @@ public void visit(NodeTraversal t, Node n, Node parent) {
found = true;
return;
}
// allow return <some expr>; instead of super(), which works at runtime as long as
// <some expr> is an object.
if (n.isReturn() && n.hasChildren()) {
found = true;
return;
}
}
}
}
21 changes: 21 additions & 0 deletions test/com/google/javascript/jscomp/CheckSuperTest.java
Expand Up @@ -114,4 +114,25 @@ public void testDotMethodInNonConstructor() {
// TODO(tbreisacher): Consider warning for this. It's valid but likely indicates a mistake.
testSame("class C extends D { foo() { super.bar(); }}");
}

public void testNoWarning_withExplicitReturnInConstructor() {
testNoWarning("class C extends D { constructor() { return {}; } }");
testNoWarning("class C extends D { constructor() { return f(); } }");
// the following cases will error at runtime but we don't bother checking them.
testNoWarning("class C extends D { constructor() { return 3; } }");
testNoWarning("class C extends D { constructor() { if (false) return {}; } }");
}

public void testWarning_withInvalidReturnInConstructor() {
// empty return
testError("class C extends D { constructor() { return; } }", MISSING_CALL_TO_SUPER);

// return in arrow function
testError(
"class C extends D { constructor() { () => { return {}; } } }", MISSING_CALL_TO_SUPER);
}

public void testInvalidThisReference_withExplicitReturnInConstructor() {
testError("class C extends D { constructor() { this.x = 3; return {}; } }", THIS_BEFORE_SUPER);
}
}

0 comments on commit efb87d0

Please sign in to comment.