[flow] Cleanup faster_error_collation

Reviewed By: panagosg7

Differential Revision: D56654465

fbshipit-source-id: 4c0ba36f6fbc907941aac005bf4b745747718244

src/commands/commandUtils.ml

@@ -892,7 +892,6 @@ module Options_flags = struct
     wait_for_recheck: bool option;
     include_suppressions: bool;
     estimate_recheck_time: bool option;
-    faster_error_collation: bool option;
     long_lived_workers: bool option;
     distributed: bool;
   }
@@ -959,7 +958,6 @@ let options_flags =
       merge_timeout
       include_suppressions
       estimate_recheck_time
-      faster_error_collation
       long_lived_workers
       distributed =
     (match merge_timeout with
@@ -988,7 +986,6 @@ let options_flags =
         merge_timeout;
         include_suppressions;
         estimate_recheck_time;
-        faster_error_collation;
         long_lived_workers;
         distributed;
       }
@@ -1038,7 +1035,6 @@ let options_flags =
       (* restarting to save time is a hack and should be removed. this should
          not be part of our public API, so not included in the docs. *)
       |> flag "--estimate-recheck-time" (optional bool) ~doc:"" ~env:"FLOW_ESTIMATE_RECHECK_TIME"
-      |> flag "--faster-error-collation" (optional bool) ~doc:"" ~env:"FLOW_FASTER_ERROR_COLLATION"
       |> flag "--long-lived-workers" (optional bool) ~doc:"" ~env:"FLOW_LONG_LIVED_WORKERS"
       |> flag "--distributed" truthy ~doc:""
     )
@@ -1400,10 +1396,6 @@ let make_options
     opt_facebook_fbs = FlowConfig.facebook_fbs flowconfig;
     opt_facebook_fbt = FlowConfig.facebook_fbt flowconfig;
     opt_facebook_module_interop = FlowConfig.facebook_module_interop flowconfig;
-    opt_faster_error_collation =
-      Option.value
-        options_flags.faster_error_collation
-        ~default:(FlowConfig.faster_error_collation flowconfig);
     opt_ignore_non_literal_requires = FlowConfig.ignore_non_literal_requires flowconfig;
     opt_include_warnings =
       options_flags.include_warnings

src/commands/config/flowConfig.ml

@@ -67,7 +67,6 @@ module Opts = struct
     facebook_fbs: string option;
     facebook_fbt: string option;
     facebook_module_interop: bool;
-    faster_error_collation: bool;
     file_watcher: file_watcher option;
     file_watcher_mergebase_with: string option;
     file_watcher_mergebase_with_git: string option;
@@ -198,7 +197,6 @@ module Opts = struct
       facebook_fbs = None;
       facebook_fbt = None;
       facebook_module_interop = false;
-      faster_error_collation = true;
       file_watcher = None;
       file_watcher_mergebase_with = None;
       file_watcher_mergebase_with_git = None;
@@ -944,7 +942,6 @@ module Opts = struct
       ("experimenta.precise_dependents", precise_dependents_parser);
       ("facebook.fbs", string (fun opts v -> Ok { opts with facebook_fbs = Some v }));
       ("facebook.fbt", string (fun opts v -> Ok { opts with facebook_fbt = Some v }));
-      ("faster_error_collation", boolean (fun opts v -> Ok { opts with faster_error_collation = v }));
       ("file_watcher", file_watcher_parser);
       ("file_watcher.mergebase_with", file_watcher_mergebase_with_parser);
       ("file_watcher.mergebase_with_git", file_watcher_mergebase_with_git_parser);
@@ -1596,8 +1593,6 @@ let facebook_fbt c = c.options.Opts.facebook_fbt
 
 let facebook_module_interop c = c.options.Opts.facebook_module_interop
 
-let faster_error_collation c = c.options.Opts.faster_error_collation
-
 let file_watcher c = c.options.Opts.file_watcher
 
 let file_watcher_mergebase_with c = c.options.Opts.file_watcher_mergebase_with

src/commands/config/flowConfig.mli

@@ -112,8 +112,6 @@ val facebook_fbt : config -> string option
 
 val facebook_module_interop : config -> bool
 
-val faster_error_collation : config -> bool
-
 val file_watcher : config -> file_watcher option
 
 val file_watcher_mergebase_with : config -> string option

src/common/options.ml

@@ -95,7 +95,6 @@ type t = {
   opt_facebook_fbs: string option;
   opt_facebook_fbt: string option;
   opt_facebook_module_interop: bool;
-  opt_faster_error_collation: bool;
   opt_file_options: Files.options;
   opt_flowconfig_hash: string;
   opt_flowconfig_name: string;
@@ -239,8 +238,6 @@ let facebook_fbt opts = opts.opt_facebook_fbt
 
 let facebook_module_interop opts = opts.opt_facebook_module_interop
 
-let faster_error_collation opts = opts.opt_faster_error_collation
-
 let file_options opts = opts.opt_file_options
 
 let flowconfig_hash opts = opts.opt_flowconfig_hash

src/common/test_utils/test_utils.ml

@@ -34,7 +34,6 @@ let make_options_flags
     ?wait_for_recheck
     ?(include_suppressions = false)
     ?estimate_recheck_time
-    ?faster_error_collation
     ?long_lived_workers
     ?(distributed = false)
     () =
@@ -58,7 +57,6 @@ let make_options_flags
     wait_for_recheck;
     include_suppressions;
     estimate_recheck_time;
-    faster_error_collation;
     long_lived_workers;
     distributed;
   }

src/server/command_handler/commandHandler.ml

@@ -271,53 +271,32 @@ let get_status ~options ~reader env =
   let lazy_stats = Rechecker.get_lazy_stats ~options env in
   let status_response =
     (* collate errors by origin *)
-    if Options.faster_error_collation options then (
-      let (errors, warnings, suppressed_errors) =
-        if Options.include_suppressions options then
-          ErrorCollator.get env
-        else
-          let (errors, warnings) = ErrorCollator.get_without_suppressed env in
-          (errors, warnings, [])
-      in
-      let warnings =
-        if Options.should_include_warnings options then
-          warnings
-        else
-          Flow_errors_utils.ConcreteLocPrintableErrorSet.empty
-      in
-      (* TODO: check status.directory *)
-      status_log errors;
-      FlowEventLogger.status_response
-        ~num_errors:(Flow_errors_utils.ConcreteLocPrintableErrorSet.cardinal errors);
-      let to_printable =
-        Flow_intermediate_error.to_printable_error
-          ~loc_of_aloc:(Parsing_heaps.Reader.loc_of_aloc ~reader)
-          ~strip_root:
-            (Base.Option.some_if (Options.should_strip_root options) (Options.root options))
-      in
-      let suppressed_errors =
-        Base.List.map suppressed_errors ~f:(fun (e, loc_set) -> (to_printable e, loc_set))
-      in
-      convert_errors ~errors ~warnings ~suppressed_errors
-    ) else
-      let (errors, warnings, suppressed_errors) =
-        if Options.include_suppressions options then
-          ErrorCollator.get_old env
-        else
-          let (errors, warnings) = ErrorCollator.get_without_suppressed env in
-          (errors, warnings, [])
-      in
-      let warnings =
-        if Options.should_include_warnings options then
-          warnings
-        else
-          Flow_errors_utils.ConcreteLocPrintableErrorSet.empty
-      in
-      (* TODO: check status.directory *)
-      status_log errors;
-      FlowEventLogger.status_response
-        ~num_errors:(Flow_errors_utils.ConcreteLocPrintableErrorSet.cardinal errors);
-      convert_errors ~errors ~warnings ~suppressed_errors
+    let (errors, warnings, suppressed_errors) =
+      if Options.include_suppressions options then
+        ErrorCollator.get env
+      else
+        let (errors, warnings) = ErrorCollator.get_without_suppressed env in
+        (errors, warnings, [])
+    in
+    let warnings =
+      if Options.should_include_warnings options then
+        warnings
+      else
+        Flow_errors_utils.ConcreteLocPrintableErrorSet.empty
+    in
+    (* TODO: check status.directory *)
+    status_log errors;
+    FlowEventLogger.status_response
+      ~num_errors:(Flow_errors_utils.ConcreteLocPrintableErrorSet.cardinal errors);
+    let to_printable =
+      Flow_intermediate_error.to_printable_error
+        ~loc_of_aloc:(Parsing_heaps.Reader.loc_of_aloc ~reader)
+        ~strip_root:(Base.Option.some_if (Options.should_strip_root options) (Options.root options))
+    in
+    let suppressed_errors =
+      Base.List.map suppressed_errors ~f:(fun (e, loc_set) -> (to_printable e, loc_set))
+    in
+    convert_errors ~errors ~warnings ~suppressed_errors
   in
   (status_response, lazy_stats)
 

src/server/env/collated_errors.ml

@@ -23,20 +23,14 @@ let empty_error_state_timestamps =
     timestamp_start_of_non_zero_subtyping_errors_all_in_one_file = None;
   }
 
-type collated_suppressed_errors =
-  | EmptyCollatedSuppressedErrors
-  | OldCollatedSuppressedErrors of
-      (Loc.t printable_error * Loc_collections.LocSet.t) list Utils_js.FilenameMap.t
-  | NewCollatedSuppressedErrors of
-      (ALoc.t Flow_intermediate_error_types.intermediate_error * Loc_collections.LocSet.t) list
-      Utils_js.FilenameMap.t
-
 type t = {
   collated_duplicate_providers_errors: (Loc.t printable_error * File_key.t * File_key.t) list;
   collated_local_errors: ConcreteLocPrintableErrorSet.t Utils_js.FilenameMap.t;
   collated_merge_errors: ConcreteLocPrintableErrorSet.t Utils_js.FilenameMap.t;
   collated_warning_map: ConcreteLocPrintableErrorSet.t Utils_js.FilenameMap.t;
-  collated_suppressed_errors: collated_suppressed_errors;
+  collated_suppressed_errors:
+    (ALoc.t Flow_intermediate_error_types.intermediate_error * Loc_collections.LocSet.t) list
+    Utils_js.FilenameMap.t;
   error_state_timestamps: error_state_timestamps;
 }
 
@@ -46,7 +40,7 @@ let empty =
     collated_local_errors = FilenameMap.empty;
     collated_merge_errors = FilenameMap.empty;
     collated_warning_map = FilenameMap.empty;
-    collated_suppressed_errors = EmptyCollatedSuppressedErrors;
+    collated_suppressed_errors = FilenameMap.empty;
     error_state_timestamps = empty_error_state_timestamps;
   }
 
@@ -71,13 +65,7 @@ let clear_all files errors =
         collated_local_errors = FilenameMap.remove file collated_local_errors;
         collated_merge_errors = FilenameMap.remove file collated_merge_errors;
         collated_warning_map = FilenameMap.remove file collated_warning_map;
-        collated_suppressed_errors =
-          (match collated_suppressed_errors with
-          | EmptyCollatedSuppressedErrors -> EmptyCollatedSuppressedErrors
-          | OldCollatedSuppressedErrors map ->
-            OldCollatedSuppressedErrors (FilenameMap.remove file map)
-          | NewCollatedSuppressedErrors map ->
-            NewCollatedSuppressedErrors (FilenameMap.remove file map));
+        collated_suppressed_errors = FilenameMap.remove file collated_suppressed_errors;
         error_state_timestamps;
       })
     files
@@ -99,13 +87,7 @@ let clear_merge files errors =
         collated_local_errors;
         collated_merge_errors = FilenameMap.remove file collated_merge_errors;
         collated_warning_map = FilenameMap.remove file collated_warning_map;
-        collated_suppressed_errors =
-          (match collated_suppressed_errors with
-          | EmptyCollatedSuppressedErrors -> EmptyCollatedSuppressedErrors
-          | OldCollatedSuppressedErrors map ->
-            OldCollatedSuppressedErrors (FilenameMap.remove file map)
-          | NewCollatedSuppressedErrors map ->
-            NewCollatedSuppressedErrors (FilenameMap.remove file map));
+        collated_suppressed_errors = FilenameMap.remove file collated_suppressed_errors;
         error_state_timestamps;
       })
     files

src/server/error_collator/errorCollator.ml

@@ -116,53 +116,21 @@ let update_collated_errors ~reader ~options ~checked_files ~all_suppressions err
   let loc_of_aloc = Parsing_heaps.Reader_dispatcher.loc_of_aloc ~reader in
   let acc_fun filename file_errs (errors, suppressed, unused) =
     let file_options = Some (Options.file_options options) in
-    if Options.faster_error_collation options then
-      let (file_errs, file_suppressed, unused) =
-        Error_suppressions.filter_suppressed_errors
-          ~root
-          ~file_options
-          ~loc_of_aloc
-          (* Use all_suppressions here to account for misplaced errors. *)
-          all_suppressions
-          file_errs
-          ~unused
-      in
-      let errors = FilenameMap.add filename file_errs errors in
-      let suppressed =
-        let open Collated_errors in
-        match suppressed with
-        | EmptyCollatedSuppressedErrors ->
-          NewCollatedSuppressedErrors (FilenameMap.singleton filename file_suppressed)
-        | NewCollatedSuppressedErrors map ->
-          NewCollatedSuppressedErrors (FilenameMap.add filename file_suppressed map)
-        | OldCollatedSuppressedErrors _ ->
-          failwith "Encountered OldCollatedSuppressedErrors under faster_error_collation=true"
-      in
-      (errors, suppressed, unused)
-    else
-      let (file_errs, file_suppressed, unused) =
-        Error_suppressions.filter_suppressed_errors_old
-          ~root
-          ~file_options
-          ~loc_of_aloc
-          (* Use all_suppressions here to account for misplaced errors. *)
-          all_suppressions
-          file_errs
-          ~unused
-      in
-      let errors = FilenameMap.add filename file_errs errors in
-      let suppressed =
-        let open Collated_errors in
-        match suppressed with
-        | EmptyCollatedSuppressedErrors ->
-          OldCollatedSuppressedErrors (FilenameMap.singleton filename file_suppressed)
-        | OldCollatedSuppressedErrors map ->
-          OldCollatedSuppressedErrors (FilenameMap.add filename file_suppressed map)
-        | NewCollatedSuppressedErrors _ ->
-          failwith "Encountered NewCollatedSuppressedErrors under faster_error_collation=false"
-      in
-      (errors, suppressed, unused)
+    let (file_errs, file_suppressed, unused) =
+      Error_suppressions.filter_suppressed_errors
+        ~root
+        ~file_options
+        ~loc_of_aloc
+        (* Use all_suppressions here to account for misplaced errors. *)
+        all_suppressions
+        file_errs
+        ~unused
+    in
+    let errors = FilenameMap.add filename file_errs errors in
+    let suppressed = FilenameMap.add filename file_suppressed suppressed in
+    (errors, suppressed, unused)
   in
+
   MonitorRPC.status_update ~event:ServerStatus.Collating_errors_start;
   let {
     collated_duplicate_providers_errors;
@@ -402,27 +370,6 @@ let get_without_suppressed env =
   in
   (errors, warnings)
 
-(* combine error maps into a single error set and a single warning set *)
-let get_old env =
-  let open Flow_errors_utils in
-  let (errors, warning_map, suppressed_errors) = get_with_separate_warnings env in
-  let warnings =
-    FilenameMap.fold
-      (fun _key -> ConcreteLocPrintableErrorSet.union)
-      warning_map
-      ConcreteLocPrintableErrorSet.empty
-  in
-  let suppressed_errors =
-    let open Collated_errors in
-    match suppressed_errors with
-    | EmptyCollatedSuppressedErrors -> FilenameMap.empty
-    | OldCollatedSuppressedErrors map -> map
-    | NewCollatedSuppressedErrors _ ->
-      failwith "Encountered NewCollatedSuppressedErrors under faster_error_collation=false"
-  in
-  let suppressed_errors = FilenameMap.fold (fun _ -> List.rev_append) suppressed_errors [] in
-  (errors, warnings, suppressed_errors)
-
 let get env =
   let open Flow_errors_utils in
   let (errors, warning_map, suppressed_errors) = get_with_separate_warnings env in
@@ -432,14 +379,6 @@ let get env =
       warning_map
       ConcreteLocPrintableErrorSet.empty
   in
-  let suppressed_errors =
-    let open Collated_errors in
-    match suppressed_errors with
-    | EmptyCollatedSuppressedErrors -> FilenameMap.empty
-    | NewCollatedSuppressedErrors map -> map
-    | OldCollatedSuppressedErrors _ ->
-      failwith "Encountered OldCollatedSuppressedErrors under faster_error_collation=true"
-  in
   let suppressed_errors = FilenameMap.fold (fun _ -> List.rev_append) suppressed_errors [] in
   (errors, warnings, suppressed_errors)
 

src/server/error_collator/errorCollator.mli

@@ -53,12 +53,6 @@ val get_without_suppressed :
   Flow_errors_utils.ConcreteLocPrintableErrorSet.t
   * Flow_errors_utils.ConcreteLocPrintableErrorSet.t
 
-val get_old :
-  ServerEnv.env ->
-  Flow_errors_utils.ConcreteLocPrintableErrorSet.t
-  * Flow_errors_utils.ConcreteLocPrintableErrorSet.t
-  * (Loc.t Flow_errors_utils.printable_error * Loc_collections.LocSet.t) list
-
 val get :
   ServerEnv.env ->
   Flow_errors_utils.ConcreteLocPrintableErrorSet.t