Remove LSP support for hh_format; hackfmt instead.

fredemmott · fredemmott · commit 45e90b056b93 · 2018-05-21T15:13:26.000-07:00
Summary: hh_format is still the default, which doens't make sense as it's been unmaintained for more than a year.

Differential Revision: D7999954
diff --git a/hphp/hack/src/client/clientLsp.ml b/hphp/hack/src/client/clientLsp.ml
@@ -1130,21 +1130,12 @@ let do_typeCoverage
 
 
 let do_formatting_common
-    (conn: server_conn)
-    (ref_unblocked_time: float ref)
     (editor_open_files: Lsp.TextDocumentItem.t SMap.t)
     (args: ServerFormatTypes.ide_action)
   : TextEdit.t list =
-  let open ServerLocalConfig in
   let open ServerFormatTypes in
-  let local_config = load ~silent:true in
-  let use_hackfmt = local_config.use_hackfmt in
   let response: ServerFormatTypes.ide_result =
-    if use_hackfmt then ServerFormat.go_ide editor_open_files args use_hackfmt
-    else
-      let command = ServerCommandTypes.HH_FORMAT (editor_open_files, args, use_hackfmt) in
-      rpc conn ref_unblocked_time command
-  in
+    ServerFormat.go_ide editor_open_files args 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 *)
@@ -1165,8 +1156,6 @@ let do_formatting_common
 
 
 let do_documentRangeFormatting
-    (conn: server_conn)
-    (ref_unblocked_time: float ref)
     (editor_open_files: Lsp.TextDocumentItem.t SMap.t)
     (params: DocumentRangeFormatting.params)
   : DocumentRangeFormatting.result =
@@ -1178,7 +1167,7 @@ let do_documentRangeFormatting
         file_range = lsp_range_to_ide params.range;
       }
   in
-  do_formatting_common conn ref_unblocked_time editor_open_files action
+  do_formatting_common editor_open_files action
 
 
 let do_signatureHelp
@@ -1194,8 +1183,6 @@ let do_signatureHelp
 
 
 let do_documentOnTypeFormatting
-    (conn: server_conn)
-    (ref_unblocked_time: float ref)
     (editor_open_files: Lsp.TextDocumentItem.t SMap.t)
     (params: DocumentOnTypeFormatting.params)
   : DocumentOnTypeFormatting.result =
@@ -1206,19 +1193,17 @@ let do_documentOnTypeFormatting
         filename = lsp_uri_to_path params.textDocument.uri;
         position = lsp_position_to_ide params.position;
       } in
-  do_formatting_common conn ref_unblocked_time editor_open_files action
+  do_formatting_common editor_open_files action
 
 
 let do_documentFormatting
-    (conn: server_conn)
-    (ref_unblocked_time: float ref)
     (editor_open_files: Lsp.TextDocumentItem.t SMap.t)
     (params: DocumentFormatting.params)
   : DocumentFormatting.result =
   let open DocumentFormatting in
   let open TextDocumentIdentifier in
   let action = ServerFormatTypes.Document (lsp_uri_to_path params.textDocument.uri) in
-  do_formatting_common conn ref_unblocked_time editor_open_files action
+  do_formatting_common editor_open_files action
 
 
 (* do_server_busy: controls the progress / action-required indicator          *)
@@ -1452,7 +1437,6 @@ let rec connect_client
 
 let do_initialize () : Initialize.result =
   let open Initialize in
-  let local_config = ServerLocalConfig.load ~silent:true in
   {
     server_capabilities = {
       textDocumentSync = {
@@ -1477,12 +1461,10 @@ let do_initialize () : Initialize.result =
       codeLensProvider = None;
       documentFormattingProvider = true;
       documentRangeFormattingProvider = true;
-      documentOnTypeFormattingProvider =
-        Option.some_if local_config.ServerLocalConfig.use_hackfmt
-          {
-            firstTriggerCharacter = ";";
-            moreTriggerCharacter = ["}"];
-          };
+      documentOnTypeFormattingProvider = Some {
+        firstTriggerCharacter = ";";
+        moreTriggerCharacter = ["}"];
+      };
       renameProvider = false;
       documentLinkProvider = None;
       executeCommandProvider = None;
@@ -2079,21 +2061,21 @@ let handle_event
   (* textDocument/formatting *)
   | Main_loop menv, Client_message c when c.method_ = "textDocument/formatting" ->
     parse_documentFormatting c.params
-    |> do_documentFormatting menv.conn ref_unblocked_time menv.editor_open_files
+    |> do_documentFormatting menv.editor_open_files
     |> print_documentFormatting |> Jsonrpc.respond to_stdout c
 
   (* textDocument/formatting *)
   | Main_loop menv, Client_message c
     when c.method_ = "textDocument/rangeFormatting" ->
     parse_documentRangeFormatting c.params
-    |> do_documentRangeFormatting menv.conn ref_unblocked_time menv.editor_open_files
+    |> do_documentRangeFormatting menv.editor_open_files
     |> print_documentRangeFormatting |> Jsonrpc.respond to_stdout c
 
   (* textDocument/onTypeFormatting *)
   | Main_loop menv, Client_message c when c.method_ = "textDocument/onTypeFormatting" ->
     cancel_if_stale client c short_timeout;
     parse_documentOnTypeFormatting c.params
-    |> do_documentOnTypeFormatting menv.conn ref_unblocked_time menv.editor_open_files
+    |> do_documentOnTypeFormatting menv.editor_open_files
     |> print_documentOnTypeFormatting |> Jsonrpc.respond to_stdout c
 
   (* textDocument/didOpen notification *)
diff --git a/hphp/hack/src/server/serverCommandTypes.ml b/hphp/hack/src/server/serverCommandTypes.ml
@@ -154,7 +154,6 @@ type _ t =
   | STATS : Stats.t t
   | KILL : unit t
   | FORMAT : ServerFormatTypes.action -> ServerFormatTypes.result t
-  | HH_FORMAT : Lsp.TextDocumentItem.t SMap.t * ServerFormatTypes.ide_action * bool -> ServerFormatTypes.ide_result t
   | TRACE_AI : Ai.TraceService.action -> string t
   | AI_QUERY : string -> string t
   | DUMP_FULL_FIDELITY_PARSE : string -> string t
diff --git a/hphp/hack/src/server/serverCommandTypesUtils.ml b/hphp/hack/src/server/serverCommandTypesUtils.ml
@@ -35,7 +35,6 @@ let debug_describe_t : type a. a t -> string = function
   | STATS                      -> "STATS"
   | KILL                       -> "KILL"
   | FORMAT                   _ -> "FORMAT"
-  | HH_FORMAT                _ -> "HH_FORMAT"
   | TRACE_AI                 _ -> "TRACE_AI"
   | AI_QUERY                 _ -> "AI_QUERY"
   | DUMP_FULL_FIDELITY_PARSE _ -> "DUMP_FULL_FIDELITY_PARSE"
diff --git a/hphp/hack/src/server/serverFormat.ml b/hphp/hack/src/server/serverFormat.ml
@@ -50,36 +50,26 @@ let go_hackfmt ?filename ~content args =
     | None -> args
   in
   Hh_logger.log "%s" (String.concat " " args);
-  let path = Path.make BuildOptions.default_hackfmt_path |> Path.to_string in
-  if Sys.file_exists path
-  then call_external_formatter path content args
-  else begin
+  let dirname = Filename.dirname Sys.argv.(0) in
+  let paths = List.map (fun x -> Path.make x |> Path.to_string) [
+    (* if running from build tree *)
+    dirname ^ "/hackfmt";
+    dirname ^ "/../hackfmt/hackfmt";
+    (* look for system installation *)
+    BuildOptions.default_hackfmt_path;
+  ] in
+  let path = List.find_opt Sys.file_exists paths in
+  match path with
+  | Some path -> call_external_formatter path content args
+  | _ ->
     Hh_logger.log "Formatter not found";
-    Error ("Could not locate formatter on provided path: " ^ path)
-  end
-
-let hh_format_result_to_response x =
-  let open Format_hack in
-  match x with
-  | Disabled_mode -> Error ("Not a Hack file")
-  | Parsing_error _ -> Error ("File has parse errors")
-  | Internal_error -> Error ("Formatter internal error")
-  | Success s -> Ok s
-
-let go_hh_format content from to_ =
-    let modes = [Some FileInfo.Mstrict; Some FileInfo.Mpartial] in
-    hh_format_result_to_response @@
-      Format_hack.region modes Path.dummy_path from to_ content
+    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_ use_hackfmt =
-    if use_hackfmt
-    then
-      let args = range_offsets_to_args from to_ in
-      go_hackfmt ?filename ~content args >>| fun lines ->
-      (String.concat "\n" lines) ^ "\n"
-    else
-    go_hh_format content from to_
+let go ?filename ~content from to_ =
+    let args = range_offsets_to_args from to_ in
+    go_hackfmt ?filename ~content args >>| fun lines ->
+    (String.concat "\n" lines) ^ "\n"
 
 (* Our formatting engine can only handle ranges that span entire rows.  *)
 (* This is signified by a range that starts at column 1 on one row,     *)
@@ -114,7 +104,6 @@ let range_regexp = Str.regexp "^\\([0-9]+\\) \\([0-9]+\\)$"
 let go_ide
   (editor_open_files: Lsp.TextDocumentItem.t SMap.t)
   (action: ServerFormatTypes.ide_action)
-  (use_hackfmt: bool)
   : ServerFormatTypes.ide_result =
   let open File_content in
   let open ServerFormatTypes in
@@ -145,14 +134,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) use_hackfmt
+    go ~filename ~content (from0 + 1) (to0 + 1)
       |> 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) use_hackfmt
+    go ~filename ~content (from0 + 1) (to0 + 1)
       |> convert_to_ide_result ~range
 
   | Position { Ide_api_types.position; _} ->
diff --git a/hphp/hack/src/server/serverLocalConfig.ml b/hphp/hack/src/server/serverLocalConfig.ml
@@ -16,7 +16,6 @@ type t = {
   watchman_subscribe: bool;
   watchman_synchronous_timeout : int; (* in seconds *)
   use_mini_state: bool;
-  use_hackfmt: bool;
   load_mini_script_timeout: int; (* in seconds *)
   (** Prefer using Ocaml implementation over load script. *)
   load_state_natively: bool;
@@ -54,7 +53,6 @@ let default = {
   watchman_subscribe = false;
   watchman_synchronous_timeout = 120;
   use_mini_state = false;
-  use_hackfmt = false;
   load_mini_script_timeout = 20;
   load_state_natively = false;
   type_decl_bucket_size = 1000;
@@ -136,8 +134,6 @@ let load_ fn ~silent =
     ~default:default.load_mini_script_timeout config in
   let load_state_natively = bool_ "load_state_natively_v4"
     ~default:default.load_state_natively config in
-  let use_hackfmt = bool_ "use_hackfmt"
-    ~default:default.use_hackfmt config in
   let state_loader_timeouts = state_loader_timeouts_
     ~default:State_loader_config.default_timeouts config in
   let use_dummy_informant = bool_ "use_dummy_informant"
@@ -185,7 +181,6 @@ let load_ fn ~silent =
     watchman_subscribe;
     watchman_synchronous_timeout;
     use_mini_state;
-    use_hackfmt;
     load_mini_script_timeout;
     load_state_natively;
     max_purgatory_clients;
diff --git a/hphp/hack/src/server/serverRpc.ml b/hphp/hack/src/server/serverRpc.ml
@@ -139,12 +139,7 @@ let handle : type a. genv -> env -> is_stale:bool -> a t -> env * a =
     | STATS -> env, Stats.get_stats ()
     | KILL -> env, ()
     | FORMAT (content, from, to_) ->
-      let open ServerLocalConfig in
-      let local_config = load ~silent:true in
-      let use_hackfmt = local_config.use_hackfmt in
-        env, ServerFormat.go content from to_ use_hackfmt
-    | HH_FORMAT (editor_open_files, action, use_hackfmt) ->
-        env, ServerFormat.go_ide editor_open_files action use_hackfmt
+        env, ServerFormat.go content from to_
     | 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/test/integration/data/lsp_exchanges/completion.expected b/hphp/hack/test/integration/data/lsp_exchanges/completion.expected
@@ -38,6 +38,12 @@
         "codeActionProvider": false,
         "documentFormattingProvider": true,
         "documentRangeFormattingProvider": true,
+        "documentOnTypeFormattingProvider": {
+          "firstTriggerCharacter": ";",
+          "moreTriggerCharacter": [
+            "}"
+          ]
+        },
         "renameProvider": false,
         "typeCoverageProvider": true,
         "rageProvider": true
diff --git a/hphp/hack/test/integration/data/lsp_exchanges/completion_legacy.expected b/hphp/hack/test/integration/data/lsp_exchanges/completion_legacy.expected
@@ -38,6 +38,12 @@
         "codeActionProvider": false,
         "documentFormattingProvider": true,
         "documentRangeFormattingProvider": true,
+        "documentOnTypeFormattingProvider": {
+          "firstTriggerCharacter": ";",
+          "moreTriggerCharacter": [
+            "}"
+          ]
+        },
         "renameProvider": false,
         "typeCoverageProvider": true,
         "rageProvider": true
diff --git a/hphp/hack/test/integration/data/lsp_exchanges/definition.expected b/hphp/hack/test/integration/data/lsp_exchanges/definition.expected
@@ -38,6 +38,12 @@
         "codeActionProvider": false,
         "documentFormattingProvider": true,
         "documentRangeFormattingProvider": true,
+        "documentOnTypeFormattingProvider": {
+          "firstTriggerCharacter": ";",
+          "moreTriggerCharacter": [
+            "}"
+          ]
+        },
         "renameProvider": false,
         "typeCoverageProvider": true,
         "rageProvider": true
diff --git a/hphp/hack/test/integration/data/lsp_exchanges/formatting.expected b/hphp/hack/test/integration/data/lsp_exchanges/formatting.expected
@@ -38,6 +38,12 @@
         "codeActionProvider": false,
         "documentFormattingProvider": true,
         "documentRangeFormattingProvider": true,
+        "documentOnTypeFormattingProvider": {
+          "firstTriggerCharacter": ";",
+          "moreTriggerCharacter": [
+            "}"
+          ]
+        },
         "renameProvider": false,
         "typeCoverageProvider": true,
         "rageProvider": true
diff --git a/hphp/hack/test/integration/data/lsp_exchanges/highlight.expected b/hphp/hack/test/integration/data/lsp_exchanges/highlight.expected
@@ -38,6 +38,12 @@
         "codeActionProvider": false,
         "documentFormattingProvider": true,
         "documentRangeFormattingProvider": true,
+        "documentOnTypeFormattingProvider": {
+          "firstTriggerCharacter": ";",
+          "moreTriggerCharacter": [
+            "}"
+          ]
+        },
         "renameProvider": false,
         "typeCoverageProvider": true,
         "rageProvider": true
diff --git a/hphp/hack/test/integration/data/lsp_exchanges/hover.expected b/hphp/hack/test/integration/data/lsp_exchanges/hover.expected
@@ -38,6 +38,12 @@
         "codeActionProvider": false,
         "documentFormattingProvider": true,
         "documentRangeFormattingProvider": true,
+        "documentOnTypeFormattingProvider": {
+          "firstTriggerCharacter": ";",
+          "moreTriggerCharacter": [
+            "}"
+          ]
+        },
         "renameProvider": false,
         "typeCoverageProvider": true,
         "rageProvider": true
diff --git a/hphp/hack/test/integration/data/lsp_exchanges/initialize_shutdown.expected b/hphp/hack/test/integration/data/lsp_exchanges/initialize_shutdown.expected
@@ -38,6 +38,12 @@
         "codeActionProvider": false,
         "documentFormattingProvider": true,
         "documentRangeFormattingProvider": true,
+        "documentOnTypeFormattingProvider": {
+          "firstTriggerCharacter": ";",
+          "moreTriggerCharacter": [
+            "}"
+          ]
+        },
         "renameProvider": false,
         "typeCoverageProvider": true,
         "rageProvider": true
diff --git a/hphp/hack/test/integration/data/lsp_exchanges/nomethod.expected b/hphp/hack/test/integration/data/lsp_exchanges/nomethod.expected
@@ -38,6 +38,12 @@
         "codeActionProvider": false,
         "documentFormattingProvider": true,
         "documentRangeFormattingProvider": true,
+        "documentOnTypeFormattingProvider": {
+          "firstTriggerCharacter": ";",
+          "moreTriggerCharacter": [
+            "}"
+          ]
+        },
         "renameProvider": false,
         "typeCoverageProvider": true,
         "rageProvider": true
diff --git a/hphp/hack/test/integration/data/lsp_exchanges/signaturehelp.expected b/hphp/hack/test/integration/data/lsp_exchanges/signaturehelp.expected
@@ -38,6 +38,12 @@
         "codeActionProvider": false,
         "documentFormattingProvider": true,
         "documentRangeFormattingProvider": true,
+        "documentOnTypeFormattingProvider": {
+          "firstTriggerCharacter": ";",
+          "moreTriggerCharacter": [
+            "}"
+          ]
+        },
         "renameProvider": false,
         "typeCoverageProvider": true,
         "rageProvider": true
