Skip to content

Commit

Permalink
[flow][refactor] Make Files.options opaque
Browse files Browse the repository at this point in the history
Summary:
These files logic is the most complicated parts of Flow that's hard to understand. To make it easier to understand, we want to easily tell how are these configs actually interpreted in the code. However, with the detail of the options fully transparent, it's hard to easily find where the options are inspected.

In this diff, I make options opaque, so that all inspections have to be done through functions, which we can grep much more easier. Mostly importantly, I did find that `declarations` in file options is only used in one place, which can help future refactors.

Changelog: [internal]

Reviewed By: panagosg7

Differential Revision: D55049081

fbshipit-source-id: 6ec1a0ca86803c306aa62a9c4ca5b0669677f315
  • Loading branch information
SamChou19815 authored and facebook-github-bot committed Mar 20, 2024
1 parent 57f47e4 commit e3a6d28
Show file tree
Hide file tree
Showing 8 changed files with 107 additions and 54 deletions.
42 changes: 24 additions & 18 deletions src/commands/commandUtils.ml
Original file line number Diff line number Diff line change
Expand Up @@ -699,24 +699,30 @@ let file_options =
|> Base.List.rev_append (FlowConfig.includes flowconfig)
|> includes_of_arg ~root ~lib_paths
in
{
Files.default_lib_dir;
ignores;
untyped;
declarations;
includes;
lib_paths;
module_file_exts = FlowConfig.module_file_exts flowconfig;
module_resource_exts = FlowConfig.module_resource_exts flowconfig;
multi_platform = FlowConfig.multi_platform flowconfig |> Base.Option.value ~default:false;
multi_platform_extensions = FlowConfig.multi_platform_extensions flowconfig;
multi_platform_ambient_supports_platform_directory_overrides =
FlowConfig.multi_platform_ambient_supports_platform_directory_overrides flowconfig
|> Base.List.map ~f:(fun (path, platforms) ->
(Files.expand_project_root_token ~root path, platforms)
);
node_resolver_dirnames = FlowConfig.node_resolver_dirnames flowconfig;
}
let module_file_exts = FlowConfig.module_file_exts flowconfig in
let module_resource_exts = FlowConfig.module_resource_exts flowconfig in
let multi_platform = FlowConfig.multi_platform flowconfig |> Base.Option.value ~default:false in
let multi_platform_extensions = FlowConfig.multi_platform_extensions flowconfig in
let multi_platform_ambient_supports_platform_directory_overrides =
FlowConfig.multi_platform_ambient_supports_platform_directory_overrides flowconfig
|> Base.List.map ~f:(fun (path, platforms) ->
(Files.expand_project_root_token ~root path, platforms)
)
in
let node_resolver_dirnames = FlowConfig.node_resolver_dirnames flowconfig in
Files.mk_options
~default_lib_dir
~ignores
~untyped
~declarations
~includes
~lib_paths
~module_file_exts
~module_resource_exts
~multi_platform
~multi_platform_extensions
~multi_platform_ambient_supports_platform_directory_overrides
~node_resolver_dirnames

let ignore_flag prev =
CommandSpec.ArgSpec.(
Expand Down
2 changes: 1 addition & 1 deletion src/commands/lsCommand.ml
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ let main
make_options ~flowconfig ~root ~ignore_flag ~include_flag ~untyped_flag ~declaration_flag
in
(* Turn on --no-flowlib by default, so that flow ls never reports flowlib files *)
let options = { options with Files.default_lib_dir = None } in
let options = Files.with_default_lib_dir ~default_lib_dir:None options in
let (_, libs) = Files.init options in
(* `flow ls` and `flow ls dir` will list out all the flow files. We want to include lib files, so
* we pass in ~libs:SSet.empty, which means we won't filter out any lib files *)
Expand Down
39 changes: 37 additions & 2 deletions src/common/files.ml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,34 @@ type options = {
node_resolver_dirnames: string list;
}

let mk_options
~default_lib_dir
~ignores
~untyped
~declarations
~includes
~lib_paths
~module_file_exts
~module_resource_exts
~multi_platform
~multi_platform_extensions
~multi_platform_ambient_supports_platform_directory_overrides
~node_resolver_dirnames =
{
default_lib_dir;
ignores;
untyped;
declarations;
includes;
lib_paths;
module_file_exts;
module_resource_exts;
multi_platform;
multi_platform_extensions;
multi_platform_ambient_supports_platform_directory_overrides;
node_resolver_dirnames;
}

let default_options =
{
default_lib_dir = None;
Expand All @@ -44,12 +72,12 @@ let default_options =

let default_lib_dir options = options.default_lib_dir

let with_default_lib_dir ~default_lib_dir options = { options with default_lib_dir }

let ignores options = options.ignores

let untyped options = options.untyped

let declarations options = options.declarations

let includes options = options.includes

let lib_paths options = options.lib_paths
Expand All @@ -58,6 +86,13 @@ let module_file_exts options = options.module_file_exts

let module_resource_exts options = options.module_resource_exts

let multi_platform options = options.multi_platform

let multi_platform_extensions options = options.multi_platform_extensions

let multi_platform_ambient_supports_platform_directory_overrides options =
options.multi_platform_ambient_supports_platform_directory_overrides

let node_resolver_dirnames options = options.node_resolver_dirnames

(* During node module resolution, we need to look for node_modules/ directories
Expand Down
41 changes: 25 additions & 16 deletions src/common/files.mli
Original file line number Diff line number Diff line change
Expand Up @@ -11,31 +11,33 @@ type lib_dir =
| Prelude of File_path.t
| Flowlib of File_path.t

type options = {
default_lib_dir: lib_dir option;
ignores: (string * Str.regexp) list;
untyped: (string * Str.regexp) list;
declarations: (string * Str.regexp) list;
includes: Path_matcher.t;
lib_paths: File_path.t list;
module_file_exts: string list;
module_resource_exts: SSet.t;
multi_platform: bool;
multi_platform_extensions: string list;
multi_platform_ambient_supports_platform_directory_overrides: (string * string list) list;
node_resolver_dirnames: string list;
}
type options

val mk_options :
default_lib_dir:lib_dir option ->
ignores:(string * Str.regexp) list ->
untyped:(string * Str.regexp) list ->
declarations:(string * Str.regexp) list ->
includes:Path_matcher.t ->
lib_paths:File_path.t list ->
module_file_exts:string list ->
module_resource_exts:SSet.t ->
multi_platform:bool ->
multi_platform_extensions:string list ->
multi_platform_ambient_supports_platform_directory_overrides:(string * string list) list ->
node_resolver_dirnames:string list ->
options

val default_options : options

val default_lib_dir : options -> lib_dir option

val with_default_lib_dir : default_lib_dir:lib_dir option -> options -> options

val ignores : options -> (string * Str.regexp) list

val untyped : options -> (string * Str.regexp) list

val declarations : options -> (string * Str.regexp) list

val includes : options -> Path_matcher.t

val lib_paths : options -> File_path.t list
Expand All @@ -44,6 +46,13 @@ val module_file_exts : options -> string list

val module_resource_exts : options -> SSet.t

val multi_platform : options -> bool

val multi_platform_extensions : options -> string list

val multi_platform_ambient_supports_platform_directory_overrides :
options -> (string * string list) list

val node_resolver_dirnames : options -> string list

val node_modules_containers : SSet.t SMap.t ref
Expand Down
25 changes: 14 additions & 11 deletions src/common/platform_set.ml
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,11 @@ let available_platforms_to_bitset ~multi_platform_extensions available_platforms
bitset

let available_platforms ~file_options ~filename ~explicit_available_platforms : t option =
let open Files in
if not file_options.multi_platform then
if not (Files.multi_platform file_options) then
None
else
let multi_platform_extensions = file_options.multi_platform_extensions in
match platform_specific_extension_and_index_opt ~options:file_options filename with
let multi_platform_extensions = Files.multi_platform_extensions file_options in
match Files.platform_specific_extension_and_index_opt ~options:file_options filename with
| Some (i, _) ->
let bitset = Bitset.all_zero (Base.List.length multi_platform_extensions) in
Bitset.set bitset i;
Expand All @@ -34,7 +33,7 @@ let available_platforms ~file_options ~filename ~explicit_available_platforms :
in
(match
Base.List.find_map
file_options.multi_platform_ambient_supports_platform_directory_overrides
(Files.multi_platform_ambient_supports_platform_directory_overrides file_options)
~f:match_directory_override
with
| None -> Some (Bitset.all_one (Base.List.length multi_platform_extensions))
Expand All @@ -52,7 +51,10 @@ let is_subset = Bitset.is_subset
let no_overlap = Bitset.no_overlap

let to_platform_string_set ~file_options bitset =
Base.List.foldi file_options.Files.multi_platform_extensions ~init:SSet.empty ~f:(fun i acc ext ->
Base.List.foldi
(Files.multi_platform_extensions file_options)
~init:SSet.empty
~f:(fun i acc ext ->
if Bitset.mem i bitset then
SSet.add (Base.String.chop_prefix_exn ~prefix:"." ext) acc
else
Expand All @@ -61,17 +63,18 @@ let to_platform_string_set ~file_options bitset =

let platform_specific_implementation_mrefs_of_possibly_interface_file
~file_options ~platform_set ~file =
let open Files in
if file_options.multi_platform && has_flow_ext file then
let file = chop_flow_ext file in
if Files.multi_platform file_options && Files.has_flow_ext file then
let file = Files.chop_flow_ext file in
let platform_set = Base.Option.value_exn platform_set in
Base.List.find_map file_options.module_file_exts ~f:(fun module_file_ext ->
Base.List.find_map (Files.module_file_exts file_options) ~f:(fun module_file_ext ->
if File_key.check_suffix file module_file_ext then
let base =
File_key.chop_suffix file module_file_ext |> File_key.to_string |> Filename.basename
in
let implementation_mrefs =
Base.List.filter_mapi file_options.multi_platform_extensions ~f:(fun i platform_ext ->
Base.List.filter_mapi
(Files.multi_platform_extensions file_options)
~f:(fun i platform_ext ->
if Bitset.mem i platform_set then
Some ("./" ^ base ^ platform_ext)
else
Expand Down
4 changes: 2 additions & 2 deletions src/parsing/docblock_parser.ml
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ let extract_docblock =
| (loc, "@jsxRuntime") :: _ :: xs ->
let acc = ((loc, InvalidJSXRuntimeAttribute) :: errors, info) in
parse_attributes ~file_options ~filename acc xs
| (loc, "@supportsPlatform") :: (_, platform) :: xs when file_options.Files.multi_platform ->
| (loc, "@supportsPlatform") :: (_, platform) :: xs when Files.multi_platform file_options ->
let acc =
if
filename
Expand All @@ -134,7 +134,7 @@ let extract_docblock =
else if
Base.List.mem
~equal:String.equal
file_options.Files.multi_platform_extensions
(Files.multi_platform_extensions file_options)
("." ^ platform)
then
let existing_platforms = Base.Option.value info.supportsPlatform ~default:[] in
Expand Down
6 changes: 3 additions & 3 deletions src/services/module/module_js.ml
Original file line number Diff line number Diff line change
Expand Up @@ -524,10 +524,10 @@ module Haste : MODULE_SYSTEM = struct
~options ~reader ?phantom_acc ~dir ~source r =
let dependency = resolve_haste_module ~options ~reader ?phantom_acc ~source ~dir r in
let file_options = Options.file_options options in
if file_options.Files.multi_platform then
if Files.multi_platform file_options then
match Option.map Parsing_heaps.read_dependency dependency with
| Some (Modulename.String mname as module_name)
when Base.List.exists file_options.Files.multi_platform_extensions ~f:(fun ext ->
when Base.List.exists (Files.multi_platform_extensions file_options) ~f:(fun ext ->
String.ends_with ~suffix:ext mname
) ->
(* If we don't allow an import to resolve a platform specific import, but we did find one,
Expand All @@ -547,7 +547,7 @@ module Haste : MODULE_SYSTEM = struct
Node.resolve_relative ~options ~reader ?phantom_acc ~source:f dir r
else
let file_options = Options.file_options options in
if not file_options.Files.multi_platform then
if not (Files.multi_platform file_options) then
lazy_seq
[
lazy (resolve_haste_module ~options ~reader ?phantom_acc ~source:f ~dir r);
Expand Down
2 changes: 1 addition & 1 deletion src/typing/type_operation_utils.ml
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ module Import_export = struct
Flow_js_utils.lookup_builtin_error cx m_name reason
|> Flow_js_utils.apply_env_errors cx loc
in
( if perform_platform_validation && Context.((metadata cx).file_options.Files.multi_platform)
( if perform_platform_validation && Files.multi_platform Context.((metadata cx).file_options)
then
match Flow_js.possible_concrete_types_for_inspection cx reason module_t with
| [ModuleT m] -> check_platform_availability cx reason m.module_available_platforms
Expand Down

0 comments on commit e3a6d28

Please sign in to comment.