From 9ffc7edc23444be8bdcc1befd3709b309780e3d1 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Thu, 16 Dec 2021 10:45:41 +0000 Subject: [PATCH] refactor(cli): use GraphData for check and emit (#12960) --- cli/emit.rs | 176 +++---- cli/graph_util.rs | 412 +++++++++++++++++ cli/lsp/cache.rs | 3 +- cli/main.rs | 21 +- cli/ops/runtime_compiler.rs | 64 ++- cli/proc_state.rs | 428 ++++++------------ cli/tests/integration/mod.rs | 15 +- cli/tests/integration/run_tests.rs | 17 + cli/tests/integration/watcher_tests.rs | 40 +- cli/tests/testdata/compiler_api_test.ts | 2 +- .../error_027_bundle_with_bare_import.ts.out | 1 + .../testdata/error_type_definitions.ts.out | 1 + cli/tests/testdata/jsx/deno-jsx-error.jsonc | 6 + .../testdata/jsx_import_source_error.out | 2 + cli/tests/testdata/reference_types_error.js | 2 + .../testdata/reference_types_error.js.out | 2 + cli/tools/test.rs | 32 +- cli/tsc.rs | 178 ++++---- 18 files changed, 885 insertions(+), 517 deletions(-) create mode 100644 cli/graph_util.rs create mode 100644 cli/tests/testdata/jsx/deno-jsx-error.jsonc create mode 100644 cli/tests/testdata/jsx_import_source_error.out create mode 100644 cli/tests/testdata/reference_types_error.js create mode 100644 cli/tests/testdata/reference_types_error.js.out diff --git a/cli/emit.rs b/cli/emit.rs index 3aa2c4794d..b7d2b7d84b 100644 --- a/cli/emit.rs +++ b/cli/emit.rs @@ -13,6 +13,8 @@ use crate::config_file::IgnoredCompilerOptions; use crate::config_file::TsConfig; use crate::diagnostics::Diagnostics; use crate::flags; +use crate::graph_util::GraphData; +use crate::graph_util::ModuleEntry; use crate::tsc; use crate::version; @@ -20,6 +22,7 @@ use deno_ast::swc; use deno_core::anyhow::anyhow; use deno_core::anyhow::Context; use deno_core::error::AnyError; +use deno_core::parking_lot::RwLock; use deno_core::serde::Deserialize; use deno_core::serde::Deserializer; use deno_core::serde::Serialize; @@ -242,34 +245,35 @@ pub(crate) fn get_ts_config( /// redirects resolved. If we aren't checking JavaScript, we need to include all /// the emittable files in the roots, so they get type checked and optionally /// emitted, otherwise they would be ignored if only imported into JavaScript. -fn get_root_names( - graph: &ModuleGraph, +fn get_tsc_roots( + roots: &[ModuleSpecifier], + graph_data: &GraphData, check_js: bool, ) -> Vec<(ModuleSpecifier, MediaType)> { if !check_js { - graph - .specifiers() + graph_data + .entries() .into_iter() - .filter_map(|(_, r)| match r { - Ok((s, mt)) => match &mt { + .filter_map(|(specifier, module_entry)| match module_entry { + ModuleEntry::Module { media_type, .. } => match &media_type { MediaType::TypeScript | MediaType::Tsx | MediaType::Mts | MediaType::Cts - | MediaType::Jsx => Some((s, mt)), + | MediaType::Jsx => Some((specifier.clone(), *media_type)), _ => None, }, _ => None, }) .collect() } else { - graph - .roots + roots .iter() - .filter_map(|s| { - graph - .get(s) - .map(|m| (m.specifier().clone(), *m.media_type())) + .filter_map(|specifier| match graph_data.get(specifier) { + Some(ModuleEntry::Module { media_type, .. }) => { + Some((specifier.clone(), *media_type)) + } + _ => None, }) .collect() } @@ -316,8 +320,11 @@ pub(crate) struct CheckOptions { pub maybe_config_specifier: Option, /// The derived tsconfig that should be used when checking. pub ts_config: TsConfig, - /// If true, existing `.tsbuildinfo` files will be ignored. + /// If true, `Check ` will be written to stdout for each root. + pub log_checks: bool, + /// If true, valid existing emits and `.tsbuildinfo` files will be ignored. pub reload: bool, + pub reload_exclusions: HashSet, } /// The result of a check or emit of a module graph. Note that the actual @@ -328,27 +335,50 @@ pub(crate) struct CheckEmitResult { pub stats: Stats, } -/// Given a module graph, type check the module graph and optionally emit -/// modules, updating the cache as appropriate. Emitting is determined by the -/// `ts_config` supplied in the options, and if emitting, the files are stored -/// in the cache. +/// Given a set of roots and graph data, type check the module graph and +/// optionally emit modules, updating the cache as appropriate. Emitting is +/// determined by the `ts_config` supplied in the options, and if emitting, the +/// files are stored in the cache. /// /// It is expected that it is determined if a check and/or emit is validated /// before the function is called. pub(crate) fn check_and_maybe_emit( - graph: Arc, + roots: &[ModuleSpecifier], + graph_data: Arc>, cache: &mut dyn Cacher, options: CheckOptions, ) -> Result { let check_js = options.ts_config.get_check_js(); - let root_names = get_root_names(&graph, check_js); + let segment_graph_data = { + let graph_data = graph_data.read(); + graph_data.graph_segment(roots).unwrap() + }; + if valid_emit( + &segment_graph_data, + cache, + &options.ts_config, + options.reload, + &options.reload_exclusions, + ) { + return Ok(Default::default()); + } + let root_names = get_tsc_roots(roots, &segment_graph_data, check_js); + if options.log_checks { + for root in roots { + let root_str = root.to_string(); + // `$deno` specifiers are internal, don't print them. + if !root_str.contains("$deno") { + log::info!("{} {}", colors::green("Check"), root); + } + } + } // 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(CacheType::TypeScriptBuildInfo, &graph.roots[0]) + cache.get(CacheType::TypeScriptBuildInfo, &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 @@ -362,7 +392,7 @@ pub(crate) fn check_and_maybe_emit( let response = tsc::exec(tsc::Request { config: options.ts_config, debug: options.debug, - graph: graph.clone(), + graph_data: graph_data.clone(), hash_data, maybe_config_specifier: options.maybe_config_specifier, maybe_tsbuildinfo, @@ -389,7 +419,7 @@ pub(crate) fn check_and_maybe_emit( if let Some(info) = &response.maybe_tsbuildinfo { // while we retrieve the build info for just the first module, it can be // used for all the roots in the graph, so we will cache it for all roots - for root in &graph.roots { + for root in roots { cache.set(CacheType::TypeScriptBuildInfo, root, info.clone())?; } } @@ -398,12 +428,16 @@ pub(crate) fn check_and_maybe_emit( assert!(specifiers.len() == 1); // The emitted specifier might not be the file specifier we want, so we // resolve it via the graph. - let specifier = graph.resolve(&specifiers[0]); - let (media_type, source) = if let Some(module) = graph.get(&specifier) { - (module.media_type(), module.maybe_source().unwrap_or("")) - } else { - log::debug!("module missing, skipping emit for {}", specifier); - continue; + let graph_data = graph_data.read(); + let specifier = graph_data.follow_redirect(&specifiers[0]); + let (source_bytes, media_type) = match graph_data.get(&specifier) { + Some(ModuleEntry::Module { + code, media_type, .. + }) => (code.as_bytes(), *media_type), + _ => { + log::debug!("skipping emit for {}", specifier); + continue; + } }; // Sometimes if `tsc` sees a CommonJS file or a JSON module, it will // _helpfully_ output it, which we don't really want to do unless @@ -420,7 +454,7 @@ pub(crate) fn check_and_maybe_emit( } match emit.media_type { MediaType::JavaScript | MediaType::Mjs | MediaType::Cjs => { - let version = get_version(source.as_bytes(), &config_bytes); + let version = get_version(source_bytes, &config_bytes); cache.set(CacheType::Version, &specifier, version)?; cache.set(CacheType::Emit, &specifier, emit.data)?; } @@ -636,11 +670,13 @@ pub(crate) fn bundle( pub(crate) struct EmitOptions { pub ts_config: TsConfig, - pub reload_exclusions: HashSet, pub reload: bool, + pub reload_exclusions: HashSet, } /// Given a module graph, emit any appropriate modules and cache them. +// TODO(nayeemrmn): This would ideally take `GraphData` like +// `check_and_maybe_emit()`, but the AST isn't stored in that. Cleanup. pub(crate) fn emit( graph: &ModuleGraph, cache: &mut dyn Cacher, @@ -693,21 +729,12 @@ pub(crate) fn emit( }) } -/// Check the sub-resource integrity of a module graph, exiting if the graph is -/// not valid. -pub(crate) fn lock(graph: &ModuleGraph) { - if let Err(err) = graph.lock() { - log::error!("{} {}", colors::red("error:"), err); - std::process::exit(10); - } -} - /// Check a module graph to determine if the graph contains anything that /// is required to be emitted to be valid. It determines what modules in the /// graph are emittable and for those that are emittable, if there is currently /// a valid emit in the cache. -pub(crate) fn valid_emit( - graph: &ModuleGraph, +fn valid_emit( + graph_data: &GraphData, cache: &dyn Cacher, ts_config: &TsConfig, reload: bool, @@ -715,47 +742,37 @@ pub(crate) fn valid_emit( ) -> bool { let config_bytes = ts_config.as_bytes(); let emit_js = ts_config.get_check_js(); - graph - .specifiers() - .iter() - .filter(|(_, r)| match r { - Ok((_, MediaType::TypeScript | MediaType::Mts | MediaType::Cts)) - | Ok((_, MediaType::Tsx)) - | Ok((_, MediaType::Jsx)) => true, - Ok((_, MediaType::JavaScript | MediaType::Mjs | MediaType::Cjs)) => { - emit_js - } - _ => false, - }) - .all(|(_, r)| { - if let Ok((s, _)) = r { - if reload && !reload_exclusions.contains(s) { - // we are reloading and the specifier isn't excluded from being - // reloaded - false - } else if let Some(version) = cache.get(CacheType::Version, s) { - if let Some(module) = graph.get(s) { - version - == get_version( - module.maybe_source().unwrap_or("").as_bytes(), - &config_bytes, - ) - } else { - // We have a source module in the graph we can't find, so the emit is - // clearly wrong - false + for (specifier, module_entry) in graph_data.entries() { + if let ModuleEntry::Module { + code, media_type, .. + } = module_entry + { + match media_type { + MediaType::TypeScript + | MediaType::Mts + | MediaType::Cts + | MediaType::Tsx + | MediaType::Jsx => {} + MediaType::JavaScript | MediaType::Mjs | MediaType::Cjs => { + if !emit_js { + continue; } - } else { - // A module that requires emitting doesn't have a version, so it doesn't - // have a valid emit - false + } + _ => continue, + } + if reload && !reload_exclusions.contains(specifier) { + return false; + } + if let Some(version) = cache.get(CacheType::Version, specifier) { + if version != get_version(code.as_bytes(), &config_bytes) { + return false; } } else { - // Something in the module graph is missing, but that doesn't mean the - // emit is invalid - true + return false; } - }) + } + } + true } /// An adapter struct to make a deno_graph::ModuleGraphError display as expected @@ -809,6 +826,7 @@ pub(crate) fn to_file_map( MediaType::JavaScript | MediaType::Mjs | MediaType::Cjs + | MediaType::Json | MediaType::Unknown ) { if let Some(module) = graph.get(&specifier) { diff --git a/cli/graph_util.rs b/cli/graph_util.rs new file mode 100644 index 0000000000..941243d905 --- /dev/null +++ b/cli/graph_util.rs @@ -0,0 +1,412 @@ +// Copyright 2018-2021 the Deno authors. All rights reserved. MIT license. + +use crate::colors; +use crate::emit::TypeLib; +use crate::errors::get_error_class_name; +use deno_core::error::custom_error; +use deno_core::error::AnyError; +use deno_core::ModuleSpecifier; +use deno_graph::Dependency; +use deno_graph::MediaType; +use deno_graph::Module; +use deno_graph::ModuleGraph; +use deno_graph::ModuleGraphError; +use deno_graph::Range; +use deno_graph::ResolutionError; +use std::collections::BTreeMap; +use std::collections::HashMap; +use std::collections::HashSet; +use std::collections::VecDeque; +use std::sync::Arc; + +#[derive(Debug, Clone)] +#[allow(clippy::large_enum_variant)] +pub(crate) enum ModuleEntry { + Module { + code: Arc, + dependencies: BTreeMap, + media_type: MediaType, + /// A set of type libs that the module has passed a type check with this + /// session. This would consist of window, worker or both. + checked_libs: HashSet, + maybe_types: Option>, + }, + Configuration { + dependencies: + BTreeMap>, + }, + Error(ModuleGraphError), + Redirect(ModuleSpecifier), +} + +/// Composes data from potentially many `ModuleGraph`s. +#[derive(Debug, Default)] +pub(crate) struct GraphData { + modules: HashMap, + /// Map of first known referrer locations for each module. Used to enhance + /// error messages. + referrer_map: HashMap, + configurations: HashSet, +} + +impl GraphData { + /// Store data from `graph` into `self`. + pub(crate) fn add_graph(&mut self, graph: &ModuleGraph, reload: bool) { + for (specifier, result) in graph.specifiers() { + if !reload && self.modules.contains_key(&specifier) { + continue; + } + if let Some(found) = graph.redirects.get(&specifier) { + let module_entry = ModuleEntry::Redirect(found.clone()); + self.modules.insert(specifier.clone(), module_entry); + continue; + } + match result { + Ok((_, media_type)) => { + let module = graph.get(&specifier).unwrap(); + let (code, dependencies, maybe_types) = match module { + Module::Es(es_module) => ( + es_module.source.clone(), + es_module.dependencies.clone(), + es_module + .maybe_types_dependency + .as_ref() + .and_then(|(_, r)| r.clone()), + ), + Module::Synthetic(synthetic_module) => match &synthetic_module + .maybe_source + { + // Synthetic modules with a source are actually JSON modules. + Some(source) => (source.clone(), Default::default(), None), + // Synthetic modules without a source are config roots. + None => { + let mut dependencies = BTreeMap::new(); + for (specifier, resolved) in &synthetic_module.dependencies { + if let Some(dep_result) = resolved { + dependencies.insert(specifier.clone(), dep_result.clone()); + if let Ok((specifier, referrer_range)) = dep_result { + let entry = self.referrer_map.entry(specifier.clone()); + entry.or_insert_with(|| referrer_range.clone()); + } + } + } + self.modules.insert( + synthetic_module.specifier.clone(), + ModuleEntry::Configuration { dependencies }, + ); + self + .configurations + .insert(synthetic_module.specifier.clone()); + continue; + } + }, + }; + if let Some(Ok((specifier, referrer_range))) = &maybe_types { + let specifier = graph.redirects.get(specifier).unwrap_or(specifier); + let entry = self.referrer_map.entry(specifier.clone()); + entry.or_insert_with(|| referrer_range.clone()); + } + for dep in dependencies.values() { + #[allow(clippy::manual_flatten)] + for resolved in [&dep.maybe_code, &dep.maybe_type] { + if let Some(Ok((specifier, referrer_range))) = resolved { + let specifier = + graph.redirects.get(specifier).unwrap_or(specifier); + let entry = self.referrer_map.entry(specifier.clone()); + entry.or_insert_with(|| referrer_range.clone()); + } + } + } + let module_entry = ModuleEntry::Module { + code, + dependencies, + media_type, + checked_libs: Default::default(), + maybe_types, + }; + self.modules.insert(specifier, module_entry); + } + Err(error) => { + let module_entry = ModuleEntry::Error(error); + self.modules.insert(specifier, module_entry); + } + } + } + } + + pub(crate) fn entries(&self) -> HashMap<&ModuleSpecifier, &ModuleEntry> { + self.modules.iter().collect() + } + + /// Walk dependencies from `roots` and return every encountered specifier. + /// Return `None` if any modules are not known. + pub(crate) fn walk<'a>( + &'a self, + roots: &[ModuleSpecifier], + follow_dynamic: bool, + follow_type_only: bool, + ) -> Option> { + let mut result = HashMap::<&'a ModuleSpecifier, &'a ModuleEntry>::new(); + let mut seen = HashSet::<&ModuleSpecifier>::new(); + let mut visiting = VecDeque::<&ModuleSpecifier>::new(); + for root in roots { + seen.insert(root); + visiting.push_back(root); + } + for root in &self.configurations { + seen.insert(root); + visiting.push_back(root); + } + while let Some(specifier) = visiting.pop_front() { + let (specifier, entry) = match self.modules.get_key_value(specifier) { + Some(pair) => pair, + None => return None, + }; + result.insert(specifier, entry); + match entry { + ModuleEntry::Module { + dependencies, + maybe_types, + .. + } => { + if follow_type_only { + if let Some(Ok((types, _))) = maybe_types { + if !seen.contains(types) { + seen.insert(types); + visiting.push_front(types); + } + } + } + for (_, dep) in dependencies.iter().rev() { + if !dep.is_dynamic || follow_dynamic { + let mut resolutions = vec![&dep.maybe_code]; + if follow_type_only { + resolutions.push(&dep.maybe_type); + } + #[allow(clippy::manual_flatten)] + for resolved in resolutions { + if let Some(Ok((dep_specifier, _))) = resolved { + if !seen.contains(dep_specifier) { + seen.insert(dep_specifier); + visiting.push_front(dep_specifier); + } + } + } + } + } + } + ModuleEntry::Configuration { dependencies } => { + for (dep_specifier, _) in dependencies.values().flatten() { + if !seen.contains(dep_specifier) { + seen.insert(dep_specifier); + visiting.push_front(dep_specifier); + } + } + } + ModuleEntry::Error(_) => {} + ModuleEntry::Redirect(specifier) => { + if !seen.contains(specifier) { + seen.insert(specifier); + visiting.push_front(specifier); + } + } + } + } + Some(result) + } + + /// Clone part of `self`, containing only modules which are dependencies of + /// `roots`. Returns `None` if any roots are not known. + pub(crate) fn graph_segment( + &self, + roots: &[ModuleSpecifier], + ) -> Option { + let mut modules = HashMap::new(); + let mut referrer_map = HashMap::new(); + let entries = match self.walk(roots, true, true) { + Some(entries) => entries, + None => return None, + }; + for (specifier, module_entry) in entries { + modules.insert(specifier.clone(), module_entry.clone()); + if let Some(referrer) = self.referrer_map.get(specifier) { + referrer_map.insert(specifier.clone(), referrer.clone()); + } + } + Some(Self { + modules, + referrer_map, + configurations: self.configurations.clone(), + }) + } + + /// Check if `roots` and their deps are available. Returns `Some(Ok(()))` if + /// so. Returns `Some(Err(_))` if there is a known module graph or resolution + /// error statically reachable from `roots`. Returns `None` if any modules are + /// not known. + pub(crate) fn check( + &self, + roots: &[ModuleSpecifier], + follow_type_only: bool, + ) -> Option> { + let entries = match self.walk(roots, false, follow_type_only) { + Some(entries) => entries, + None => return None, + }; + for (specifier, module_entry) in entries { + match module_entry { + ModuleEntry::Module { + dependencies, + maybe_types, + .. + } => { + if follow_type_only { + if let Some(Err(error)) = maybe_types { + let range = error.range(); + if !range.specifier.as_str().contains("$deno") { + return Some(Err(custom_error( + get_error_class_name(&error.clone().into()), + format!("{}\n at {}", error.to_string(), range), + ))); + } + return Some(Err(error.clone().into())); + } + } + for (_, dep) in dependencies.iter() { + if !dep.is_dynamic { + let mut resolutions = vec![&dep.maybe_code]; + if follow_type_only { + resolutions.push(&dep.maybe_type); + } + #[allow(clippy::manual_flatten)] + for resolved in resolutions { + if let Some(Err(error)) = resolved { + let range = error.range(); + if !range.specifier.as_str().contains("$deno") { + return Some(Err(custom_error( + get_error_class_name(&error.clone().into()), + format!("{}\n at {}", error.to_string(), range), + ))); + } + return Some(Err(error.clone().into())); + } + } + } + } + } + ModuleEntry::Configuration { dependencies } => { + for resolved_result in dependencies.values() { + if let Err(error) = resolved_result { + let range = error.range(); + if !range.specifier.as_str().contains("$deno") { + return Some(Err(custom_error( + get_error_class_name(&error.clone().into()), + format!("{}\n at {}", error.to_string(), range), + ))); + } + return Some(Err(error.clone().into())); + } + } + } + ModuleEntry::Error(error) => { + if !roots.contains(specifier) { + if let Some(range) = self.referrer_map.get(specifier) { + if !range.specifier.as_str().contains("$deno") { + let message = error.to_string(); + return Some(Err(custom_error( + get_error_class_name(&error.clone().into()), + format!("{}\n at {}", message, range), + ))); + } + } + } + return Some(Err(error.clone().into())); + } + _ => {} + } + } + Some(Ok(())) + } + + /// Mark `roots` and all of their dependencies as type checked under `lib`. + /// Assumes that all of those modules are known. + pub(crate) fn set_type_checked( + &mut self, + roots: &[ModuleSpecifier], + lib: &TypeLib, + ) { + let specifiers: Vec = match self.walk(roots, true, true) { + Some(entries) => entries.into_keys().cloned().collect(), + None => unreachable!("contains module not in graph data"), + }; + for specifier in specifiers { + if let ModuleEntry::Module { checked_libs, .. } = + self.modules.get_mut(&specifier).unwrap() + { + checked_libs.insert(lib.clone()); + } + } + } + + /// Check if `roots` are all marked as type checked under `lib`. + pub(crate) fn is_type_checked( + &self, + roots: &[ModuleSpecifier], + lib: &TypeLib, + ) -> bool { + roots.iter().all(|r| { + let found = self.follow_redirect(r); + match self.modules.get(&found) { + Some(ModuleEntry::Module { checked_libs, .. }) => { + checked_libs.contains(lib) + } + _ => false, + } + }) + } + + /// If `specifier` is known and a redirect, return the found specifier. + /// Otherwise return `specifier`. + pub(crate) fn follow_redirect( + &self, + specifier: &ModuleSpecifier, + ) -> ModuleSpecifier { + match self.modules.get(specifier) { + Some(ModuleEntry::Redirect(s)) => s.clone(), + _ => specifier.clone(), + } + } + + pub(crate) fn get<'a>( + &'a self, + specifier: &ModuleSpecifier, + ) -> Option<&'a ModuleEntry> { + self.modules.get(specifier) + } +} + +impl From<&ModuleGraph> for GraphData { + fn from(graph: &ModuleGraph) -> Self { + let mut graph_data = GraphData::default(); + graph_data.add_graph(graph, false); + graph_data + } +} + +/// Like `graph.valid()`, but enhanced with referrer info. +pub(crate) fn graph_valid( + graph: &ModuleGraph, + follow_type_only: bool, +) -> Result<(), AnyError> { + GraphData::from(graph) + .check(&graph.roots, follow_type_only) + .unwrap() +} + +/// Calls `graph.lock()` and exits on errors. +pub(crate) fn graph_lock_or_exit(graph: &ModuleGraph) { + if let Err(err) = graph.lock() { + log::error!("{} {}", colors::red("error:"), err); + std::process::exit(10); + } +} diff --git a/cli/lsp/cache.rs b/cli/lsp/cache.rs index 2471284201..b26c0ba4f5 100644 --- a/cli/lsp/cache.rs +++ b/cli/lsp/cache.rs @@ -4,6 +4,7 @@ use crate::cache::CacherLoader; use crate::cache::FetchCacher; use crate::config_file::ConfigFile; use crate::flags::Flags; +use crate::graph_util::graph_valid; use crate::proc_state::ProcState; use crate::resolver::ImportMapResolver; use crate::resolver::JsxResolver; @@ -82,7 +83,7 @@ impl CacheServer { ) .await; - if tx.send(graph.valid().map_err(|err| err.into())).is_err() { + if tx.send(graph_valid(&graph, true)).is_err() { log::warn!("cannot send to client"); } } diff --git a/cli/main.rs b/cli/main.rs index bffe6f329a..9a613fb644 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -18,6 +18,7 @@ mod flags; mod flags_allow_net; mod fmt_errors; mod fs_util; +mod graph_util; mod http_cache; mod http_util; mod lockfile; @@ -58,6 +59,8 @@ use crate::flags::TestFlags; use crate::flags::UninstallFlags; use crate::flags::UpgradeFlags; use crate::fmt_errors::PrettyJsError; +use crate::graph_util::graph_lock_or_exit; +use crate::graph_util::graph_valid; use crate::module_loader::CliModuleLoader; use crate::proc_state::ProcState; use crate::resolver::ImportMapResolver; @@ -70,6 +73,7 @@ use deno_core::error::AnyError; use deno_core::futures::future::FutureExt; use deno_core::futures::Future; use deno_core::located_script_name; +use deno_core::parking_lot::RwLock; use deno_core::resolve_url_or_path; use deno_core::serde_json; use deno_core::serde_json::json; @@ -705,15 +709,10 @@ async fn create_graph_and_maybe_check( .await, ); - // Ensure that all non-dynamic, non-type only imports are properly loaded and - // if not, error with the first issue encountered. - graph.valid().map_err(emit::GraphError::from)?; - // If there was a locker, validate the integrity of all the modules in the - // locker. - emit::lock(graph.as_ref()); + graph_valid(&graph, ps.flags.check != CheckFlag::None)?; + graph_lock_or_exit(&graph); if ps.flags.check != CheckFlag::None { - graph.valid_types_only().map_err(emit::GraphError::from)?; let lib = if ps.flags.unstable { emit::TypeLib::UnstableDenoWindow } else { @@ -727,14 +726,14 @@ async fn create_graph_and_maybe_check( ps.maybe_config_file.as_ref(), None, )?; - log::info!("{} {}", colors::green("Check"), graph.roots[0]); if let Some(ignored_options) = maybe_ignored_options { eprintln!("{}", ignored_options); } let maybe_config_specifier = ps.maybe_config_file.as_ref().map(|cf| cf.specifier.clone()); let check_result = emit::check_and_maybe_emit( - graph.clone(), + &graph.roots, + Arc::new(RwLock::new(graph.as_ref().into())), &mut cache, emit::CheckOptions { check: ps.flags.check.clone(), @@ -742,7 +741,9 @@ async fn create_graph_and_maybe_check( emit_with_diagnostics: false, maybe_config_specifier, ts_config, + log_checks: true, reload: ps.flags.reload, + reload_exclusions: Default::default(), }, )?; debug!("{}", check_result.stats); @@ -1056,7 +1057,7 @@ async fn run_with_watch(flags: Flags, script: String) -> Result { None, ) .await; - graph.valid()?; + graph_valid(&graph, ps.flags.check != flags::CheckFlag::None)?; // Find all local files in graph let mut paths_to_watch: Vec = graph diff --git a/cli/ops/runtime_compiler.rs b/cli/ops/runtime_compiler.rs index af99f12caa..fb5b078783 100644 --- a/cli/ops/runtime_compiler.rs +++ b/cli/ops/runtime_compiler.rs @@ -6,6 +6,7 @@ use crate::diagnostics::Diagnostics; use crate::emit; use crate::errors::get_error_class_name; use crate::flags; +use crate::graph_util::graph_valid; use crate::proc_state::ProcState; use crate::resolver::ImportMapResolver; use crate::resolver::JsxResolver; @@ -14,6 +15,7 @@ use deno_core::anyhow::Context; use deno_core::error::custom_error; use deno_core::error::generic_error; use deno_core::error::AnyError; +use deno_core::parking_lot::RwLock; use deno_core::resolve_url_or_path; use deno_core::serde_json; use deno_core::serde_json::Value; @@ -215,16 +217,15 @@ async fn op_emit( ) .await, ); + let check = args.check.unwrap_or(true); // There are certain graph errors that we want to return as an error of an op, // versus something that gets returned as a diagnostic of the op, this is // handled here. - if let Err(err) = graph.valid() { - let err: AnyError = err.into(); + if let Err(err) = graph_valid(&graph, check) { if get_error_class_name(&err) == "PermissionDenied" { return Err(err); } } - let check = args.check.unwrap_or(true); let debug = ps.flags.log_level == Some(log::Level::Debug); let tsc_emit = check && args.bundle.is_none(); let (ts_config, maybe_ignored_options) = emit::get_ts_config( @@ -233,43 +234,32 @@ async fn op_emit( args.compiler_options.as_ref(), )?; let (files, mut diagnostics, stats) = if check && args.bundle.is_none() { - let (diagnostics, stats) = if args.sources.is_none() - && emit::valid_emit( - graph.as_ref(), - cache.as_cacher(), - &ts_config, - ps.flags.reload, - &HashSet::default(), - ) { - log::debug!( - "cache is valid for \"{}\", skipping check/emit", - root_specifier - ); - (Diagnostics::default(), emit::Stats::default()) - } else { - let emit_result = emit::check_and_maybe_emit( - graph.clone(), - cache.as_mut_cacher(), - emit::CheckOptions { - check: flags::CheckFlag::All, - debug, - emit_with_diagnostics: true, - maybe_config_specifier: None, - ts_config, - reload: true, - }, - )?; - (emit_result.diagnostics, emit_result.stats) - }; + let emit_result = emit::check_and_maybe_emit( + &graph.roots, + Arc::new(RwLock::new(graph.as_ref().into())), + cache.as_mut_cacher(), + emit::CheckOptions { + check: flags::CheckFlag::All, + debug, + emit_with_diagnostics: true, + maybe_config_specifier: None, + ts_config, + log_checks: false, + reload: ps.flags.reload || args.sources.is_some(), + // TODO(nayeemrmn): Determine reload exclusions. + reload_exclusions: Default::default(), + }, + )?; let files = emit::to_file_map(graph.as_ref(), cache.as_mut_cacher()); - (files, diagnostics, stats) + (files, emit_result.diagnostics, emit_result.stats) } else if let Some(bundle) = &args.bundle { let (diagnostics, stats) = if check { if ts_config.get_declaration() { return Err(custom_error("TypeError", "The bundle option is set, but the compiler option of `declaration` is true which is not currently supported.")); } let emit_result = emit::check_and_maybe_emit( - graph.clone(), + &graph.roots, + Arc::new(RwLock::new(graph.as_ref().into())), cache.as_mut_cacher(), emit::CheckOptions { check: flags::CheckFlag::All, @@ -277,7 +267,10 @@ async fn op_emit( emit_with_diagnostics: true, maybe_config_specifier: None, ts_config: ts_config.clone(), - reload: true, + log_checks: false, + reload: ps.flags.reload || args.sources.is_some(), + // TODO(nayeemrmn): Determine reload exclusions. + reload_exclusions: Default::default(), }, )?; (emit_result.diagnostics, emit_result.stats) @@ -305,8 +298,9 @@ async fn op_emit( graph.as_ref(), cache.as_mut_cacher(), emit::EmitOptions { - reload: ps.flags.reload, ts_config, + reload: ps.flags.reload || args.sources.is_some(), + // TODO(nayeemrmn): Determine reload exclusions. reload_exclusions: HashSet::default(), }, )?; diff --git a/cli/proc_state.rs b/cli/proc_state.rs index d82f8d017f..b6797d6635 100644 --- a/cli/proc_state.rs +++ b/cli/proc_state.rs @@ -1,7 +1,6 @@ // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license. use crate::cache; -use crate::cache::Cacher; use crate::colors; use crate::compat; use crate::compat::NodeEsmResolver; @@ -9,10 +8,12 @@ use crate::config_file::ConfigFile; use crate::config_file::MaybeImportsResult; use crate::deno_dir; use crate::emit; -use crate::errors::get_error_class_name; use crate::file_fetcher::CacheSetting; use crate::file_fetcher::FileFetcher; use crate::flags; +use crate::graph_util::graph_lock_or_exit; +use crate::graph_util::GraphData; +use crate::graph_util::ModuleEntry; use crate::http_cache; use crate::lockfile::as_maybe_locker; use crate::lockfile::Lockfile; @@ -25,7 +26,9 @@ use deno_core::anyhow::anyhow; use deno_core::anyhow::Context; use deno_core::error::custom_error; use deno_core::error::AnyError; +use deno_core::futures; use deno_core::parking_lot::Mutex; +use deno_core::parking_lot::RwLock; use deno_core::resolve_url; use deno_core::url::Url; use deno_core::CompiledWasmModuleStore; @@ -34,10 +37,10 @@ use deno_core::ModuleSpecifier; use deno_core::ModuleType; use deno_core::SharedArrayBufferStore; use deno_graph::create_graph; -use deno_graph::Dependency; +use deno_graph::source::CacheInfo; +use deno_graph::source::LoadFuture; +use deno_graph::source::Loader; use deno_graph::MediaType; -use deno_graph::ModuleGraphError; -use deno_graph::Range; use deno_runtime::deno_broadcast_channel::InMemoryBroadcastChannel; use deno_runtime::deno_tls::rustls; use deno_runtime::deno_tls::rustls::RootCertStore; @@ -48,10 +51,7 @@ use deno_runtime::deno_web::BlobStore; use deno_runtime::inspector_server::InspectorServer; use deno_runtime::permissions::Permissions; use import_map::ImportMap; -use std::collections::BTreeMap; -use std::collections::HashMap; use std::collections::HashSet; -use std::collections::VecDeque; use std::env; use std::fs::File; use std::io::BufReader; @@ -64,104 +64,13 @@ use std::sync::Arc; #[derive(Clone)] pub struct ProcState(Arc); -#[derive(Debug, Clone)] -#[allow(clippy::large_enum_variant)] -enum ModuleEntry { - Module { - code: String, - media_type: MediaType, - dependencies: BTreeMap, - }, - Error(ModuleGraphError), - Redirect(ModuleSpecifier), -} - -#[derive(Default)] -struct GraphData { - modules: HashMap, - /// A set of type libs that each module has passed a type check with this - /// session. This would consist of window, worker or both. - checked_libs_map: HashMap>, - /// Map of first known referrer locations for each module. Used to enhance - /// error messages. - referrer_map: HashMap, -} - -impl GraphData { - /// Check if `roots` are ready to be loaded by V8. Returns `Some(Ok(()))` if - /// prepared. Returns `Some(Err(_))` if there is a known module graph error - /// statically reachable from `roots`. Returns `None` if sufficient graph data - /// is yet to be supplied. - fn check_if_prepared( - &self, - roots: &[ModuleSpecifier], - ) -> Option> { - let mut seen = HashSet::<&ModuleSpecifier>::new(); - let mut visiting = VecDeque::<&ModuleSpecifier>::new(); - for root in roots { - visiting.push_back(root); - } - while let Some(specifier) = visiting.pop_front() { - match self.modules.get(specifier) { - Some(ModuleEntry::Module { dependencies, .. }) => { - for (_, dep) in dependencies.iter().rev() { - for resolved in [&dep.maybe_code, &dep.maybe_type] { - if !dep.is_dynamic { - match resolved { - Some(Ok((dep_specifier, _))) => { - if !dep.is_dynamic && !seen.contains(dep_specifier) { - seen.insert(dep_specifier); - visiting.push_front(dep_specifier); - } - } - Some(Err(error)) => { - let range = error.range(); - if !range.specifier.as_str().contains("$deno") { - return Some(Err(custom_error( - get_error_class_name(&error.clone().into()), - format!("{}\n at {}", error.to_string(), range), - ))); - } - return Some(Err(error.clone().into())); - } - None => {} - } - } - } - } - } - Some(ModuleEntry::Error(error)) => { - if !roots.contains(specifier) { - if let Some(range) = self.referrer_map.get(specifier) { - if !range.specifier.as_str().contains("$deno") { - let message = error.to_string(); - return Some(Err(custom_error( - get_error_class_name(&error.clone().into()), - format!("{}\n at {}", message, range), - ))); - } - } - } - return Some(Err(error.clone().into())); - } - Some(ModuleEntry::Redirect(specifier)) => { - seen.insert(specifier); - visiting.push_front(specifier); - } - None => return None, - } - } - Some(Ok(())) - } -} - pub struct Inner { /// Flags parsed from `argv` contents. pub flags: flags::Flags, pub dir: deno_dir::DenoDir, pub coverage_dir: Option, pub file_fetcher: FileFetcher, - graph_data: Arc>, + graph_data: Arc>, pub lockfile: Option>>, pub maybe_config_file: Option, pub maybe_import_map: Option>, @@ -404,8 +313,8 @@ impl ProcState { /// This method must be called for a module or a static importer of that /// module before attempting to `load()` it from a `JsRuntime`. It will - /// populate `self.graph_data` in memory with the necessary source code or - /// report any module graph / type checking errors. + /// populate `self.graph_data` in memory with the necessary source code, write + /// emits where necessary or report any module graph / type checking errors. pub(crate) async fn prepare_module_load( &self, roots: Vec, @@ -425,16 +334,13 @@ impl ProcState { roots }; if !reload_on_watch { - let graph_data = self.graph_data.lock(); + let graph_data = self.graph_data.read(); if self.flags.check == flags::CheckFlag::None - || roots.iter().all(|root| { - graph_data - .checked_libs_map - .get(root) - .map_or(false, |checked_libs| checked_libs.contains(&lib)) - }) + || graph_data.is_type_checked(&roots, &lib) { - if let Some(result) = graph_data.check_if_prepared(&roots) { + if let Some(result) = + graph_data.check(&roots, self.flags.check != flags::CheckFlag::None) + { return result; } } @@ -453,11 +359,45 @@ impl ProcState { } else { None }; + + struct ProcStateLoader<'a> { + inner: &'a mut cache::FetchCacher, + graph_data: Arc>, + reload: bool, + } + impl Loader for ProcStateLoader<'_> { + fn get_cache_info( + &self, + specifier: &ModuleSpecifier, + ) -> Option { + self.inner.get_cache_info(specifier) + } + fn load( + &mut self, + specifier: &ModuleSpecifier, + is_dynamic: bool, + ) -> LoadFuture { + let graph_data = self.graph_data.read(); + let found_specifier = graph_data.follow_redirect(specifier); + match graph_data.get(&found_specifier) { + Some(_) if !self.reload => Box::pin(futures::future::ready(( + specifier.clone(), + Err(anyhow!("")), + ))), + _ => self.inner.load(specifier, is_dynamic), + } + } + } + let mut loader = ProcStateLoader { + inner: &mut cache, + graph_data: self.graph_data.clone(), + reload: reload_on_watch, + }; let graph = create_graph( roots.clone(), is_dynamic, maybe_imports, - &mut cache, + &mut loader, maybe_resolver, maybe_locker, None, @@ -465,15 +405,23 @@ impl ProcState { .await; // If there was a locker, validate the integrity of all the modules in the // locker. - emit::lock(&graph); + graph_lock_or_exit(&graph); // Determine any modules that have already been emitted this session and // should be skipped. let reload_exclusions: HashSet = { - let graph_data = self.graph_data.lock(); - graph_data.modules.keys().cloned().collect() + let graph_data = self.graph_data.read(); + graph_data.entries().into_keys().cloned().collect() }; + { + let mut graph_data = self.graph_data.write(); + graph_data.add_graph(&graph, reload_on_watch); + graph_data + .check(&roots, self.flags.check != flags::CheckFlag::None) + .unwrap()?; + } + let config_type = if self.flags.check == flags::CheckFlag::None { emit::ConfigType::Emit } else { @@ -485,166 +433,49 @@ impl ProcState { let (ts_config, maybe_ignored_options) = emit::get_ts_config(config_type, self.maybe_config_file.as_ref(), None)?; - let graph = Arc::new(graph); - let mut type_check_result = Ok(()); - - if emit::valid_emit( - graph.as_ref(), - &cache, - &ts_config, - self.flags.reload, - &reload_exclusions, - ) { - if let Some(root) = graph.roots.get(0) { - log::debug!("specifier \"{}\" and dependencies have valid emit, skipping checking and emitting", root); - } else { - log::debug!("rootless graph, skipping checking and emitting"); - } - } else { - if let Some(ignored_options) = maybe_ignored_options { - log::warn!("{}", ignored_options); - } - let emit_result = if self.flags.check == flags::CheckFlag::None { - let options = emit::EmitOptions { - ts_config, - reload_exclusions, - reload: self.flags.reload, - }; - emit::emit(graph.as_ref(), &mut cache, options)? - } else { - // here, we are type checking, so we want to error here if any of the - // type only dependencies are missing or we have other errors with them - // where as if we are not type checking, we shouldn't care about these - // errors, and they don't get returned in `graph.valid()` above. - graph.valid_types_only()?; - - let maybe_config_specifier = self - .maybe_config_file - .as_ref() - .map(|cf| cf.specifier.clone()); - let options = emit::CheckOptions { - check: self.flags.check.clone(), - debug: self.flags.log_level == Some(log::Level::Debug), - emit_with_diagnostics: false, - maybe_config_specifier, - ts_config, - reload: self.flags.reload, - }; - for root in &graph.roots { - let root_str = root.to_string(); - // `$deno` specifiers are internal specifiers, printing out that - // they are being checked is confusing to a user, since they don't - // actually exist, so we will simply indicate that a generated module - // is being checked instead of the cryptic internal module - if !root_str.contains("$deno") { - log::info!("{} {}", colors::green("Check"), root); - } else { - log::info!("{} a generated module", colors::green("Check")) - } - } - emit::check_and_maybe_emit(graph.clone(), &mut cache, options)? - }; - log::debug!("{}", emit_result.stats); - if !emit_result.diagnostics.is_empty() { - type_check_result = Err(anyhow!(emit_result.diagnostics)); - } + if let Some(ignored_options) = maybe_ignored_options { + log::warn!("{}", ignored_options); } - { - let mut graph_data = self.graph_data.lock(); - let mut specifiers = graph.specifiers(); - // Set specifier results for redirects. - // TODO(nayeemrmn): This should be done in `ModuleGraph::specifiers()`. - for (specifier, found) in &graph.redirects { - let actual = specifiers.get(found).unwrap().clone(); - specifiers.insert(specifier.clone(), actual); - } - for (specifier, result) in &specifiers { - if let Some(found) = graph.redirects.get(specifier) { - let module_entry = ModuleEntry::Redirect(found.clone()); - graph_data.modules.insert(specifier.clone(), module_entry); - continue; - } - match result { - Ok((_, media_type)) => { - let module = graph.get(specifier).unwrap(); - // If there was a type check error, supply dummy code. It shouldn't - // be used since preparation will fail. - let code = if type_check_result.is_err() { - "".to_string() - // Check to see if there is an emitted file in the cache. - } else if let Some(code) = - cache.get(cache::CacheType::Emit, specifier) - { - code - // Then if the file is JavaScript (or unknown) and wasn't emitted, - // we will load the original source code in the module. - } else if matches!( - media_type, - MediaType::JavaScript - | MediaType::Unknown - | MediaType::Cjs - | MediaType::Mjs - | MediaType::Json - ) { - module.maybe_source().unwrap_or("").to_string() - // The emit may also be missing when a declaration file is in the - // graph. There shouldn't be any runtime statements in the source - // file and if there was, users would be shown a `TS1036` - // diagnostic. So just return an empty emit. - } else if !emit::is_emittable(media_type, true) { - "".to_string() - } else { - unreachable!( - "unexpected missing emit: {} media type: {}", - specifier, media_type - ) - }; - let dependencies = - module.maybe_dependencies().cloned().unwrap_or_default(); - graph_data.modules.insert( - specifier.clone(), - ModuleEntry::Module { - code, - dependencies, - media_type: *media_type, - }, - ); - if let Some(dependencies) = module.maybe_dependencies() { - for dep in dependencies.values() { - #[allow(clippy::manual_flatten)] - for resolved in [&dep.maybe_code, &dep.maybe_type] { - if let Some(Ok((specifier, referrer_range))) = resolved { - let specifier = - graph.redirects.get(specifier).unwrap_or(specifier); - let entry = - graph_data.referrer_map.entry(specifier.clone()); - entry.or_insert_with(|| referrer_range.clone()); - } - } - } - } - } - Err(error) => { - let module_entry = ModuleEntry::Error(error.clone()); - graph_data.modules.insert(specifier.clone(), module_entry); - } - } + if self.flags.check == flags::CheckFlag::None { + let options = emit::EmitOptions { + ts_config, + reload: self.flags.reload, + reload_exclusions, + }; + let emit_result = emit::emit(&graph, &mut cache, options)?; + log::debug!("{}", emit_result.stats); + } else { + let maybe_config_specifier = self + .maybe_config_file + .as_ref() + .map(|cf| cf.specifier.clone()); + let options = emit::CheckOptions { + check: self.flags.check.clone(), + debug: self.flags.log_level == Some(log::Level::Debug), + emit_with_diagnostics: false, + maybe_config_specifier, + ts_config, + log_checks: true, + reload: self.flags.reload, + reload_exclusions, + }; + let emit_result = emit::check_and_maybe_emit( + &roots, + self.graph_data.clone(), + &mut cache, + options, + )?; + if !emit_result.diagnostics.is_empty() { + return Err(anyhow!(emit_result.diagnostics)); } + log::debug!("{}", emit_result.stats); + } - graph_data.check_if_prepared(&roots).unwrap()?; - type_check_result?; - - if self.flags.check != flags::CheckFlag::None { - for specifier in specifiers.keys() { - let checked_libs = graph_data - .checked_libs_map - .entry(specifier.clone()) - .or_default(); - checked_libs.insert(lib.clone()); - } - } + if self.flags.check != flags::CheckFlag::None { + let mut graph_data = self.graph_data.write(); + graph_data.set_type_checked(&roots, &lib); } // any updates to the lockfile should be updated now @@ -662,12 +493,9 @@ impl ProcState { referrer: &str, ) -> Result { if let Ok(referrer) = deno_core::resolve_url_or_path(referrer) { - let graph_data = self.graph_data.lock(); - let found_referrer = match graph_data.modules.get(&referrer) { - Some(ModuleEntry::Redirect(r)) => r, - _ => &referrer, - }; - let maybe_resolved = match graph_data.modules.get(found_referrer) { + let graph_data = self.graph_data.read(); + let found_referrer = graph_data.follow_redirect(&referrer); + let maybe_resolved = match graph_data.get(&found_referrer) { Some(ModuleEntry::Module { dependencies, .. }) => dependencies .get(specifier) .and_then(|dep| dep.maybe_code.clone()), @@ -725,23 +553,43 @@ impl ProcState { is_dynamic ); - let graph_data = self.graph_data.lock(); - let found_specifier = match graph_data.modules.get(&specifier) { - Some(ModuleEntry::Redirect(s)) => s, - _ => &specifier, - }; - match graph_data.modules.get(found_specifier) { + let graph_data = self.graph_data.read(); + let found = graph_data.follow_redirect(&specifier); + match graph_data.get(&found) { Some(ModuleEntry::Module { code, media_type, .. - }) => Ok(ModuleSource { - code: code.clone(), - module_url_specified: specifier.to_string(), - module_url_found: found_specifier.to_string(), - module_type: match media_type { - MediaType::Json => ModuleType::Json, - _ => ModuleType::JavaScript, - }, - }), + }) => { + let code = match media_type { + MediaType::JavaScript + | MediaType::Unknown + | MediaType::Cjs + | MediaType::Mjs + | MediaType::Json => code.as_ref().clone(), + MediaType::Dts => "".to_string(), + _ => { + let emit_path = self + .dir + .gen_cache + .get_cache_filename_with_extension(&found, "js") + .unwrap_or_else(|| { + unreachable!("Unable to get cache filename: {}", &found) + }); + match self.dir.gen_cache.get(&emit_path) { + Ok(b) => String::from_utf8(b).unwrap(), + Err(_) => unreachable!("Unexpected missing emit: {}", found), + } + } + }; + Ok(ModuleSource { + code, + module_url_specified: specifier.to_string(), + module_url_found: found.to_string(), + module_type: match media_type { + MediaType::Json => ModuleType::Json, + _ => ModuleType::JavaScript, + }, + }) + } _ => Err(anyhow!( "Loading unprepared module: {}", specifier.to_string() diff --git a/cli/tests/integration/mod.rs b/cli/tests/integration/mod.rs index 9cd1b2c11d..b04f552e81 100644 --- a/cli/tests/integration/mod.rs +++ b/cli/tests/integration/mod.rs @@ -382,13 +382,14 @@ fn ts_reload() { // check the output of the the bundle program. let output_path = hello_ts.canonicalize().unwrap(); - assert!(std::str::from_utf8(&output.stderr) - .unwrap() - .trim() - .contains(&format!( - "host.getSourceFile(\"{}\", Latest)", - url::Url::from_file_path(&output_path).unwrap().as_str() - ))); + assert!( + dbg!(std::str::from_utf8(&output.stderr).unwrap().trim()).contains( + &format!( + "host.getSourceFile(\"{}\", Latest)", + url::Url::from_file_path(&output_path).unwrap().as_str() + ) + ) + ); } #[test] diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs index c04e4731c1..d36d0de1b4 100644 --- a/cli/tests/integration/run_tests.rs +++ b/cli/tests/integration/run_tests.rs @@ -1583,6 +1583,23 @@ itest!(worker_close_in_wasm_reactions { output: "worker_close_in_wasm_reactions.js.out", }); +itest!(reference_types_error { + args: "run reference_types_error.js", + output: "reference_types_error.js.out", + exit_code: 1, +}); + +itest!(reference_types_error_no_check { + args: "run --no-check reference_types_error.js", + output_str: Some(""), +}); + +itest!(jsx_import_source_error { + args: "run --config jsx/deno-jsx-error.jsonc jsx_import_source_no_pragma.tsx", + output: "jsx_import_source_error.out", + exit_code: 1, +}); + #[test] fn no_validate_asm() { let output = util::deno_cmd() diff --git a/cli/tests/integration/watcher_tests.rs b/cli/tests/integration/watcher_tests.rs index 02367d7808..f0431e3019 100644 --- a/cli/tests/integration/watcher_tests.rs +++ b/cli/tests/integration/watcher_tests.rs @@ -394,7 +394,7 @@ fn bundle_js_watch() { use std::path::PathBuf; // Test strategy extends this of test bundle_js by adding watcher let t = TempDir::new().unwrap(); - let file_to_watch = t.path().join("file_to_watch.js"); + let file_to_watch = t.path().join("file_to_watch.ts"); write(&file_to_watch, "console.log('Hello world');").unwrap(); assert!(file_to_watch.is_file()); let t = TempDir::new().unwrap(); @@ -418,7 +418,7 @@ fn bundle_js_watch() { let next_line = stderr_lines.next().unwrap(); assert_contains!(&next_line, CLEAR_SCREEN); assert_contains!(&next_line, "Bundle started"); - assert_contains!(stderr_lines.next().unwrap(), "file_to_watch.js"); + assert_contains!(stderr_lines.next().unwrap(), "file_to_watch.ts"); assert_contains!(stderr_lines.next().unwrap(), "mod6.bundle.js"); let file = PathBuf::from(&bundle); assert!(file.is_file()); @@ -430,7 +430,7 @@ fn bundle_js_watch() { let next_line = stderr_lines.next().unwrap(); assert_contains!(&next_line, CLEAR_SCREEN); assert_contains!(&next_line, "File change detected!"); - assert_contains!(stderr_lines.next().unwrap(), "file_to_watch.js"); + assert_contains!(stderr_lines.next().unwrap(), "file_to_watch.ts"); assert_contains!(stderr_lines.next().unwrap(), "mod6.bundle.js"); let file = PathBuf::from(&bundle); assert!(file.is_file()); @@ -449,7 +449,7 @@ fn bundle_js_watch() { #[test] fn bundle_watch_not_exit() { let t = TempDir::new().unwrap(); - let file_to_watch = t.path().join("file_to_watch.js"); + let file_to_watch = t.path().join("file_to_watch.ts"); write(&file_to_watch, "syntax error ^^").unwrap(); let target_file = t.path().join("target.js"); @@ -482,7 +482,7 @@ fn bundle_watch_not_exit() { let next_line = stderr_lines.next().unwrap(); assert_contains!(&next_line, CLEAR_SCREEN); assert_contains!(&next_line, "File change detected!"); - assert_contains!(stderr_lines.next().unwrap(), "file_to_watch.js"); + assert_contains!(stderr_lines.next().unwrap(), "file_to_watch.ts"); assert_contains!(stderr_lines.next().unwrap(), "target.js"); wait_for("Bundle finished", &mut stderr_lines); @@ -967,3 +967,33 @@ fn test_watch_doc() { assert_contains!(skip_restarting_line(&mut stderr_lines), "foo.ts$3-6"); check_alive_then_kill(child); } + +#[test] +fn test_watch_module_graph_error_referrer() { + let t = TempDir::new().unwrap(); + let file_to_watch = t.path().join("file_to_watch.js"); + write(&file_to_watch, "import './nonexistent.js';").unwrap(); + let mut child = util::deno_cmd() + .current_dir(util::testdata_path()) + .arg("run") + .arg("--watch") + .arg("--unstable") + .arg(&file_to_watch) + .env("NO_COLOR", "1") + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .unwrap(); + let (_, mut stderr_lines) = child_lines(&mut child); + let line1 = stderr_lines.next().unwrap(); + assert_contains!(&line1, CLEAR_SCREEN); + assert_contains!(&line1, "Process started"); + let line2 = stderr_lines.next().unwrap(); + assert_contains!(&line2, "error: Cannot load module"); + assert_contains!(&line2, "nonexistent.js"); + let line3 = stderr_lines.next().unwrap(); + assert_contains!(&line3, " at "); + assert_contains!(&line3, "file_to_watch.js"); + wait_for("Process failed", &mut stderr_lines); + check_alive_then_kill(child); +} diff --git a/cli/tests/testdata/compiler_api_test.ts b/cli/tests/testdata/compiler_api_test.ts index b9755c29ae..f51b5647cb 100644 --- a/cli/tests/testdata/compiler_api_test.ts +++ b/cli/tests/testdata/compiler_api_test.ts @@ -307,7 +307,7 @@ Deno.test({ ); assertEquals(diagnostics.length, 0); assert(!ignoredOptions); - assertEquals(stats.length, 12); + assertEquals(stats.length, 0); assertEquals( Object.keys(files).sort(), ["deno:///bundle.js.map", "deno:///bundle.js"].sort(), diff --git a/cli/tests/testdata/error_027_bundle_with_bare_import.ts.out b/cli/tests/testdata/error_027_bundle_with_bare_import.ts.out index 12f38c1c5c..e2edd118a1 100644 --- a/cli/tests/testdata/error_027_bundle_with_bare_import.ts.out +++ b/cli/tests/testdata/error_027_bundle_with_bare_import.ts.out @@ -1 +1,2 @@ [WILDCARD]error: Relative import path "foo" not prefixed with / or ./ or ../ + at file:///[WILDCARD]/error_027_bundle_with_bare_import.ts:[WILDCARD] diff --git a/cli/tests/testdata/error_type_definitions.ts.out b/cli/tests/testdata/error_type_definitions.ts.out index a0a536f782..5e1d73ca45 100644 --- a/cli/tests/testdata/error_type_definitions.ts.out +++ b/cli/tests/testdata/error_type_definitions.ts.out @@ -1 +1,2 @@ [WILDCARD]error: Relative import path "baz" not prefixed with / or ./ or ../ + at [WILDCARD]/type_definitions/bar.d.ts:[WILDCARD] diff --git a/cli/tests/testdata/jsx/deno-jsx-error.jsonc b/cli/tests/testdata/jsx/deno-jsx-error.jsonc new file mode 100644 index 0000000000..37cb4dd912 --- /dev/null +++ b/cli/tests/testdata/jsx/deno-jsx-error.jsonc @@ -0,0 +1,6 @@ +{ + "compilerOptions": { + "jsx": "react-jsx", + "jsxImportSource": "./nonexistent" + } +} diff --git a/cli/tests/testdata/jsx_import_source_error.out b/cli/tests/testdata/jsx_import_source_error.out new file mode 100644 index 0000000000..b9758a99e6 --- /dev/null +++ b/cli/tests/testdata/jsx_import_source_error.out @@ -0,0 +1,2 @@ +error: Cannot load module "file:///[WILDCARD]/nonexistent/jsx-runtime". + at file:///[WILDCARD]/deno-jsx-error.jsonc:1:1 diff --git a/cli/tests/testdata/reference_types_error.js b/cli/tests/testdata/reference_types_error.js new file mode 100644 index 0000000000..68b6c21364 --- /dev/null +++ b/cli/tests/testdata/reference_types_error.js @@ -0,0 +1,2 @@ +/// +export const a = 1; diff --git a/cli/tests/testdata/reference_types_error.js.out b/cli/tests/testdata/reference_types_error.js.out new file mode 100644 index 0000000000..89b450520c --- /dev/null +++ b/cli/tests/testdata/reference_types_error.js.out @@ -0,0 +1,2 @@ +error: Cannot load module "file:///[WILDCARD]/nonexistent.d.ts". + at file:///[WILDCARD]/reference_types_error.js:1:23 diff --git a/cli/tools/test.rs b/cli/tools/test.rs index c660d6043a..7db50a8072 100644 --- a/cli/tools/test.rs +++ b/cli/tools/test.rs @@ -9,10 +9,12 @@ use crate::emit; use crate::file_fetcher::File; use crate::file_watcher; use crate::file_watcher::ResolutionResult; +use crate::flags::CheckFlag; use crate::flags::Flags; use crate::fs_util::collect_specifiers; use crate::fs_util::is_supported_test_ext; use crate::fs_util::is_supported_test_path; +use crate::graph_util::graph_valid; use crate::located_script_name; use crate::lockfile; use crate::ops; @@ -1080,6 +1082,7 @@ pub async fn run_tests_with_watch( let include = include.unwrap_or_else(|| vec![".".to_string()]); let paths_to_watch: Vec<_> = include.iter().map(PathBuf::from).collect(); + let no_check = ps.flags.check == CheckFlag::None; let resolver = |changed: Option>| { let mut cache = cache::FetchCacher::new( @@ -1149,7 +1152,7 @@ pub async fn run_tests_with_watch( None, ) .await; - graph.valid()?; + graph_valid(&graph, !no_check)?; // TODO(@kitsonk) - This should be totally derivable from the graph. for specifier in test_modules { @@ -1159,21 +1162,32 @@ pub async fn run_tests_with_watch( // This needs to be accessible to skip getting dependencies if they're already there, // otherwise this will cause a stack overflow with circular dependencies output: &mut HashSet<&'a ModuleSpecifier>, + no_check: bool, ) { if let Some(Module::Es(module)) = maybe_module { for dep in module.dependencies.values() { if let Some(specifier) = &dep.get_code() { if !output.contains(specifier) { output.insert(specifier); - - get_dependencies(graph, graph.get(specifier), output); + get_dependencies( + graph, + graph.get(specifier), + output, + no_check, + ); } } - if let Some(specifier) = &dep.get_type() { - if !output.contains(specifier) { - output.insert(specifier); - - get_dependencies(graph, graph.get(specifier), output); + if !no_check { + if let Some(specifier) = &dep.get_type() { + if !output.contains(specifier) { + output.insert(specifier); + get_dependencies( + graph, + graph.get(specifier), + output, + no_check, + ); + } } } } @@ -1183,7 +1197,7 @@ pub async fn run_tests_with_watch( // This test module and all it's dependencies let mut modules = HashSet::new(); modules.insert(&specifier); - get_dependencies(&graph, graph.get(&specifier), &mut modules); + get_dependencies(&graph, graph.get(&specifier), &mut modules, no_check); paths_to_watch.extend( modules diff --git a/cli/tsc.rs b/cli/tsc.rs index f76fb18fb1..e87cb7c7d1 100644 --- a/cli/tsc.rs +++ b/cli/tsc.rs @@ -3,6 +3,8 @@ use crate::config_file::TsConfig; use crate::diagnostics::Diagnostics; use crate::emit; +use crate::graph_util::GraphData; +use crate::graph_util::ModuleEntry; use deno_ast::MediaType; use deno_core::anyhow::anyhow; @@ -10,6 +12,7 @@ use deno_core::anyhow::Context; use deno_core::error::AnyError; use deno_core::located_script_name; use deno_core::op_sync; +use deno_core::parking_lot::RwLock; use deno_core::resolve_url_or_path; use deno_core::serde::de; use deno_core::serde::Deserialize; @@ -22,7 +25,6 @@ use deno_core::ModuleSpecifier; use deno_core::OpFn; use deno_core::RuntimeOptions; use deno_core::Snapshot; -use deno_graph::ModuleGraph; use std::collections::HashMap; use std::path::PathBuf; use std::sync::Arc; @@ -185,7 +187,7 @@ pub struct Request { pub config: TsConfig, /// Indicates to the tsc runtime if debug logging should occur. pub debug: bool, - pub graph: Arc, + pub(crate) graph_data: Arc>, pub hash_data: Vec>, pub maybe_config_specifier: Option, pub maybe_tsbuildinfo: Option, @@ -211,7 +213,7 @@ struct State { data_url_map: HashMap, hash_data: Vec>, emitted_files: Vec, - graph: Arc, + graph_data: Arc>, maybe_config_specifier: Option, maybe_tsbuildinfo: Option, maybe_response: Option, @@ -220,7 +222,7 @@ struct State { impl State { pub fn new( - graph: Arc, + graph_data: Arc>, hash_data: Vec>, maybe_config_specifier: Option, maybe_tsbuildinfo: Option, @@ -231,7 +233,7 @@ impl State { data_url_map, hash_data, emitted_files: Default::default(), - graph, + graph_data, maybe_config_specifier, maybe_tsbuildinfo, maybe_response: None, @@ -335,11 +337,15 @@ struct ExistsArgs { } fn op_exists(state: &mut State, args: ExistsArgs) -> Result { + let graph_data = state.graph_data.read(); if let Ok(specifier) = normalize_specifier(&args.specifier) { if specifier.scheme() == "asset" || specifier.scheme() == "data" { Ok(true) } else { - Ok(state.graph.contains(&specifier)) + Ok(matches!( + graph_data.get(&graph_data.follow_redirect(&specifier)), + Some(ModuleEntry::Module { .. }) + )) } } else { Ok(false) @@ -401,9 +407,16 @@ fn op_load(state: &mut State, args: Value) -> Result { } else { specifier }; - let maybe_source = if let Some(module) = state.graph.get(&specifier) { - media_type = *module.media_type(); - module.maybe_source().map(String::from) + let graph_data = state.graph_data.read(); + let maybe_source = if let Some(ModuleEntry::Module { + code, + media_type: mt, + .. + }) = + graph_data.get(&graph_data.follow_redirect(&specifier)) + { + media_type = *mt; + Some(code.as_ref().clone()) } else { media_type = MediaType::Unknown; None @@ -427,27 +440,6 @@ pub struct ResolveArgs { pub specifiers: Vec, } -fn resolve_specifier( - state: &mut State, - specifier: &ModuleSpecifier, -) -> (String, String) { - let media_type = state - .graph - .get(specifier) - .map_or(&MediaType::Unknown, |m| m.media_type()); - let specifier_str = match specifier.scheme() { - "data" | "blob" => { - let specifier_str = hash_url(specifier, media_type); - state - .data_url_map - .insert(specifier_str.clone(), specifier.clone()); - specifier_str - } - _ => specifier.to_string(), - }; - (specifier_str, media_type.as_ts_extension().into()) -} - fn op_resolve(state: &mut State, args: Value) -> Result { let v: ResolveArgs = serde_json::from_value(args) .context("Invalid request from JavaScript for \"op_resolve\".")?; @@ -468,32 +460,62 @@ fn op_resolve(state: &mut State, args: Value) -> Result { MediaType::from(specifier).as_ts_extension().to_string(), )); } else { - // here, we try to resolve the specifier via the referrer, but if we can't - // we will try to resolve the specifier via the configuration file, if - // present, finally defaulting to a "placeholder" specifier. This handles - // situations like the jsxImportSource, which tsc tries to resolve the - // import source from a JSX module, but the module graph only contains the - // import as a dependency of the configuration file. - let resolved_dependency = if let Some(resolved_specifier) = state - .graph - .resolve_dependency(specifier, &referrer, true) - .cloned() - { - resolve_specifier(state, &resolved_specifier) - } else if let Some(resolved_specifier) = state - .maybe_config_specifier - .as_ref() - .map(|cf| state.graph.resolve_dependency(specifier, cf, true).cloned()) - .flatten() - { - resolve_specifier(state, &resolved_specifier) - } else { - ( + let graph_data = state.graph_data.read(); + let referrer = graph_data.follow_redirect(&referrer); + let resolved_dep = match graph_data.get(&referrer) { + Some(ModuleEntry::Module { dependencies, .. }) => { + dependencies.get(specifier).and_then(|d| { + d.maybe_type.as_ref().or_else(|| d.maybe_code.as_ref()) + }) + } + Some(ModuleEntry::Configuration { dependencies }) => { + dependencies.get(specifier) + } + _ => None, + }; + let maybe_result = match resolved_dep { + Some(Ok((specifier, _))) => { + let specifier = graph_data.follow_redirect(specifier); + match graph_data.get(&specifier) { + Some(ModuleEntry::Module { + media_type, + maybe_types, + .. + }) => match maybe_types { + Some(Ok((types, _))) => { + let types = graph_data.follow_redirect(types); + match graph_data.get(&types) { + Some(ModuleEntry::Module { media_type, .. }) => { + Some((types, media_type)) + } + _ => None, + } + } + _ => Some((specifier, media_type)), + }, + _ => None, + } + } + _ => None, + }; + let result = match maybe_result { + Some((specifier, media_type)) => { + let specifier_str = match specifier.scheme() { + "data" | "blob" => { + let specifier_str = hash_url(&specifier, media_type); + state.data_url_map.insert(specifier_str.clone(), specifier); + specifier_str + } + _ => specifier.to_string(), + }; + (specifier_str, media_type.as_ts_extension().into()) + } + None => ( "deno:///missing_dependency.d.ts".to_string(), ".d.ts".to_string(), - ) + ), }; - resolved.push(resolved_dependency); + resolved.push(result); } } @@ -553,7 +575,7 @@ pub(crate) fn exec(request: Request) -> Result { let op_state = runtime.op_state(); let mut op_state = op_state.borrow_mut(); op_state.put(State::new( - request.graph, + request.graph_data, request.hash_data.clone(), request.maybe_config_specifier.clone(), request.maybe_tsbuildinfo.clone(), @@ -656,20 +678,18 @@ mod tests { let hash_data = maybe_hash_data.unwrap_or_else(|| vec![b"".to_vec()]); let fixtures = test_util::testdata_path().join("tsc2"); let mut loader = MockLoader { fixtures }; - let graph = Arc::new( - deno_graph::create_graph( - vec![specifier], - false, - None, - &mut loader, - None, - None, - None, - ) - .await, - ); + let graph = deno_graph::create_graph( + vec![specifier], + false, + None, + &mut loader, + None, + None, + None, + ) + .await; State::new( - graph, + Arc::new(RwLock::new((&graph).into())), hash_data, None, maybe_tsbuildinfo, @@ -684,18 +704,16 @@ mod tests { let hash_data = vec![b"something".to_vec()]; let fixtures = test_util::testdata_path().join("tsc2"); let mut loader = MockLoader { fixtures }; - let graph = Arc::new( - deno_graph::create_graph( - vec![specifier.clone()], - false, - None, - &mut loader, - None, - None, - None, - ) - .await, - ); + let graph = deno_graph::create_graph( + vec![specifier.clone()], + false, + None, + &mut loader, + None, + None, + None, + ) + .await; let config = TsConfig::new(json!({ "allowJs": true, "checkJs": false, @@ -716,7 +734,7 @@ mod tests { let request = Request { config, debug: false, - graph, + graph_data: Arc::new(RwLock::new((&graph).into())), hash_data, maybe_config_specifier: None, maybe_tsbuildinfo: None,