From d886a3d40d46a6eeacad67aab5f065171f8f31df Mon Sep 17 00:00:00 2001
From: Jake Bailey <jakebailey@fb.com>
Date: Fri, 11 Aug 2017 11:27:27 -0700
Subject: [PATCH] Refactor SwitchStatement handling

Summary: This handling was needlessly factored out into a helper function, which meant that we needed to throw exceptions when encountering SwitchStatement children under other circumstances. I don't see any reason not to just inline this handling into our main transform function.

Differential Revision: D5567179

fbshipit-source-id: 4decd93867abe51874c48a5e38ee34698b12d3b1
---
 hphp/hack/src/hackfmt/hack_format.ml               | 104 +++++++--------------
 hphp/hack/test/hackfmt/tests/switch_statement.php  |  19 ++++
 .../test/hackfmt/tests/switch_statement.php.exp    |  19 ++++
 3 files changed, 73 insertions(+), 69 deletions(-)
 create mode 100644 hphp/hack/test/hackfmt/tests/switch_statement.php
 create mode 100644 hphp/hack/test/hackfmt/tests/switch_statement.php.exp

diff --git a/hphp/hack/src/hackfmt/hack_format.ml b/hphp/hack/src/hackfmt/hack_format.ml
index ae3ad4fe30f..fd87f7fbfd1 100644
--- a/hphp/hack/src/hackfmt/hack_format.ml
+++ b/hphp/hack/src/hackfmt/hack_format.ml
@@ -736,23 +736,47 @@ let rec t node =
   | SwitchStatement x ->
     let (kw, left_p, expr, right_p, left_b, sections, right_b) =
       get_switch_statement_children x in
+    let sections = syntax_node_to_list sections in
+    Fmt [
+      t kw;
+      Space;
+      delimited_nest left_p right_p [t expr];
+      Space;
+      braced_block_nest left_b right_b (List.map sections t);
+      Newline;
+    ]
+  | SwitchSection x ->
+    let (labels, statements, fallthrough) = get_switch_section_children x in
+    let labels = syntax_node_to_list labels in
+    let statements = syntax_node_to_list statements in
+    Fmt [
+      Fmt (List.map labels t);
+      BlockNest (List.map statements t);
+      t fallthrough;
+    ]
+  | CaseLabel x ->
+    let (kw, expr, colon) = get_case_label_children x in
     Fmt [
       t kw;
       Space;
-      t left_p;
       Split;
-      WithRule (Rule.Parental, Fmt [
-        Nest [t expr];
-        t right_p;
-      ]);
-      handle_switch_body left_b sections right_b;
+      t expr;
+      t colon;
+      Newline;
+    ]
+  | DefaultLabel x ->
+    let (kw, colon) = get_default_label_children x in
+    Fmt [
+      t kw;
+      t colon;
       Newline;
     ]
-  | SwitchSection _
-  | CaseLabel _
-  | DefaultLabel _
-  | SwitchFallthrough _ ->
-    failwith "SwitchStatement children should be handled by handle_switch_body"
+  | SwitchFallthrough x ->
+    let (kw, semi) = get_switch_fallthrough_children x in
+    Fmt [
+      t kw;
+      t semi;
+    ]
   | ReturnStatement x ->
     let (kw, expr, semi) = get_return_statement_children x in
     transform_keyword_expression_statement kw expr semi
@@ -1699,64 +1723,6 @@ and handle_possible_chaining (obj, arrow1, member1) argish =
       ])
   | _ -> failwith "Expected a chain of at least length 1"
 
-and handle_switch_body left_b sections right_b =
-  let handle_fallthrough fallthrough =
-    match syntax fallthrough with
-    | SwitchFallthrough x ->
-      let (kw, semi) = get_switch_fallthrough_children x in
-      [
-        t kw;
-        t semi;
-      ]
-    | _ -> []
-  in
-  let handle_label label =
-    match syntax label with
-    | CaseLabel x ->
-      let (kw, expr, colon) = get_case_label_children x in
-      Fmt [
-        t kw;
-        Space;
-        Split;
-        t expr;
-        t colon;
-        Newline;
-      ]
-    | DefaultLabel x ->
-      let (kw, colon) = get_default_label_children x in
-      Fmt [
-        t kw;
-        t colon;
-        Newline;
-      ]
-    | _ -> Nothing
-  in
-  let handle_statement statement =
-    BlockNest [
-      t statement;
-    ]
-  in
-  let handle_section section =
-    match syntax section with
-    | SwitchSection s ->
-      Fmt (
-        (List.map
-          (syntax_node_to_list s.switch_section_labels)
-          ~f:handle_label)
-        @ (List.map
-          (syntax_node_to_list s.switch_section_statements)
-          ~f:handle_statement)
-        @ handle_fallthrough s.switch_section_fallthrough
-      )
-    | _ -> Nothing
-  in
-  Fmt [
-    Space;
-    braced_block_nest left_b right_b (
-      List.map (syntax_node_to_list sections) handle_section
-    )
-  ]
-
 and transform_fn_decl_name async coroutine kw amp name type_params leftp =
   [
     t async;
diff --git a/hphp/hack/test/hackfmt/tests/switch_statement.php b/hphp/hack/test/hackfmt/tests/switch_statement.php
new file mode 100644
index 00000000000..15c4a4424c4
--- /dev/null
+++ b/hphp/hack/test/hackfmt/tests/switch_statement.php
@@ -0,0 +1,19 @@
+<?hh
+
+switch ($foo->color) {
+  case FooColor::RED:
+    handleRedFoo($foo);
+    break;
+  case FooColor::GREEN:
+  case FooColor::BLUE:
+    handleGreenOrBlueFoo($foo);
+    break;
+  case FooColor::ORANGE:
+    logOrangeFooEvent($foo);
+    // FALLTHROUGH
+  case FooColor::YELLOW:
+    handleYellowishFoo($foo);
+    break;
+  default:
+    handleOtherFoo($foo);
+}
diff --git a/hphp/hack/test/hackfmt/tests/switch_statement.php.exp b/hphp/hack/test/hackfmt/tests/switch_statement.php.exp
new file mode 100644
index 00000000000..36cd89971c7
--- /dev/null
+++ b/hphp/hack/test/hackfmt/tests/switch_statement.php.exp
@@ -0,0 +1,19 @@
+<?hh
+
+switch ($foo->color) {
+  case FooColor::RED:
+    handleRedFoo($foo);
+    break;
+  case FooColor::GREEN:
+  case FooColor::BLUE:
+    handleGreenOrBlueFoo($foo);
+    break;
+  case FooColor::ORANGE:
+    logOrangeFooEvent($foo);
+  // FALLTHROUGH
+  case FooColor::YELLOW:
+    handleYellowishFoo($foo);
+    break;
+  default:
+    handleOtherFoo($foo);
+}
-- 
2.11.4.GIT