Skip to content

Commit

Permalink
Prevent calls to super outside of a constructor
Browse files Browse the repository at this point in the history 
Closes #2452

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=157253710
  • Loading branch information
@ChadKillingsworth @blickly
ChadKillingsworth authored and blickly committed May 26, 2017
1 parent 9803c43 commit fdf120f
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 154 deletions.
171 changes: 96 additions & 75 deletions src/com/google/javascript/jscomp/Es6ConvertSuper.java
View file
Expand Up @@ -35,6 +35,11 @@
*/ */
public final class Es6ConvertSuper extends NodeTraversal.AbstractPostOrderCallback public final class Es6ConvertSuper extends NodeTraversal.AbstractPostOrderCallback
implements HotSwapCompilerPass { implements HotSwapCompilerPass {

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

private final AbstractCompiler compiler; private final AbstractCompiler compiler;


public Es6ConvertSuper(AbstractCompiler compiler) { public Es6ConvertSuper(AbstractCompiler compiler) {
Expand Down Expand Up @@ -111,108 +116,124 @@ private boolean isInterface(Node classNode) {
} }


private void visitSuper(Node node, Node parent) { private void visitSuper(Node node, Node parent) {
Node enclosingCall = parent; Preconditions.checkState(node.isSuper());
Node potentialCallee = node; Node exprRoot = node;
if (!parent.isCall()) {
enclosingCall = parent.getParent(); if (exprRoot.getParent().isGetElem() || exprRoot.getParent().isGetProp()) {
potentialCallee = parent; exprRoot = exprRoot.getParent();
} }
if (!enclosingCall.isCall()
|| enclosingCall.getFirstChild() != potentialCallee Node enclosingMemberDef =
|| enclosingCall.getFirstChild().isGetElem()) { 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 (parent.isCall()) {
// super(...)
visitSuperCall(node, parent, enclosingMemberDef);
} else if (parent.isGetProp()) {
if (parent.getFirstChild() == node) {
if (parent.getParent().isCall() && NodeUtil.isCallOrNewTarget(parent)) {
// super.something(...)
visitSuperPropertyCall(node, parent, enclosingMemberDef);
} else {
// super.something
compiler.report(JSError.make(node, CANNOT_CONVERT_YET,
"Only calls to super or to a method of super are supported."));
}
} else {
// super.something used in some other way
compiler.report(JSError.make(node, CANNOT_CONVERT_YET,
"Only calls to super or to a method of super are supported."));
}
} else if (parent.isGetElem()) {
compiler.report(JSError.make(node, CANNOT_CONVERT_YET,
"Only calls to super or to a method of super are supported."));
} else if (parent.isNew()) {
// new super(...)
compiler.report(JSError.make(node, INVALID_SUPER_CALL));
} else {
// some other use of super we don't support yet
compiler.report(JSError.make(node, CANNOT_CONVERT_YET, compiler.report(JSError.make(node, CANNOT_CONVERT_YET,
"Only calls to super or to a method of super are supported.")); "Only calls to super or to a method of super are supported."));
return;
} }
}

private void visitSuperCall(Node node, Node parent, Node enclosingMemberDef) {
Preconditions.checkState(parent.isCall(), parent);
Preconditions.checkState(node.isSuper(), node);

Node clazz = NodeUtil.getEnclosingClass(node); Node clazz = NodeUtil.getEnclosingClass(node);
Node superName = clazz.getSecondChild(); Node superName = clazz.getSecondChild();
if (!superName.isQualifiedName()) { if (!superName.isQualifiedName()) {
// This will be reported as an error in Es6ToEs3Converter. // This will be reported as an error in Es6ToEs3Converter.
return; return;
} }


Node enclosingMemberDef = NodeUtil.getEnclosingNode( if (enclosingMemberDef.isMemberFunctionDef()
node, && enclosingMemberDef.getString().equals("constructor")) {
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) {
// Calls to super() constructors will be transpiled by Es6ConvertSuperConstructorCalls later. // Calls to super() constructors will be transpiled by Es6ConvertSuperConstructorCalls later.
if (node.isFromExterns() || isInterface(clazz)) { 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 // 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. // implementation that should be instantiated.
// A call to super() shouldn't actually exist for these cases and is problematic to // A call to super() shouldn't actually exist for these cases and is problematic to
// transpile, so just drop it. // transpile, so just drop it.
Node enclosingStatement = NodeUtil.getEnclosingStatement(node); NodeUtil.getEnclosingStatement(node).detach();
Node enclosingScope = enclosingStatement.getParent(); compiler.reportCodeChange();
enclosingStatement.detach();
compiler.reportChangeToEnclosingScope(enclosingScope);
} }
// Calls to super() constructors will be transpiled by Es6ConvertSuperConstructorCalls // Calls to super() constructors will be transpiled by Es6ConvertSuperConstructorCalls
// later. // later.
return; return;
} } else {
if (enclosingMemberDef.isStaticMember()) { // super can only be directly called in a constructor
Node callTarget; compiler.report(JSError.make(node, INVALID_SUPER_CALL));
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());
}
callTarget = IR.getprop(potentialCallee, IR.string("call"));
enclosingCall.addChildToFront(callTarget);
enclosingCall.addChildAfter(IR.thisNode(), callTarget);
enclosingCall.useSourceInfoIfMissingFromForTree(enclosingCall);
compiler.reportChangeToEnclosingScope(enclosingCall);
return; return;
} }
}


String methodName; private void visitSuperPropertyCall(Node node, Node parent, Node enclosingMemberDef) {
Node callName = enclosingCall.removeFirstChild(); Preconditions.checkState(parent.isGetProp(), parent);
if (callName.isSuper()) { Preconditions.checkState(node.isSuper(), node);
// TODO(bradfordcsmith): This should report an error since this is not allowed by the Node grandparent = parent.getParent();
// current spec. Preconditions.checkState(grandparent.isCall());
methodName = enclosingMemberDef.getString();
} else { Node clazz = NodeUtil.getEnclosingClass(node);
methodName = callName.getLastChild().getString(); Node superName = clazz.getSecondChild();
if (!superName.isQualifiedName()) {
// This will be reported as an error in Es6ToEs3Converter.
return;
} }
Node baseCall = baseCall(
superName.getQualifiedName(), methodName, enclosingCall.removeChildren());
baseCall.useSourceInfoIfMissingFromForTree(enclosingCall);
enclosingCall.replaceWith(baseCall);
compiler.reportChangeToEnclosingScope(baseCall);
}


private Node baseCall(String baseClass, String methodName, Node arguments) { Node callTarget = parent;
Preconditions.checkNotNull(baseClass); if (enclosingMemberDef.isStaticMember()) {
Preconditions.checkNotNull(methodName); callTarget.replaceChild(node, superName.cloneTree());
String baseMethodName; callTarget = IR.getprop(callTarget.detach(), IR.string("call"));
baseMethodName = Joiner.on('.').join(baseClass, "prototype", methodName, "call"); grandparent.addChildToFront(callTarget);
Node methodCall = NodeUtil.newQName(compiler, baseMethodName); grandparent.addChildAfter(IR.thisNode(), callTarget);
Node callNode = IR.call(methodCall, IR.thisNode()); grandparent.useSourceInfoIfMissingFromForTree(parent);
if (arguments != null) { } else {
callNode.addChildrenToBack(arguments); 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"));
grandparent.addChildToFront(callTarget);
grandparent.addChildAfter(IR.thisNode(), callTarget);
grandparent.putBooleanProp(Node.FREE_CALL, false);
grandparent.useSourceInfoIfMissingFromForTree(parent);
} }
return callNode; compiler.reportChangeToEnclosingScope(grandparent);
} }


@Override @Override
Expand Down
46 changes: 8 additions & 38 deletions test/com/google/javascript/jscomp/Es6RewriteClassTest.java
View file
Expand Up @@ -16,6 +16,7 @@
package com.google.javascript.jscomp; package com.google.javascript.jscomp;


import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import static com.google.javascript.jscomp.Es6ConvertSuper.INVALID_SUPER_CALL;
import static com.google.javascript.jscomp.Es6RewriteClass.CLASS_REASSIGNMENT; import static com.google.javascript.jscomp.Es6RewriteClass.CLASS_REASSIGNMENT;
import static com.google.javascript.jscomp.Es6RewriteClass.CONFLICTING_GETTER_SETTER_TYPE; import static com.google.javascript.jscomp.Es6RewriteClass.CONFLICTING_GETTER_SETTER_TYPE;
import static com.google.javascript.jscomp.Es6RewriteClass.DYNAMIC_EXTENDS_TYPE; import static com.google.javascript.jscomp.Es6RewriteClass.DYNAMIC_EXTENDS_TYPE;
Expand Down Expand Up @@ -776,17 +777,8 @@ public void testSuperCall() {
" $jscomp.inherits(D, C);", " $jscomp.inherits(D, C);",
"};")); "};"));


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


public void testSuperKnownNotToChangeThis() { public void testSuperKnownNotToChangeThis() {
Expand Down Expand Up @@ -1196,39 +1188,17 @@ public void testSuperGet() {
} }


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


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


public void testSuperCallNonConstructor() { public void testSuperCallNonConstructor() {


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


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


public void testStaticThis() { public void testStaticThis() {
Expand Down
49 changes: 8 additions & 41 deletions test/com/google/javascript/jscomp/Es6ToEs3ConverterTest.java
View file
Expand Up @@ -16,6 +16,7 @@
package com.google.javascript.jscomp; package com.google.javascript.jscomp;


import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import static com.google.javascript.jscomp.Es6ConvertSuper.INVALID_SUPER_CALL;
import static com.google.javascript.jscomp.Es6RewriteClass.CLASS_REASSIGNMENT; import static com.google.javascript.jscomp.Es6RewriteClass.CLASS_REASSIGNMENT;
import static com.google.javascript.jscomp.Es6RewriteClass.CONFLICTING_GETTER_SETTER_TYPE; import static com.google.javascript.jscomp.Es6RewriteClass.CONFLICTING_GETTER_SETTER_TYPE;
import static com.google.javascript.jscomp.Es6RewriteClass.DYNAMIC_EXTENDS_TYPE; import static com.google.javascript.jscomp.Es6RewriteClass.DYNAMIC_EXTENDS_TYPE;
Expand Down Expand Up @@ -876,18 +877,8 @@ public void testSuperCall() {
" $jscomp.inherits(D, C);", " $jscomp.inherits(D, C);",
"};")); "};"));


test( testError(
"class D {} class C extends D { constructor() {}; f() {super();} }", "class D {} class C extends D { constructor() {} f() {super();} }", INVALID_SUPER_CALL);
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);",
"}"));
} }


public void testSuperKnownNotToChangeThis() { public void testSuperKnownNotToChangeThis() {
Expand Down Expand Up @@ -1297,11 +1288,9 @@ public void testSuperGet() {
} }


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

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


public void testSuperSpread() { public void testSuperSpread() {
Expand All @@ -1326,31 +1315,9 @@ public void testSuperSpread() {
} }


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


test( testError("class S extends B { f() { super(); } }", INVALID_SUPER_CALL);
"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);",
"}"));
} }


public void testStaticThis() { public void testStaticThis() {
Expand Down

0 comments on commit fdf120f

Please sign in to comment.