[flow] No longer make libdef parsing error stop the world

Summary:
Currently, if we fail to parse libdef files (determined by failed to find them in the parsing heap), we will stop all work (no checking at all).

I managed to trace the behavior back to D2176600 when it's first introduced. It's still unclear to me why we have to do this. Maybe we want to show all the libdef errors up front rather than make it hidden in a sea of other errors, but it would be better achieved at the error printing stage.

Since the end of last year, the handling of libdef has switched to a new system: it's treated mostly as a normal file, except that we will run a special type sig merge to extract the signatures of the globals. Under the new system, the stop-the-world behavior seems even more weird and inconsistent, especially consider that we only stop the world for parse but not checking error for libdef.

Therefore, I decide to remove this behavior in this diff.

Changelog: [internal]

Reviewed By: panagosg7

Differential Revision: D54989487

fbshipit-source-id: 3dd2977dce25d12f93399502d004b02bb87474e0

src/codemods/utils/codemod_runner.ml

@@ -613,7 +613,7 @@ module TypedRunner (TypedRunnerConfig : TYPED_RUNNER_CONFIG) : STEP_RUNNER = str
     let should_print_summary = Options.should_profile options in
     Profiling_js.with_profiling_lwt ~label:"Codemod" ~should_print_summary (fun profiling ->
         extract_flowlibs_or_exit options;
-        let%lwt (_libs_ok, env) = Types_js.init ~profiling ~workers options in
+        let%lwt env = Types_js.init ~profiling ~workers options in
         (* Create roots set based on file list *)
         let roots =
           get_target_filename_set
@@ -784,7 +784,7 @@ module UntypedFlowInitRunner (C : UNTYPED_FLOW_INIT_RUNNER_CONFIG) : STEP_RUNNER
         (* Parse all files (even non @flow files) *)
         let options = { options with Options.opt_all = true } in
         extract_flowlibs_or_exit options;
-        let%lwt (_libs_ok, env) = Types_js.init ~profiling ~workers options in
+        let%lwt env = Types_js.init ~profiling ~workers options in
 
         let file_options = Options.file_options options in
         let all = Options.all options in

src/server/server.ml

@@ -67,17 +67,11 @@ let init ~profiling ?focus_targets genv =
 
   extract_flowlibs_or_exit options;
 
-  let%lwt (libs_ok, env) = Types_js.init ~profiling ~workers options in
-  (* If any libs errored, skip typechecking and just show lib errors. Note
-   * that `init` above has done all parsing, not just lib parsing, resolved
-   * and committed modules, etc.
-   *
-   * Furthermore, if we're in lazy mode, we forego typechecking until later,
+  let%lwt env = Types_js.init ~profiling ~workers options in
+  (* If we're in lazy mode, we forego typechecking until later,
    * when it proceeds on an as-needed basis. *)
   let%lwt (env, first_internal_error) =
-    if not libs_ok then
-      Lwt.return (env, None)
-    else if Options.lazy_mode options then
+    if Options.lazy_mode options then
       Types_js.libdef_check_for_lazy_init ~profiling ~workers ~options env
     else
       Types_js.full_check_for_init ~profiling ~workers ?focus_targets ~options env

src/services/inference/init_js.ml

@@ -27,55 +27,51 @@ module ErrorSet = Flow_error.ErrorSet
    returns (success, parse and signature errors, exports)
 *)
 let load_lib_files ~ccx ~options ~reader files =
-  let%lwt (ok, errors, ordered_asts) =
+  let%lwt (errors, ordered_asts) =
     files
     |> Lwt_list.fold_left_s
-         (fun (ok_acc, errors_acc, asts_acc) file ->
+         (fun (errors_acc, asts_acc) file ->
            let lib_file = File_key.LibFile file in
            match Parsing_heaps.Mutator_reader.get_ast ~reader lib_file with
            | Some ast ->
              (* construct ast list in reverse override order *)
              let asts_acc = ast :: asts_acc in
-             Lwt.return (ok_acc, errors_acc, asts_acc)
+             Lwt.return (errors_acc, asts_acc)
            | None ->
              Hh_logger.info "Failed to find %s in parsing heap." (File_key.show lib_file);
-             Lwt.return (false, errors_acc, asts_acc))
-         (true, ErrorSet.empty, [])
+             Lwt.return (errors_acc, asts_acc))
+         (ErrorSet.empty, [])
   in
   let (builtin_exports, master_cx, cx_opt) =
-    if ok then
-      let sig_opts = Type_sig_options.builtin_options options in
-      let (builtins, master_cx) = Merge_js.merge_lib_files ~sig_opts ordered_asts in
-      let cx_opt =
-        match master_cx with
-        | Context.EmptyMasterContext -> None
-        | Context.NonEmptyMasterContext { builtin_leader_file_key; _ } ->
-          let metadata =
-            Context.(
-              let metadata = metadata_of_options options in
-              { metadata with checked = false }
-            )
-          in
-          let mk_builtins = Merge_js.mk_builtins metadata master_cx in
-          let cx =
-            Context.make
-              ccx
-              metadata
-              builtin_leader_file_key
-              (lazy (ALoc.empty_table builtin_leader_file_key))
-              (fun mref -> Error (Reason.InternalModuleName mref))
-              mk_builtins
-          in
-          Some cx
-      in
-      (Exports.of_builtins builtins, master_cx, cx_opt)
-    else
-      (Exports.empty, Context.EmptyMasterContext, None)
+    let sig_opts = Type_sig_options.builtin_options options in
+    let (builtins, master_cx) = Merge_js.merge_lib_files ~sig_opts ordered_asts in
+    let cx_opt =
+      match master_cx with
+      | Context.EmptyMasterContext -> None
+      | Context.NonEmptyMasterContext { builtin_leader_file_key; _ } ->
+        let metadata =
+          Context.(
+            let metadata = metadata_of_options options in
+            { metadata with checked = false }
+          )
+        in
+        let mk_builtins = Merge_js.mk_builtins metadata master_cx in
+        let cx =
+          Context.make
+            ccx
+            metadata
+            builtin_leader_file_key
+            (lazy (ALoc.empty_table builtin_leader_file_key))
+            (fun mref -> Error (Reason.InternalModuleName mref))
+            mk_builtins
+        in
+        Some cx
+    in
+    (Exports.of_builtins builtins, master_cx, cx_opt)
   in
-  Lwt.return (ok, master_cx, cx_opt, errors, builtin_exports)
+  Lwt.return (master_cx, cx_opt, errors, builtin_exports)
 
 type init_result = {
-  ok: bool;
   errors: ErrorSet.t FilenameMap.t;
   warnings: ErrorSet.t FilenameMap.t;
   suppressions: Error_suppressions.t;
@@ -103,7 +99,7 @@ let error_set_to_filemap err_set =
 let init ~options ~reader lib_files =
   let ccx = Context.make_ccx () in
 
-  let%lwt (ok, master_cx, cx_opt, parse_and_sig_errors, exports) =
+  let%lwt (master_cx, cx_opt, parse_and_sig_errors, exports) =
     load_lib_files ~ccx ~options ~reader lib_files
   in
 
@@ -135,4 +131,4 @@ let init ~options ~reader lib_files =
     (error_set_to_filemap errors, error_set_to_filemap warnings, suppressions)
   in
 
-  Lwt.return { ok; errors; warnings; suppressions; exports; master_cx }
+  Lwt.return { errors; warnings; suppressions; exports; master_cx }

src/services/inference/init_js.mli

@@ -6,7 +6,6 @@
  *)
 
 type init_result = {
-  ok: bool;
   errors: Flow_error.ErrorSet.t Utils_js.FilenameMap.t;
   warnings: Flow_error.ErrorSet.t Utils_js.FilenameMap.t;
   suppressions: Error_suppressions.t;

src/services/inference/types_js.ml

@@ -638,8 +638,7 @@ let ensure_parsed_or_trigger_recheck ~options ~profiling ~workers ~reader files
 let init_libs ~options ~profiling ~local_errors ~warnings ~suppressions ~reader ordered_libs =
   with_memory_timer_lwt ~options "InitLibs" profiling (fun () ->
       let%lwt {
-            Init_js.ok;
-            errors = lib_errors;
+            Init_js.errors = lib_errors;
             warnings = lib_warnings;
             suppressions = lib_suppressions;
             exports;
@@ -648,8 +647,7 @@ let init_libs ~options ~profiling ~local_errors ~warnings ~suppressions ~reader
         Init_js.init ~options ~reader ordered_libs
       in
       Lwt.return
-        ( ok,
-          FilenameMap.union lib_errors local_errors,
+        ( FilenameMap.union lib_errors local_errors,
           FilenameMap.union lib_warnings warnings,
           Error_suppressions.update_suppressions lib_suppressions suppressions,
           exports,
@@ -2002,7 +2000,7 @@ let init_from_saved_state ~profiling ~workers ~saved_state ~updates ?env options
           Bucket.of_list []
     )
   in
-  let%lwt (libs_ok, local_errors, warnings, suppressions, lib_exports, master_cx) =
+  let%lwt (local_errors, warnings, suppressions, lib_exports, master_cx) =
     let suppressions = Error_suppressions.empty in
     let warnings = FilenameMap.empty in
     init_libs ~options ~profiling ~local_errors ~warnings ~suppressions ~reader ordered_libs
@@ -2100,15 +2098,14 @@ let init_from_saved_state ~profiling ~workers ~saved_state ~updates ?env options
       ~connections
       ~master_cx
   in
-  Lwt.return (env, libs_ok)
+  Lwt.return env
 
-let handle_updates_since_saved_state ~profiling ~workers ~options ~libs_ok updates env =
+let handle_updates_since_saved_state ~profiling ~workers ~options updates env =
   let should_force_recheck = Options.saved_state_force_recheck options in
   (* We know that all the files in updates have changed since the saved state was generated. We
      * have two ways to deal with them: *)
   if (not (Options.lazy_mode options)) || should_force_recheck then begin
-    if CheckedSet.is_empty updates || not libs_ok then
-      (* Don't recheck if the libs are not ok *)
+    if CheckedSet.is_empty updates then
       Lwt.return env
     else
       (* In non-lazy mode, we return updates here. They will immediately be rechecked. Due to
@@ -2215,7 +2212,7 @@ let init_from_scratch ~profiling ~workers options =
 
   Hh_logger.info "Loading libraries";
   MonitorRPC.status_update ~event:ServerStatus.Load_libraries_start;
-  let%lwt (libs_ok, local_errors, warnings, suppressions, lib_exports, master_cx) =
+  let%lwt (local_errors, warnings, suppressions, lib_exports, master_cx) =
     let suppressions = Error_suppressions.empty in
     init_libs ~options ~profiling ~local_errors ~warnings ~suppressions ~reader ordered_libs
   in
@@ -2279,7 +2276,7 @@ let init_from_scratch ~profiling ~workers options =
       ~connections:Persistent_connection.empty
       ~master_cx
   in
-  Lwt.return (env, libs_ok)
+  Lwt.return env
 
 let exit_if_no_fallback ?msg options =
   if Options.saved_state_no_fallback options then Exit.(exit ?msg Invalid_saved_state)
@@ -2343,27 +2340,23 @@ let load_saved_state ~profiling ~workers options =
 
 let init ~profiling ~workers options =
   let start_time = Unix.gettimeofday () in
-  let%lwt (env, libs_ok) =
+  let%lwt env =
     match%lwt load_saved_state ~profiling ~workers options with
     | Error msg ->
       (* Either there is no saved state or we failed to load it for some reason *)
       Hh_logger.info "Failed to load saved state: %s" msg;
       init_from_scratch ~profiling ~workers options
     | Ok (saved_state, updates) ->
       (* We loaded a saved state successfully! We are awesome! *)
-      let%lwt (env, libs_ok) =
-        init_from_saved_state ~profiling ~workers ~saved_state ~updates options
-      in
-      let%lwt env =
-        handle_updates_since_saved_state ~profiling ~workers ~options ~libs_ok updates env
-      in
-      Lwt.return (env, libs_ok)
+      let%lwt env = init_from_saved_state ~profiling ~workers ~saved_state ~updates options in
+      let%lwt env = handle_updates_since_saved_state ~profiling ~workers ~options updates env in
+      Lwt.return env
   in
   let init_time = Unix.gettimeofday () -. start_time in
   let%lwt () =
     Recheck_stats.init ~options ~init_time ~parsed_count:(FilenameSet.cardinal env.ServerEnv.files)
   in
-  Lwt.return (libs_ok, env)
+  Lwt.return env
 
 let reinit ~profiling ~workers ~options ~updates ~files_to_force ~will_be_checked_files env =
   match%lwt
@@ -2385,7 +2378,7 @@ let reinit ~profiling ~workers ~options ~updates ~files_to_force ~will_be_checke
   | Ok (saved_state, updates_since_saved_state) ->
     (* We loaded a saved state successfully! We are awesome! *)
     Hh_logger.info "Reinitializing from saved state";
-    let%lwt (env, libs_ok) =
+    let%lwt env =
       init_from_saved_state
         ~profiling
         ~workers
@@ -2394,8 +2387,6 @@ let reinit ~profiling ~workers ~options ~updates ~files_to_force ~will_be_checke
         ~env
         options
     in
-    (* TODO: what do we do if they're not? exit? *)
-    ignore libs_ok;
 
     let updates_since_saved_state =
       (* In lazy mode, we just want to merge the upstream changes since saved

src/services/inference/types_js.mli

@@ -11,7 +11,7 @@ val init :
   profiling:Profiling_js.running ->
   workers:MultiWorkerLwt.worker list option ->
   Options.t ->
-  (bool (* libs_ok *) * ServerEnv.env) Lwt.t
+  ServerEnv.env Lwt.t
 
 val calc_deps :
   options:Options.t ->

tests/liberr_parse/a.js

@@ -1,5 +1,4 @@
 /**
  * @flow
  */
-// error will not appear in output, because lib parse errors forego merge
-var x: string = 0;
+var x: string = 0; // error: number ~> string

tests/liberr_parse/liberr_parse.exp

@@ -6,5 +6,19 @@ Unexpected identifier, expected the token `,`
         ^^^^^^^
 
 
+Error -------------------------------------------------------------------------------------------------------- a.js:4:17
 
-Found 1 error
+Cannot assign `0` to `x` because number [1] is incompatible with string [2]. [incompatible-type]
+
+   a.js:4:17
+   4| var x: string = 0; // error: number ~> string
+                      ^ [1]
+
+References:
+   a.js:4:8
+   4| var x: string = 0; // error: number ~> string
+             ^^^^^^ [2]
+
+
+
+Found 2 errors