Skip to content

Commit

Permalink
Automated rollback.
Browse files Browse the repository at this point in the history
*** Original change description ***

Report a warning if a class extends another class, but doesn't goog.require() it.

***
-------------
Created by MOE: http://code.google.com/p/moe-java
MOE_MIGRATED_REVID=85108939
  • Loading branch information
tbreisacher authored and dimvar committed Jan 30, 2015
1 parent cb3d5a7 commit 486345c
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 49 deletions.
56 changes: 26 additions & 30 deletions src/com/google/javascript/jscomp/CheckRequiresForConstructors.java
Expand Up @@ -25,9 +25,9 @@
import com.google.javascript.rhino.Token;
import com.google.javascript.rhino.jstype.JSType;

import java.util.HashMap;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.Map;
import java.util.List;
import java.util.Set;

/**
Expand Down Expand Up @@ -99,7 +99,7 @@ private static String getOutermostClassName(String className) {
private class CheckRequiresForConstructorsCallback implements Callback {
private final Set<String> constructors = new HashSet<>();
private final Set<String> requires = new HashSet<>();
private final Map<String, Node> usages = new HashMap<>();
private final List<Node> newAndImplementsNodes = new ArrayList<>();

@Override
public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
Expand All @@ -118,7 +118,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
if (NodeUtil.isStatement(n)) {
maybeAddConstructor(t, n);
}
maybeAddJsDocUsages(t, n);
maybeAddImplements(t, n);
break;
case Token.CALL:
visitCallNode(n, parent);
Expand All @@ -133,10 +133,13 @@ public void visit(NodeTraversal t, Node n, Node parent) {

private void visitScriptNode(NodeTraversal t) {
Set<String> classNames = new HashSet<>();
for (Map.Entry<String, Node> entry : usages.entrySet()) {
String className = entry.getKey();
Node node = entry.getValue();

for (Node node : newAndImplementsNodes) {
String className;
if (node.isNew()) {
className = node.getFirstChild().getQualifiedName();
} else {
className = node.getString();
}
String outermostClassName = getOutermostClassName(className);
// The parent namespace is also checked as part of the requires so that classes
// used by goog.module are still checked properly. This may cause missing requires
Expand Down Expand Up @@ -165,7 +168,7 @@ private void visitScriptNode(NodeTraversal t) {
}
// for the next script, if there is one, we don't want the new, ctor, and
// require nodes to spill over.
this.usages.clear();
this.newAndImplementsNodes.clear();
this.requires.clear();
this.constructors.clear();
}
Expand Down Expand Up @@ -199,7 +202,7 @@ private void visitNewNode(NodeTraversal t, Node n) {
if (var != null && (var.isLocal() || var.isExtern())) {
return;
}
usages.put(n.getFirstChild().getQualifiedName(), n);
newAndImplementsNodes.add(n);
}

private void maybeAddConstructor(NodeTraversal t, Node n) {
Expand All @@ -220,31 +223,24 @@ private void maybeAddConstructor(NodeTraversal t, Node n) {
}
}

private void maybeAddJsDocUsages(NodeTraversal t, Node n) {
private void maybeAddImplements(NodeTraversal t, Node n) {
JSDocInfo info = NodeUtil.getBestJSDocInfo(n);
if (info != null) {
for (JSTypeExpression expr : info.getImplementedInterfaces()) {
maybeAddJsDocUsage(t, n, expr);
}
if (info.getBaseType() != null) {
maybeAddJsDocUsage(t, n, info.getBaseType());
}
}
}

private void maybeAddJsDocUsage(NodeTraversal t, Node n, JSTypeExpression expr) {
Node typeNode = expr.getRoot();
Preconditions.checkState(typeNode.getType() == Token.BANG);
Node child = typeNode.getFirstChild();
Preconditions.checkState(child.isString());
Node implementsNode = expr.getRoot();
Preconditions.checkState(implementsNode.getType() == Token.BANG);
Node child = implementsNode.getFirstChild();
Preconditions.checkState(child.isString());

String rootName = Splitter.on('.').split(child.getString()).iterator().next();
Scope.Var var = t.getScope().getVar(rootName);
if (var != null && var.isExtern()) {
return;
}

String rootName = Splitter.on('.').split(child.getString()).iterator().next();
Scope.Var var = t.getScope().getVar(rootName);
if (var != null && var.isExtern()) {
return;
newAndImplementsNodes.add(child);
}
}

usages.put(child.getString(), n);
}
}
}
Expand Up @@ -111,25 +111,6 @@ public void testPassWithImplements() {
testSame(js);
}

public void testFailWithExtends() {
String[] js = new String[] {
"var goog = {};\n"
+ "goog.provide('example.Foo');\n"
+ "/** @constructor */ example.Foo = function() {};",

"/** @constructor @extends {example.Foo} */ var Ctor = function() {};"
};
String warning = "'example.Foo' used but not goog.require'd";
test(js, js, null, MISSING_REQUIRE_WARNING, warning);
}

public void testPassWithExtends() {
String js = "goog.require('example.Foo');"
+ "/** @constructor @extends {example.Foo} */"
+ "var Ctor = function() {};";
testSame(js);
}

public void testPassWithLocalFunctions() {
String js =
"/** @constructor */ function tempCtor() {}; var foo = new tempCtor();";
Expand Down

0 comments on commit 486345c

Please sign in to comment.