Skip to content

Commit

Permalink
Merge the two super-checking passes together.
Browse files Browse the repository at this point in the history
It was incorrect for one of them to be considered a "transpilation" pass when in fact it ought to run even in non-transpiling compilations.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=184609654
  • Loading branch information
tbreisacher authored and blickly committed Feb 6, 2018
1 parent 6e44c2a commit 4fef6f0
Show file tree
Hide file tree
Showing 8 changed files with 199 additions and 288 deletions.
97 changes: 0 additions & 97 deletions src/com/google/javascript/jscomp/CheckMissingSuper.java

This file was deleted.

143 changes: 143 additions & 0 deletions src/com/google/javascript/jscomp/CheckSuper.java
@@ -0,0 +1,143 @@
/*
* 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.Callback;
import com.google.javascript.rhino.Node;

/**
* Check for errors related to the `super` keyword.
*/
final class CheckSuper implements HotSwapCompilerPass, Callback {
static final DiagnosticType MISSING_CALL_TO_SUPER =
DiagnosticType.error("JSC_MISSING_CALL_TO_SUPER", "constructor is missing a call to super()");

static final DiagnosticType THIS_BEFORE_SUPER =
DiagnosticType.error("JSC_THIS_BEFORE_SUPER", "cannot access this before calling super()");

static final DiagnosticType INVALID_SUPER_CALL = DiagnosticType.error(
"JSC_INVALID_SUPER_CALL",
"super() not allowed except in the constructor of a subclass");

static final DiagnosticType INVALID_SUPER_CALL_WITH_SUGGESTION = DiagnosticType.error(
"JSC_INVALID_SUPER_CALL_WITH_SUGGESTION",
"super() not allowed here. Did you mean super.{0}?");

private final AbstractCompiler compiler;

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

@Override
public void process(Node externs, Node root) {
NodeTraversal.traverseEs6(compiler, root, this);
}

@Override
public void hotSwapScript(Node scriptRoot, Node originalRoot) {
NodeTraversal.traverseEs6(compiler, scriptRoot, this);
}

@Override
public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
if (n.isClass()) {
return visitClass(t, n);
}
return true;
}

@Override
public void visit(NodeTraversal t, Node n, Node parent) {
if (n.isSuper()) {
visitSuper(t, n, parent);
}
}

/**
* @return Whether to continue traversing this class
*/
private boolean visitClass(NodeTraversal t, Node n) {
Node superclass = n.getSecondChild();
if (superclass.isEmpty()) {
return true;
}

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

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

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

private void visitSuper(NodeTraversal t, Node n, Node parent) {
Node classNode = NodeUtil.getEnclosingClass(n);
if (classNode == null || classNode.getSecondChild().isEmpty()) {
t.report(n, INVALID_SUPER_CALL);
return;
}

if (parent.isCall()) {
Node fn = NodeUtil.getEnclosingFunction(parent);
if (fn == null) {
t.report(n, INVALID_SUPER_CALL);
return;
}

Node memberDef = fn.getParent();
if (memberDef.isMemberFunctionDef()) {
if (memberDef.matchesQualifiedName("constructor")) {
// No error.
} else {
t.report(n, INVALID_SUPER_CALL_WITH_SUGGESTION, memberDef.getString());
}
} else {
t.report(n, INVALID_SUPER_CALL);
}
}
}

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

@Override
public boolean shouldTraverse(NodeTraversal nodeTraversal, Node n, Node parent) {
// Stop traversal once the super() call is found. Also don't traverse into nested functions
// since this and super() references may not be applicable within those scopes.
return !found && !n.isFunction();
}

@Override
public void visit(NodeTraversal t, Node n, Node parent) {
if (n.isThis()) {
t.report(n, THIS_BEFORE_SUPER);
}
if (n.isSuper() && parent.isCall()) {
found = true;
return;
}
}
}
}
10 changes: 5 additions & 5 deletions src/com/google/javascript/jscomp/DefaultPassConfig.java
Expand Up @@ -165,7 +165,7 @@ protected List<PassFactory> getTranspileOnlyPasses() {
TranspilationPasses.addEs6ModulePass(passes);
}

passes.add(checkMissingSuper);
passes.add(checkSuper);
passes.add(checkVariableReferencesForTranspileOnly);

// It's important that the Dart super accessors pass run *before* es6ConvertSuper,
Expand Down Expand Up @@ -324,7 +324,7 @@ protected List<PassFactory> getChecks() {
checks.add(declaredGlobalExternsOnWindow);
}

checks.add(checkMissingSuper);
checks.add(checkSuper);

if (options.closurePass) {
checks.add(closureGoogScopeAliases);
Expand Down Expand Up @@ -1885,11 +1885,11 @@ protected FeatureSet featureSet() {
};

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

@Override
Expand Down
77 changes: 0 additions & 77 deletions src/com/google/javascript/jscomp/Es6SuperCheck.java

This file was deleted.

5 changes: 2 additions & 3 deletions src/com/google/javascript/jscomp/LintPassConfig.java
Expand Up @@ -68,15 +68,14 @@ protected CompilerPass create(AbstractCompiler compiler) {
new CheckJSDocStyle(compiler),
new CheckJSDoc(compiler),
new CheckMissingSemicolon(compiler),
new CheckMissingSuper(compiler),
new CheckSuper(compiler),
new CheckPrimitiveAsObject(compiler),
new ClosureCheckModule(compiler),
new CheckRequiresAndProvidesSorted(compiler),
new CheckSideEffects(
compiler, /* report */ true, /* protectSideEffectFreeCode */ false),
new CheckUnusedLabels(compiler),
new CheckUselessBlocks(compiler),
new Es6SuperCheck(compiler)));
new CheckUselessBlocks(compiler)));
}

@Override
Expand Down
14 changes: 0 additions & 14 deletions src/com/google/javascript/jscomp/TranspilationPasses.java
Expand Up @@ -61,7 +61,6 @@ public static void addEs2016Passes(List<PassFactory> passes) {
*/
public static void addEs6EarlyPasses(List<PassFactory> passes) {
passes.add(es6NormalizeShorthandProperties);
passes.add(es6SuperCheck);
passes.add(es6ConvertSuper);
passes.add(es6RenameVariablesInParamLists);
passes.add(es6SplitVariableDeclarations);
Expand Down Expand Up @@ -191,19 +190,6 @@ protected FeatureSet featureSet() {
}
};

private static final PassFactory es6SuperCheck =
new PassFactory("es6SuperCheck", true) {
@Override
protected CompilerPass create(final AbstractCompiler compiler) {
return new Es6SuperCheck(compiler);
}

@Override
protected FeatureSet featureSet() {
return ES8;
}
};

static final HotSwapPassFactory es6ExtractClasses =
new HotSwapPassFactory(PassNames.ES6_EXTRACT_CLASSES) {
@Override
Expand Down

0 comments on commit 4fef6f0

Please sign in to comment.