From 0a67a3965fd7c0041b818a5cb1068dee19ae25b1 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 14 Apr 2023 18:05:46 -0400 Subject: [PATCH] refactor: add `TypeChecker` struct (#18709) Adds a `TypeChecker` struct and pushes more shared functionality into it. --- cli/graph_util.rs | 62 +++-------- cli/module_loader.rs | 78 +++++--------- cli/proc_state.rs | 14 ++- cli/tools/check.rs | 237 +++++++++++++++++++++++++------------------ 4 files changed, 185 insertions(+), 206 deletions(-) diff --git a/cli/graph_util.rs b/cli/graph_util.rs index 60b1b80d03..dacd3be174 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -2,19 +2,17 @@ use crate::args::CliOptions; use crate::args::Lockfile; -use crate::args::TsConfigType; use crate::args::TsTypeLib; use crate::args::TypeCheckMode; use crate::cache; -use crate::cache::DenoDir; use crate::cache::ParsedSourceCache; -use crate::cache::TypeCheckCache; use crate::colors; use crate::errors::get_error_class_name; use crate::file_fetcher::FileFetcher; use crate::npm::NpmPackageResolver; use crate::resolver::CliGraphResolver; use crate::tools::check; +use crate::tools::check::TypeChecker; use deno_core::anyhow::bail; use deno_core::error::custom_error; @@ -170,10 +168,9 @@ pub struct ModuleGraphBuilder { npm_resolver: Arc, parsed_source_cache: Arc, lockfile: Option>>, - caches: Arc, emit_cache: cache::EmitCache, file_fetcher: Arc, - deno_dir: DenoDir, + type_checker: Arc, } impl ModuleGraphBuilder { @@ -184,10 +181,9 @@ impl ModuleGraphBuilder { npm_resolver: Arc, parsed_source_cache: Arc, lockfile: Option>>, - caches: Arc, emit_cache: cache::EmitCache, file_fetcher: Arc, - deno_dir: DenoDir, + type_checker: Arc, ) -> Self { Self { options, @@ -195,10 +191,9 @@ impl ModuleGraphBuilder { npm_resolver, parsed_source_cache, lockfile, - caches, emit_cache, file_fetcher, - deno_dir, + type_checker, } } @@ -270,51 +265,24 @@ impl ModuleGraphBuilder { ) .await?; - graph_valid_with_cli_options(&graph, &graph.roots, &self.options)?; let graph = Arc::new(graph); + graph_valid_with_cli_options(&graph, &graph.roots, &self.options)?; if let Some(lockfile) = &self.lockfile { graph_lock_or_exit(&graph, &mut lockfile.lock()); } if self.options.type_check_mode() != TypeCheckMode::None { - // node built-in specifiers use the @types/node package to determine - // types, so inject that now after the lockfile has been written - if graph.has_node_specifier { - self - .npm_resolver - .inject_synthetic_types_node_package() - .await?; - } - - let ts_config_result = - self - .options - .resolve_ts_config_for_emit(TsConfigType::Check { + self + .type_checker + .check( + graph.clone(), + check::CheckOptions { lib: self.options.ts_type_lib_window(), - })?; - if let Some(ignored_options) = ts_config_result.maybe_ignored_options { - log::warn!("{}", ignored_options); - } - let maybe_config_specifier = self.options.maybe_config_file_specifier(); - let cache = - TypeCheckCache::new(self.caches.type_checking_cache_db(&self.deno_dir)); - let check_result = check::check( - graph.clone(), - &cache, - self.npm_resolver.clone(), - check::CheckOptions { - type_check_mode: self.options.type_check_mode(), - debug: self.options.log_level() == Some(log::Level::Debug), - maybe_config_specifier, - ts_config: ts_config_result.ts_config, - log_checks: true, - reload: self.options.reload_flag(), - }, - )?; - log::debug!("{}", check_result.stats); - if !check_result.diagnostics.is_empty() { - return Err(check_result.diagnostics.into()); - } + log_ignored_options: true, + reload: self.options.reload_flag(), + }, + ) + .await?; } Ok(graph) diff --git a/cli/module_loader.rs b/cli/module_loader.rs index c2dfaad8f1..e681821cda 100644 --- a/cli/module_loader.rs +++ b/cli/module_loader.rs @@ -2,13 +2,9 @@ use crate::args::CliOptions; use crate::args::DenoSubcommand; -use crate::args::TsConfigType; use crate::args::TsTypeLib; use crate::args::TypeCheckMode; -use crate::cache::Caches; -use crate::cache::DenoDir; use crate::cache::ParsedSourceCache; -use crate::cache::TypeCheckCache; use crate::emit::Emitter; use crate::graph_util::graph_lock_or_exit; use crate::graph_util::graph_valid_with_cli_options; @@ -24,6 +20,7 @@ use crate::proc_state::FileWatcherReporter; use crate::proc_state::ProcState; use crate::resolver::CliGraphResolver; use crate::tools::check; +use crate::tools::check::TypeChecker; use crate::util::progress_bar::ProgressBar; use crate::util::text_encoding::code_without_source_map; use crate::util::text_encoding::source_map_from_code; @@ -66,45 +63,39 @@ use std::sync::Arc; pub struct ModuleLoadPreparer { options: Arc, - caches: Arc, - deno_dir: DenoDir, graph_container: Arc, lockfile: Option>>, maybe_file_watcher_reporter: Option, module_graph_builder: Arc, - npm_resolver: Arc, parsed_source_cache: Arc, progress_bar: ProgressBar, resolver: Arc, + type_checker: Arc, } impl ModuleLoadPreparer { #[allow(clippy::too_many_arguments)] pub fn new( options: Arc, - caches: Arc, - deno_dir: DenoDir, graph_container: Arc, lockfile: Option>>, maybe_file_watcher_reporter: Option, module_graph_builder: Arc, - npm_resolver: Arc, parsed_source_cache: Arc, progress_bar: ProgressBar, resolver: Arc, + type_checker: Arc, ) -> Self { Self { options, - caches, - deno_dir, graph_container, lockfile, maybe_file_watcher_reporter, module_graph_builder, - npm_resolver, parsed_source_cache, progress_bar, resolver, + type_checker, } } @@ -166,61 +157,40 @@ impl ModuleLoadPreparer { ) .await?; - // If there is a lockfile, validate the integrity of all the modules. + graph_valid_with_cli_options(graph, &roots, &self.options)?; + + // If there is a lockfile... if let Some(lockfile) = &self.lockfile { - graph_lock_or_exit(graph, &mut lockfile.lock()); + let mut lockfile = lockfile.lock(); + // validate the integrity of all the modules + graph_lock_or_exit(graph, &mut lockfile); + // update it with anything new + lockfile.write()?; } - graph_valid_with_cli_options(graph, &roots, &self.options)?; // save the graph and get a reference to the new graph let graph = graph_update_permit.commit(); - if graph.has_node_specifier - && self.options.type_check_mode() != TypeCheckMode::None - { - self - .npm_resolver - .inject_synthetic_types_node_package() - .await?; - } - drop(_pb_clear_guard); // type check if necessary if self.options.type_check_mode() != TypeCheckMode::None && !self.graph_container.is_type_checked(&roots, lib) { - // todo(dsherret): consolidate this with what's done in graph_util - log::debug!("Type checking."); - let maybe_config_specifier = self.options.maybe_config_file_specifier(); let graph = Arc::new(graph.segment(&roots)); - let options = check::CheckOptions { - type_check_mode: self.options.type_check_mode(), - debug: self.options.log_level() == Some(log::Level::Debug), - maybe_config_specifier, - ts_config: self - .options - .resolve_ts_config_for_emit(TsConfigType::Check { lib })? - .ts_config, - log_checks: true, - reload: self.options.reload_flag() - && !roots.iter().all(|r| reload_exclusions.contains(r)), - }; - let check_cache = - TypeCheckCache::new(self.caches.type_checking_cache_db(&self.deno_dir)); - let check_result = - check::check(graph, &check_cache, self.npm_resolver.clone(), options)?; + self + .type_checker + .check( + graph, + check::CheckOptions { + lib, + log_ignored_options: false, + reload: self.options.reload_flag() + && !roots.iter().all(|r| reload_exclusions.contains(r)), + }, + ) + .await?; self.graph_container.set_type_checked(&roots, lib); - if !check_result.diagnostics.is_empty() { - return Err(anyhow!(check_result.diagnostics)); - } - log::debug!("{}", check_result.stats); - } - - // any updates to the lockfile should be updated now - if let Some(ref lockfile) = self.lockfile { - let g = lockfile.lock(); - g.write()?; } log::debug!("Prepared module load."); diff --git a/cli/proc_state.rs b/cli/proc_state.rs index b3784362ad..728363bc2e 100644 --- a/cli/proc_state.rs +++ b/cli/proc_state.rs @@ -25,6 +25,7 @@ use crate::npm::NpmPackageResolver; use crate::npm::NpmResolution; use crate::npm::PackageJsonDepsInstaller; use crate::resolver::CliGraphResolver; +use crate::tools::check::TypeChecker; use crate::util::progress_bar::ProgressBar; use crate::util::progress_bar::ProgressBarStyle; @@ -305,30 +306,33 @@ impl ProcState { file_fetcher.clone(), npm_resolver.clone(), )); + let type_checker = Arc::new(TypeChecker::new( + dir.clone(), + caches.clone(), + cli_options.clone(), + npm_resolver.clone(), + )); let module_graph_builder = Arc::new(ModuleGraphBuilder::new( cli_options.clone(), resolver.clone(), npm_resolver.clone(), parsed_source_cache.clone(), lockfile.clone(), - caches.clone(), emit_cache.clone(), file_fetcher.clone(), - dir.clone(), + type_checker.clone(), )); let graph_container: Arc = Default::default(); let module_load_preparer = Arc::new(ModuleLoadPreparer::new( cli_options.clone(), - caches.clone(), - dir.clone(), graph_container.clone(), lockfile.clone(), maybe_file_watcher_reporter.clone(), module_graph_builder.clone(), - npm_resolver.clone(), parsed_source_cache.clone(), progress_bar.clone(), resolver.clone(), + type_checker, )); Ok(ProcState(Arc::new(Inner { diff --git a/cli/tools/check.rs b/cli/tools/check.rs index dcc9b28430..eee82adcfb 100644 --- a/cli/tools/check.rs +++ b/cli/tools/check.rs @@ -12,136 +12,172 @@ use deno_runtime::colors; use once_cell::sync::Lazy; use regex::Regex; +use crate::args::CliOptions; use crate::args::TsConfig; +use crate::args::TsConfigType; +use crate::args::TsTypeLib; use crate::args::TypeCheckMode; +use crate::cache::Caches; +use crate::cache::DenoDir; use crate::cache::FastInsecureHasher; use crate::cache::TypeCheckCache; use crate::npm::NpmPackageResolver; use crate::tsc; -use crate::tsc::Diagnostics; -use crate::tsc::Stats; use crate::version; /// Options for performing a check of a module graph. Note that the decision to /// emit or not is determined by the `ts_config` settings. pub struct CheckOptions { - /// The check flag from the option which can effect the filtering of - /// diagnostics in the emit result. - pub type_check_mode: TypeCheckMode, - /// Set the debug flag on the TypeScript type checker. - pub debug: bool, - /// The module specifier to the configuration file, passed to tsc so that - /// configuration related diagnostics are properly formed. - pub maybe_config_specifier: Option, - /// The derived tsconfig that should be used when checking. - pub ts_config: TsConfig, - /// If true, `Check ` will be written to stdout for each root. - pub log_checks: bool, + /// Default type library to type check with. + pub lib: TsTypeLib, + /// Whether to log about any ignored compiler options. + pub log_ignored_options: bool, /// If true, valid `.tsbuildinfo` files will be ignored and type checking /// will always occur. pub reload: bool, } -/// The result of a check of a module graph. -#[derive(Debug, Default)] -pub struct CheckResult { - pub diagnostics: Diagnostics, - pub stats: Stats, +pub struct TypeChecker { + deno_dir: DenoDir, + caches: Arc, + cli_options: Arc, + npm_resolver: Arc, } -/// Given a set of roots and graph data, type check the module graph. -/// -/// It is expected that it is determined if a check and/or emit is validated -/// before the function is called. -pub fn check( - graph: Arc, - cache: &TypeCheckCache, - npm_resolver: Arc, - options: CheckOptions, -) -> Result { - let check_js = options.ts_config.get_check_js(); - let check_hash = match get_check_hash(&graph, &options) { - CheckHashResult::NoFiles => return Ok(Default::default()), - CheckHashResult::Hash(hash) => hash, - }; - - // do not type check if we know this is type checked - if !options.reload && cache.has_check_hash(check_hash) { - return Ok(Default::default()); +impl TypeChecker { + pub fn new( + deno_dir: DenoDir, + caches: Arc, + cli_options: Arc, + npm_resolver: Arc, + ) -> Self { + Self { + deno_dir, + caches, + cli_options, + npm_resolver, + } } - if options.log_checks { + /// Type check the module graph. + /// + /// It is expected that it is determined if a check and/or emit is validated + /// before the function is called. + pub async fn check( + &self, + graph: Arc, + options: CheckOptions, + ) -> Result<(), AnyError> { + // node built-in specifiers use the @types/node package to determine + // types, so inject that now (the caller should do this after the lockfile + // has been written) + if graph.has_node_specifier { + self + .npm_resolver + .inject_synthetic_types_node_package() + .await?; + } + + log::debug!("Type checking."); + let ts_config_result = self + .cli_options + .resolve_ts_config_for_emit(TsConfigType::Check { lib: options.lib })?; + if options.log_ignored_options { + if let Some(ignored_options) = ts_config_result.maybe_ignored_options { + log::warn!("{}", ignored_options); + } + } + + let ts_config = ts_config_result.ts_config; + let type_check_mode = self.cli_options.type_check_mode(); + let debug = self.cli_options.log_level() == Some(log::Level::Debug); + let cache = + TypeCheckCache::new(self.caches.type_checking_cache_db(&self.deno_dir)); + let check_js = ts_config.get_check_js(); + let check_hash = match get_check_hash(&graph, type_check_mode, &ts_config) { + CheckHashResult::NoFiles => return Ok(()), + CheckHashResult::Hash(hash) => hash, + }; + + // do not type check if we know this is type checked + if !options.reload && cache.has_check_hash(check_hash) { + return Ok(()); + } + for root in &graph.roots { let root_str = root.as_str(); log::info!("{} {}", colors::green("Check"), root_str); } - } - let root_names = get_tsc_roots(&graph, check_js); - // while there might be multiple roots, we can't "merge" the build info, so we - // try to retrieve the build info for first root, which is the most common use - // case. - let maybe_tsbuildinfo = if options.reload { - None - } else { - cache.get_tsbuildinfo(&graph.roots[0]) - }; - // to make tsc build info work, we need to consistently hash modules, so that - // tsc can better determine if an emit is still valid or not, so we provide - // that data here. - let hash_data = { - let mut hasher = FastInsecureHasher::new(); - hasher.write(&options.ts_config.as_bytes()); - hasher.write_str(version::deno()); - hasher.finish() - }; + let root_names = get_tsc_roots(&graph, check_js); + // while there might be multiple roots, we can't "merge" the build info, so we + // try to retrieve the build info for first root, which is the most common use + // case. + let maybe_tsbuildinfo = if options.reload { + None + } else { + cache.get_tsbuildinfo(&graph.roots[0]) + }; + // to make tsc build info work, we need to consistently hash modules, so that + // tsc can better determine if an emit is still valid or not, so we provide + // that data here. + let hash_data = { + let mut hasher = FastInsecureHasher::new(); + hasher.write(&ts_config.as_bytes()); + hasher.write_str(version::deno()); + hasher.finish() + }; - let response = tsc::exec(tsc::Request { - config: options.ts_config, - debug: options.debug, - graph: graph.clone(), - hash_data, - maybe_npm_resolver: Some(npm_resolver.clone()), - maybe_tsbuildinfo, - root_names, - check_mode: options.type_check_mode, - })?; + let response = tsc::exec(tsc::Request { + config: ts_config, + debug, + graph: graph.clone(), + hash_data, + maybe_npm_resolver: Some(self.npm_resolver.clone()), + maybe_tsbuildinfo, + root_names, + check_mode: type_check_mode, + })?; - let diagnostics = if options.type_check_mode == TypeCheckMode::Local { - response.diagnostics.filter(|d| { - if let Some(file_name) = &d.file_name { - if !file_name.starts_with("http") { - if ModuleSpecifier::parse(file_name) - .map(|specifier| !npm_resolver.in_npm_package(&specifier)) - .unwrap_or(true) - { - Some(d.clone()) + let diagnostics = if type_check_mode == TypeCheckMode::Local { + response.diagnostics.filter(|d| { + if let Some(file_name) = &d.file_name { + if !file_name.starts_with("http") { + if ModuleSpecifier::parse(file_name) + .map(|specifier| !self.npm_resolver.in_npm_package(&specifier)) + .unwrap_or(true) + { + Some(d.clone()) + } else { + None + } } else { None } } else { - None + Some(d.clone()) } - } else { - Some(d.clone()) - } - }) - } else { - response.diagnostics - }; + }) + } else { + response.diagnostics + }; - if let Some(tsbuildinfo) = response.maybe_tsbuildinfo { - cache.set_tsbuildinfo(&graph.roots[0], &tsbuildinfo); + if let Some(tsbuildinfo) = response.maybe_tsbuildinfo { + cache.set_tsbuildinfo(&graph.roots[0], &tsbuildinfo); + } + + if diagnostics.is_empty() { + cache.add_check_hash(check_hash); + } + + log::debug!("{}", response.stats); + + if diagnostics.is_empty() { + Ok(()) + } else { + Err(diagnostics.into()) + } } - - if diagnostics.is_empty() { - cache.add_check_hash(check_hash); - } - - Ok(CheckResult { - diagnostics, - stats: response.stats, - }) } enum CheckHashResult { @@ -153,17 +189,18 @@ enum CheckHashResult { /// be used to tell fn get_check_hash( graph: &ModuleGraph, - options: &CheckOptions, + type_check_mode: TypeCheckMode, + ts_config: &TsConfig, ) -> CheckHashResult { let mut hasher = FastInsecureHasher::new(); - hasher.write_u8(match options.type_check_mode { + hasher.write_u8(match type_check_mode { TypeCheckMode::All => 0, TypeCheckMode::Local => 1, TypeCheckMode::None => 2, }); - hasher.write(&options.ts_config.as_bytes()); + hasher.write(&ts_config.as_bytes()); - let check_js = options.ts_config.get_check_js(); + let check_js = ts_config.get_check_js(); let mut sorted_modules = graph.modules().collect::>(); sorted_modules.sort_by_key(|m| m.specifier().as_str()); // make it deterministic let mut has_file = false;