Rollforward "Add more destructuring unit tests in TypedScopeCreator"

Unlike the original change, this just ignores any type JSDoc on qualified
names in assignments unless the assignment is of the form
  /** @type {number} */ a.b.c = rhs;

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=206247764

src/com/google/javascript/jscomp/NodeUtil.java

@@ -4237,6 +4237,7 @@ private static void getLhsNodesHelper(Node n, List<Node> lhsNodes) {
       case DEFAULT_VALUE:
       case CATCH:
       case REST:
+      case CAST:
         getLhsNodesHelper(n.getFirstChild(), lhsNodes);
         return;
       case IMPORT_SPEC:

src/com/google/javascript/jscomp/TypedScopeCreator.java

@@ -2055,7 +2055,7 @@ void maybeDeclareQualifiedName(NodeTraversal t, JSDocInfo info,
         // caught when we try to declare it in the current scope.
         new SlotDefiner()
             .forDeclarationNode(n)
-            .readVariableNameFromDeclarationNode()
+            .forVariableName(qName)
             .inScope(getLValueRootScope(n))
             .withType(valueType)
             .allowLaterTypeInference(inferred)
@@ -2327,6 +2327,11 @@ void visitPostorder(NodeTraversal t, Node n, Node parent) {
 
         case ASSIGN:
           // Handle initialization of properties.
+          // We only allow qualified name declarations of the form
+          //   /** @type {number} */ a.b.c = rhs;
+          // TODO(b/77597706): Ensure that CheckJSDoc warns for JSDoc on assignments not to
+          // qualified names, e.g.
+          //   /** @type {number} */ [a.b.c] = someArr;
           Node firstChild = n.getFirstChild();
           if (firstChild.isGetProp() && firstChild.isQualifiedName()) {
             maybeDeclareQualifiedName(t, n.getJSDocInfo(), firstChild, n, firstChild.getNext());

test/com/google/javascript/jscomp/NodeUtilTest.java

@@ -3380,25 +3380,44 @@ public void testGetDeclaredTypeExpression4() {
     assertThat(typeExpr.getRoot().getFirstChild().getString()).isEqualTo("number");
   }
 
-  public void testFindLhsNodesInNode() {
+  public void testFindLhsNodesInNodeWithNameDeclaration() {
     assertThat(findLhsNodesInNode("var x;")).hasSize(1);
     assertThat(findLhsNodesInNode("var x, y;")).hasSize(2);
     assertThat(findLhsNodesInNode("var f = function(x, y, z) {};")).hasSize(1);
+  }
+
+  public void testFindLhsNodesInNodeWithArrayPatternDeclaration() {
     assertThat(findLhsNodesInNode("var [x=a => a, y = b=>b+1] = arr;")).hasSize(2);
     assertThat(findLhsNodesInNode("var [x=a => a, y = b=>b+1, ...z] = arr;")).hasSize(3);
     assertThat(findLhsNodesInNode("var [ , , , y = b=>b+1, ...z] = arr;")).hasSize(2);
+  }
+
+  public void testFindLhsNodesInNodeWithObjectPatternDeclaration() {
     assertThat(findLhsNodesInNode("var {x = a=>a, y = b=>b+1} = obj;")).hasSize(2);
     assertThat(findLhsNodesInNode("var {p1: x = a=>a, p2: y = b=>b+1} = obj;")).hasSize(2);
     assertThat(findLhsNodesInNode("var {[pname]: x = a=>a, [p2name]: y} = obj;")).hasSize(2);
     assertThat(findLhsNodesInNode("var {lhs1 = a, p2: [lhs2, lhs3 = b] = [notlhs]} = obj;"))
         .hasSize(3);
+  }
+
+  public void testFindLhsNodesInNodeWithCastOnLhs() {
+    Iterable<Node> lhsNodes = findLhsNodesInNode("/** @type {*} */ (a.b) = 3;");
+    assertThat(lhsNodes).hasSize(1);
+    Iterator<Node> nodeIterator = lhsNodes.iterator();
+    assertNode(nodeIterator.next()).matchesQualifiedName("a.b");
+  }
+
+  public void testFindLhsNodesInNodeWithArrayPatternAssign() {
     assertThat(findLhsNodesInNode("[this.x] = rhs;")).hasSize(1);
     assertThat(findLhsNodesInNode("[this.x, y] = rhs;")).hasSize(2);
     assertThat(findLhsNodesInNode("[this.x, y, this.z] = rhs;")).hasSize(3);
     assertThat(findLhsNodesInNode("[y, this.z] = rhs;")).hasSize(2);
     assertThat(findLhsNodesInNode("[x[y]] = rhs;")).hasSize(1);
     assertThat(findLhsNodesInNode("[x.y.z] = rhs;")).hasSize(1);
+    assertThat(findLhsNodesInNode("[ /** @type {*} */ (x.y.z) ] = rhs;")).hasSize(1);
+  }
 
+  public void testFindLhsNodesInNodeWithComplexAssign() {
     assertThat(findLhsNodesInNode("x += 1;")).hasSize(1);
     assertThat(findLhsNodesInNode("x.y += 1;")).hasSize(1);
     assertThat(findLhsNodesInNode("x -= 1;")).hasSize(1);

test/com/google/javascript/jscomp/TypedScopeCreatorTest.java

@@ -228,6 +228,46 @@ 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() {
@@ -252,6 +292,111 @@ public void testVarDeclarationNestedPatterns() {
     assertFalse(lengthVar.isTypeInferred());
   }
 
+  // The following testAssign* tests check that we never treat qualified names in destructuring
+  // patterns as declared. CheckJSDoc will warn on those cases, so TypedScopeCreator just ignores
+  // them. The only way to 'declare' a qualified name is:
+  //    /** @type {number} */ a.b.c = rhs;
+
+  public void testAssignWithJSDocForObjPatWithOneVariable() {
+    // Ignore the JSDoc on the assignment
+    testSame("const ns = {}; (/** @type {number} */ {a: ns.a} = {a: 1});");
+
+    TypedVar aVar = globalScope.getVar("ns.a");
+    assertNull(aVar);
+    }
+
+  public void testAssignWithJSDocForObjPatWithMultipleVariables() {
+    // Ignore the JSDoc on the assignment
+    testSame(
+        "const ns = {}; (/** @type {number} */ {a: ns.a, b: ns.b} = {a: 1});");
+
+    TypedVar aVar = globalScope.getVar("ns.a");
+    assertNull(aVar);
+  }
+
+  public void testAssignObjPatNormalProp() {
+    // CheckJSDoc will warn on the inline type annotation here, typechecking just ignores it.
+    testSame("const ns = {}; ({a: /** number */ ns.a} = {a: 1});");
+
+    TypedVar aVar = globalScope.getVar("ns.a");
+    assertNull(aVar);
+  }
+
+  public void testAssignObjPatComputedProp() {
+    // CheckJSDoc will warn on the inline type annotation here, typechecking just ignores it.
+    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 = globalScope.getVar("ns.a");
+    assertNull(aVar);
+  }
+
+  public void testAssignArrayPatWithQualifiedName() {
+    // CheckJSDoc will warn on the inline type annotation here, typechecking just ignores it.
+    testSame("const ns = {}; [ /** number */ ns.a ] = [1];");
+
+    TypedVar aVar = globalScope.getVar("ns.a");
+    assertNull(aVar);
+  }
+
+  public void testAssignArrayPatWithQualifiedNameAndDefaultValue() {
+    // CheckJSDoc will warn on the inline type annotation here, typechecking just ignores it.
+    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 testCastOnLhsDoesntDeclareProperty() {
+    testSame("const ns = {}; /** @type {null} */ (ns.a) = null;");
+
+    assertNull(globalScope.getVar("ns.a"));
+  }
+
+  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(