diff options
Diffstat (limited to 'cli/tools')
| -rw-r--r-- | cli/tools/doc.rs | 143 | ||||
| -rw-r--r-- | cli/tools/lint.rs | 207 | ||||
| -rw-r--r-- | cli/tools/registry/diagnostics.rs | 152 | ||||
| -rw-r--r-- | cli/tools/registry/graph.rs | 57 | ||||
| -rw-r--r-- | cli/tools/registry/mod.rs | 110 |
5 files changed, 444 insertions, 225 deletions
diff --git a/cli/tools/doc.rs b/cli/tools/doc.rs index 4a59ec986..2cb9ddfba 100644 --- a/cli/tools/doc.rs +++ b/cli/tools/doc.rs @@ -5,6 +5,16 @@ use crate::args::DocHtmlFlag; use crate::args::DocSourceFileFlag; use crate::args::Flags; use crate::colors; +use crate::diagnostics::Diagnostic; +use crate::diagnostics::DiagnosticLevel; +use crate::diagnostics::DiagnosticLocation; +use crate::diagnostics::DiagnosticSnippet; +use crate::diagnostics::DiagnosticSnippetHighlight; +use crate::diagnostics::DiagnosticSnippetHighlightStyle; +use crate::diagnostics::DiagnosticSnippetSource; +use crate::diagnostics::DiagnosticSourcePos; +use crate::diagnostics::DiagnosticSourceRange; +use crate::diagnostics::SourceTextParsedSourceStore; use crate::display::write_json_to_stdout; use crate::display::write_to_stdout_ignore_sigpipe; use crate::factory::CliFactory; @@ -23,7 +33,10 @@ use deno_graph::ModuleAnalyzer; use deno_graph::ModuleParser; use deno_graph::ModuleSpecifier; use doc::DocDiagnostic; +use doc::DocDiagnosticKind; use indexmap::IndexMap; +use lsp_types::Url; +use std::borrow::Cow; use std::collections::BTreeMap; use std::rc::Rc; @@ -129,7 +142,7 @@ pub async fn doc(flags: Flags, doc_flags: DocFlags) -> Result<(), AnyError> { if doc_flags.lint { let diagnostics = doc_parser.take_diagnostics(); - check_diagnostics(&diagnostics)?; + check_diagnostics(&**parsed_source_cache, &diagnostics)?; } doc_nodes_by_url @@ -291,7 +304,118 @@ fn print_docs_to_stdout( write_to_stdout_ignore_sigpipe(details.as_bytes()).map_err(AnyError::from) } -fn check_diagnostics(diagnostics: &[DocDiagnostic]) -> Result<(), AnyError> { +impl Diagnostic for DocDiagnostic { + fn level(&self) -> DiagnosticLevel { + DiagnosticLevel::Error + } + + fn code(&self) -> impl std::fmt::Display + '_ { + match self.kind { + DocDiagnosticKind::MissingJsDoc => "missing-jsdoc", + DocDiagnosticKind::MissingExplicitType => "missing-explicit-type", + DocDiagnosticKind::MissingReturnType => "missing-return-type", + DocDiagnosticKind::PrivateTypeRef { .. } => "private-type-ref", + } + } + + fn message(&self) -> impl std::fmt::Display + '_ { + match &self.kind { + DocDiagnosticKind::MissingJsDoc => { + Cow::Borrowed("exported symbol is missing JSDoc documentation") + } + DocDiagnosticKind::MissingExplicitType => { + Cow::Borrowed("exported symbol is missing an explicit type annotation") + } + DocDiagnosticKind::MissingReturnType => Cow::Borrowed( + "exported function is missing an explicit return type annotation", + ), + DocDiagnosticKind::PrivateTypeRef { + reference, name, .. + } => Cow::Owned(format!( + "public type '{name}' references private type '{reference}'", + )), + } + } + + fn location(&self) -> DiagnosticLocation { + let specifier = Url::parse(&self.location.filename).unwrap(); + DiagnosticLocation::PositionInFile { + specifier: Cow::Owned(specifier), + source_pos: DiagnosticSourcePos::ByteIndex(self.location.byte_index), + } + } + + fn snippet(&self) -> Option<DiagnosticSnippet<'_>> { + let specifier = Url::parse(&self.location.filename).unwrap(); + Some(DiagnosticSnippet { + source: DiagnosticSnippetSource::Specifier(Cow::Owned(specifier)), + highlight: DiagnosticSnippetHighlight { + style: DiagnosticSnippetHighlightStyle::Error, + range: DiagnosticSourceRange { + start: DiagnosticSourcePos::ByteIndex(self.location.byte_index), + end: DiagnosticSourcePos::ByteIndex(self.location.byte_index + 1), + }, + description: None, + }, + }) + } + + fn hint(&self) -> Option<impl std::fmt::Display + '_> { + match &self.kind { + DocDiagnosticKind::PrivateTypeRef { .. } => { + Some("make the referenced type public or remove the reference") + } + _ => None, + } + } + fn snippet_fixed(&self) -> Option<DiagnosticSnippet<'_>> { + match &self.kind { + DocDiagnosticKind::PrivateTypeRef { + reference_location, .. + } => { + let specifier = Url::parse(&reference_location.filename).unwrap(); + Some(DiagnosticSnippet { + source: DiagnosticSnippetSource::Specifier(Cow::Owned(specifier)), + highlight: DiagnosticSnippetHighlight { + style: DiagnosticSnippetHighlightStyle::Hint, + range: DiagnosticSourceRange { + start: DiagnosticSourcePos::ByteIndex( + reference_location.byte_index, + ), + end: DiagnosticSourcePos::ByteIndex( + reference_location.byte_index + 1, + ), + }, + description: Some(Cow::Borrowed("this is the referenced type")), + }, + }) + } + _ => None, + } + } + + fn info(&self) -> std::borrow::Cow<'_, [std::borrow::Cow<'_, str>]> { + match &self.kind { + DocDiagnosticKind::MissingJsDoc => Cow::Borrowed(&[]), + DocDiagnosticKind::MissingExplicitType => Cow::Borrowed(&[]), + DocDiagnosticKind::MissingReturnType => Cow::Borrowed(&[]), + DocDiagnosticKind::PrivateTypeRef { .. } => { + Cow::Borrowed(&[Cow::Borrowed( + "to ensure documentation is complete all types that are exposed in the public API must be public", + )]) + } + } + } + + fn docs_url(&self) -> Option<impl std::fmt::Display + '_> { + None::<&str> + } +} + +fn check_diagnostics( + parsed_source_cache: &dyn deno_graph::ParsedSourceStore, + diagnostics: &[DocDiagnostic], +) -> Result<(), AnyError> { if diagnostics.is_empty() { return Ok(()); } @@ -309,18 +433,13 @@ fn check_diagnostics(diagnostics: &[DocDiagnostic]) -> Result<(), AnyError> { .push(diagnostic); } - for (filename, diagnostics_by_lc) in diagnostic_groups { - for (line, diagnostics_by_col) in diagnostics_by_lc { - for (col, diagnostics) in diagnostics_by_col { + for (_, diagnostics_by_lc) in diagnostic_groups { + for (_, diagnostics_by_col) in diagnostics_by_lc { + for (_, diagnostics) in diagnostics_by_col { for diagnostic in diagnostics { - log::warn!("{}", diagnostic.message()); + let sources = SourceTextParsedSourceStore(parsed_source_cache); + eprintln!("{}", diagnostic.display(&sources)); } - log::warn!( - " at {}:{}:{}\n", - colors::cyan(filename.as_str()), - colors::yellow(&line.to_string()), - colors::yellow(&(col + 1).to_string()) - ) } } } diff --git a/cli/tools/lint.rs b/cli/tools/lint.rs index 20fd12ce2..8de8160de 100644 --- a/cli/tools/lint.rs +++ b/cli/tools/lint.rs @@ -8,6 +8,16 @@ use crate::args::LintOptions; use crate::args::LintReporterKind; use crate::args::LintRulesConfig; use crate::colors; +use crate::diagnostics::Diagnostic; +use crate::diagnostics::DiagnosticLevel; +use crate::diagnostics::DiagnosticLocation; +use crate::diagnostics::DiagnosticSnippet; +use crate::diagnostics::DiagnosticSnippetHighlight; +use crate::diagnostics::DiagnosticSnippetHighlightStyle; +use crate::diagnostics::DiagnosticSnippetSource; +use crate::diagnostics::DiagnosticSourcePos; +use crate::diagnostics::DiagnosticSourceRange; +use crate::diagnostics::SourceTextStore; use crate::factory::CliFactory; use crate::tools::fmt::run_parallelized; use crate::util::file_watcher; @@ -16,22 +26,25 @@ use crate::util::fs::FileCollector; use crate::util::path::is_script_ext; use crate::util::sync::AtomicFlag; use deno_ast::MediaType; +use deno_ast::ModuleSpecifier; +use deno_ast::ParsedSource; +use deno_ast::SourceTextInfo; use deno_config::glob::FilePatterns; use deno_core::anyhow::bail; use deno_core::error::generic_error; use deno_core::error::AnyError; -use deno_core::error::JsStackFrame; use deno_core::serde_json; +use deno_core::url; use deno_lint::diagnostic::LintDiagnostic; use deno_lint::linter::LintFileOptions; use deno_lint::linter::Linter; use deno_lint::linter::LinterBuilder; use deno_lint::rules; use deno_lint::rules::LintRule; -use deno_runtime::fmt_errors::format_location; use log::debug; use log::info; use serde::Serialize; +use std::borrow::Cow; use std::fs; use std::io::stdin; use std::io::Read; @@ -42,7 +55,7 @@ use std::sync::Mutex; use crate::cache::IncrementalCache; -static STDIN_FILE_NAME: &str = "_stdin.ts"; +static STDIN_FILE_NAME: &str = "$deno$stdin.ts"; fn create_reporter(kind: LintReporterKind) -> Box<dyn LintReporter + Send> { match kind { @@ -110,9 +123,10 @@ pub async fn lint(flags: Flags, lint_flags: LintFlags) -> Result<(), AnyError> { let reporter_kind = lint_options.reporter_kind; let reporter_lock = Arc::new(Mutex::new(create_reporter(reporter_kind))); let lint_rules = get_config_rules_err_empty(lint_options.rules)?; - let r = lint_stdin(lint_rules); - let success = - handle_lint_result(STDIN_FILE_NAME, r, reporter_lock.clone()); + let file_path = cli_options.initial_cwd().join(STDIN_FILE_NAME); + let file_path = file_path.to_string_lossy(); + let r = lint_stdin(&file_path, lint_rules); + let success = handle_lint_result(&file_path, r, reporter_lock.clone()); reporter_lock.lock().unwrap().close(1); success } else { @@ -173,10 +187,11 @@ async fn lint_files( } let r = lint_file(&file_path, file_text, lint_rules); - if let Ok((file_diagnostics, file_text)) = &r { + if let Ok((file_diagnostics, file_source)) = &r { if file_diagnostics.is_empty() { // update the incremental cache if there were no diagnostics - incremental_cache.update_file(&file_path, file_text) + incremental_cache + .update_file(&file_path, file_source.text_info().text_str()) } } @@ -262,27 +277,28 @@ fn lint_file( file_path: &Path, source_code: String, lint_rules: Vec<&'static dyn LintRule>, -) -> Result<(Vec<LintDiagnostic>, String), AnyError> { +) -> Result<(Vec<LintDiagnostic>, ParsedSource), AnyError> { let filename = file_path.to_string_lossy().to_string(); let media_type = MediaType::from_path(file_path); let linter = create_linter(lint_rules); - let (_, file_diagnostics) = linter.lint_file(LintFileOptions { + let (source, file_diagnostics) = linter.lint_file(LintFileOptions { filename, media_type, source_code: source_code.clone(), })?; - Ok((file_diagnostics, source_code)) + Ok((file_diagnostics, source)) } /// Lint stdin and write result to stdout. /// Treats input as TypeScript. /// Compatible with `--json` flag. fn lint_stdin( + file_path: &str, lint_rules: Vec<&'static dyn LintRule>, -) -> Result<(Vec<LintDiagnostic>, String), AnyError> { +) -> Result<(Vec<LintDiagnostic>, ParsedSource), AnyError> { let mut source_code = String::new(); if stdin().read_to_string(&mut source_code).is_err() { return Err(generic_error("Failed to read from stdin")); @@ -290,18 +306,18 @@ fn lint_stdin( let linter = create_linter(lint_rules); - let (_, file_diagnostics) = linter.lint_file(LintFileOptions { - filename: STDIN_FILE_NAME.to_string(), + let (source, file_diagnostics) = linter.lint_file(LintFileOptions { + filename: file_path.to_string(), source_code: source_code.clone(), media_type: MediaType::TypeScript, })?; - Ok((file_diagnostics, source_code)) + Ok((file_diagnostics, source)) } fn handle_lint_result( file_path: &str, - result: Result<(Vec<LintDiagnostic>, String), AnyError>, + result: Result<(Vec<LintDiagnostic>, ParsedSource), AnyError>, reporter_lock: Arc<Mutex<Box<dyn LintReporter + Send>>>, ) -> bool { let mut reporter = reporter_lock.lock().unwrap(); @@ -310,7 +326,7 @@ fn handle_lint_result( Ok((mut file_diagnostics, source)) => { sort_diagnostics(&mut file_diagnostics); for d in file_diagnostics.iter() { - reporter.visit_diagnostic(d, source.split('\n').collect()); + reporter.visit_diagnostic(d, &source); } file_diagnostics.is_empty() } @@ -322,7 +338,7 @@ fn handle_lint_result( } trait LintReporter { - fn visit_diagnostic(&mut self, d: &LintDiagnostic, source_lines: Vec<&str>); + fn visit_diagnostic(&mut self, d: &LintDiagnostic, source: &ParsedSource); fn visit_error(&mut self, file_path: &str, err: &AnyError); fn close(&mut self, check_count: usize); } @@ -343,29 +359,77 @@ impl PrettyLintReporter { } } +impl Diagnostic for LintDiagnostic { + fn level(&self) -> DiagnosticLevel { + DiagnosticLevel::Error + } + + fn code(&self) -> impl std::fmt::Display + '_ { + &self.code + } + + fn message(&self) -> impl std::fmt::Display + '_ { + &self.message + } + + fn location(&self) -> DiagnosticLocation { + let specifier = url::Url::from_file_path(&self.filename).unwrap(); + DiagnosticLocation::PositionInFile { + specifier: Cow::Owned(specifier), + source_pos: DiagnosticSourcePos::ByteIndex(self.range.start.byte_index), + } + } + + fn snippet(&self) -> Option<DiagnosticSnippet<'_>> { + let specifier = url::Url::from_file_path(&self.filename).unwrap(); + let range = DiagnosticSourceRange { + start: DiagnosticSourcePos::ByteIndex(self.range.start.byte_index), + end: DiagnosticSourcePos::ByteIndex(self.range.end.byte_index), + }; + Some(DiagnosticSnippet { + source: DiagnosticSnippetSource::Specifier(Cow::Owned(specifier)), + highlight: DiagnosticSnippetHighlight { + range, + style: DiagnosticSnippetHighlightStyle::Error, + description: None, + }, + }) + } + + fn hint(&self) -> Option<impl std::fmt::Display + '_> { + self.hint.as_ref().map(|h| h as &dyn std::fmt::Display) + } + + fn snippet_fixed(&self) -> Option<DiagnosticSnippet<'_>> { + None // todo + } + + fn info(&self) -> Cow<'_, [std::borrow::Cow<'_, str>]> { + Cow::Borrowed(&[]) + } + + fn docs_url(&self) -> Option<impl std::fmt::Display + '_> { + Some(format!("https://lint.deno.land/#{}", &self.code)) + } +} + +struct OneSource<'a>(&'a ParsedSource); + +impl SourceTextStore for OneSource<'_> { + fn get_source_text<'a>( + &'a self, + _specifier: &ModuleSpecifier, + ) -> Option<Cow<'a, SourceTextInfo>> { + Some(Cow::Borrowed(self.0.text_info())) + } +} + impl LintReporter for PrettyLintReporter { - fn visit_diagnostic(&mut self, d: &LintDiagnostic, source_lines: Vec<&str>) { + fn visit_diagnostic(&mut self, d: &LintDiagnostic, source: &ParsedSource) { self.lint_count += 1; - let pretty_message = format!("({}) {}", colors::red(&d.code), &d.message); - - let message = format_diagnostic( - &d.code, - &pretty_message, - &source_lines, - &d.range, - d.hint.as_ref(), - &format_location(&JsStackFrame::from_location( - Some(d.filename.clone()), - // todo(dsherret): these should use "display positions" - // which take into account the added column index of tab - // indentation - Some(d.range.start.line_index as i64 + 1), - Some(d.range.start.column_index as i64 + 1), - )), - ); - - eprintln!("{message}\n"); + let sources = OneSource(source); + eprintln!("{}", d.display(&sources)); } fn visit_error(&mut self, file_path: &str, err: &AnyError) { @@ -399,7 +463,7 @@ impl CompactLintReporter { } impl LintReporter for CompactLintReporter { - fn visit_diagnostic(&mut self, d: &LintDiagnostic, _source_lines: Vec<&str>) { + fn visit_diagnostic(&mut self, d: &LintDiagnostic, _source: &ParsedSource) { self.lint_count += 1; eprintln!( @@ -432,69 +496,6 @@ impl LintReporter for CompactLintReporter { } } -pub fn format_diagnostic( - diagnostic_code: &str, - message_line: &str, - source_lines: &[&str], - range: &deno_lint::diagnostic::Range, - maybe_hint: Option<&String>, - formatted_location: &str, -) -> String { - let mut lines = vec![]; - - for (i, line) in source_lines - .iter() - .enumerate() - .take(range.end.line_index + 1) - .skip(range.start.line_index) - { - lines.push(line.to_string()); - if range.start.line_index == range.end.line_index { - lines.push(format!( - "{}{}", - " ".repeat(range.start.column_index), - colors::red( - &"^".repeat(range.end.column_index - range.start.column_index) - ) - )); - } else { - let line_len = line.len(); - if range.start.line_index == i { - lines.push(format!( - "{}{}", - " ".repeat(range.start.column_index), - colors::red(&"^".repeat(line_len - range.start.column_index)) - )); - } else if range.end.line_index == i { - lines - .push(colors::red(&"^".repeat(range.end.column_index)).to_string()); - } else if line_len != 0 { - lines.push(colors::red(&"^".repeat(line_len)).to_string()); - } - } - } - - let hint = if let Some(hint) = maybe_hint { - format!(" {} {}\n", colors::cyan("hint:"), hint) - } else { - "".to_string() - }; - let help = format!( - " {} for further information visit https://lint.deno.land/#{}", - colors::cyan("help:"), - diagnostic_code - ); - - format!( - "{message_line}\n{snippets}\n at {formatted_location}\n\n{hint}{help}", - message_line = message_line, - snippets = lines.join("\n"), - formatted_location = formatted_location, - hint = hint, - help = help - ) -} - #[derive(Serialize)] struct JsonLintReporter { diagnostics: Vec<LintDiagnostic>, @@ -511,7 +512,7 @@ impl JsonLintReporter { } impl LintReporter for JsonLintReporter { - fn visit_diagnostic(&mut self, d: &LintDiagnostic, _source_lines: Vec<&str>) { + fn visit_diagnostic(&mut self, d: &LintDiagnostic, _source: &ParsedSource) { self.diagnostics.push(d.clone()); } diff --git a/cli/tools/registry/diagnostics.rs b/cli/tools/registry/diagnostics.rs new file mode 100644 index 000000000..f6d81f5a8 --- /dev/null +++ b/cli/tools/registry/diagnostics.rs @@ -0,0 +1,152 @@ +// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. + +use std::borrow::Cow; +use std::fmt::Display; +use std::sync::Arc; +use std::sync::Mutex; + +use deno_ast::swc::common::util::take::Take; +use deno_core::anyhow::anyhow; +use deno_core::error::AnyError; +use deno_graph::FastCheckDiagnostic; +use deno_graph::ParsedSourceStore; + +use crate::diagnostics::Diagnostic; +use crate::diagnostics::DiagnosticLevel; +use crate::diagnostics::DiagnosticLocation; +use crate::diagnostics::DiagnosticSnippet; +use crate::diagnostics::DiagnosticSnippetHighlight; +use crate::diagnostics::DiagnosticSnippetHighlightStyle; +use crate::diagnostics::DiagnosticSnippetSource; +use crate::diagnostics::DiagnosticSourcePos; +use crate::diagnostics::DiagnosticSourceRange; +use crate::diagnostics::SourceTextParsedSourceStore; + +#[derive(Clone, Default)] +pub struct PublishDiagnosticsCollector { + diagnostics: Arc<Mutex<Vec<PublishDiagnostic>>>, +} + +impl PublishDiagnosticsCollector { + pub fn print_and_error( + &self, + sources: &dyn ParsedSourceStore, + ) -> Result<(), AnyError> { + let mut errors = 0; + let diagnostics = self.diagnostics.lock().unwrap().take(); + let sources = SourceTextParsedSourceStore(sources); + for diagnostic in diagnostics { + eprintln!("{}", diagnostic.display(&sources)); + if matches!(diagnostic.level(), DiagnosticLevel::Error) { + errors += 1; + } + } + if errors > 0 { + Err(anyhow!( + "Found {} problem{}", + errors, + if errors == 1 { "" } else { "s" } + )) + } else { + Ok(()) + } + } + + pub fn push(&self, diagnostic: PublishDiagnostic) { + self.diagnostics.lock().unwrap().push(diagnostic); + } +} + +pub enum PublishDiagnostic { + FastCheck { diagnostic: FastCheckDiagnostic }, +} + +impl Diagnostic for PublishDiagnostic { + fn level(&self) -> DiagnosticLevel { + match self { + PublishDiagnostic::FastCheck { + diagnostic: FastCheckDiagnostic::UnsupportedJavaScriptEntrypoint { .. }, + } => DiagnosticLevel::Warning, + PublishDiagnostic::FastCheck { .. } => DiagnosticLevel::Error, + } + } + + fn code(&self) -> impl Display + '_ { + match &self { + PublishDiagnostic::FastCheck { diagnostic, .. } => diagnostic.code(), + } + } + + fn message(&self) -> impl Display + '_ { + match &self { + PublishDiagnostic::FastCheck { diagnostic, .. } => diagnostic.to_string(), // todo + } + } + + fn location(&self) -> DiagnosticLocation { + match &self { + PublishDiagnostic::FastCheck { diagnostic } => match diagnostic.range() { + Some(range) => DiagnosticLocation::PositionInFile { + specifier: Cow::Borrowed(diagnostic.specifier()), + source_pos: DiagnosticSourcePos::SourcePos(range.range.start), + }, + None => DiagnosticLocation::File { + specifier: Cow::Borrowed(diagnostic.specifier()), + }, + }, + } + } + + fn snippet(&self) -> Option<DiagnosticSnippet<'_>> { + match &self { + PublishDiagnostic::FastCheck { diagnostic } => { + diagnostic.range().map(|range| DiagnosticSnippet { + source: DiagnosticSnippetSource::Specifier(Cow::Borrowed( + diagnostic.specifier(), + )), + highlight: DiagnosticSnippetHighlight { + style: DiagnosticSnippetHighlightStyle::Error, + range: DiagnosticSourceRange { + start: DiagnosticSourcePos::SourcePos(range.range.start), + end: DiagnosticSourcePos::SourcePos(range.range.end), + }, + description: diagnostic.range_description().map(Cow::Borrowed), + }, + }) + } + } + } + + fn hint(&self) -> Option<impl Display + '_> { + match &self { + PublishDiagnostic::FastCheck { diagnostic } => { + Some(diagnostic.fix_hint()) + } + } + } + + fn snippet_fixed(&self) -> Option<DiagnosticSnippet<'_>> { + None + } + + fn info(&self) -> Cow<'_, [Cow<'_, str>]> { + match &self { + PublishDiagnostic::FastCheck { diagnostic } => { + let infos = diagnostic + .additional_info() + .iter() + .map(|s| Cow::Borrowed(*s)) + .collect(); + Cow::Owned(infos) + } + } + } + + fn docs_url(&self) -> Option<impl Display + '_> { + match &self { + PublishDiagnostic::FastCheck { diagnostic } => { + Some(format!("https://jsr.io/go/{}", diagnostic.code())) + } + } + } +} diff --git a/cli/tools/registry/graph.rs b/cli/tools/registry/graph.rs index b64350887..29c825242 100644 --- a/cli/tools/registry/graph.rs +++ b/cli/tools/registry/graph.rs @@ -12,6 +12,9 @@ use deno_core::error::AnyError; use deno_graph::FastCheckDiagnostic; use deno_graph::ModuleGraph; +use super::diagnostics::PublishDiagnostic; +use super::diagnostics::PublishDiagnosticsCollector; + #[derive(Debug)] pub struct MemberRoots { pub name: String, @@ -61,11 +64,13 @@ pub fn resolve_config_file_roots_from_exports( Ok(exports) } -pub fn surface_fast_check_type_graph_errors( +/// Collects diagnostics from the module graph for the given packages. +/// Returns true if any diagnostics were collected. +pub fn collect_fast_check_type_graph_diagnostics( graph: &ModuleGraph, packages: &[MemberRoots], -) -> Result<(), AnyError> { - let mut diagnostic_count = 0; + diagnostics_collector: &PublishDiagnosticsCollector, +) -> bool { let mut seen_diagnostics = HashSet::new(); let mut seen_modules = HashSet::with_capacity(graph.specifiers_count()); for package in packages { @@ -85,31 +90,18 @@ pub fn surface_fast_check_type_graph_errors( }; if let Some(diagnostic) = esm_module.fast_check_diagnostic() { for diagnostic in diagnostic.flatten_multiple() { + if !seen_diagnostics.insert(diagnostic.message_with_range_for_test()) + { + continue; + } + diagnostics_collector.push(PublishDiagnostic::FastCheck { + diagnostic: diagnostic.clone(), + }); if matches!( diagnostic, FastCheckDiagnostic::UnsupportedJavaScriptEntrypoint { .. } ) { - // ignore JS packages for fast check - log::warn!( - concat!( - "{} Package '{}' is a JavaScript package without a corresponding ", - "declaration file. This may lead to a non-optimal experience for ", - "users of your package. For performance reasons, it's recommended ", - "to ship a corresponding TypeScript declaration file or to ", - "convert to TypeScript.", - ), - deno_runtime::colors::yellow("Warning"), - package.name, - ); break 'analyze_package; // no need to keep analyzing this package - } else { - let message = diagnostic.message_with_range_for_test(); - if !seen_diagnostics.insert(message.clone()) { - continue; - } - - log::error!("\n{}", message); - diagnostic_count += 1; } } } @@ -133,22 +125,5 @@ pub fn surface_fast_check_type_graph_errors( } } - if diagnostic_count > 0 { - // for the time being, tell the user why we have these errors and the benefit they bring - log::error!( - concat!( - "\nFixing these fast check errors is required to make the code fast check compatible ", - "which enables type checking your package's TypeScript code with the same ", - "performance as if you had distributed declaration files. Do any of these ", - "errors seem too restrictive or incorrect? Please open an issue if so to ", - "help us improve: https://github.com/denoland/deno/issues\n", - ) - ); - bail!( - "Had {} fast check error{}.", - diagnostic_count, - if diagnostic_count == 1 { "" } else { "s" } - ) - } - Ok(()) + !seen_diagnostics.is_empty() } diff --git a/cli/tools/registry/mod.rs b/cli/tools/registry/mod.rs index 3b8ffcd04..8eaf61258 100644 --- a/cli/tools/registry/mod.rs +++ b/cli/tools/registry/mod.rs @@ -32,15 +32,17 @@ use crate::factory::CliFactory; use crate::graph_util::ModuleGraphBuilder; use crate::http_util::HttpClient; use crate::tools::check::CheckOptions; +use crate::tools::registry::diagnostics::PublishDiagnosticsCollector; +use crate::tools::registry::graph::collect_fast_check_type_graph_diagnostics; use crate::tools::registry::graph::get_workspace_member_roots; use crate::tools::registry::graph::resolve_config_file_roots_from_exports; -use crate::tools::registry::graph::surface_fast_check_type_graph_errors; use crate::tools::registry::graph::MemberRoots; use crate::util::display::human_size; use crate::util::import_map::ImportMapUnfurler; mod api; mod auth; +mod diagnostics; mod graph; mod publish_order; mod tar; @@ -166,47 +168,6 @@ pub enum Permission<'s> { }, } -/// Prints diagnostics like so: -/// ``` -/// -/// Warning -/// ├╌ Dynamic import was not analyzable... -/// ├╌╌ at file:///dev/foo/bar/foo.ts:4:5 -/// | -/// ├╌ Dynamic import was not analyzable... -/// ├╌╌ at file:///dev/foo/bar/foo.ts:4:5 -/// | -/// ├╌ Dynamic import was not analyzable... -/// └╌╌ at file:///dev/foo/bar/foo.ts:4:5 -/// -/// ``` -fn print_diagnostics(diagnostics: &[String]) { - if !diagnostics.is_empty() { - let len = diagnostics.len(); - log::warn!(""); - log::warn!("{}", crate::colors::yellow("Warning")); - for (i, diagnostic) in diagnostics.iter().enumerate() { - let last_diagnostic = i == len - 1; - let lines = diagnostic.split('\n').collect::<Vec<_>>(); - let lines_len = lines.len(); - if i != 0 { - log::warn!("|"); - } - for (j, line) in lines.iter().enumerate() { - let last_line = j == lines_len - 1; - if j == 0 { - log::warn!("├╌ {}", line); - } else if last_line && last_diagnostic { - log::warn!("└╌╌ {}", line); - } else { - log::warn!("├╌╌ {}", line); - } - } - } - log::warn!(""); - } -} - async fn get_auth_headers( client: &reqwest::Client, registry_url: String, @@ -484,11 +445,6 @@ async fn perform_publish( .values() .cloned() .collect::<Vec<_>>(); - let diagnostics = packages - .iter() - .flat_map(|p| p.tarball.diagnostics.iter().cloned()) - .collect::<Vec<_>>(); - print_diagnostics(&diagnostics); ensure_scopes_and_packages_exist( client, @@ -679,6 +635,7 @@ async fn publish_package( async fn prepare_packages_for_publishing( cli_factory: &CliFactory, + diagnostics_collector: &PublishDiagnosticsCollector, deno_json: ConfigFile, import_map: Arc<ImportMap>, ) -> Result< @@ -700,6 +657,7 @@ async fn prepare_packages_for_publishing( module_graph_builder, type_checker, cli_options, + diagnostics_collector, &[MemberRoots { name: get_deno_json_package_name(&deno_json)?, dir_url: deno_json.specifier.join("./").unwrap().clone(), @@ -724,6 +682,7 @@ async fn prepare_packages_for_publishing( module_graph_builder, type_checker, cli_options, + diagnostics_collector, &roots, ) .await?; @@ -765,6 +724,7 @@ async fn build_and_check_graph_for_publish( module_graph_builder: &ModuleGraphBuilder, type_checker: &TypeChecker, cli_options: &CliOptions, + diagnostics_collector: &PublishDiagnosticsCollector, packages: &[MemberRoots], ) -> Result<Arc<deno_graph::ModuleGraph>, deno_core::anyhow::Error> { let graph = Arc::new( @@ -784,28 +744,34 @@ async fn build_and_check_graph_for_publish( ); graph.valid()?; log::info!("Checking fast check type graph for errors..."); - surface_fast_check_type_graph_errors(&graph, packages)?; - log::info!("Ensuring type checks..."); - let diagnostics = type_checker - .check_diagnostics( - graph.clone(), - CheckOptions { - lib: cli_options.ts_type_lib_window(), - log_ignored_options: false, - reload: cli_options.reload_flag(), - }, - ) - .await?; - if !diagnostics.is_empty() { - bail!( - concat!( - "{:#}\n\n", - "You may have discovered a bug in Deno's fast check implementation. ", - "Fast check is still early days and we would appreciate if you log a ", - "bug if you believe this is one: https://github.com/denoland/deno/issues/" - ), - diagnostics - ); + let has_fast_check_diagnostics = collect_fast_check_type_graph_diagnostics( + &graph, + packages, + diagnostics_collector, + ); + if !has_fast_check_diagnostics { + log::info!("Ensuring type checks..."); + let diagnostics = type_checker + .check_diagnostics( + graph.clone(), + CheckOptions { + lib: cli_options.ts_type_lib_window(), + log_ignored_options: false, + reload: cli_options.reload_flag(), + }, + ) + .await?; + if !diagnostics.is_empty() { + bail!( + concat!( + "{:#}\n\n", + "You may have discovered a bug in Deno's fast check implementation. ", + "Fast check is still early days and we would appreciate if you log a ", + "bug if you believe this is one: https://github.com/denoland/deno/issues/" + ), + diagnostics + ); + } } Ok(graph) } @@ -836,14 +802,20 @@ pub async fn publish( ); }; + let diagnostics_collector = PublishDiagnosticsCollector::default(); + let (publish_order_graph, prepared_package_by_name) = prepare_packages_for_publishing( &cli_factory, + &diagnostics_collector, config_file.clone(), import_map, ) .await?; + diagnostics_collector + .print_and_error(&**cli_factory.parsed_source_cache())?; + if prepared_package_by_name.is_empty() { bail!("No packages to publish"); } |