From 7d44315ee6158560082419f55c38aee9e9d5ff8a Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 7 Jun 2023 10:09:10 -0400 Subject: [PATCH] refactor: helpers methods on `TypeCheckMode` (#19393) --- cli/args/flags.rs | 20 ++++++++++++++++++++ cli/factory.rs | 11 +++-------- cli/graph_util.rs | 16 ++++------------ cli/module_loader.rs | 3 +-- cli/tools/bench.rs | 8 +------- cli/tools/bundle.rs | 3 +-- cli/tools/compile.rs | 7 +++---- cli/tools/test.rs | 8 +------- 8 files changed, 34 insertions(+), 42 deletions(-) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index c4d8a3f87e..2e4d826f92 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -9,6 +9,7 @@ use clap::Command; use clap::ValueHint; use deno_core::resolve_url_or_path; use deno_core::url::Url; +use deno_graph::GraphKind; use deno_runtime::permissions::parse_sys_kind; use log::debug; use log::Level; @@ -255,6 +256,25 @@ pub enum TypeCheckMode { Local, } +impl TypeCheckMode { + /// Gets if type checking will occur under this mode. + pub fn is_true(&self) -> bool { + match self { + Self::None => false, + Self::Local | Self::All => true, + } + } + + /// Gets the corresponding module `GraphKind` that should be created + /// for the current `TypeCheckMode`. + pub fn as_graph_kind(&self) -> GraphKind { + match self.is_true() { + true => GraphKind::All, + false => GraphKind::CodeOnly, + } + } +} + impl Default for TypeCheckMode { fn default() -> Self { Self::None diff --git a/cli/factory.rs b/cli/factory.rs index 78aefe7804..c4331652e1 100644 --- a/cli/factory.rs +++ b/cli/factory.rs @@ -8,7 +8,6 @@ use crate::args::Lockfile; use crate::args::PackageJsonDepsProvider; use crate::args::StorageKeyResolver; use crate::args::TsConfigType; -use crate::args::TypeCheckMode; use crate::cache::Caches; use crate::cache::DenoDir; use crate::cache::DenoDirProvider; @@ -541,14 +540,10 @@ impl CliFactory { pub fn graph_container(&self) -> &Arc { self.services.graph_container.get_or_init(|| { let graph_kind = match self.options.sub_command() { + // todo(dsherret): ideally the graph container would not be used + // for deno cache because it doesn't dynamically load modules DenoSubcommand::Cache(_) => GraphKind::All, - _ => { - if self.options.type_check_mode() == TypeCheckMode::None { - GraphKind::CodeOnly - } else { - GraphKind::All - } - } + _ => self.options.type_check_mode().as_graph_kind(), }; Arc::new(ModuleGraphContainer::new(graph_kind)) }) diff --git a/cli/graph_util.rs b/cli/graph_util.rs index 55052b9d0b..530b0a9745 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -3,7 +3,6 @@ use crate::args::CliOptions; use crate::args::Lockfile; use crate::args::TsTypeLib; -use crate::args::TypeCheckMode; use crate::cache; use crate::cache::ParsedSourceCache; use crate::colors; @@ -57,7 +56,7 @@ pub fn graph_valid_with_cli_options( roots, GraphValidOptions { is_vendoring: false, - follow_type_only: options.type_check_mode() != TypeCheckMode::None, + follow_type_only: options.type_check_mode().is_true(), check_js: options.check_js(), }, ) @@ -229,9 +228,7 @@ impl ModuleGraphBuilder { ) .await?; - if graph.has_node_specifier - && self.options.type_check_mode() != TypeCheckMode::None - { + if graph.has_node_specifier && self.options.type_check_mode().is_true() { self .npm_resolver .inject_synthetic_types_node_package() @@ -251,12 +248,7 @@ impl ModuleGraphBuilder { let graph_resolver = cli_resolver.as_graph_resolver(); let graph_npm_resolver = cli_resolver.as_graph_npm_resolver(); let analyzer = self.parsed_source_cache.as_analyzer(); - let should_type_check = - self.options.type_check_mode() != TypeCheckMode::None; - let graph_kind = match should_type_check { - true => GraphKind::All, - false => GraphKind::CodeOnly, - }; + let graph_kind = self.options.type_check_mode().as_graph_kind(); let mut graph = ModuleGraph::new(graph_kind); self .build_graph_with_npm_resolution( @@ -280,7 +272,7 @@ impl ModuleGraphBuilder { graph_lock_or_exit(&graph, &mut lockfile.lock()); } - if should_type_check { + if self.options.type_check_mode().is_true() { self .type_checker .check( diff --git a/cli/module_loader.rs b/cli/module_loader.rs index 3352cb951f..804c9a162b 100644 --- a/cli/module_loader.rs +++ b/cli/module_loader.rs @@ -3,7 +3,6 @@ use crate::args::CliOptions; use crate::args::DenoSubcommand; use crate::args::TsTypeLib; -use crate::args::TypeCheckMode; use crate::cache::ParsedSourceCache; use crate::emit::Emitter; use crate::graph_util::graph_lock_or_exit; @@ -169,7 +168,7 @@ impl ModuleLoadPreparer { drop(_pb_clear_guard); // type check if necessary - if self.options.type_check_mode() != TypeCheckMode::None + if self.options.type_check_mode().is_true() && !self.graph_container.is_type_checked(&roots, lib) { let graph = Arc::new(graph.segment(&roots)); diff --git a/cli/tools/bench.rs b/cli/tools/bench.rs index 6461e544f3..a7b75d8be8 100644 --- a/cli/tools/bench.rs +++ b/cli/tools/bench.rs @@ -2,7 +2,6 @@ use crate::args::BenchOptions; use crate::args::CliOptions; -use crate::args::TypeCheckMode; use crate::colors; use crate::display::write_json_to_stdout; use crate::factory::CliFactory; @@ -31,7 +30,6 @@ use deno_core::task::spawn; use deno_core::task::spawn_blocking; use deno_core::v8; use deno_core::ModuleSpecifier; -use deno_graph::GraphKind; use deno_runtime::permissions::Permissions; use deno_runtime::permissions::PermissionsContainer; use deno_runtime::tokio_util::create_and_run_current_thread; @@ -694,11 +692,7 @@ pub async fn run_benchmarks_with_watch( // file would have impact on other files, which is undesirable. let permissions = Permissions::from_options(&cli_options.permissions_options())?; - let type_check = cli_options.type_check_mode() != TypeCheckMode::None; - let graph_kind = match type_check { - true => GraphKind::All, - false => GraphKind::CodeOnly, - }; + let graph_kind = cli_options.type_check_mode().as_graph_kind(); let resolver = |changed: Option>| { let paths_to_watch = bench_options.files.include.clone(); diff --git a/cli/tools/bundle.rs b/cli/tools/bundle.rs index 759882c833..f38948776d 100644 --- a/cli/tools/bundle.rs +++ b/cli/tools/bundle.rs @@ -12,7 +12,6 @@ use crate::args::BundleFlags; use crate::args::CliOptions; use crate::args::Flags; use crate::args::TsConfigType; -use crate::args::TypeCheckMode; use crate::factory::CliFactory; use crate::graph_util::error_for_any_npm_specifier; use crate::util; @@ -157,7 +156,7 @@ fn bundle_module_graph( let ts_config_result = cli_options.resolve_ts_config_for_emit(TsConfigType::Bundle)?; - if cli_options.type_check_mode() == TypeCheckMode::None { + if !cli_options.type_check_mode().is_true() { if let Some(ignored_options) = ts_config_result.maybe_ignored_options { log::warn!("{}", ignored_options); } diff --git a/cli/tools/compile.rs b/cli/tools/compile.rs index 540c23fc86..c53ae4e028 100644 --- a/cli/tools/compile.rs +++ b/cli/tools/compile.rs @@ -2,7 +2,6 @@ use crate::args::CompileFlags; use crate::args::Flags; -use crate::args::TypeCheckMode; use crate::factory::CliFactory; use crate::standalone::is_standalone_binary; use crate::util::path::path_has_trailing_slash; @@ -50,15 +49,15 @@ pub async fn compile( .await?, ) .unwrap(); - let graph = if cli_options.type_check_mode() == TypeCheckMode::None { - graph - } else { + let graph = if cli_options.type_check_mode().is_true() { // In this case, the previous graph creation did type checking, which will // create a module graph with types information in it. We don't want to // store that in the eszip so create a code only module graph from scratch. module_graph_builder .create_graph(GraphKind::CodeOnly, module_roots) .await? + } else { + graph }; let parser = parsed_source_cache.as_capturing_parser(); diff --git a/cli/tools/test.rs b/cli/tools/test.rs index bc8f685999..ebe4deb9ae 100644 --- a/cli/tools/test.rs +++ b/cli/tools/test.rs @@ -3,7 +3,6 @@ use crate::args::CliOptions; use crate::args::FilesConfig; use crate::args::TestOptions; -use crate::args::TypeCheckMode; use crate::colors; use crate::display; use crate::factory::CliFactory; @@ -39,7 +38,6 @@ use deno_core::task::spawn_blocking; use deno_core::url::Url; use deno_core::v8; use deno_core::ModuleSpecifier; -use deno_graph::GraphKind; use deno_runtime::deno_io::Stdio; use deno_runtime::deno_io::StdioPipe; use deno_runtime::fmt_errors::format_js_error; @@ -1707,11 +1705,7 @@ pub async fn run_tests_with_watch( // file would have impact on other files, which is undesirable. let permissions = Permissions::from_options(&cli_options.permissions_options())?; - let type_check = cli_options.type_check_mode() != TypeCheckMode::None; - let graph_kind = match type_check { - true => GraphKind::All, - false => GraphKind::CodeOnly, - }; + let graph_kind = cli_options.type_check_mode().as_graph_kind(); let log_level = cli_options.log_level(); let resolver = |changed: Option>| {