Skip to content

Commit

Permalink
[flow] Include the full set of libdefs in saved state
Browse files Browse the repository at this point in the history
Summary:
The only reason we are excluding the builtin ones is because its relative path against the root might be unstable across machines. However, the problem has been addressed with the infra added in D69577613, and we are fully capable of normalizing, storing, and denormalizing all libdef paths, so let's reduce the complexity and just store everything.

Changelog: [internal]

Reviewed By: panagosg7

Differential Revision: D69871981

fbshipit-source-id: 2024b362b78bc33ab531e02e2652eab9fe8c89b4
  • Loading branch information
SamChou19815 authored and facebook-github-bot committed Feb 21, 2025
1 parent 95bcff8 commit 7b624f5
Show file tree
Hide file tree
Showing 10 changed files with 77 additions and 53 deletions.
9 changes: 2 additions & 7 deletions src/common/files.ml
Original file line number Diff line number Diff line change
Expand Up @@ -546,14 +546,9 @@ let is_in_flowlib (options : options) : string -> bool =
let root = File_path.make root_str in
is_prefix (File_path.to_string root)

let init ?(flowlibs_only = false) (options : options) =
let init (options : options) =
let node_module_filter = is_node_module options in
let libs =
if flowlibs_only then
[]
else
options.lib_paths
in
let libs = options.lib_paths in
let (libs, filter) =
match options.default_lib_dir with
| None -> (libs, is_valid_path ~options)
Expand Down
2 changes: 1 addition & 1 deletion src/common/files.mli
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ val is_in_flowlib : options -> string -> bool

val get_all_watched_extensions : options -> SSet.t

val init : ?flowlibs_only:bool -> options -> string list * SSet.t
val init : options -> string list * SSet.t

(* regexp for Filename constants *)
val dir_sep : Str.regexp
Expand Down
10 changes: 2 additions & 8 deletions src/services/inference/types_js.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1874,18 +1874,13 @@ let init_with_initial_state
let init_from_saved_state ~profiling ~workers ~saved_state ~updates ?env options =
with_transaction "init" @@ fun transaction reader ->
let is_init = Option.is_none env in
let file_options = Options.file_options options in

(* We don't want to walk the file system for the checked in files. But we still need to find the
* flowlibs *)
let (ordered_flowlib_libs, _) = Files.init ~flowlibs_only:true file_options in

let {
Saved_state.flowconfig_hash = _;
parsed_heaps;
unparsed_heaps;
package_heaps;
ordered_non_flowlib_libs;
ordered_libs;
local_errors;
node_modules_containers;
dependency_graph;
Expand Down Expand Up @@ -2077,7 +2072,6 @@ let init_from_saved_state ~profiling ~workers ~saved_state ~updates ?env options

if verify then assert_valid_hashes updates invalid_hashes;

let ordered_libs = List.rev_append (List.rev ordered_flowlib_libs) ordered_non_flowlib_libs in
let%lwt (env, libs_ok) =
init_with_initial_state
~profiling
Expand Down Expand Up @@ -2299,7 +2293,7 @@ let load_saved_state ~profiling ~workers options =
let updates =
Recheck_updates.process_updates
~options
~libs:(SSet.of_list saved_state.Saved_state.ordered_non_flowlib_libs)
~libs:(SSet.of_list saved_state.Saved_state.ordered_libs)
changed_files
in
let updates =
Expand Down
61 changes: 25 additions & 36 deletions src/services/saved_state/saved_state.ml
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ type saved_state_data = {
parsed_heaps: (File_key.t * parsed_file_data) list;
unparsed_heaps: (File_key.t * unparsed_file_data) list;
package_heaps: (File_key.t * package_file_data) list; (** package.json info *)
ordered_non_flowlib_libs: string list;
ordered_libs: string list;
(* Why store local errors and not merge_errors/suppressions/etc? Well, I have a few reasons:
*
* 1. Much smaller data structure. The whole env.errors data structure can be hundreds of MBs
Expand Down Expand Up @@ -170,15 +170,14 @@ end = struct
* *)
let normalize_path t path = intern t (Files.relative_path t.root path)

let normalize_libdef_path t p =
match t.builtin_flowlib_root with
| Some flowlib_root when String.starts_with ~prefix:flowlib_root p ->
intern t (builtin_flow_lib_marker ^ Files.relative_path flowlib_root p)
| _ -> normalize_path t p

let normalize_file_key t = function
| File_key.LibFile p ->
let p =
match t.builtin_flowlib_root with
| Some flowlib_root when String.starts_with ~prefix:flowlib_root p ->
intern t (builtin_flow_lib_marker ^ Files.relative_path flowlib_root p)
| _ -> normalize_path t p
in
File_key.LibFile p
| File_key.LibFile p -> File_key.LibFile (normalize_libdef_path t p)
| File_key.SourceFile p -> File_key.SourceFile (normalize_path t p)
| File_key.JsonFile p -> File_key.JsonFile (normalize_path t p)
| File_key.ResourceFile p -> File_key.ResourceFile (normalize_path t p)
Expand Down Expand Up @@ -325,13 +324,6 @@ end = struct
let relative_fn = normalize_file_key t fn in
(relative_fn, relative_file_data) :: unparsed_heaps

(* The builtin flowlibs are excluded from the saved state. The server which loads the saved state
* will extract and typecheck its own builtin flowlibs *)
let is_not_in_flowlib ~options =
let file_options = Options.file_options options in
let is_in_flowlib = Files.is_in_flowlib file_options in
(fun f -> not (is_in_flowlib f))

let normalize_error_set t = Flow_error.ErrorSet.map (normalize_error t)

(* Collect all the data for all the files *)
Expand Down Expand Up @@ -365,11 +357,7 @@ end = struct
[]
)
in
let ordered_non_flowlib_libs =
env.ServerEnv.ordered_libs
|> List.filter (is_not_in_flowlib ~options)
|> Base.List.map ~f:(normalize_path t)
in
let ordered_libs = Base.List.map env.ServerEnv.ordered_libs ~f:(normalize_libdef_path t) in
let local_errors =
FilenameMap.fold
(fun fn error_set acc ->
Expand Down Expand Up @@ -417,7 +405,7 @@ end = struct
parsed_heaps;
unparsed_heaps;
package_heaps;
ordered_non_flowlib_libs;
ordered_libs;
local_errors;
node_modules_containers;
dependency_graph;
Expand Down Expand Up @@ -526,6 +514,8 @@ end = struct

val denormalize_path : t -> string -> string

val denormalize_libdef_path : t -> string -> string

val denormalize_file_key_no_cache : t -> File_key.t -> File_key.t

val denormalize_file_key : t -> File_key.t -> File_key.t
Expand All @@ -545,17 +535,16 @@ end = struct
* these calls does not even save a single word in the denormalized saved state object. *)
let denormalize_path { root; _ } path = Files.absolute_path root path

let denormalize_libdef_path t p =
match t.builtin_flowlib_root with
| Some flowlib_root ->
(match Base.String.chop_prefix ~prefix:builtin_flow_lib_marker p with
| Some p -> Files.absolute_path flowlib_root p
| None -> denormalize_path t p)
| None -> denormalize_path t p

let denormalize_file_key_no_cache t = function
| File_key.LibFile p ->
let p =
match t.builtin_flowlib_root with
| Some flowlib_root ->
(match Base.String.chop_prefix ~prefix:builtin_flow_lib_marker p with
| Some p -> Files.absolute_path flowlib_root p
| None -> denormalize_path t p)
| None -> denormalize_path t p
in
File_key.LibFile p
| File_key.LibFile p -> File_key.LibFile (denormalize_libdef_path t p)
| File_key.SourceFile p -> File_key.SourceFile (denormalize_path t p)
| File_key.JsonFile p -> File_key.JsonFile (denormalize_path t p)
| File_key.ResourceFile p -> File_key.ResourceFile (denormalize_path t p)
Expand Down Expand Up @@ -686,7 +675,7 @@ end = struct
parsed_heaps;
unparsed_heaps;
package_heaps;
ordered_non_flowlib_libs;
ordered_libs;
local_errors;
node_modules_containers;
dependency_graph;
Expand All @@ -712,8 +701,8 @@ end = struct
let progress_fn = progress_fn (List.length unparsed_heaps) in
denormalize_unparsed_heaps ~workers ~denormalizer ~progress_fn unparsed_heaps
in
let ordered_non_flowlib_libs =
Base.List.map ~f:(FileDenormalizer.denormalize_path denormalizer) ordered_non_flowlib_libs
let ordered_libs =
Base.List.map ~f:(FileDenormalizer.denormalize_libdef_path denormalizer) ordered_libs
in
let local_errors =
FilenameMap.fold
Expand All @@ -738,7 +727,7 @@ end = struct
parsed_heaps;
unparsed_heaps;
package_heaps;
ordered_non_flowlib_libs;
ordered_libs;
local_errors;
node_modules_containers;
dependency_graph;
Expand Down
2 changes: 1 addition & 1 deletion src/services/saved_state/saved_state.mli
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ type saved_state_data = {
parsed_heaps: (File_key.t * parsed_file_data) list;
unparsed_heaps: (File_key.t * unparsed_file_data) list;
package_heaps: (File_key.t * package_file_data) list;
ordered_non_flowlib_libs: string list;
ordered_libs: string list;
local_errors: Flow_error.ErrorSet.t Utils_js.FilenameMap.t;
node_modules_containers: SSet.t SMap.t;
dependency_graph: saved_state_dependency_graph;
Expand Down
3 changes: 3 additions & 0 deletions tests/saved_state_different_root/.flowconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[options]
all=true
lazy_mode=true
2 changes: 2 additions & 0 deletions tests/saved_state_different_root/.testconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
shell: test.sh
skip_saved_state: true
Empty file.
21 changes: 21 additions & 0 deletions tests/saved_state_different_root/saved_state_different_root.exp
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
Generate saved-state
Use saved state in a different root
Error ----------------------------------------------------------------------------------------------------- test.js:1:19

Cannot assign `new Object()` to `a` because `Object` [1] is incompatible with string [2]. [incompatible-type]

test.js:1:19
1| const a: string = new Object()
^^^^^^^^^^^^ [1]

References:
test.js:1:10
1| const a: string = new Object()
^^^^^^ [2]



Found 1 error

The Flow server is currently in lazy mode and is only checking 2/2 files.
To learn more, visit flow.org/en/docs/lang/lazy-modes
20 changes: 20 additions & 0 deletions tests/saved_state_different_root/test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#!/bin/bash
# Copyright (c) Meta Platforms, Inc. and affiliates.
#
# This source code is licensed under the MIT license found in the
# LICENSE file in the root directory of this source tree.

echo "Generate saved-state"
"$FLOW" stop
"$FLOW" start --saved-state-fetcher none
"$FLOW" save-state --out new_root/.flow.saved_state >> /dev/null
"$FLOW" stop

echo "" > new_root/.flow.saved_state_file_changes
echo "Use saved state in a different root"
mv .flowconfig new_root
cd new_root
"$FLOW" start --saved-state-fetcher local --wait
echo "const a: string = new Object()" > test.js
"$FLOW" force-recheck test.js
assert_errors "$FLOW" status --strip-root

0 comments on commit 7b624f5

Please sign in to comment.