diff --git a/cli/graph_util.rs b/cli/graph_util.rs index 8a68399193..17f7d6739f 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -52,14 +52,14 @@ use std::ops::Deref; use std::path::PathBuf; use std::sync::Arc; -#[derive(Clone, Copy)] +#[derive(Clone)] pub struct GraphValidOptions { pub check_js: bool, pub kind: GraphKind, - pub is_vendoring: bool, - /// Whether to exit the process for lockfile errors. - /// Otherwise, surfaces lockfile errors as errors. - pub exit_lockfile_errors: bool, + /// Whether to exit the process for integrity check errors such as + /// lockfile checksum mismatches and JSR integrity failures. + /// Otherwise, surfaces integrity errors as errors. + pub exit_integrity_errors: bool, } /// Check if `roots` and their deps are available. Returns `Ok(())` if @@ -75,17 +75,54 @@ pub fn graph_valid( roots: &[ModuleSpecifier], options: GraphValidOptions, ) -> Result<(), AnyError> { - if options.exit_lockfile_errors { - graph_exit_lock_errors(graph); + if options.exit_integrity_errors { + graph_exit_integrity_errors(graph); } - let mut errors = graph + let mut errors = graph_walk_errors( + graph, + fs, + roots, + GraphWalkErrorsOptions { + check_js: options.check_js, + kind: options.kind, + }, + ); + if let Some(error) = errors.next() { + Err(error) + } else { + // finally surface the npm resolution result + if let Err(err) = &graph.npm_dep_graph_result { + return Err(custom_error( + get_error_class_name(err), + format_deno_graph_error(err.as_ref().deref()), + )); + } + Ok(()) + } +} + +#[derive(Clone)] +pub struct GraphWalkErrorsOptions { + pub check_js: bool, + pub kind: GraphKind, +} + +/// Walks the errors found in the module graph that should be surfaced to users +/// and enhances them with CLI information. +pub fn graph_walk_errors<'a>( + graph: &'a ModuleGraph, + fs: &'a Arc, + roots: &'a [ModuleSpecifier], + options: GraphWalkErrorsOptions, +) -> impl Iterator + 'a { + graph .walk( roots.iter(), deno_graph::WalkOptions { check_js: options.check_js, kind: options.kind, - follow_dynamic: options.is_vendoring, + follow_dynamic: false, prefer_fast_check_graph: false, }, ) @@ -109,7 +146,7 @@ pub fn graph_valid( ) } ModuleGraphError::ModuleError(error) => { - enhanced_lockfile_error_message(error) + enhanced_integrity_error_message(error) .or_else(|| enhanced_sloppy_imports_error_message(fs, error)) .unwrap_or_else(|| format_deno_graph_error(error)) } @@ -132,56 +169,18 @@ pub fn graph_valid( return None; } - if options.is_vendoring { - // warn about failing dynamic imports when vendoring, but don't fail completely - if matches!( - error, - ModuleGraphError::ModuleError(ModuleError::MissingDynamic(_, _)) - ) { - log::warn!("Ignoring: {}", message); - return None; - } - - // ignore invalid downgrades and invalid local imports when vendoring - match &error { - ModuleGraphError::ResolutionError(err) - | ModuleGraphError::TypesResolutionError(err) => { - if matches!( - err, - ResolutionError::InvalidDowngrade { .. } - | ResolutionError::InvalidLocalImport { .. } - ) { - return None; - } - } - ModuleGraphError::ModuleError(_) => {} - } - } - Some(custom_error(get_error_class_name(&error.into()), message)) - }); - if let Some(error) = errors.next() { - Err(error) - } else { - // finally surface the npm resolution result - if let Err(err) = &graph.npm_dep_graph_result { - return Err(custom_error( - get_error_class_name(err), - format_deno_graph_error(err.as_ref().deref()), - )); - } - Ok(()) - } + }) } -pub fn graph_exit_lock_errors(graph: &ModuleGraph) { +pub fn graph_exit_integrity_errors(graph: &ModuleGraph) { for error in graph.module_errors() { - exit_for_lockfile_error(error); + exit_for_integrity_error(error); } } -fn exit_for_lockfile_error(err: &ModuleError) { - if let Some(err_message) = enhanced_lockfile_error_message(err) { +fn exit_for_integrity_error(err: &ModuleError) { + if let Some(err_message) = enhanced_integrity_error_message(err) { log::error!("{} {}", colors::red("error:"), err_message); std::process::exit(10); } @@ -719,14 +718,13 @@ impl ModuleGraphBuilder { &self.fs, roots, GraphValidOptions { - is_vendoring: false, kind: if self.options.type_check_mode().is_true() { GraphKind::All } else { GraphKind::CodeOnly }, check_js: self.options.check_js(), - exit_lockfile_errors: true, + exit_integrity_errors: true, }, ) } @@ -780,7 +778,7 @@ fn enhanced_sloppy_imports_error_message( } } -fn enhanced_lockfile_error_message(err: &ModuleError) -> Option { +fn enhanced_integrity_error_message(err: &ModuleError) -> Option { match err { ModuleError::LoadingErr( specifier, diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index aed4b47efc..216b14de62 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -274,10 +274,9 @@ impl LanguageServer { factory.fs(), &roots, graph_util::GraphValidOptions { - is_vendoring: false, kind: GraphKind::All, check_js: false, - exit_lockfile_errors: false, + exit_integrity_errors: false, }, )?; diff --git a/cli/tools/doc.rs b/cli/tools/doc.rs index 0ba3b84fbb..8f37632c84 100644 --- a/cli/tools/doc.rs +++ b/cli/tools/doc.rs @@ -7,7 +7,9 @@ use crate::args::Flags; use crate::colors; use crate::display; use crate::factory::CliFactory; -use crate::graph_util::graph_exit_lock_errors; +use crate::graph_util::graph_exit_integrity_errors; +use crate::graph_util::graph_walk_errors; +use crate::graph_util::GraphWalkErrorsOptions; use crate::tsc::get_types_declaration_file_text; use crate::util::fs::collect_specifiers; use deno_ast::diagnostics::Diagnostic; @@ -107,7 +109,7 @@ pub async fn doc( } DocSourceFileFlag::Paths(ref source_files) => { let module_graph_creator = factory.module_graph_creator().await?; - let maybe_lockfile = cli_options.maybe_lockfile(); + let fs = factory.fs(); let module_specifiers = collect_specifiers( FilePatterns { @@ -127,8 +129,18 @@ pub async fn doc( .create_graph(GraphKind::TypesOnly, module_specifiers.clone()) .await?; - if maybe_lockfile.is_some() { - graph_exit_lock_errors(&graph); + graph_exit_integrity_errors(&graph); + let errors = graph_walk_errors( + &graph, + fs, + &module_specifiers, + GraphWalkErrorsOptions { + check_js: false, + kind: GraphKind::TypesOnly, + }, + ); + for error in errors { + log::warn!("{} {}", colors::yellow("Warning"), error); } let doc_parser = doc::DocParser::new( diff --git a/cli/tools/info.rs b/cli/tools/info.rs index 174785631e..b92887d53a 100644 --- a/cli/tools/info.rs +++ b/cli/tools/info.rs @@ -29,7 +29,7 @@ use crate::args::Flags; use crate::args::InfoFlags; use crate::display; use crate::factory::CliFactory; -use crate::graph_util::graph_exit_lock_errors; +use crate::graph_util::graph_exit_integrity_errors; use crate::npm::CliNpmResolver; use crate::npm::ManagedCliNpmResolver; use crate::util::checksum; @@ -75,7 +75,7 @@ pub async fn info( // write out the lockfile if there is one if let Some(lockfile) = &maybe_lockfile { - graph_exit_lock_errors(&graph); + graph_exit_integrity_errors(&graph); lockfile.write_if_changed()?; } diff --git a/tests/specs/permission/allow_import/doc.out b/tests/specs/permission/allow_import/doc.out index bc748d7260..6a57eafe69 100644 --- a/tests/specs/permission/allow_import/doc.out +++ b/tests/specs/permission/allow_import/doc.out @@ -1,4 +1,5 @@ -[# todo(dsherret): we should probably at least show a warning here] +Warning Requires import access to "localhost:4545", run again with the --allow-import flag + at file:///[WILDLINE]/doc.ts:1:15 Defined in file:///[WILDLINE]/doc.ts:3:1 class Test