Pass LSP spaces vs tabs and indent width options through to hackfmt

fredemmott · fredemmott · commit a7cdefc35ed4 · 2018-05-21T15:13:49.000-07:00
Summary:
Also flipped default `insertSpaces` to `true` to keep default behavior
the same.

Differential Revision: D8018959
diff --git a/hphp/hack/src/client/clientLsp.ml b/hphp/hack/src/client/clientLsp.ml
@@ -1131,11 +1131,12 @@ let do_typeCoverage
 
 let do_formatting_common
     (editor_open_files: Lsp.TextDocumentItem.t SMap.t)
-    (args: ServerFormatTypes.ide_action)
+    (action: ServerFormatTypes.ide_action)
+    (options: DocumentFormatting.formattingOptions)
   : TextEdit.t list =
   let open ServerFormatTypes in
   let response: ServerFormatTypes.ide_result =
-    ServerFormat.go_ide editor_open_files args in
+    ServerFormat.go_ide editor_open_files action options in
   match response with
   | Error "File failed to parse without errors" ->
     (* If LSP issues a formatting request at a given line+char, but we can't *)
@@ -1167,7 +1168,7 @@ let do_documentRangeFormatting
         file_range = lsp_range_to_ide params.range;
       }
   in
-  do_formatting_common editor_open_files action
+  do_formatting_common editor_open_files action params.options
 
 
 let do_signatureHelp
@@ -1193,7 +1194,7 @@ let do_documentOnTypeFormatting
         filename = lsp_uri_to_path params.textDocument.uri;
         position = lsp_position_to_ide params.position;
       } in
-  do_formatting_common editor_open_files action
+  do_formatting_common editor_open_files action params.options
 
 
 let do_documentFormatting
@@ -1203,7 +1204,7 @@ let do_documentFormatting
   let open DocumentFormatting in
   let open TextDocumentIdentifier in
   let action = ServerFormatTypes.Document (lsp_uri_to_path params.textDocument.uri) in
-  do_formatting_common editor_open_files action
+  do_formatting_common editor_open_files action params.options
 
 
 (* do_server_busy: controls the progress / action-required indicator          *)
diff --git a/hphp/hack/src/server/serverFormat.ml b/hphp/hack/src/server/serverFormat.ml
@@ -41,6 +41,14 @@ let call_external_formatter
     cmd
     args
 
+let formatting_options_to_args (options: Lsp.DocumentFormatting.formattingOptions) =
+  let open Lsp.DocumentFormatting in
+  let args = [ "--indent-width"; string_of_int options.tabSize ] in
+  if not options.insertSpaces then
+    args@[ "--tabs" ]
+  else
+    args
+
 let range_offsets_to_args from to_ =
   ["--range"; string_of_int from; string_of_int to_]
 
@@ -66,8 +74,10 @@ let go_hackfmt ?filename ~content args =
     Error ("Could not locate formatter - looked in: " ^ (String.concat " " paths))
 
 (* This function takes 1-based offsets, and 'to_' is exclusive. *)
-let go ?filename ~content from to_ =
-    let args = range_offsets_to_args from to_ in
+let go ?filename ~content from to_ options =
+    let format_args = formatting_options_to_args options in
+    let range_args = range_offsets_to_args from to_ in
+    let args = format_args @ range_args in
     go_hackfmt ?filename ~content args >>| fun lines ->
     (String.concat "\n" lines) ^ "\n"
 
@@ -104,6 +114,7 @@ let range_regexp = Str.regexp "^\\([0-9]+\\) \\([0-9]+\\)$"
 let go_ide
   (editor_open_files: Lsp.TextDocumentItem.t SMap.t)
   (action: ServerFormatTypes.ide_action)
+  (options: Lsp.DocumentFormatting.formattingOptions)
   : ServerFormatTypes.ide_result =
   let open File_content in
   let open ServerFormatTypes in
@@ -134,14 +145,14 @@ let go_ide
     let ed = offset_to_position content to0 in
     let range = {st = {line = 1; column = 1;}; ed;} in
     (* hackfmt currently takes one-indexed integers for range formatting. *)
-    go ~filename ~content (from0 + 1) (to0 + 1)
+    go ~filename ~content (from0 + 1) (to0 + 1) options
       |> convert_to_ide_result ~range
 
   | Range range ->
     let file_range = range.Ide_api_types.file_range |> Ide_api_types.ide_range_to_fc in
     let (range, from0, to0) =
       expand_range_to_whole_rows content file_range in
-    go ~filename ~content (from0 + 1) (to0 + 1)
+    go ~filename ~content (from0 + 1) (to0 + 1) options
       |> convert_to_ide_result ~range
 
   | Position { Ide_api_types.position; _} ->
@@ -150,6 +161,7 @@ let go_ide
     let position = position |> Ide_api_types.ide_pos_to_fc in
     let offset = get_offset content position in
     let args = ["--at-char"; string_of_int offset] in
+    let args = args @ (formatting_options_to_args options) in
     go_hackfmt ~filename ~content args >>= fun lines ->
 
     (* `hackfmt --at-char` returns the range that was formatted, as well as the
diff --git a/hphp/hack/src/server/serverRpc.ml b/hphp/hack/src/server/serverRpc.ml
@@ -139,7 +139,12 @@ let handle : type a. genv -> env -> is_stale:bool -> a t -> env * a =
     | STATS -> env, Stats.get_stats ()
     | KILL -> env, ()
     | FORMAT (content, from, to_) ->
-        env, ServerFormat.go content from to_
+        let legacy_format_options =
+          { Lsp.DocumentFormatting.
+            tabSize = 2;
+            insertSpaces = true;
+          } in
+        env, ServerFormat.go content from to_ legacy_format_options
     | TRACE_AI action ->
         env, Ai.TraceService.go action Typing_check_utils.check_defs
            (ServerArgs.ai_mode genv.options) env.tcopt
diff --git a/hphp/hack/src/utils/lsp_fmt.ml b/hphp/hack/src/utils/lsp_fmt.ml
@@ -248,7 +248,7 @@ let parse_formattingOptions (json: json option)
   : DocumentFormatting.formattingOptions =
   { DocumentFormatting.
     tabSize = Jget.int_d json "tabSize" 2;
-    insertSpaces = Jget.bool_d json "insertSpaces" false;
+    insertSpaces = Jget.bool_d json "insertSpaces" true;
   }
 
 let print_symbolInformation (info: SymbolInformation.t) : json =
diff --git a/hphp/hack/test/integration/data/lsp_exchanges/formatting.expected b/hphp/hack/test/integration/data/lsp_exchanges/formatting.expected
@@ -65,7 +65,7 @@
             "character": 0
           }
         },
-        "newText": "<?hh //strict\n\nfunction x(): string {\n  /* @lint-ignore TXT2 3 tabs on purpose */\n  $a = \"this\";\n\n  /* @lint-ignore TXT2 2 tabs on purpose */\n  $b = \"is\";\n\n  /* lint-ignore TXT2 1 tab on purpose */\n  $c = \"messy\"; // 1 tab\n\n  $d = \".\"; // 4 spaces\n  return \"$a\".\"$b\".\"$c\".\"d\";\n}\n"
+        "newText": "<?hh //strict\n\nfunction x(): string {\n     /* @lint-ignore TXT2 3 tabs on purpose */\n     $a = \"this\";\n\n     /* @lint-ignore TXT2 2 tabs on purpose */\n     $b = \"is\";\n\n     /* lint-ignore TXT2 1 tab on purpose */\n     $c = \"messy\"; // 1 tab\n\n     $d = \".\"; // 4 spaces\n     return \"$a\".\"$b\".\"$c\".\"d\";\n}\n"
       }
     ]
   },
