Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Prevent calls to super outside of a constructor #2452

Closed
wants to merge 2 commits into from
Closed

Prevent calls to super outside of a constructor #2452

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c3eed57
Prevent calls to super outside of a constructor
ChadKillingsworth May 3, 2017
5d65215
More clearly delineate the cases for super transpilation.
ChadKillingsworth May 23, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
171 changes: 96 additions & 75 deletions src/com/google/javascript/jscomp/Es6ConvertSuper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@
*/
public final class Es6ConvertSuper extends NodeTraversal.AbstractPostOrderCallback
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;

public Es6ConvertSuper(AbstractCompiler compiler) {
Expand Down Expand Up @@ -109,108 +114,124 @@ 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;
Preconditions.checkState(node.isSuper());
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 (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,
"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());
Preconditions.checkState(node.isSuper());

Node clazz = NodeUtil.getEnclosingClass(node);
Node superName = clazz.getSecondChild();
if (!superName.isQualifiedName()) {
// This will be reported as an error in Es6ToEs3Converter.
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")) {
// 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());
}
callTarget = IR.getprop(potentialCallee, IR.string("call"));
enclosingCall.addChildToFront(callTarget);
enclosingCall.addChildAfter(IR.thisNode(), callTarget);
enclosingCall.useSourceInfoIfMissingFromForTree(enclosingCall);
compiler.reportChangeToEnclosingScope(enclosingCall);
} else {
// super can only be directly called in a constructor
compiler.report(JSError.make(node, INVALID_SUPER_CALL));
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();
private void visitSuperPropertyCall(Node node, Node parent, Node enclosingMemberDef) {
Preconditions.checkState(parent.isGetProp());
Preconditions.checkState(node.isSuper());
Node grandparent = parent.getParent();
Preconditions.checkState(grandparent.isCall());

Node clazz = NodeUtil.getEnclosingClass(node);
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) {
Preconditions.checkNotNull(baseClass);
Preconditions.checkNotNull(methodName);
String baseMethodName;
baseMethodName = Joiner.on('.').join(baseClass, "prototype", methodName, "call");
Node methodCall = NodeUtil.newQName(compiler, baseMethodName);
Node callNode = IR.call(methodCall, IR.thisNode());
if (arguments != null) {
callNode.addChildrenToBack(arguments);
Node callTarget = parent;
if (enclosingMemberDef.isStaticMember()) {
callTarget.replaceChild(node, superName.cloneTree());
callTarget = IR.getprop(callTarget.detach(), IR.string("call"));
grandparent.addChildToFront(callTarget);
grandparent.addChildAfter(IR.thisNode(), callTarget);
grandparent.useSourceInfoIfMissingFromForTree(parent);
} else {
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
Expand Down
48 changes: 9 additions & 39 deletions test/com/google/javascript/jscomp/Es6RewriteClassTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@
package com.google.javascript.jscomp;

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.CONFLICTING_GETTER_SETTER_TYPE;
import static com.google.javascript.jscomp.Es6RewriteClass.DYNAMIC_EXTENDS_TYPE;
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.CANNOT_CONVERT_YET;

import com.google.javascript.jscomp.CompilerOptions.LanguageMode;

Expand Down Expand Up @@ -776,17 +777,8 @@ public void testSuperCall() {
" $jscomp.inherits(D, C);",
"};"));

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

public void testSuperKnownNotToChangeThis() {
Expand Down Expand Up @@ -1196,39 +1188,17 @@ 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() {var s = new super;} }", INVALID_SUPER_CALL);

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(str) {var s = new super(str);} }", INVALID_SUPER_CALL);
}

public void testSuperCallNonConstructor() {

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

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

public void testStaticThis() {
Expand Down
51 changes: 9 additions & 42 deletions test/com/google/javascript/jscomp/Es6ToEs3ConverterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@
package com.google.javascript.jscomp;

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.CONFLICTING_GETTER_SETTER_TYPE;
import static com.google.javascript.jscomp.Es6RewriteClass.DYNAMIC_EXTENDS_TYPE;
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.CANNOT_CONVERT_YET;
import static com.google.javascript.jscomp.TypeCheck.INSTANTIATE_ABSTRACT_CLASS;

import com.google.javascript.jscomp.CompilerOptions.LanguageMode;
Expand Down Expand Up @@ -876,18 +877,8 @@ 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_CALL);
}

public void testSuperKnownNotToChangeThis() {
Expand Down Expand Up @@ -1297,11 +1288,9 @@ 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_CALL);
testError(
"class D {} class C extends D { f(str) {var s = new super(str);} }", INVALID_SUPER_CALL);
}

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

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

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_CALL);
}

public void testStaticThis() {
Expand Down