Skip to content

Commit

Permalink
Prevent calls to super outside of a constructor
Browse files Browse the repository at this point in the history
  • Loading branch information
@ChadKillingsworth
ChadKillingsworth committed Apr 24, 2017
1 parent 8829c16 commit cc8e8c3
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 103 deletions.
127 changes: 65 additions & 62 deletions src/com/google/javascript/jscomp/Es6ConvertSuper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.google.javascript.jscomp;

import static com.google.javascript.jscomp.Es6ToEs3Converter.CANNOT_CONVERT_YET;
import static com.google.javascript.jscomp.Es6ToEs3Converter.INVALID_SUPER;

import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
Expand Down Expand Up @@ -109,95 +110,97 @@ private boolean isInterface(Node classNode) {
}

private void visitSuper(Node node, Node parent) {
Node enclosingCall = parent;
Node potentialCallee = node;
if (!parent.isCall()) {
enclosingCall = parent.getParent();
potentialCallee = parent;
Node exprRoot = node;

if (exprRoot.getParent().isGetElem() || exprRoot.getParent().isGetProp()) {
exprRoot = exprRoot.getParent();
}
if (!enclosingCall.isCall()
|| enclosingCall.getFirstChild() != potentialCallee
|| enclosingCall.getFirstChild().isGetElem()) {

Node enclosingMemberDef =
NodeUtil.getEnclosingNode(
exprRoot,
new Predicate<Node>() {
@Override
public boolean apply(Node n) {
switch (n.getToken()) {
case MEMBER_FUNCTION_DEF:
case GETTER_DEF:
case SETTER_DEF:
return true;
default:
return false;
}
}
});

if (exprRoot.getParent().isNew()) {
compiler.report(JSError.make(node, INVALID_SUPER));
} else if (NodeUtil.isCallOrNewTarget(exprRoot)
&& (exprRoot.isSuper() || exprRoot.getFirstChild().isSuper())) {
visitSuperCall(node, exprRoot, enclosingMemberDef);
} else {
compiler.report(JSError.make(node, CANNOT_CONVERT_YET,
"Only calls to super or to a method of super are supported."));
return;
}
}

private void visitSuperCall(Node node, Node callTarget, Node enclosingMemberDef) {
Node enclosingCall = callTarget.getParent();
Preconditions.checkState(enclosingCall.isCall());

Node clazz = NodeUtil.getEnclosingClass(node);
Node superName = clazz.getSecondChild();
if (!superName.isQualifiedName()) {
// This will be reported as an error in Es6ToEs3Converter.
return;
} else if (callTarget.isGetElem()) {
compiler.report(JSError.make(node, CANNOT_CONVERT_YET,
"Only calls to super or to a method of super are supported."));
return;
}

Node enclosingMemberDef = NodeUtil.getEnclosingNode(
node,
new Predicate<Node>() {
@Override
public boolean apply(Node n) {
switch (n.getToken()) {
case MEMBER_FUNCTION_DEF:
case GETTER_DEF:
case SETTER_DEF:
return true;
default:
return false;
}
}
});
if (enclosingMemberDef.getString().equals("constructor")
&& parent.isCall()
&& parent.getFirstChild() == node) {
if (enclosingMemberDef.isMemberFunctionDef()
&& enclosingMemberDef.getString().equals("constructor")
&& callTarget.isSuper()) {
// Calls to super() constructors will be transpiled by Es6ConvertSuperConstructorCalls later.
if (node.isFromExterns() || isInterface(clazz)) {
// If a class is defined in an externs file or as an interface, it's only a stub, not an
// implementation that should be instantiated.
// A call to super() shouldn't actually exist for these cases and is problematic to
// transpile, so just drop it.
Node enclosingStatement = NodeUtil.getEnclosingStatement(node);
Node enclosingScope = enclosingStatement.getParent();
enclosingStatement.detach();
compiler.reportChangeToEnclosingScope(enclosingScope);
NodeUtil.getEnclosingStatement(node).detach();
compiler.reportCodeChange();
}
// Calls to super() constructors will be transpiled by Es6ConvertSuperConstructorCalls
// later.
return;
}
if (enclosingMemberDef.isStaticMember()) {
Node callTarget;
potentialCallee.detach();
if (potentialCallee == node) {
// of the form super()
// TODO(bradfordcsmith): This should report an error since this is not allowed by the
// current spec.
potentialCallee =
IR.getprop(superName.cloneTree(), IR.string(enclosingMemberDef.getString()));
enclosingCall.putBooleanProp(Node.FREE_CALL, false);
} else {
// of the form super.method()
potentialCallee.replaceChild(node, superName.cloneTree());
} else if (enclosingMemberDef.isStaticMember()) {
if (callTarget.isSuper()) {
compiler.report(JSError.make(node, INVALID_SUPER));
return;
}
callTarget = IR.getprop(potentialCallee, IR.string("call"));
// of the form super.method()
callTarget.replaceChild(node, superName.cloneTree());
callTarget = IR.getprop(callTarget.detach(), IR.string("call"));
enclosingCall.addChildToFront(callTarget);
enclosingCall.addChildAfter(IR.thisNode(), callTarget);
enclosingCall.useSourceInfoIfMissingFromForTree(enclosingCall);
compiler.reportChangeToEnclosingScope(enclosingCall);
return;
}

String methodName;
Node callName = enclosingCall.removeFirstChild();
if (callName.isSuper()) {
// TODO(bradfordcsmith): This should report an error since this is not allowed by the
// current spec.
methodName = enclosingMemberDef.getString();
} else {
methodName = callName.getLastChild().getString();
if (callTarget.isSuper()) {
compiler.report(JSError.make(node, INVALID_SUPER));
return;
}

String newPropName = Joiner.on('.').join(superName.getQualifiedName(), "prototype");
Node newProp = NodeUtil.newQName(compiler, newPropName);
node.replaceWith(newProp);
callTarget = IR.getprop(callTarget.detach(), IR.string("call"));
enclosingCall.addChildToFront(callTarget);
enclosingCall.addChildAfter(IR.thisNode(), callTarget);
enclosingCall.putBooleanProp(Node.FREE_CALL, false);
enclosingCall.useSourceInfoIfMissingFromForTree(enclosingCall);
}
Node baseCall = baseCall(
superName.getQualifiedName(), methodName, enclosingCall.removeChildren());
baseCall.useSourceInfoIfMissingFromForTree(enclosingCall);
enclosingCall.replaceWith(baseCall);
compiler.reportChangeToEnclosingScope(baseCall);
compiler.reportCodeChange();
}

private Node baseCall(String baseClass, String methodName, Node arguments) {
Expand Down
4 changes: 4 additions & 0 deletions src/com/google/javascript/jscomp/Es6ToEs3Converter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ public final class Es6ToEs3Converter implements NodeTraversal.Callback, HotSwapC
"JSC_CANNOT_CONVERT_YET",
"ES6 transpilation of ''{0}'' is not yet implemented.");

static final DiagnosticType INVALID_SUPER =
DiagnosticType.error(
"JSC_INVALID_SUPER", "Calls to super cannot be used outside of a constructor.");

static final DiagnosticType DYNAMIC_EXTENDS_TYPE = DiagnosticType.error(
"JSC_DYNAMIC_EXTENDS_TYPE",
"The class in an extends clause must be a qualified name.");
Expand Down
47 changes: 6 additions & 41 deletions test/com/google/javascript/jscomp/Es6ToEs3ConverterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static com.google.javascript.jscomp.Es6ToEs3Converter.CANNOT_CONVERT;
import static com.google.javascript.jscomp.Es6ToEs3Converter.CANNOT_CONVERT_YET;
import static com.google.javascript.jscomp.Es6ToEs3Converter.CONFLICTING_GETTER_SETTER_TYPE;
import static com.google.javascript.jscomp.Es6ToEs3Converter.INVALID_SUPER;
import static com.google.javascript.jscomp.TypeCheck.INSTANTIATE_ABSTRACT_CLASS;

import com.google.javascript.jscomp.CompilerOptions.LanguageMode;
Expand Down Expand Up @@ -880,18 +881,7 @@ public void testSuperCall() {
" $jscomp.inherits(D, C);",
"};"));

test(
"class D {} class C extends D { constructor() {}; f() {super();} }",
LINE_JOINER.join(

"/** @constructor @struct */",
"var D = function() {};",
"/** @constructor @struct @extends {D} */",
"var C = function() {}",
"$jscomp.inherits(C, D);",
"C.prototype.f = function() {",
" D.prototype.f.call(this);",
"}"));
testError("class D {} class C extends D { constructor() {} f() {super();} }", INVALID_SUPER);
}

public void testSuperKnownNotToChangeThis() {
Expand Down Expand Up @@ -1301,11 +1291,8 @@ public void testSuperGet() {
}

public void testSuperNew() {
testError("class D {} class C extends D { f() {var s = new super;} }",
CANNOT_CONVERT_YET);

testError("class D {} class C extends D { f(str) {var s = new super(str);} }",
CANNOT_CONVERT_YET);
testError("class D {} class C extends D { f() {var s = new super;} }", INVALID_SUPER);
testError("class D {} class C extends D { f(str) {var s = new super(str);} }", INVALID_SUPER);
}

public void testSuperSpread() {
Expand All @@ -1330,31 +1317,9 @@ public void testSuperSpread() {
}

public void testSuperCallNonConstructor() {
testError("class S extends B { static f() { super(); } }", INVALID_SUPER);

test(
"class S extends B { static f() { super(); } }",
LINE_JOINER.join(
"/** @constructor @struct",
" * @extends {B}",
" * @param {...?} var_args",
" */",
"var S = function(var_args) { return B.apply(this, arguments) || this; };",
"$jscomp.inherits(S, B);",
"/** @this {?} */",
"S.f=function() { B.f.call(this) }"));

test(
"class S extends B { f() { super(); } }",
LINE_JOINER.join(
"/** @constructor @struct",
" * @extends {B}",
" * @param {...?} var_args",
" */",
"var S = function(var_args) { return B.apply(this, arguments) || this; };",
"$jscomp.inherits(S, B);",
"S.prototype.f=function() {",
" B.prototype.f.call(this);",
"}"));
testError("class S extends B { f() { super(); } }", INVALID_SUPER);
}

public void testStaticThis() {
Expand Down

0 comments on commit cc8e8c3

Please sign in to comment.