Skip to content

Commit

Permalink
Make the .i.js generation pass completely local.
Browse files Browse the repository at this point in the history 
This should make it faster as well as simpler, as it can work on a single file
at a time without looking at externs.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=145702601
  • Loading branch information
@blickly
blickly committed Jan 27, 2017
1 parent 9b376d4 commit 670cb87
Show file tree
Hide file tree
Showing 2 changed files with 119 additions and 58 deletions.
143 changes: 85 additions & 58 deletions src/com/google/javascript/jscomp/ConvertToTypedInterface.java
View file
Expand Up @@ -16,7 +16,7 @@
package com.google.javascript.jscomp;

import com.google.common.base.Preconditions;
import com.google.javascript.jscomp.NodeTraversal.AbstractModuleCallback;
import com.google.common.collect.Iterables;
import com.google.javascript.jscomp.NodeTraversal.AbstractShallowStatementCallback;
import com.google.javascript.rhino.IR;
import com.google.javascript.rhino.JSDocInfo;
Expand All @@ -25,7 +25,6 @@
import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.Token;
import com.google.javascript.rhino.jstype.JSType;

import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
Expand Down Expand Up @@ -174,38 +173,60 @@ private static JSDocInfo getJSDocForName(Var decl, JSDocInfo oldJSDoc) {

}

private static class RemoveCode extends AbstractModuleCallback {
private final AbstractCompiler compiler;
private final Set<String> globalSeenNames = new HashSet<>();
private final List<Node> globalConstructorsToProcess = new ArrayList();
private Node currentModule = null;
private Set<String> moduleSeenNames;
private List<Node> moduleConstructorsToProcess;
/**
* Class to keep track of what has been seen so far in a given file.
*
* This is cleared after each file to make sure that the analysis is working on a per-file basis.
*/
private static class FileInfo {
private final Set<String> prefixNames = new HashSet<>();
private final Set<String> seenNames = new HashSet<>();
private final List<Node> constructorsToProcess = new ArrayList<>();

boolean isNameProcessed(String fullyQualifiedName) {
return seenNames.contains(fullyQualifiedName);
}

RemoveCode(AbstractCompiler compiler) {
this.compiler = compiler;
boolean isPrefixProvided(String fullyQualifiedName) {
for (String prefix : Iterables.concat(seenNames, prefixNames)) {
if (fullyQualifiedName.startsWith(prefix)) {
return true;
}
}
return false;
}

void process(Node externs, Node root) {
NodeTraversal.traverseRootsEs6(compiler, this, externs, root);
void markConstructorToProcess(Node ctorNode) {
Preconditions.checkArgument(ctorNode.isFunction());
constructorsToProcess.add(ctorNode);
}

processConstructors(globalConstructorsToProcess);
void markNameProcessed(String fullyQualifiedName) {
seenNames.add(fullyQualifiedName);
}

@Override
public void enterModule(NodeTraversal t, Node scopeRoot) {
currentModule = scopeRoot;
moduleSeenNames = new HashSet<>();
moduleConstructorsToProcess = new ArrayList<>();
void markPrefixDefined(String namespacePrefix) {
prefixNames.add(namespacePrefix);
}

@Override
public void exitModule(NodeTraversal t, Node scopeRoot) {
processConstructors(moduleConstructorsToProcess);
void clear() {
prefixNames.clear();
seenNames.clear();
constructorsToProcess.clear();
}

}

private static class RemoveCode implements NodeTraversal.Callback {
private final AbstractCompiler compiler;
private final FileInfo currentFile = new FileInfo();

currentModule = null;
moduleSeenNames = null;
moduleConstructorsToProcess = null;
RemoveCode(AbstractCompiler compiler) {
this.compiler = compiler;
}

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

private void processConstructors(List<Node> constructorNodes) {
Expand All @@ -217,15 +238,22 @@ private void processConstructors(List<Node> constructorNodes) {
@Override
public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
switch (n.getToken()) {
case SCRIPT:
currentFile.clear();
break;
case CLASS:
case FUNCTION: {
if (parent.isCall()) {
Preconditions.checkState(!parent.getFirstChild().matchesQualifiedName("goog.scope"),
parent);
}
if (NodeUtil.isStatementParent(parent)) {
currentFile.markNameProcessed(n.getFirstChild().getString());
}
Node body = n.getLastChild();
if (body.isBlock() && body.hasChildren()) {
if (body.isNormalBlock() && body.hasChildren()) {
if (isConstructor(n)) {
markConstructorToProcess(n);
currentFile.markConstructorToProcess(n);
return false;
}
n.getLastChild().removeChildren();
Expand All @@ -247,6 +275,7 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
t.report(n, UNSUPPORTED_GOOG_SCOPE);
return false;
} else if (callee.matchesQualifiedName("goog.provide")) {
currentFile.markPrefixDefined(expr.getLastChild().getString());
Node childBefore;
while (null != (childBefore = n.getPrevious())
&& childBefore.getBooleanProp(Node.IS_NAMESPACE)) {
Expand Down Expand Up @@ -304,6 +333,9 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
@Override
public void visit(NodeTraversal t, Node n, Node parent) {
switch (n.getToken()) {
case SCRIPT:
processConstructors(currentFile.constructorsToProcess);
break;
case TRY:
case DEFAULT_CASE:
parent.replaceChild(n, n.getFirstChild().detach());
Expand Down Expand Up @@ -346,31 +378,6 @@ public void visit(NodeTraversal t, Node n, Node parent) {
}
}

private boolean isNameProcessed(String fullyQualifiedName) {
if (currentModule == null) {
return globalSeenNames.contains(fullyQualifiedName);
} else {
return moduleSeenNames.contains(fullyQualifiedName);
}
}

private void markConstructorToProcess(Node ctorNode) {
Preconditions.checkArgument(ctorNode.isFunction());
if (currentModule == null) {
globalConstructorsToProcess.add(ctorNode);
} else {
moduleConstructorsToProcess.add(ctorNode);
}
}

private void markNameProcessed(String fullyQualifiedName) {
if (currentModule == null) {
globalSeenNames.add(fullyQualifiedName);
} else {
moduleSeenNames.add(fullyQualifiedName);
}
}

private void processConstructor(final Node function) {
final String className = getClassName(function);
if (className == null) {
Expand All @@ -391,7 +398,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
}
String pname = name.getLastChild().getString();
String fullyQualifiedName = className + ".prototype." + pname;
if (isNameProcessed(fullyQualifiedName)) {
if (currentFile.isNameProcessed(fullyQualifiedName)) {
return;
}
JSDocInfo jsdoc = NodeUtil.getBestJSDocInfo(name);
Expand All @@ -406,7 +413,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
// TODO(blickly): Preserve the declaration order of the this properties.
insertionPoint.getParent().addChildAfter(newProtoAssignStmt, insertionPoint);
compiler.reportCodeChange();
markNameProcessed(fullyQualifiedName);
currentFile.markNameProcessed(fullyQualifiedName);
}
}
});
Expand Down Expand Up @@ -455,11 +462,15 @@ private RemovalType shouldRemove(Node nameNode) {
}
if (jsdoc == null
|| !jsdoc.containsDeclaration()) {
if (isNameProcessed(nameNode.getQualifiedName())) {
String fullyQualifiedName = nameNode.getQualifiedName();
if (currentFile.isNameProcessed(fullyQualifiedName)) {
return RemovalType.REMOVE_ALL;
}
jsdocNode.setJSDocInfo(getAllTypeJSDoc());
return RemovalType.REMOVE_RHS;
if (isDeclaration(nameNode) || currentFile.isPrefixProvided(fullyQualifiedName)) {
jsdocNode.setJSDocInfo(getAllTypeJSDoc());
return RemovalType.REMOVE_RHS;
}
return RemovalType.REMOVE_ALL;
}

This comment has been minimized.

Sign in to view

Copy link
@nguyenhoan

nguyenhoan Feb 18, 2017 •
edited

Hi @blickly
We are a team of researchers from Iowa State, The University of Texs at Dallas and Oregon State University, USA. We are investigating common/repeated code changes.
We have four short questions regarding the change in the image below which is part of this commit.
image

Questions:

Q1- Is the change at these lines similar to any other changes (from other locations of the same commit or from other commits)? (yes, no, not sure)

Q2- Can you briefly describe the change and why you made it? (for example, checking parameter before calling the method to avoid a Null Pointer Exception)

Q3- Can you give it a name? (for example, Null Check)

Q4- Would you like to have this change automated by a tool? (Yes, No, Already automated)

The data collected from the answers will never be associated with you or your project. Our questions are about recurring code changes from the developer community, not about personal information. All the data is merged across recurring changes from GitHub repositories. We will publish aggregated data from the trends of the whole community.
We have a long tradition of developing refactoring tools and contributing them freely to the Eclipse, Netbeans, Android Studio under their respective FLOSS licenses. For example, look at some of our recently released refactoring tools: http://refactoring.info/tools/

Thank you,
Hoan Nguyen https://sites.google.com/site/nguyenanhhoan/
Michael Hilton http://web.engr.oregonstate.edu/~hiltonm/
Tien Nguyen http://www.utdallas.edu/~tien.n.nguyen/
Danny Dig http://eecs.oregonstate.edu/people/dig-danny
if (isInferrableConst(jsdoc, nameNode)) {
jsdocNode.setJSDocInfo(pullJsdocTypeFromAst(compiler, jsdoc, nameNode));
Expand All @@ -485,7 +496,7 @@ private void processName(Node nameNode, Node statement) {
maybeRemoveRhs(nameNode, statement, jsdocNode.getJSDocInfo());
break;
}
markNameProcessed(nameNode.getQualifiedName());
currentFile.markNameProcessed(nameNode.getQualifiedName());
}

private void removeNode(Node n) {
Expand Down Expand Up @@ -521,6 +532,22 @@ private void removeEnumValues(Node objLit) {
}
}

// TODO(blickly): Move to NodeUtil if it makes more sense there.
private static boolean isDeclaration(Node nameNode) {
Preconditions.checkArgument(nameNode.isQualifiedName());
Node parent = nameNode.getParent();
switch (parent.getToken()) {
case VAR:
case LET:
case CONST:
case CLASS:
case FUNCTION:
return true;
default:
return false;
}
}

private static boolean isInferrableConst(JSDocInfo jsdoc, Node nameNode) {
boolean isConst =
nameNode.getParent().isConst() || (jsdoc != null && jsdoc.hasConstAnnotation());
Expand Down
34 changes: 34 additions & 0 deletions test/com/google/javascript/jscomp/ConvertToTypedInterfaceTest.java
View file
Expand Up @@ -331,6 +331,40 @@ public void testGoogModules() {
});
}

public void testCrossFileModifications() {
test(
LINE_JOINER.join(
"goog.module('a.b.c');",
"othermodule.modify.something = othermodule.modify.something + 1;"),
"goog.module('a.b.c');");

testEs6(
LINE_JOINER.join(
"goog.module('a.b.c');",
"class Foo {}",
"Foo.something = othermodule.modify.something + 1;",
"exports = Foo;"),
LINE_JOINER.join(
"goog.module('a.b.c');",
"class Foo {}",
"/** @const {*} */ Foo.something",
"exports = Foo;"));

test(
LINE_JOINER.join(
"goog.provide('a.b.c');",
"otherfile.modify.something = otherfile.modify.something + 1;"),
"goog.provide('a.b.c');");

test(
LINE_JOINER.join(
"goog.provide('a.b.c');",
"a.b.c.something = otherfile.modify.something + 1;"),
LINE_JOINER.join(
"goog.provide('a.b.c');",
"/** @const {*} */ a.b.c.something;"));
}

public void testRemoveCalls() {
test("alert('hello'); window.clearTimeout();", "");

Expand Down

0 comments on commit 670cb87

Please sign in to comment.