Rollback "Handle destructuring in assigns in TypedScopeCreator"

It broke internal Google code. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=205874340
google · Jul 25, 2018 · c6853d0 · c6853d0
1 parent 30250ac
commit c6853d0
Show file tree

Hide file tree

Showing 2 changed files with 5 additions and 154 deletions.
diff --git a/src/com/google/javascript/jscomp/TypedScopeCreator.java b/src/com/google/javascript/jscomp/TypedScopeCreator.java
@@ -1946,31 +1946,6 @@ private void applyDelegateRelationship(
       }
     }
 
-    /** Calls maybeDeclareQualifiedName on all names in the LHS of an assign */
-    void defineQualifiedNamesFromAssign(NodeTraversal t, Node assign) {
-      Node lhs = assign.getFirstChild();
-      if (lhs.isGetProp() && lhs.isQualifiedName()) {
-        maybeDeclareQualifiedName(t, assign.getJSDocInfo(), lhs, assign, lhs.getNext());
-        return;
-      }
-      // Handle destructuring assigns
-      List<Node> lhsNodes = NodeUtil.findLhsNodesInNode(assign);
-      JSDocInfo info = assign.getJSDocInfo();
-
-      if (lhsNodes.size() > 1 && info != null) {
-        t.report(assign, TypeCheck.MULTIPLE_VAR_DEF);
-        info = null;
-      }
-
-      for (Node target : lhsNodes) {
-        if (target.isGetProp() && target.isQualifiedName()) {
-          // TODO(b/77597706): make maybeDeclareQualifiedName able to handle a destructuring rhs
-            // and handle inline JSDoc info (i.e. JSDoc on `target` instead of on `assign`)
-          maybeDeclareQualifiedName(t, info, target, target.getParent(), null);
-        }
-      }
-    }
-
     /**
      * Declare the symbol for a qualified name in the global scope.
      *
@@ -2080,7 +2055,7 @@ void maybeDeclareQualifiedName(NodeTraversal t, JSDocInfo info,
         // caught when we try to declare it in the current scope.
         new SlotDefiner()
             .forDeclarationNode(n)
-            .forVariableName(qName)
+            .readVariableNameFromDeclarationNode()
             .inScope(getLValueRootScope(n))
             .withType(valueType)
             .allowLaterTypeInference(inferred)
@@ -2352,7 +2327,10 @@ void visitPostorder(NodeTraversal t, Node n, Node parent) {
 
         case ASSIGN:
           // Handle initialization of properties.
-          defineQualifiedNamesFromAssign(t, n);
+          Node firstChild = n.getFirstChild();
+          if (firstChild.isGetProp() && firstChild.isQualifiedName()) {
+            maybeDeclareQualifiedName(t, n.getJSDocInfo(), firstChild, n, firstChild.getNext());
+          }
           break;
 
         case CATCH:

diff --git a/test/com/google/javascript/jscomp/TypedScopeCreatorTest.java b/test/com/google/javascript/jscomp/TypedScopeCreatorTest.java
@@ -228,46 +228,6 @@ public void testVarDeclarationArrayPatRest() {
     assertFalse(aVar.isTypeInferred());
   }
 
-  public void testConstDeclarationObjectPatternInfersType_forAliasedConstructor() {
-    disableTypeInfoValidation();
-    testSame(
-        lines(
-            "const ns = {};",
-            "/** @constructor */",
-            "ns.Foo = function() {}",
-            "",
-            "const {Foo} = ns;",
-            "// const /** !Foo */ fooInstance = new Foo();"));
-
-    TypedVar fooVar = checkNotNull(globalScope.getVar("Foo"));
-    // TODO(b/77597706): treat `Foo` as an alias type for `ns.Foo`
-    assertNull(fooVar.getType()); // type should be `function(new:ns.Foo): undefined`
-  }
-
-  public void testConstDeclarationObjectPatternInfersType() {
-    disableTypeInfoValidation();
-    testSame(
-        lines(
-            "const /** {a: number} */ obj = {a: 3};", // preserve newline
-            "const {a} = obj;"));
-
-    // TODO(b/77597706): Infer `a` to have type number
-    TypedVar aVar = checkNotNull(globalScope.getVar("a"));
-    assertNull(aVar.getType());
-  }
-
-  public void testConstDeclarationArrayPatternInfersType() {
-    disableTypeInfoValidation();
-    testSame(
-        lines(
-            "const /** !Iterable<number> */ arr = [1, 2, 3];", // preserve newline
-            "const [a] = arr;"));
-
-    // TODO(b/77597706): Infer `a` to have type number
-    TypedVar aVar = checkNotNull(globalScope.getVar("a"));
-    assertNull(aVar.getType());
-  }
-
   // TODO(bradfordcsmith): Add Object rest test case.
 
   public void testVarDeclarationNestedPatterns() {
@@ -292,93 +252,6 @@ public void testVarDeclarationNestedPatterns() {
     assertFalse(lengthVar.isTypeInferred());
   }
 
-  public void testAssignWithJSDocForObjPatWithOneVariable() {
-    testSame("const ns = {}; (/** @type {number} */ {a: ns.a} = {a: 1});");
-    TypedVar aVar = checkNotNull(globalScope.getVar("ns.a"));
-    assertType(aVar.getType()).toStringIsEqualTo("number");
-    assertFalse(aVar.isTypeInferred());
-  }
-
-  public void testAssignWithJSDocForObjPatWithMultipleVariables() {
-    // We don't allow statement-level JSDoc when multiple variables are declared.
-    testWarning(
-        "const ns = {}; (/** @type {number} */ {a: ns.a, b: ns.b} = {a: 1});",
-        TypeCheck.MULTIPLE_VAR_DEF);
-  }
-
-  public void testAssignObjPatNormalProp() {
-    // TODO(b/77597706): this doesn't work because /** number */ ns.a does not parse as inline JSDoc
-    testSame("const ns = {}; ({a: /** number */ ns.a} = {a: 1});");
-
-    TypedVar aVar = globalScope.getVar("ns.a");
-    assertNull(aVar);
-  }
-
-  public void testAssignObjPatComputedProp() {
-    // TODO(b/77597706): this doesn't work because /** number */ ns.a does not parse as inline JSDoc
-    // Either forbid the JSDoc or parse it as an inline type.
-    testSame("const ns = {}; ({['a']: /** number */ ns.a} = {a: 1});");
-
-    TypedVar aVar = globalScope.getVar("ns.a");
-    assertNull(aVar);
-  }
-
-  public void testAssignArrayPatWithJSDocOnAssign() {
-    testSame("const ns = {}; /** @type {number} */ [ ns.a ] = [1];");
-
-    TypedVar aVar = checkNotNull(globalScope.getVar("ns.a"));
-    assertType(aVar.getType()).toStringIsEqualTo("number");
-    assertFalse(aVar.isTypeInferred());
-  }
-
-  public void testAssignArrayPat() {
-    // TODO(b/77597706): this doesn't work because /** number */ ns.a does not parse as inline JSDoc
-    // fix the parser and TypedScopeCreator, then re-enable this test
-    testSame("const ns = {}; [ /** number */ ns.a ] = [1];");
-
-    TypedVar aVar = globalScope.getVar("ns.a");
-    assertNull(aVar);
-  }
-
-  public void testForOfWithObjectPatVarDeclarationWithShorthand() {
-    testSame("for (var {/** number */ a} of {}) {}");
-
-    TypedVar aVar = checkNotNull(globalScope.getVar("a"));
-    assertType(aVar.getType()).toStringIsEqualTo("number");
-    assertFalse(aVar.isTypeInferred());
-  }
-
-  public void testForOfWithArrayPatVarDeclaration() {
-    testSame("for (var [/** number */ a] of []) {}");
-
-    TypedVar aVar = checkNotNull(globalScope.getVar("a"));
-    assertType(aVar.getType()).toStringIsEqualTo("number");
-    assertFalse(aVar.isTypeInferred());
-  }
-
-  public void testForOfWithObjectPatConstDeclarationShadowedInLoop() {
-    testSame(
-        lines(
-            "for (const {/** number */ a} of {}) {",
-            "  const /** string */ a = 'foo';",
-            "  IN_LOOP: a;",
-            "}"));
-
-    assertScope(globalScope).doesNotDeclare("a");
-    TypedScope loopBlockScope = getLabeledStatement("IN_LOOP").enclosingScope;
-    TypedScope loopInitializerScope = loopBlockScope.getParent();
-    assertThat(loopInitializerScope).isNotEqualTo(globalScope);
-
-    TypedVar aVarloopBlock = loopBlockScope.getVar("a");
-    assertType(aVarloopBlock.getType()).toStringIsEqualTo("string");
-    assertFalse(aVarloopBlock.isTypeInferred());
-
-
-    TypedVar aVarLoopInit = loopInitializerScope.getVar("a");
-    assertType(aVarLoopInit.getType()).toStringIsEqualTo("number");
-    assertFalse(aVarLoopInit.isTypeInferred());
-  }
-
   public void testDeclarativelyUnboundVarsWithoutTypes() {
     testSame(
         lines(