From 27561dd48542be97e1641ebbe0ae03e4acc0e93e Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Tue, 26 Nov 2024 07:44:54 +0000 Subject: [PATCH] smart diagnostics concatenation --- cli/graph_container.rs | 5 +- cli/graph_util.rs | 1 + cli/module_loader.rs | 3 +- cli/tools/check.rs | 56 +++++++++++++++---- cli/tsc/diagnostics.rs | 2 +- .../check/check_workspace/check_discover.out | 2 + 6 files changed, 54 insertions(+), 15 deletions(-) diff --git a/cli/graph_container.rs b/cli/graph_container.rs index c463d71a6a..ba8b649698 100644 --- a/cli/graph_container.rs +++ b/cli/graph_container.rs @@ -13,6 +13,7 @@ use deno_runtime::deno_permissions::PermissionsContainer; use crate::args::CliOptions; use crate::module_loader::ModuleLoadPreparer; +use crate::tools::check::MaybeDiagnostics; use crate::util::fs::collect_specifiers; use crate::util::path::is_script_ext; @@ -69,7 +70,7 @@ impl MainModuleGraphContainer { &self, specifiers: &[ModuleSpecifier], ext_overwrite: Option<&String>, - ) -> Result<(), AnyError> { + ) -> Result<(), MaybeDiagnostics> { let mut graph_permit = self.acquire_update_permit().await; let graph = graph_permit.graph_mut(); self @@ -99,7 +100,7 @@ impl MainModuleGraphContainer { log::warn!("{} No matching files found.", colors::yellow("Warning")); } - self.check_specifiers(&specifiers, None).await + Ok(self.check_specifiers(&specifiers, None).await?) } pub fn collect_specifiers( diff --git a/cli/graph_util.rs b/cli/graph_util.rs index 6ed0506dd7..059c069c53 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -371,6 +371,7 @@ impl ModuleGraphCreator { }, ) .await + .map_err(AnyError::from) } } diff --git a/cli/module_loader.rs b/cli/module_loader.rs index 035ae4264b..eb7c4cfc30 100644 --- a/cli/module_loader.rs +++ b/cli/module_loader.rs @@ -33,6 +33,7 @@ use crate::resolver::ModuleCodeStringSource; use crate::resolver::NotSupportedKindInNpmError; use crate::resolver::NpmModuleLoader; use crate::tools::check; +use crate::tools::check::MaybeDiagnostics; use crate::tools::check::TypeChecker; use crate::util::progress_bar::ProgressBar; use crate::util::text_encoding::code_without_source_map; @@ -117,7 +118,7 @@ impl ModuleLoadPreparer { lib: TsTypeLib, permissions: PermissionsContainer, ext_overwrite: Option<&String>, - ) -> Result<(), AnyError> { + ) -> Result<(), MaybeDiagnostics> { log::debug!("Preparing module load."); let _pb_clear_guard = self.progress_bar.clear_guard(); diff --git a/cli/tools/check.rs b/cli/tools/check.rs index cad3cc4239..125d58b30f 100644 --- a/cli/tools/check.rs +++ b/cli/tools/check.rs @@ -4,6 +4,8 @@ use std::collections::BTreeMap; use std::collections::BTreeSet; use std::collections::HashSet; use std::collections::VecDeque; +use std::error::Error; +use std::fmt; use std::sync::Arc; use deno_ast::MediaType; @@ -107,7 +109,8 @@ pub async fn check( ); let dir_count = by_workspace_directory.len(); let initial_cwd = cli_options.initial_cwd().to_path_buf(); - let mut check_errors = vec![]; + let mut diagnostics = vec![]; + let mut all_errors = vec![]; let mut found_specifiers = false; for (dir_url, (workspace_directory, patterns)) in by_workspace_directory { let (npmrc, _) = @@ -170,18 +173,24 @@ pub async fn check( .check_specifiers(&specifiers_for_typecheck, None) .await { - check_errors.push(err); + match err { + MaybeDiagnostics::Diagnostics(Diagnostics(d)) => { + diagnostics.extend(d) + } + MaybeDiagnostics::Other(err) => all_errors.push(err), + } } } if !found_specifiers { log::warn!("{} No matching files found.", colors::yellow("Warning")); } - if !check_errors.is_empty() { - // TODO(nayeemrmn): More integrated way of concatenating diagnostics from - // different checks. + if !diagnostics.is_empty() { + all_errors.push(AnyError::from(Diagnostics(diagnostics))); + } + if !all_errors.is_empty() { return Err(anyhow!( "{}", - check_errors + all_errors .into_iter() .map(|e| e.to_string()) .collect::>() @@ -225,9 +234,11 @@ pub async fn check( specifiers }; - main_graph_container - .check_specifiers(&specifiers_for_typecheck, None) - .await + Ok( + main_graph_container + .check_specifiers(&specifiers_for_typecheck, None) + .await?, + ) } /// Options for performing a check of a module graph. Note that the decision to @@ -249,6 +260,29 @@ pub struct CheckOptions { pub type_check_mode: TypeCheckMode, } +#[derive(Debug)] +pub enum MaybeDiagnostics { + Diagnostics(Diagnostics), + Other(AnyError), +} + +impl fmt::Display for MaybeDiagnostics { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + MaybeDiagnostics::Diagnostics(d) => d.fmt(f), + MaybeDiagnostics::Other(err) => err.fmt(f), + } + } +} + +impl Error for MaybeDiagnostics {} + +impl From for MaybeDiagnostics { + fn from(err: AnyError) -> Self { + MaybeDiagnostics::Other(err) + } +} + pub struct TypeChecker { caches: Arc, cjs_tracker: Arc, @@ -285,14 +319,14 @@ impl TypeChecker { &self, graph: ModuleGraph, options: CheckOptions, - ) -> Result, AnyError> { + ) -> Result, MaybeDiagnostics> { let (graph, mut diagnostics) = self.check_diagnostics(graph, options).await?; diagnostics.emit_warnings(); if diagnostics.is_empty() { Ok(graph) } else { - Err(diagnostics.into()) + Err(MaybeDiagnostics::Diagnostics(diagnostics)) } } diff --git a/cli/tsc/diagnostics.rs b/cli/tsc/diagnostics.rs index d3795706eb..1a987f0e0f 100644 --- a/cli/tsc/diagnostics.rs +++ b/cli/tsc/diagnostics.rs @@ -274,7 +274,7 @@ impl fmt::Display for Diagnostic { } #[derive(Clone, Debug, Default, Eq, PartialEq)] -pub struct Diagnostics(Vec); +pub struct Diagnostics(pub Vec); impl Diagnostics { #[cfg(test)] diff --git a/tests/specs/check/check_workspace/check_discover.out b/tests/specs/check/check_workspace/check_discover.out index 14c733c9c0..9a8ce47280 100644 --- a/tests/specs/check/check_workspace/check_discover.out +++ b/tests/specs/check/check_workspace/check_discover.out @@ -7,3 +7,5 @@ TS2304 [ERROR]: Cannot find name 'localStorage'. localStorage; ~~~~~~~~~~~~ at file:///[WILDCARD]/member/mod.ts:2:1 + +Found 2 errors.