Skip to content

Commit

Permalink
Start pruning non-import destructuring var/const/let statements.
Browse files Browse the repository at this point in the history
Since we don't currently have a way to annotate them as having declared types,
it's impossible for destructuring declarations to meet our requirements
about having declared types at .i.js generation time.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=171606192
  • Loading branch information
blickly authored and Tyler Breisacher committed Oct 10, 2017
1 parent 4cd3eae commit 3db3af1
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 11 deletions.
43 changes: 32 additions & 11 deletions src/com/google/javascript/jscomp/ConvertToTypedInterface.java
Expand Up @@ -228,13 +228,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
case VAR: case VAR:
case LET: case LET:
case CONST: case CONST:
while (n.hasMoreThanOneChild()) { splitNameDeclarationsAndRemoveDestructuring(n, t);
Node nameToSplit = n.getLastChild().detach();
Node rhs = nameToSplit.hasChildren() ? nameToSplit.removeFirstChild() : null;
Node newDeclaration = IR.declaration(nameToSplit, rhs, n.getToken()).srcref(n);
parent.addChildAfter(newDeclaration, n);
t.reportCodeChange();
}
break; break;
case BLOCK: case BLOCK:
if (!parent.isFunction()) { if (!parent.isFunction()) {
Expand All @@ -247,6 +241,33 @@ public void visit(NodeTraversal t, Node n, Node parent) {
break; break;
} }
} }

/**
* Does two simplifications to const/let/var nodes.
* 1. Splits them so that each declaration is a separate statement.
* 2. Removes non-import destructuring statements, which we assume are not type declarations.
*/
static void splitNameDeclarationsAndRemoveDestructuring(Node n, NodeTraversal t) {
checkArgument(NodeUtil.isNameDeclaration(n));
while (n.hasChildren()) {
Node lhsToSplit = n.getLastChild();
if (lhsToSplit.isDestructuringLhs() && !isImportRhs(lhsToSplit.getLastChild())) {
// Remove destructuring statements, which we assume are not type declarations
NodeUtil.markFunctionsDeleted(lhsToSplit, t.getCompiler());
NodeUtil.removeChild(n, lhsToSplit);
t.reportCodeChange();
continue;
}
if (n.hasOneChild()) {
return;
}
// A name declaration with more than one LHS is split into separate declarations.
Node rhs = lhsToSplit.hasChildren() ? lhsToSplit.removeFirstChild() : null;
Node newDeclaration = IR.declaration(lhsToSplit.detach(), rhs, n.getToken()).srcref(n);
n.getParent().addChildAfter(newDeclaration, n);
t.reportCodeChange();
}
}
} }


private static class PropagateConstJsdoc extends NodeTraversal.AbstractPostOrderCallback { private static class PropagateConstJsdoc extends NodeTraversal.AbstractPostOrderCallback {
Expand Down Expand Up @@ -306,7 +327,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
void recordNameDeclaration(NodeTraversal t, Node decl) { void recordNameDeclaration(NodeTraversal t, Node decl) {
checkArgument(NodeUtil.isNameDeclaration(decl)); checkArgument(NodeUtil.isNameDeclaration(decl));
Node rhs = decl.getFirstChild().getLastChild(); Node rhs = decl.getFirstChild().getLastChild();
boolean isImport = rhs != null && isImportRhs(rhs); boolean isImport = isImportRhs(rhs);
for (Node name : NodeUtil.getLhsNodesOfDeclaration(decl)) { for (Node name : NodeUtil.getLhsNodesOfDeclaration(decl)) {
if (isImport) { if (isImport) {
currentFile.recordImport(name.getString()); currentFile.recordImport(name.getString());
Expand Down Expand Up @@ -427,7 +448,7 @@ private static class FileInfo {
private final List<PotentialDeclaration> declarations = new ArrayList<>(); private final List<PotentialDeclaration> declarations = new ArrayList<>();


void recordDeclaration(Node qnameNode, Scope scope) { void recordDeclaration(Node qnameNode, Scope scope) {
checkArgument(qnameNode.isQualifiedName()); checkArgument(qnameNode.isQualifiedName(), qnameNode);
declarations.add(PotentialDeclaration.from(qnameNode, scope)); declarations.add(PotentialDeclaration.from(qnameNode, scope));
} }


Expand Down Expand Up @@ -741,8 +762,8 @@ private static boolean isConstructor(Node functionNode) {
return jsdoc != null && jsdoc.isConstructor(); return jsdoc != null && jsdoc.isConstructor();
} }


private static boolean isImportRhs(Node rhs) { private static boolean isImportRhs(@Nullable Node rhs) {
if (!rhs.isCall()) { if (rhs == null || !rhs.isCall()) {
return false; return false;
} }
Node callee = rhs.getFirstChild(); Node callee = rhs.getFirstChild();
Expand Down
24 changes: 24 additions & 0 deletions test/com/google/javascript/jscomp/ConvertToTypedInterfaceTest.java
Expand Up @@ -1077,6 +1077,30 @@ public void testGoogScopeLeftoversAreRemoved() {
"a.b.c.d.e.f.g.Foo = class {};")); "a.b.c.d.e.f.g.Foo = class {};"));
} }


public void testDestructuringDoesntCrash() {
test(
LINE_JOINER.join(
"goog.module('a.b.c');",
"",
"const Enum = goog.require('Enum');",
"const Foo = goog.require('Foo');",
"",
"const {A, B} = Enum;",
"",
"/** @type {Foo} */",
"exports.foo = use(A, B);",
""),
LINE_JOINER.join(
"goog.module('a.b.c');",
"",
"const Enum = goog.require('Enum');",
"const Foo = goog.require('Foo');",
"",
"/** @type {Foo} */",
"exports.foo;",
""));
}

public void testDescAnnotationCountsAsTyped() { public void testDescAnnotationCountsAsTyped() {
test( test(
LINE_JOINER.join( LINE_JOINER.join(
Expand Down

0 comments on commit 3db3af1

Please sign in to comment.