From e3a6d287ee11b0c3248e972bfed2b471e600755d Mon Sep 17 00:00:00 2001 From: Sam Zhou Date: Wed, 20 Mar 2024 09:53:37 -0700 Subject: [PATCH] [flow][refactor] Make `Files.options` opaque 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 --- src/commands/commandUtils.ml | 42 +++++++++++++++++------------- src/commands/lsCommand.ml | 2 +- src/common/files.ml | 39 +++++++++++++++++++++++++-- src/common/files.mli | 41 +++++++++++++++++------------ src/common/platform_set.ml | 25 ++++++++++-------- src/parsing/docblock_parser.ml | 4 +-- src/services/module/module_js.ml | 6 ++--- src/typing/type_operation_utils.ml | 2 +- 8 files changed, 107 insertions(+), 54 deletions(-) diff --git a/src/commands/commandUtils.ml b/src/commands/commandUtils.ml index 295a4070b3f..56a538c7254 100644 --- a/src/commands/commandUtils.ml +++ b/src/commands/commandUtils.ml @@ -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.( diff --git a/src/commands/lsCommand.ml b/src/commands/lsCommand.ml index 66e9a3c09f7..10f3d18f467 100644 --- a/src/commands/lsCommand.ml +++ b/src/commands/lsCommand.ml @@ -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 *) diff --git a/src/common/files.ml b/src/common/files.ml index 6fb4b1b6cd6..81fa4a474fc 100644 --- a/src/common/files.ml +++ b/src/common/files.ml @@ -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; @@ -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 @@ -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 diff --git a/src/common/files.mli b/src/common/files.mli index 594c476debb..0e7fca6cf12 100644 --- a/src/common/files.mli +++ b/src/common/files.mli @@ -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 @@ -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 diff --git a/src/common/platform_set.ml b/src/common/platform_set.ml index 4752d405f34..54f42d5da46 100644 --- a/src/common/platform_set.ml +++ b/src/common/platform_set.ml @@ -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; @@ -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)) @@ -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 @@ -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 diff --git a/src/parsing/docblock_parser.ml b/src/parsing/docblock_parser.ml index 0df95e059b6..b96aac43367 100644 --- a/src/parsing/docblock_parser.ml +++ b/src/parsing/docblock_parser.ml @@ -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 @@ -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 diff --git a/src/services/module/module_js.ml b/src/services/module/module_js.ml index 3712b07033b..594097c5af5 100644 --- a/src/services/module/module_js.ml +++ b/src/services/module/module_js.ml @@ -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, @@ -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); diff --git a/src/typing/type_operation_utils.ml b/src/typing/type_operation_utils.ml index 0c4c47dd847..ad548d0928e 100644 --- a/src/typing/type_operation_utils.ml +++ b/src/typing/type_operation_utils.ml @@ -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