diff --git a/src/com/google/javascript/jscomp/ConvertToTypedInterface.java b/src/com/google/javascript/jscomp/ConvertToTypedInterface.java index 3c4a9e3bd90..960b094e3e9 100644 --- a/src/com/google/javascript/jscomp/ConvertToTypedInterface.java +++ b/src/com/google/javascript/jscomp/ConvertToTypedInterface.java @@ -16,8 +16,8 @@ package com.google.javascript.jscomp; import com.google.common.base.Preconditions; +import com.google.javascript.jscomp.NodeTraversal.AbstractModuleCallback; import com.google.javascript.jscomp.NodeTraversal.AbstractShallowStatementCallback; -import com.google.javascript.jscomp.NodeTraversal.Callback; import com.google.javascript.rhino.IR; import com.google.javascript.rhino.JSDocInfo; import com.google.javascript.rhino.JSDocInfoBuilder; @@ -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.HashSet; import java.util.Set; @@ -58,7 +57,7 @@ class ConvertToTypedInterface implements CompilerPass { @Override public void process(Node externs, Node root) { NodeTraversal.traverseEs6(compiler, root, new PropagateConstJsdoc(compiler)); - NodeTraversal.traverseEs6(compiler, root, new RemoveCode(compiler)); + NodeTraversal.traverseRootsEs6(compiler, new RemoveCode(compiler), externs, root); } private static class PropagateConstJsdoc extends NodeTraversal.AbstractPostOrderCallback { @@ -74,14 +73,14 @@ public void visit(NodeTraversal t, Node n, Node parent) { case EXPR_RESULT: if (NodeUtil.isExprAssign(n)) { Node expr = n.getFirstChild(); - processName(t, expr.getFirstChild()); + propagateJsdocAtName(t, expr.getFirstChild()); } break; case VAR: case CONST: case LET: if (n.getChildCount() == 1) { - processName(t, n.getFirstChild()); + propagateJsdocAtName(t, n.getFirstChild()); } break; default: @@ -89,7 +88,7 @@ public void visit(NodeTraversal t, Node n, Node parent) { } } - private void processName(NodeTraversal t, Node nameNode) { + private void propagateJsdocAtName(NodeTraversal t, Node nameNode) { Node jsdocNode = NodeUtil.getBestJSDocInfoNode(nameNode); JSDocInfo jsdoc = jsdocNode.getJSDocInfo(); if (!isInferrableConst(jsdoc, nameNode)) { @@ -168,14 +167,28 @@ private static JSDocInfo getJSDocForName(Var decl, JSDocInfo oldJSDoc) { } - private static class RemoveCode implements Callback { + private static class RemoveCode extends AbstractModuleCallback { private final AbstractCompiler compiler; - private final Set seenNames = new HashSet<>(); + private final Set globalSeenNames = new HashSet<>(); + private Node currentModule = null; + private Set moduleSeenNames; RemoveCode(AbstractCompiler compiler) { this.compiler = compiler; } + @Override + public void enterModule(NodeTraversal t, Node scopeRoot) { + currentModule = scopeRoot; + moduleSeenNames = new HashSet<>(); + } + + @Override + public void exitModule(NodeTraversal t, Node scopeRoot) { + currentModule = null; + moduleSeenNames = null; + } + @Override public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) { switch (n.getType()) { @@ -213,12 +226,17 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) { parent.removeChild(childBefore); compiler.reportCodeChange(); } - } else if (!callee.matchesQualifiedName("goog.require")) { + } else if (!callee.matchesQualifiedName("goog.require") + && !callee.matchesQualifiedName("goog.module")) { n.detachFromParent(); compiler.reportCodeChange(); } break; case ASSIGN: + if (t.inModuleScope() && expr.getFirstChild().matchesQualifiedName("exports")) { + // Module exports shouldn't be renamed + break; + } processName(expr.getFirstChild(), n); break; case GETPROP: @@ -296,6 +314,22 @@ 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 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) { @@ -303,7 +337,9 @@ private void processConstructor(final Node function) { } final Node insertionPoint = NodeUtil.getEnclosingStatement(function); NodeTraversal.traverseEs6( - compiler, function.getLastChild(), new AbstractShallowStatementCallback() { + compiler, + function.getLastChild(), + new AbstractShallowStatementCallback() { @Override public void visit(NodeTraversal t, Node n, Node parent) { if (n.isExprResult()) { @@ -314,7 +350,7 @@ public void visit(NodeTraversal t, Node n, Node parent) { } String pname = name.getLastChild().getString(); String fullyQualifiedName = className + ".prototype." + pname; - if (seenNames.contains(fullyQualifiedName)) { + if (isNameProcessed(fullyQualifiedName)) { return; } JSDocInfo jsdoc = NodeUtil.getBestJSDocInfo(name); @@ -329,7 +365,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(); - seenNames.add(fullyQualifiedName); + markNameProcessed(fullyQualifiedName); } } }); @@ -345,7 +381,6 @@ private RemovalType shouldRemove(Node nameNode) { Node jsdocNode = NodeUtil.getBestJSDocInfoNode(nameNode); JSDocInfo jsdoc = jsdocNode.getJSDocInfo(); Node rhs = NodeUtil.getRValueOfLValue(nameNode); - // System.err.println("RHS of " + nameNode + " is " + rhs); if (rhs == null || rhs.isFunction() || (rhs.isQualifiedName() && rhs.matchesQualifiedName("goog.abstractMethod")) @@ -357,7 +392,7 @@ private RemovalType shouldRemove(Node nameNode) { } if (jsdoc == null || !jsdoc.containsDeclaration()) { - if (seenNames.contains(nameNode.getQualifiedName())) { + if (isNameProcessed(nameNode.getQualifiedName())) { return RemovalType.REMOVE_ALL; } jsdocNode.setJSDocInfo(getAllTypeJSDoc()); @@ -387,7 +422,7 @@ private void processName(Node nameNode, Node statement) { maybeRemoveRhs(nameNode, statement, jsdocNode.getJSDocInfo()); break; } - seenNames.add(nameNode.getQualifiedName()); + markNameProcessed(nameNode.getQualifiedName()); } private void removeNode(Node n) { diff --git a/test/com/google/javascript/jscomp/ConvertToTypedInterfaceTest.java b/test/com/google/javascript/jscomp/ConvertToTypedInterfaceTest.java index 14d2ffca2ef..b07ded40011 100644 --- a/test/com/google/javascript/jscomp/ConvertToTypedInterfaceTest.java +++ b/test/com/google/javascript/jscomp/ConvertToTypedInterfaceTest.java @@ -39,6 +39,10 @@ public void testInferAnnotatedTypeFromTypeInference() { "/** @constructor */ function Foo() {} \n /** @const {number} */ Foo.prototype.x;"); } + public void testExternsDefinitionsRespected() { + test("/** @type {number} */ var x;", "x = 7;", "", null, null); + } + public void testSimpleConstJsdocPropagation() { test("/** @const */ var x = 5;", "/** @const {number} */ var x;"); test("/** @const */ var x = true;", "/** @const {boolean} */ var x;"); @@ -216,6 +220,70 @@ public void testRemoveUnnecessaryBodies() { "class Foo { method(/** string */ s) {} }"); } + public void testGoogModules() { + testSame( + LINE_JOINER.join( + "goog.module('x.y.z');", + "", + "/** @constructor */ function Foo() {};", + "", + "exports = Foo;")); + + testSame( + new String[] { + LINE_JOINER.join( + "goog.module('a.b.c');", + "/** @constructor */ function Foo() {}", + "Foo.prototoype.display = function() {};", + "exports = Foo;"), + LINE_JOINER.join( + "goog.module('x.y.z');", + "/** @constructor */ function Foo() {}", + "Foo.prototoype.display = function() {};", + "exports = Foo;"), + }); + + testSame( + new String[] { + LINE_JOINER.join( + "/** @constructor */ function Foo() {}", + "Foo.prototoype.display = function() {};"), + LINE_JOINER.join( + "goog.module('x.y.z');", + "/** @constructor */ function Foo() {}", + "Foo.prototoype.display = function() {};", + "exports = Foo;"), + }); + + test( + new String[] { + LINE_JOINER.join( + "goog.module('a.b.c');", + "/** @constructor */ function Foo() {", + " /** @type {number} */ this.x = 5;", + "}", + "exports = Foo;"), + LINE_JOINER.join( + "goog.module('x.y.z');", + "/** @constructor */ function Foo() {", + " /** @type {number} */ this.x = 99;", + "}", + "exports = Foo;"), + }, + new String[] { + LINE_JOINER.join( + "goog.module('a.b.c');", + "/** @constructor */ function Foo() {}", + "/** @type {number} */ Foo.prototype.x;", + "exports = Foo;"), + LINE_JOINER.join( + "goog.module('x.y.z');", + "/** @constructor */ function Foo() {}", + "/** @type {number} */ Foo.prototype.x;", + "exports = Foo;"), + }); + } + public void testRemoveCalls() { test("alert('hello'); window.clearTimeout();", "");