Introduce the node SWITCH_BODY to encapsulate the cases of a switch statement.

The cases of a switch are currently children of the `SWITCH` node, along with the switch condition.  This leads to some difficulties correctly modeling the new scope introduced by a switch. It holds all cases, but not the switch condition itself.

Fixes a bug where the compiler incorrectly reported a variable in a switch condition was an "early reference".

Introducing SWITCH_BODY makes it easier to model scope, and also in my opinion makes it easier to write code iterating over all cases in a switch - it just needs to look at all children of the SWITCH_BODY, instead of having to special case skipping the switch condition.

Why not use a BLOCK? various places in the compiler assume that BLOCK implies "children of this node can be statements" - if we used a BLOCK here, we'd have to update those places to say "children can be statements unless in a SWITCH".

PiperOrigin-RevId: 715051442

Author: lauraharker

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

@@ -1753,10 +1753,12 @@ private void validateNoCatchBinding(Node n) {
   private void validateSwitch(Node n) {
     validateNodeType(Token.SWITCH, n);
     validateProperties(n);
-    validateMinimumChildCount(n, 1);
+    validateChildCount(n, 2);
     validateExpression(n.getFirstChild());
+    validateNodeType(Token.SWITCH_BODY, n.getSecondChild());
+    Node cases = n.getSecondChild();
     int defaults = 0;
-    for (Node c = n.getSecondChild(); c != null; c = c.getNext()) {
+    for (Node c = cases.getFirstChild(); c != null; c = c.getNext()) {
       validateSwitchMember(c);
       if (c.isDefaultCase()) {
         defaults++;

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

@@ -1300,8 +1300,12 @@ protected void add(Node node, Context context, boolean printComments) {
         add("switch(");
         add(first);
         add(")");
+        add(last, context);
+        break;
+
+      case SWITCH_BODY:
         cc.beginBlock();
-        addAllSiblings(first.getNext());
+        addAllSiblings(first);
         cc.endBlock(context == Context.STATEMENT);
         break;
 

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

@@ -438,6 +438,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
       case WITH:
         handleWith(n);
         return;
+      case SWITCH_BODY:
       case LABEL:
       case CLASS_MEMBERS:
       case MEMBER_FUNCTION_DEF:
@@ -532,38 +533,40 @@ private void handleFor(Node forNode) {
   private void handleSwitch(Node node) {
     // Transfer to the first non-DEFAULT CASE. if there are none, transfer
     // to the DEFAULT or the EMPTY node.
-    Node next = getNextSiblingOfType(node.getSecondChild(), Token.CASE, Token.EMPTY);
+    Node switchBody = node.getLastChild();
+    createEdge(node, Branch.UNCOND, switchBody);
+    Node next = getNextSiblingOfType(switchBody.getFirstChild(), Token.CASE, Token.EMPTY);
     if (next != null) { // Has at least one CASE or EMPTY
-      createEdge(node, Branch.UNCOND, next);
+      createEdge(switchBody, Branch.UNCOND, next);
     } else { // Has no CASE but possibly a DEFAULT
-      if (node.getSecondChild() != null) {
-        createEdge(node, Branch.UNCOND, node.getSecondChild());
+      if (switchBody.hasChildren()) {
+        createEdge(switchBody, Branch.UNCOND, switchBody.getFirstChild());
       } else { // No CASE, no DEFAULT
-        createEdge(node, Branch.UNCOND, computeFollowNode(node, this));
+        createEdge(switchBody, Branch.UNCOND, computeFollowNode(node, this));
       }
     }
     connectToPossibleExceptionHandler(node, node.getFirstChild());
   }
 
-  private void handleCase(Node node) {
+  private void handleCase(Node caseNode) {
     // Case is a bit tricky....First it goes into the body if condition is true.
-    createEdge(node, Branch.ON_TRUE, node.getSecondChild());
+    createEdge(caseNode, Branch.ON_TRUE, caseNode.getSecondChild());
 
     // Look for the next CASE, skipping over DEFAULT.
-    Node next = getNextSiblingOfType(node.getNext(), Token.CASE);
+    Node next = getNextSiblingOfType(caseNode.getNext(), Token.CASE);
     if (next != null) { // Found a CASE
       checkState(next.isCase());
-      createEdge(node, Branch.ON_FALSE, next);
+      createEdge(caseNode, Branch.ON_FALSE, next);
     } else { // No more CASE found, go back and search for a DEFAULT.
-      Node parent = node.getParent();
-      Node deflt = getNextSiblingOfType(parent.getSecondChild(), Token.DEFAULT_CASE);
+      Node switchBody = caseNode.getParent();
+      Node deflt = getNextSiblingOfType(switchBody.getFirstChild(), Token.DEFAULT_CASE);
       if (deflt != null) { // Has a DEFAULT
-        createEdge(node, Branch.ON_FALSE, deflt);
+        createEdge(caseNode, Branch.ON_FALSE, deflt);
       } else { // No DEFAULT found, go to the follow of the SWITCH.
-        createEdge(node, Branch.ON_FALSE, computeFollowNode(node, this));
+        createEdge(caseNode, Branch.ON_FALSE, computeFollowNode(caseNode, this));
       }
     }
-    connectToPossibleExceptionHandler(node, node.getFirstChild());
+    connectToPossibleExceptionHandler(caseNode, caseNode.getFirstChild());
   }
 
   private void handleDefault(Node node) {

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

@@ -146,6 +146,7 @@ public static boolean isEnteringNewCfgNode(Node n) {
       case ROOT:
       case SCRIPT:
       case TRY:
+      case SWITCH_BODY:
         return true;
       case FUNCTION:
         // A function node represents the start of a function where the name
@@ -176,11 +177,10 @@ public static boolean isEnteringNewCfgNode(Node n) {
         // TODO(user): Investigate how we should handle the case where
         // we have a very complex expression inside the FOR-IN header.
         return n != parent.getFirstChild();
-      case SWITCH:
       case CASE:
       case CATCH:
       case WITH:
-        return n != parent.getFirstChild();
+        return n != parent.getFirstChild(); // exclude the condition
       default:
         return false;
     }

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

@@ -969,7 +969,8 @@ void transpileSwitch(Node n, TranspilationContext.@Nullable Case breakCase) {
 
       // Are all "switch" cases unmarked?
       boolean hasGeneratorMarker = false;
-      for (Node caseSection = n.getSecondChild();
+      Node switchBody = n.getSecondChild();
+      for (Node caseSection = switchBody.getFirstChild();
           caseSection != null;
           caseSection = caseSection.getNext()) {
         if (caseSection.isGeneratorMarker()) {
@@ -1000,7 +1001,7 @@ class SwitchCase {
 
       // We don't have to transpile unmarked cases at the beginning of "switch".
       boolean canSkipUnmarkedCases = true;
-      for (Node caseSection = n.getSecondChild();
+      for (Node caseSection = switchBody.getFirstChild();
           caseSection != null;
           caseSection = caseSection.getNext()) {
         if (!caseSection.isDefaultCase() && caseSection.getFirstChild().isGeneratorMarker()) {
@@ -1046,6 +1047,7 @@ class SwitchCase {
 
       // Transpile the barebone of original "switch" statement
       n.setGeneratorMarker(false);
+      switchBody.setGeneratorMarker(false);
       transpileUnmarkedNode(n);
       context.writeJumpTo(endCase, n); // TODO(skill): do not always add this.
 
@@ -1359,10 +1361,11 @@ public void finalizeTransformation(Node generatorBody) {
         Node switchNode =
             IR.switchNode(getContextField(generatorBody, "nextAddress")).srcref(generatorBody);
         generatorBody.addChildToBack(switchNode);
+        Node switchBody = switchNode.getSecondChild().srcref(generatorBody);
 
         // Populate "switch" statement with "case"s.
         for (Case currentCase : allCases) {
-          switchNode.addChildToBack(currentCase.createCaseNode());
+          switchBody.addChildToBack(currentCase.createCaseNode());
         }
         allCases.clear();
       }

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

@@ -924,6 +924,7 @@ private static boolean isConditionalOp(Node n) {
         case BLOCK:
         case LABEL:
         case CASE:
+        case SWITCH_BODY:
         case DEFAULT_CASE:
         case DEFAULT_VALUE:
         case PARAM_LIST:

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

@@ -411,9 +411,11 @@ private void splitObject(Var v, Reference init, ReferenceCollection referenceInf
         // Assuming scopeRoot is a BLOCK, then we want to insert at the top of the block, before the
         // first statement.
         vnode = scopeRoot.getFirstChild();
-        // Some scope-creating nodes might not be BLOCK nodes (e.g. SWITCH)
+        // Some scope-creating nodes might not be BLOCK nodes
         if (!NodeUtil.isStatement(vnode) && NodeUtil.isStatement(scopeRoot)) {
           vnode = scopeRoot;
+        } else if (scopeRoot.isSwitchBody()) {
+          vnode = scopeRoot.getParent();
         }
       } else {
         // Find the beginning of the function body / script.

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

@@ -200,8 +200,9 @@ void tryMinimizeExits(Node n, Token exitType, @Nullable String labelName) {
   void tryMinimizeSwitchExits(Node n, Token exitType, @Nullable String labelName) {
     checkState(n.isSwitch());
     // Skipping the switch condition, visit all the children.
-    for (Node c = n.getSecondChild(); c != null; c = c.getNext()) {
-      if (c != n.getLastChild()) {
+    Node switchBody = n.getSecondChild();
+    for (Node c = switchBody.getFirstChild(); c != null; c = c.getNext()) {
+      if (c != switchBody.getLastChild()) {
         tryMinimizeSwitchCaseExits(c, exitType, labelName);
       } else {
         // Last case, the last case block can be optimized more aggressively.

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

@@ -656,7 +656,7 @@ private void traverseAtScope(AbstractScope<?, ?> s) {
         }
         break;
       case BLOCK:
-      case SWITCH:
+      case SWITCH_BODY:
         if (callback.shouldTraverse(this, n, null)) {
           pushScope(s);
 

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

@@ -2450,7 +2450,7 @@ static boolean isControlStructureCodeBlock(Node parent, Node n) {
       case IF:
       case SWITCH:
       case CASE:
-        return parent.getFirstChild() != n;
+        return parent.getFirstChild() != n; // exclude the condition
       case DEFAULT_CASE:
         return true;
       default:
@@ -2515,7 +2515,7 @@ static boolean createsBlockScope(Node n) {
       case FOR_IN:
       case FOR_OF:
       case FOR_AWAIT_OF:
-      case SWITCH:
+      case SWITCH_BODY:
       case CLASS:
         return true;
       default:
@@ -4533,7 +4533,10 @@ public boolean apply(Node n) {
    * traversing into nested functions, so it will return true for e.g. a BLOCK or SWITCH statement.
    */
   public static boolean isShallowStatementTree(Node n) {
-    return n == null || NodeUtil.isControlStructure(n) || NodeUtil.isStatementBlock(n);
+    return n == null
+        || NodeUtil.isControlStructure(n)
+        || NodeUtil.isStatementBlock(n)
+        || n.isSwitchBody();
   }
 
   /** A predicate for matching statements without exiting the current scope. */

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

@@ -511,31 +511,32 @@ private void removeIfUnnamedBreak(Node maybeBreak) {
     }
   }
 
-  private Node tryRemoveSwitchWithSingleCase(Node n, boolean shouldHoistCondition) {
-    Node caseBlock = n.getLastChild().getLastChild();
+  private Node tryRemoveSwitchWithSingleCase(Node switchNode, boolean shouldHoistCondition) {
+    Node switchBody = switchNode.getSecondChild();
+    Node caseBlock = switchBody.getOnlyChild().getLastChild();
     removeIfUnnamedBreak(caseBlock.getLastChild());
     // Back off if the switch contains statements like "if (a) { break; }"
     if (NodeUtil.has(caseBlock, MATCH_UNNAMED_BREAK, NodeUtil.MATCH_NOT_FUNCTION)) {
-      return n;
+      return switchNode;
     }
     if (shouldHoistCondition) {
-      Node switchBlock = caseBlock.getGrandparent();
-      IR.exprResult(n.removeFirstChild()).srcref(n).insertBefore(switchBlock);
+      IR.exprResult(switchNode.removeFirstChild()).srcref(switchNode).insertBefore(switchNode);
     }
-    n.replaceWith(caseBlock.detach());
+    switchNode.replaceWith(caseBlock.detach());
     reportChangeToEnclosingScope(caseBlock);
     return caseBlock;
   }
 
   private Node tryRemoveSwitch(Node n) {
-    if (n.hasOneChild()) {
+    Node switchBody = n.getSecondChild();
+    if (!switchBody.hasChildren()) {
       // Remove the switch if there are no remaining cases
       Node condition = n.removeFirstChild();
       Node replacement = IR.exprResult(condition).srcref(n);
       n.replaceWith(replacement);
       reportChangeToEnclosingScope(replacement);
       return replacement;
-    } else if (n.hasTwoChildren() && n.getLastChild().isDefaultCase()) {
+    } else if (switchBody.hasOneChild() && switchBody.getOnlyChild().isDefaultCase()) {
       if (n.getFirstChild().isCall() || n.getFirstChild().isOptChainCall()) {
         // Before removing switch, we must preserve the switch condition if it is a call
         return tryRemoveSwitchWithSingleCase(n, true);
@@ -551,16 +552,17 @@ private Node tryRemoveSwitch(Node n) {
   private Node tryOptimizeSwitch(Node n) {
     checkState(n.isSwitch(), n);
 
+    Node switchBody = n.getSecondChild();
     Node defaultCase = tryOptimizeDefaultCase(n);
 
     // Generally, it is unsafe to remove other cases when the default case is not the last one.
-    if (defaultCase == null || n.getLastChild().isDefaultCase()) {
+    if (defaultCase == null || switchBody.getLastChild().isDefaultCase()) {
       Node cond = n.getFirstChild();
       Node prev = null;
       Node next = null;
       Node cur;
 
-      for (cur = cond.getNext(); cur != null; cur = next) {
+      for (cur = switchBody.getFirstChild(); cur != null; cur = next) {
         next = cur.getNext();
         if (!mayHaveSideEffects(cur.getFirstChild()) && isUselessCase(cur, prev, defaultCase)) {
           removeCase(n, cur);
@@ -574,7 +576,7 @@ private Node tryOptimizeSwitch(Node n) {
         Node caseLabel;
         Tri caseMatches = Tri.TRUE;
         // Remove cases until you find one that may match
-        for (cur = cond.getNext(); cur != null; cur = next) {
+        for (cur = switchBody.getFirstChild(); cur != null; cur = next) {
           next = cur.getNext();
           caseLabel = cur.getFirstChild();
           caseMatches = PeepholeFoldConstants.evaluateComparison(this, Token.SHEQ, cond, caseLabel);
@@ -637,23 +639,21 @@ private Node tryOptimizeSwitch(Node n) {
   private @Nullable Node tryOptimizeDefaultCase(Node n) {
     checkState(n.isSwitch(), n);
 
-    Node lastNonRemovable = n.getFirstChild(); // The switch condition
+    Node switchBody = n.getSecondChild();
+    Node lastNonRemovable = null; // the most recently iterated case known to not be removable.
 
-    // The first child is the switch conditions skip it when looking for cases.
-    for (Node c = n.getSecondChild(); c != null; c = c.getNext()) {
+    for (Node c = switchBody.getFirstChild(); c != null; c = c.getNext()) {
       if (c.isDefaultCase()) {
-        // Remove cases that fall-through to the default case
-        Node caseToRemove = lastNonRemovable.getNext();
+        // Remove any cases that fall-through to the default case
+        Node caseToRemove =
+            lastNonRemovable != null ? lastNonRemovable.getNext() : switchBody.getFirstChild();
         for (Node next; caseToRemove != c; caseToRemove = next) {
           next = caseToRemove.getNext();
           removeCase(n, caseToRemove);
         }
 
-        // Don't use the switch condition as the previous case.
-        Node prevCase = (lastNonRemovable == n.getFirstChild()) ? null : lastNonRemovable;
-
         // Remove the default case if we can
-        if (isUselessCase(c, prevCase, c)) {
+        if (isUselessCase(c, lastNonRemovable, c)) {
           removeCase(n, c);
           return null;
         }
@@ -690,8 +690,8 @@ private boolean isUselessCase(
     checkState(previousCase == null || previousCase.getNext() == caseNode);
     // A case isn't useless if a previous case falls through to it unless it happens to be the last
     // case in the switch.
-    Node switchNode = caseNode.getParent();
-    if (switchNode.getLastChild() != caseNode && previousCase != null) {
+    Node switchBody = caseNode.getParent();
+    if (switchBody.getLastChild() != caseNode && previousCase != null) {
       Node previousBlock = previousCase.getLastChild();
       if (!previousBlock.hasChildren() || !isExit(previousBlock.getLastChild())) {
         return false;
@@ -818,7 +818,8 @@ private static boolean isSwitchExit(Node n) {
 
     boolean hasDefaultCase = false;
 
-    for (Node switchCase = n.getSecondChild();
+    Node switchBody = n.getSecondChild();
+    for (Node switchCase = switchBody.getFirstChild();
         switchCase != null;
         switchCase = switchCase.getNext()) {
       if (switchCase.isDefaultCase()) {

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

@@ -309,7 +309,7 @@ private static boolean isDefinitelyRValue(Node rvalue) {
       case IF:
       case SWITCH:
       case WHILE:
-        return rvalue.isFirstChildOf(parent);
+        return rvalue.isFirstChildOf(parent); // the condition is always an r-value
 
       case EXPR_RESULT:
         // Extern declarations are sometimes stubs. These must be considered L-values with no

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

@@ -293,6 +293,7 @@ private static boolean isBlockBoundary(Node n, Node parent) {
         case WHILE:
         case WITH:
         case CLASS:
+        case SWITCH_BODY:
           // NOTE: TRY has up to 3 child blocks:
           // TRY
           //   BLOCK
@@ -307,7 +308,6 @@ private static boolean isBlockBoundary(Node n, Node parent) {
         case HOOK:
         case IF:
         case OR:
-        case SWITCH:
         case COALESCE:
         case OPTCHAIN_GETPROP:
         case OPTCHAIN_GETELEM:

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

@@ -463,7 +463,7 @@ private void traverseNode(Node n, Scope scope) {
         traverseCall(n, scope);
         break;
 
-      case SWITCH:
+      case SWITCH_BODY:
       case BLOCK:
         // This case if for if there are let and const variables in block scopes.
         // Otherwise other variables will be hoisted up into the global scope and already be

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

@@ -151,7 +151,7 @@ void populate() {
         case FOR_OF:
         case FOR_AWAIT_OF:
         case FOR_IN:
-        case SWITCH:
+        case SWITCH_BODY:
           scanVars(n, null, scope);
           return;