Skip to content

Commit

Permalink
Add a check for missing super() calls in constructors.
Browse files Browse the repository at this point in the history
Fixes #1160
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=117986475
  • Loading branch information
tbreisacher authored and blickly committed Mar 25, 2016
1 parent bc96344 commit 25dd075
Show file tree
Hide file tree
Showing 6 changed files with 164 additions and 6 deletions.
89 changes: 89 additions & 0 deletions src/com/google/javascript/jscomp/CheckMissingSuper.java
@@ -0,0 +1,89 @@
/*
* Copyright 2016 The Closure Compiler Authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.google.javascript.jscomp;

import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback;
import com.google.javascript.jscomp.NodeTraversal.Callback;
import com.google.javascript.rhino.Node;

/**
* Report an error if the user forgot to call super() in the constructor of an ES6 class.
* We don't require that the super() call be the first statement in the constructor, because doing
* so would break J2CL-compiled code.
*/
final class CheckMissingSuper extends AbstractPostOrderCallback implements HotSwapCompilerPass {
static final DiagnosticType MISSING_CALL_TO_SUPER =
DiagnosticType.error("JSC_MISSING_CALL_TO_SUPER", "constructor is missing a call to super()");

private final AbstractCompiler compiler;

public CheckMissingSuper(AbstractCompiler compiler) {
this.compiler = compiler;
}

@Override
public void process(Node externs, Node root) {
if (!compiler.getOptions().getLanguageIn().isEs6OrHigher()) {
return;
}
NodeTraversal.traverseEs6(compiler, root, this);
}

@Override
public void hotSwapScript(Node scriptRoot, Node originalRoot) {
process(null, scriptRoot);
}

@Override
public void visit(NodeTraversal t, Node n, Node parent) {
if (n.isClass()) {
Node superclass = n.getSecondChild();
if (superclass.isEmpty()) {
return;
}

Node constructor =
NodeUtil.getFirstPropMatchingKey(NodeUtil.getClassMembers(n), "constructor");
if (constructor == null) {
return;
}

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

if (!finder.found) {
t.report(constructor, MISSING_CALL_TO_SUPER);
}
}
}

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

@Override
public boolean shouldTraverse(NodeTraversal nodeTraversal, Node n, Node parent) {
return !found;
}

@Override
public void visit(NodeTraversal t, Node n, Node parent) {
if (n.isSuper() && parent.isCall()) {
found = true;
return;
}
}
}
}
10 changes: 10 additions & 0 deletions src/com/google/javascript/jscomp/DefaultPassConfig.java
Expand Up @@ -221,6 +221,7 @@ protected List<PassFactory> getChecks() {
checks.add(convertEs6TypedToEs6);
}

checks.add(checkMissingSuper);
checks.add(checkVariableReferences);

if (!options.skipNonTranspilationPasses && options.closurePass) {
Expand Down Expand Up @@ -1432,6 +1433,15 @@ protected HotSwapCompilerPass create(AbstractCompiler compiler) {
}
};

/** Checks that references to variables look reasonable. */
private final HotSwapPassFactory checkMissingSuper =
new HotSwapPassFactory("checkMissingSuper", true) {
@Override
protected HotSwapCompilerPass create(AbstractCompiler compiler) {
return new CheckMissingSuper(compiler);
}
};

/** Pre-process goog.testing.ObjectPropertyString. */
private final PassFactory objectPropertyStringPreprocess =
new PassFactory("ObjectPropertyStringPreprocess", true) {
Expand Down
1 change: 1 addition & 0 deletions src/com/google/javascript/jscomp/LintPassConfig.java
Expand Up @@ -69,6 +69,7 @@ protected CompilerPass create(AbstractCompiler compiler) {
new CheckJSDocStyle(compiler),
new CheckJSDoc(compiler),
new CheckMissingSemicolon(compiler),
new CheckMissingSuper(compiler),
new CheckRequiresAndProvidesSorted(compiler),
new CheckUnusedLabels(compiler),
new CheckUselessBlocks(compiler),
Expand Down
17 changes: 11 additions & 6 deletions src/com/google/javascript/jscomp/NodeUtil.java
Expand Up @@ -515,6 +515,10 @@ public static String getNearestFunctionName(Node n) {
return null;
}

public static Node getClassMembers(Node n) {
Preconditions.checkArgument(n.isClass());
return n.getLastChild();
}

/**
* Returns true if this is an immutable value.
Expand Down Expand Up @@ -1988,11 +1992,12 @@ public static Node getEnclosingNode(Node n, Predicate<Node> pred) {
}

/**
* @return The first property in the objlit that matches the key.
* @return The first property in the objlit or class members, that matches the key.
*/
@Nullable static Node getFirstPropMatchingKey(Node objlit, String keyName) {
Preconditions.checkState(objlit.isObjectLit());
for (Node keyNode : objlit.children()) {
@Nullable
static Node getFirstPropMatchingKey(Node n, String keyName) {
Preconditions.checkState(n.isObjectLit() || n.isClassMembers());
for (Node keyNode : n.children()) {
if ((keyNode.isStringKey() || keyNode.isMemberFunctionDef())
&& keyNode.getString().equals(keyName)) {
return keyNode.getFirstChild();
Expand Down Expand Up @@ -2505,8 +2510,8 @@ public static boolean isCallOrNew(Node node) {
/**
* Return a BLOCK node for the given FUNCTION node.
*/
static Node getFunctionBody(Node fn) {
Preconditions.checkArgument(fn.isFunction());
public static Node getFunctionBody(Node fn) {
Preconditions.checkArgument(fn.isFunction(), fn);
return fn.getLastChild();
}

Expand Down
10 changes: 10 additions & 0 deletions src/com/google/javascript/refactoring/ErrorToFixMapper.java
Expand Up @@ -75,6 +75,8 @@ public static SuggestedFix getFixForJsError(JSError error, AbstractCompiler comp
return removeNode(error);
case "JSC_INEXISTENT_PROPERTY":
return getFixForInexistentProperty(error);
case "JSC_MISSING_CALL_TO_SUPER":
return getFixForMissingSuper(error);
case "JSC_MISSING_REQUIRE_WARNING":
return getFixForMissingRequire(error, compiler);
case "JSC_DUPLICATE_REQUIRE_WARNING":
Expand Down Expand Up @@ -114,6 +116,14 @@ private static SuggestedFix getFixForMissingSemicolon(JSError error) {
.build();
}

private static SuggestedFix getFixForMissingSuper(JSError error) {
Node body = NodeUtil.getFunctionBody(error.node);
return new SuggestedFix.Builder()
.setOriginalMatchedNode(error.node)
.addChildToFront(body, "super();")
.build();
}

private static SuggestedFix getFixForInexistentProperty(JSError error) {
Matcher m = DID_YOU_MEAN.matcher(error.description);
if (m.matches()) {
Expand Down
43 changes: 43 additions & 0 deletions test/com/google/javascript/jscomp/CheckMissingSuperTest.java
@@ -0,0 +1,43 @@
/*
* Copyright 2016 The Closure Compiler Authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.google.javascript.jscomp;

import static com.google.javascript.jscomp.CheckMissingSuper.MISSING_CALL_TO_SUPER;

public final class CheckMissingSuperTest extends Es6CompilerTestCase {
@Override
protected CompilerPass getProcessor(Compiler compiler) {
return new CheckMissingSuper(compiler);
}

public void testMissingSuper() {
testErrorEs6("class C extends D { constructor() {} }", MISSING_CALL_TO_SUPER);
testErrorEs6("class C extends D { constructor() { super.foo(); } }", MISSING_CALL_TO_SUPER);
}

public void testNoWarning() {
testSameEs6("class C extends D { constructor() { super(); } }");
testSameEs6("class C { constructor() {} }");
testSameEs6("class C extends D {}");
}

// We could require that the super() call is the first statement in the constructor, except that
// doing so breaks J2CL-compiled code, which needs to do the static initialization for the class
// before anything else.
public void testNoWarning_J2CL() {
testSameEs6("class C extends D { constructor() { C.init(); super(); } }");
}
}

0 comments on commit 25dd075

Please sign in to comment.