diff --git a/Cargo.lock b/Cargo.lock index a83b5138706944..4c3440e8c2a9d4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1037,6 +1037,7 @@ dependencies = [ "trust-dns-server", "twox-hash", "typed-arena", + "unicode-width", "uuid", "walkdir", "winapi", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 6fdf7c51a262ca..1a74469eab75ad 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -131,6 +131,7 @@ tokio-util.workspace = true tower-lsp.workspace = true twox-hash = "=1.6.3" typed-arena = "=2.0.1" +unicode-width = "0.1" uuid = { workspace = true, features = ["serde"] } walkdir = "=2.3.2" zeromq = { version = "=0.3.4", default-features = false, features = ["tcp-transport", "tokio-runtime"] } diff --git a/cli/diagnostics.rs b/cli/diagnostics.rs new file mode 100644 index 00000000000000..9a6994df73dc14 --- /dev/null +++ b/cli/diagnostics.rs @@ -0,0 +1,642 @@ +use std::borrow::Cow; +use std::fmt; +use std::fmt::Display; +use std::fmt::Write as _; + +use deno_ast::ModuleSpecifier; +use deno_ast::SourcePos; +use deno_ast::SourceRange; +use deno_ast::SourceRanged; +use deno_ast::SourceTextInfo; +use deno_graph::ParsedSourceStore; +use deno_runtime::colors; +use unicode_width::UnicodeWidthStr; + +pub trait SourceTextStore { + fn get_source_text<'a>( + &'a self, + specifier: &ModuleSpecifier, + ) -> Option>; +} + +pub struct SourceTextParsedSourceStore<'a>(pub &'a dyn ParsedSourceStore); + +impl SourceTextStore for SourceTextParsedSourceStore<'_> { + fn get_source_text<'a>( + &'a self, + specifier: &ModuleSpecifier, + ) -> Option> { + let parsed_source = self.0.get_parsed_source(specifier)?; + Some(Cow::Owned(parsed_source.text_info().clone())) + } +} + +pub enum DiagnosticLevel { + Error, + Warning, +} + +#[derive(Clone, Copy, Debug)] +pub struct DiagnosticSourceRange { + pub start: DiagnosticSourcePos, + pub end: DiagnosticSourcePos, +} + +#[derive(Clone, Copy, Debug)] +pub enum DiagnosticSourcePos { + SourcePos(SourcePos), + ByteIndex(usize), +} + +impl DiagnosticSourcePos { + fn pos(&self, source: &SourceTextInfo) -> SourcePos { + match self { + DiagnosticSourcePos::SourcePos(pos) => *pos, + DiagnosticSourcePos::ByteIndex(index) => source.range().start() + *index, + } + } +} + +#[derive(Clone, Debug)] +pub enum DiagnosticLocation<'a> { + /// The diagnostic is relevant to an entire file. + File { + /// The specifier of the module that contains the diagnostic. + specifier: Cow<'a, ModuleSpecifier>, + }, + /// The diagnostic is relevant to a specific position in a file. + /// + /// This variant will get the relevant `SouceTextInfo` from the cache using + /// the given specifier, and will then calculate the line and column numbers + /// from the given `SourcePos`. + PositionInFile { + /// The specifier of the module that contains the diagnostic. + specifier: Cow<'a, ModuleSpecifier>, + /// The source position of the diagnostic. + source_pos: DiagnosticSourcePos, + }, +} + +impl<'a> DiagnosticLocation<'a> { + fn specifier(&self) -> &ModuleSpecifier { + match self { + DiagnosticLocation::File { specifier } => specifier, + DiagnosticLocation::PositionInFile { specifier, .. } => specifier, + } + } + + /// Return the line and column number of the diagnostic. + /// + /// The line number is 1-indexed. + /// + /// The column number is 1-indexed. This is the number of UTF-16 code units + /// from the start of the line to the diagnostic. + /// Why UTF-16 code units? Because that's what VS Code understands, and + /// everyone uses VS Code. :) + fn position(&self, sources: &dyn SourceTextStore) -> Option<(usize, usize)> { + match self { + DiagnosticLocation::File { .. } => None, + DiagnosticLocation::PositionInFile { + specifier, + source_pos, + } => { + let source = sources.get_source_text(specifier).expect( + "source text should be in the cache if the location is in a file", + ); + let pos = source_pos.pos(&source); + let line_index = source.line_index(pos); + let line_start_pos = source.line_start(line_index); + let content = source.range_text(&SourceRange::new(line_start_pos, pos)); + let line = line_index + 1; + let column = content.encode_utf16().count() + 1; + Some((line, column)) + } + } + } +} + +pub struct DiagnosticSnippet<'a> { + /// The source text for this snippet. The + pub source: DiagnosticSnippetSource<'a>, + /// The piece of the snippet that should be highlighted. + pub highlight: DiagnosticSnippetHighlight<'a>, +} + +pub struct DiagnosticSnippetHighlight<'a> { + /// The range of the snippet that should be highlighted. + pub range: DiagnosticSourceRange, + /// The style of the highlight. + pub style: DiagnosticSnippetHighlightStyle, + /// An optional inline description of the highlight. + pub description: Option>, +} + +pub enum DiagnosticSnippetHighlightStyle { + /// The highlight is an error. This will place red carets under the highlight. + Error, + #[allow(dead_code)] + /// The highlight is a warning. This will place yellow carets under the + /// highlight. + Warning, + #[allow(dead_code)] + /// The highlight shows code additions. This will place green + signs under + /// the highlight and will highlight the code in green. + Addition, + /// The highlight shows a hint. This will place blue dashes under the + /// highlight. + Hint, +} + +impl DiagnosticSnippetHighlightStyle { + fn style_underline( + &self, + s: impl std::fmt::Display, + ) -> impl std::fmt::Display { + match self { + DiagnosticSnippetHighlightStyle::Error => colors::red_bold(s), + DiagnosticSnippetHighlightStyle::Warning => colors::yellow_bold(s), + DiagnosticSnippetHighlightStyle::Addition => colors::green_bold(s), + DiagnosticSnippetHighlightStyle::Hint => colors::intense_blue(s), + } + } + + fn underline_char(&self) -> char { + match self { + DiagnosticSnippetHighlightStyle::Error => '^', + DiagnosticSnippetHighlightStyle::Warning => '^', + DiagnosticSnippetHighlightStyle::Addition => '+', + DiagnosticSnippetHighlightStyle::Hint => '-', + } + } +} + +pub enum DiagnosticSnippetSource<'a> { + /// The specifier of the module that should be displayed in this snippet. The + /// contents of the file will be retrieved from the `SourceTextStore`. + Specifier(Cow<'a, ModuleSpecifier>), + #[allow(dead_code)] + /// The source text that should be displayed in this snippet. + /// + /// This should be used if the text of the snippet is not available in the + /// `SourceTextStore`. + SourceTextInfo(Cow<'a, deno_ast::SourceTextInfo>), +} + +impl<'a> DiagnosticSnippetSource<'a> { + fn to_source_text_info( + &self, + sources: &'a dyn SourceTextStore, + ) -> Cow<'a, SourceTextInfo> { + match self { + DiagnosticSnippetSource::Specifier(specifier) => { + sources.get_source_text(specifier).expect( + "source text should be in the cache if snippet source is a specifier", + ) + } + DiagnosticSnippetSource::SourceTextInfo(info) => info.clone(), + } + } +} + +/// Returns the text of the line with the given number. +fn line_text(source: &SourceTextInfo, line_number: usize) -> &str { + source.line_text(line_number - 1) +} + +/// Returns the text of the line that contains the given position, split at the +/// given position. +fn line_text_split( + source: &SourceTextInfo, + pos: DiagnosticSourcePos, +) -> (&str, &str) { + let pos = pos.pos(&source); + let line_index = source.line_index(pos); + let line_start_pos = source.line_start(line_index); + let line_end_pos = source.line_end(line_index); + let before = source.range_text(&SourceRange::new(line_start_pos, pos)); + let after = source.range_text(&SourceRange::new(pos, line_end_pos)); + (before, after) +} + +/// Returns the text of the line that contains the given positions, split at the +/// given positions. +/// +/// If the positions are on different lines, this will panic. +fn line_text_split3( + source: &SourceTextInfo, + start_pos: DiagnosticSourcePos, + end_pos: DiagnosticSourcePos, +) -> (&str, &str, &str) { + let start_pos = start_pos.pos(&source); + let end_pos = end_pos.pos(&source); + let line_index = source.line_index(start_pos); + assert_eq!( + line_index, + source.line_index(end_pos), + "start and end must be on the same line" + ); + let line_start_pos = source.line_start(line_index); + let line_end_pos = source.line_end(line_index); + let before = source.range_text(&SourceRange::new(line_start_pos, start_pos)); + let between = source.range_text(&SourceRange::new(start_pos, end_pos)); + let after = source.range_text(&SourceRange::new(end_pos, line_end_pos)); + (before, between, after) +} + +/// Returns the line number (1 indexed) of the line that contains the given +/// position. +fn line_number(source: &SourceTextInfo, pos: DiagnosticSourcePos) -> usize { + source.line_index(pos.pos(&source)) + 1 +} + +pub trait Diagnostic { + /// The level of the diagnostic. + fn level(&self) -> DiagnosticLevel; + + /// The diagnostic code, like `no-explicit-any` or `ban-untagged-ignore`. + fn code(&self) -> impl fmt::Display + '_; + + /// The human-readable diagnostic message. + fn message(&self) -> impl fmt::Display + '_; + + /// The location this diagnostic is associated with. + fn location(&self) -> DiagnosticLocation; + + /// A snippet showing the source code associated with the diagnostic. + fn snippet(&self) -> Option>; + + /// A hint for fixing the diagnostic. + fn hint(&self) -> Option; + + /// A snippet showing how the diagnostic can be fixed. + fn snippet_fixed(&self) -> Option>; + + fn info(&self) -> Cow<'_, [Cow<'_, str>]>; + + /// An optional URL to the documentation for the diagnostic. + fn docs_url(&self) -> Option; + + fn display<'a>( + &'a self, + sources: &'a dyn SourceTextStore, + ) -> DiagnosticDisplay<'a, Self> { + DiagnosticDisplay { + diagnostic: self, + sources, + } + } +} + +struct RepeatingCharFmt(char, usize); +impl fmt::Display for RepeatingCharFmt { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + for _ in 0..self.1 { + f.write_char(self.0)?; + } + Ok(()) + } +} + +/// How many spaces a tab should be displayed as. 2 is the default used for +/// `deno fmt`, so we'll use that here. +const TAB_WIDTH: usize = 2; + +struct ReplaceTab<'a>(&'a str); +impl fmt::Display for ReplaceTab<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut written = 0; + for (i, c) in self.0.char_indices() { + if c == '\t' { + self.0[written..i].fmt(f)?; + RepeatingCharFmt(' ', TAB_WIDTH).fmt(f)?; + written = i + 1; + } + } + self.0[written..].fmt(f)?; + Ok(()) + } +} + +/// The width of the string as displayed, assuming tabs are 2 spaces wide. +/// +/// This display width assumes that zero-width-joined characters are the width +/// of their consituent characters. This means that "Person: Red Hair" (which is +/// represented as "Person" + "ZWJ" + "Red Hair") will have a width of 4. +/// +/// Whether this is correct is unfortunately dependent on the font / terminal +/// being used. Here is a list of what terminals consider the length of +/// "Person: Red Hair" to be: +/// +/// | Terminal | Rendered Width | +/// | ---------------- | -------------- | +/// | Windows Terminal | 5 chars | +/// | iTerm (macOS) | 2 chars | +/// | Terminal (macOS) | 2 chars | +/// | VS Code terminal | 4 chars | +/// | GNOME Terminal | 4 chars | +/// +/// If we really wanted to, we could try and detect the terminal being used and +/// adjust the width accordingly. However, this is probably not worth the +/// effort. +fn display_width(str: &str) -> usize { + str.width_cjk() + (str.chars().filter(|c| *c == '\t').count() * TAB_WIDTH) +} + +pub struct DiagnosticDisplay<'a, T: Diagnostic + ?Sized> { + diagnostic: &'a T, + sources: &'a dyn SourceTextStore, +} + +impl Display for DiagnosticDisplay<'_, T> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + print_diagnostic(f, self.sources, self.diagnostic) + } +} + +// error[missing-return-type]: missing explicit return type on public function +// at /mnt/artemis/Projects/github.com/denoland/deno/test.ts:1:16 +// | +// 1 | export function test() { +// | ^^^^ +// = hint: add an explicit return type to the function +// | +// 1 | export function test(): string { +// | ^^^^^^^^ +// +// info: all functions that are exported from a module must have an explicit return type to support fast check and documentation generation. +// docs: https://jsr.io/d/missing-return-type +fn print_diagnostic( + io: &mut dyn std::fmt::Write, + sources: &dyn SourceTextStore, + diagnostic: &(impl Diagnostic + ?Sized), +) -> Result<(), std::fmt::Error> { + match diagnostic.level() { + DiagnosticLevel::Error => { + write!( + io, + "{}", + colors::red_bold(format_args!("error[{}]", diagnostic.code())) + )?; + } + DiagnosticLevel::Warning => { + write!( + io, + "{}", + colors::yellow(format_args!("warning[{}]", diagnostic.code())) + )?; + } + } + + writeln!(io, ": {}", colors::bold(diagnostic.message()))?; + + let mut max_line_number_digits = 1; + if let Some(snippet) = diagnostic.snippet() { + let source = snippet.source.to_source_text_info(sources); + let last_line = line_number(&source, snippet.highlight.range.end); + max_line_number_digits = max_line_number_digits.max(last_line.ilog10() + 1); + } + if let Some(snippet) = diagnostic.snippet_fixed() { + let source = snippet.source.to_source_text_info(sources); + let last_line = line_number(&source, snippet.highlight.range.end); + max_line_number_digits = max_line_number_digits.max(last_line.ilog10() + 1); + } + + let location = diagnostic.location(); + write!( + io, + "{}{}", + RepeatingCharFmt(' ', max_line_number_digits as usize), + colors::intense_blue("-->"), + )?; + let location_specifier = location.specifier(); + if let Ok(path) = location_specifier.to_file_path() { + write!(io, " {}", colors::cyan(path.display()))?; + } else { + write!(io, " {}", colors::cyan(location_specifier.as_str()))?; + } + if let Some((line, column)) = location.position(sources) { + write!( + io, + "{}", + colors::yellow(format_args!(":{}:{}", line, column)) + )?; + } + writeln!(io)?; + + if let Some(snippet) = diagnostic.snippet() { + let indent = print_snippet(io, sources, &snippet, max_line_number_digits)?; + Some(indent) + } else { + None + }; + + if let Some(hint) = diagnostic.hint() { + write!( + io, + "{} {} ", + RepeatingCharFmt(' ', max_line_number_digits as usize), + colors::intense_blue("=") + )?; + writeln!(io, "{}: {}", colors::bold("hint"), hint)?; + } + + if let Some(snippet) = diagnostic.snippet_fixed() { + print_snippet(io, sources, &snippet, max_line_number_digits)?; + } + + writeln!(io)?; + + let mut needs_final_newline = false; + for info in diagnostic.info().iter() { + needs_final_newline = true; + writeln!(io, " {}: {}", colors::intense_blue("info"), info)?; + } + if let Some(docs_url) = diagnostic.docs_url() { + needs_final_newline = true; + writeln!(io, " {}: {}", colors::intense_blue("docs"), docs_url)?; + } + + if needs_final_newline { + writeln!(io)?; + } + + Ok(()) +} + +/// Prints a snippet to the given writer and returns the line number indent. +fn print_snippet( + io: &mut dyn std::fmt::Write, + sources: &dyn SourceTextStore, + snippet: &DiagnosticSnippet<'_>, + max_line_number_digits: u32, +) -> Result<(), std::fmt::Error> { + let DiagnosticSnippet { source, highlight } = snippet; + + fn print_padded( + io: &mut dyn std::fmt::Write, + text: impl std::fmt::Display, + padding: u32, + ) -> Result<(), std::fmt::Error> { + for _ in 0..padding { + write!(io, " ")?; + } + write!(io, "{}", text)?; + Ok(()) + } + + let source = source.to_source_text_info(sources); + + let start_line_number = line_number(&source, highlight.range.start); + let end_line_number = line_number(&source, highlight.range.end); + + print_padded(io, colors::intense_blue(" | "), max_line_number_digits)?; + writeln!(io)?; + for line_number in start_line_number..=end_line_number { + print_padded( + io, + colors::intense_blue(format_args!("{} | ", line_number)), + max_line_number_digits - line_number.ilog10() - 1, + )?; + + let padding_width; + let highlight_width; + if line_number == start_line_number && start_line_number == end_line_number + { + let (before, between, after) = + line_text_split3(&source, highlight.range.start, highlight.range.end); + write!(io, "{}", ReplaceTab(before))?; + match highlight.style { + DiagnosticSnippetHighlightStyle::Addition => { + write!(io, "{}", colors::green(ReplaceTab(between)))?; + } + _ => { + write!(io, "{}", ReplaceTab(between))?; + } + } + writeln!(io, "{}", ReplaceTab(after))?; + padding_width = display_width(before); + highlight_width = display_width(between); + } else if line_number == start_line_number { + let (before, after) = line_text_split(&source, highlight.range.start); + write!(io, "{}", ReplaceTab(before))?; + match highlight.style { + DiagnosticSnippetHighlightStyle::Addition => { + write!(io, "{}", colors::green(ReplaceTab(after)))?; + } + _ => { + write!(io, "{}", ReplaceTab(after))?; + } + } + writeln!(io)?; + padding_width = display_width(before); + highlight_width = display_width(after); + } else if line_number == end_line_number { + let (before, after) = line_text_split(&source, highlight.range.end); + match highlight.style { + DiagnosticSnippetHighlightStyle::Addition => { + write!(io, "{}", colors::green(ReplaceTab(before)))?; + } + _ => { + write!(io, "{}", ReplaceTab(before))?; + } + } + write!(io, "{}", ReplaceTab(after))?; + writeln!(io)?; + padding_width = 0; + highlight_width = display_width(before); + } else { + let line = line_text(&source, line_number); + writeln!(io, "{}", ReplaceTab(line))?; + padding_width = 0; + highlight_width = display_width(line); + } + + print_padded(io, colors::intense_blue(" | "), max_line_number_digits)?; + write!(io, "{}", RepeatingCharFmt(' ', padding_width))?; + let underline = + RepeatingCharFmt(highlight.style.underline_char(), highlight_width); + write!(io, "{}", highlight.style.style_underline(underline))?; + + if line_number == end_line_number { + if let Some(description) = &highlight.description { + write!(io, " {}", highlight.style.style_underline(description))?; + } + } + + writeln!(io)?; + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use std::borrow::Cow; + + use deno_ast::ModuleSpecifier; + use deno_ast::SourceTextInfo; + + use super::SourceTextStore; + + struct TestSource { + specifier: ModuleSpecifier, + text_info: SourceTextInfo, + } + + impl SourceTextStore for TestSource { + fn get_source_text<'a>( + &'a self, + specifier: &ModuleSpecifier, + ) -> Option> { + if specifier == &self.specifier { + Some(Cow::Borrowed(&self.text_info)) + } else { + None + } + } + } + + #[test] + fn test_display_width() { + assert_eq!(super::display_width("abc"), 3); + assert_eq!(super::display_width("\t"), 2); + assert_eq!(super::display_width("\t\t123"), 7); + assert_eq!(super::display_width("πŸŽ„"), 2); + assert_eq!(super::display_width("πŸŽ„πŸŽ„"), 4); + assert_eq!(super::display_width("πŸ§‘β€πŸ¦°"), 4); + } + + #[test] + fn test_position_in_file_from_text_info_simple() { + let specifier: ModuleSpecifier = "file:///dev/test.ts".parse().unwrap(); + let text_info = SourceTextInfo::new("foo\nbar\nbaz".into()); + let pos = text_info.line_start(1); + let sources = TestSource { + specifier: specifier.clone(), + text_info, + }; + let location = super::DiagnosticLocation::PositionInFile { + specifier: Cow::Borrowed(&specifier), + source_pos: super::DiagnosticSourcePos::SourcePos(pos), + }; + let position = location.position(&sources).unwrap(); + assert_eq!(position, (2, 1)) + } + + #[test] + fn test_position_in_file_from_text_info_emoji() { + let specifier: ModuleSpecifier = "file:///dev/test.ts".parse().unwrap(); + let text_info = SourceTextInfo::new("πŸ§‘β€πŸ¦°text".into()); + let pos = text_info.line_start(0) + 11; // the end of the emoji + let sources = TestSource { + specifier: specifier.clone(), + text_info, + }; + let location = super::DiagnosticLocation::PositionInFile { + specifier: Cow::Borrowed(&specifier), + source_pos: super::DiagnosticSourcePos::SourcePos(pos), + }; + let position = location.position(&sources).unwrap(); + assert_eq!(position, (1, 6)) + } +} diff --git a/cli/main.rs b/cli/main.rs index fb88ad137a7607..2a46c1beb061f8 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -25,6 +25,7 @@ mod tsc; mod util; mod version; mod worker; +mod diagnostics; use crate::args::flags_from_vec; use crate::args::DenoSubcommand; diff --git a/cli/tools/doc.rs b/cli/tools/doc.rs index 4a59ec986b515a..2cb9ddfbae9756 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> { + 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 { + match &self.kind { + DocDiagnosticKind::PrivateTypeRef { .. } => { + Some("make the referenced type public or remove the reference") + } + _ => None, + } + } + fn snippet_fixed(&self) -> Option> { + 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 { + 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 20fd12ce2e1d2d..3c8af5031f54ea 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; @@ -173,10 +186,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,19 +276,19 @@ fn lint_file( file_path: &Path, source_code: String, lint_rules: Vec<&'static dyn LintRule>, -) -> Result<(Vec, String), AnyError> { +) -> Result<(Vec, 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. @@ -282,7 +296,7 @@ fn lint_file( /// Compatible with `--json` flag. fn lint_stdin( lint_rules: Vec<&'static dyn LintRule>, -) -> Result<(Vec, String), AnyError> { +) -> Result<(Vec, 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 +304,18 @@ fn lint_stdin( let linter = create_linter(lint_rules); - let (_, file_diagnostics) = linter.lint_file(LintFileOptions { + let (source, file_diagnostics) = linter.lint_file(LintFileOptions { filename: STDIN_FILE_NAME.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, String), AnyError>, + result: Result<(Vec, ParsedSource), AnyError>, reporter_lock: Arc>>, ) -> bool { let mut reporter = reporter_lock.lock().unwrap(); @@ -310,7 +324,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 +336,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 +357,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> { + 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 { + self.hint.as_ref().map(|h| h as &dyn std::fmt::Display) + } + + fn snippet_fixed(&self) -> Option> { + None // todo + } + + fn info(&self) -> Cow<'_, [std::borrow::Cow<'_, str>]> { + Cow::Borrowed(&[]) + } + + fn docs_url(&self) -> Option { + 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> { + 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 +461,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 +494,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, @@ -511,7 +510,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 00000000000000..ad53acd67a6d72 --- /dev/null +++ b/cli/tools/registry/diagnostics.rs @@ -0,0 +1,151 @@ +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>>, +} + +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> { + match &self { + PublishDiagnostic::FastCheck { diagnostic } => match diagnostic.range() { + Some(range) => Some(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), + }, + }), + None => None, + }, + } + } + + fn hint(&self) -> Option { + match &self { + PublishDiagnostic::FastCheck { diagnostic } => { + Some(diagnostic.fix_hint()) + } + } + } + + fn snippet_fixed(&self) -> Option> { + None + } + + fn info(&self) -> Cow<'_, [Cow<'_, str>]> { + match &self { + PublishDiagnostic::FastCheck { diagnostic } => { + let infos = diagnostic + .additional_info() + .into_iter() + .map(|s| Cow::Borrowed(*s)) + .collect(); + Cow::Owned(infos) + } + } + } + + fn docs_url(&self) -> Option { + 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 b64350887c21e1..29c825242c325b 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 3b8ffcd043f750..8eaf6125836a5e 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::>(); - 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::>(); - let diagnostics = packages - .iter() - .flat_map(|p| p.tarball.diagnostics.iter().cloned()) - .collect::>(); - 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, ) -> 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, 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"); } diff --git a/runtime/colors.rs b/runtime/colors.rs index f8b18b977179bb..ff9ee24dbc7dfc 100644 --- a/runtime/colors.rs +++ b/runtime/colors.rs @@ -80,7 +80,7 @@ impl fmt::Write for StdIoStdFmtWriter<'_> { } } -struct Style { +pub struct Style { colorspec: ColorSpec, inner: I, } @@ -101,65 +101,68 @@ impl fmt::Display for Style { } #[inline] -fn style<'a>( - s: impl fmt::Display + 'a, - colorspec: ColorSpec, -) -> impl fmt::Display + 'a { +fn style<'a, S: fmt::Display + 'a>(s: S, colorspec: ColorSpec) -> Style { Style { colorspec, inner: s, } } -pub fn red_bold<'a>(s: impl fmt::Display + 'a) -> impl fmt::Display + 'a { +pub fn red_bold<'a, S: fmt::Display + 'a>(s: S) -> Style { let mut style_spec = ColorSpec::new(); style_spec.set_fg(Some(Red)).set_bold(true); style(s, style_spec) } -pub fn green_bold<'a>(s: impl fmt::Display + 'a) -> impl fmt::Display + 'a { +pub fn green_bold<'a, S: fmt::Display + 'a>(s: S) -> Style { let mut style_spec = ColorSpec::new(); style_spec.set_fg(Some(Green)).set_bold(true); style(s, style_spec) } -pub fn italic<'a>(s: impl fmt::Display + 'a) -> impl fmt::Display + 'a { +pub fn yellow_bold<'a, S: fmt::Display + 'a>(s: S) -> Style { + let mut style_spec = ColorSpec::new(); + style_spec.set_fg(Some(Yellow)).set_bold(true); + style(s, style_spec) +} + +pub fn italic<'a, S: fmt::Display + 'a>(s: S) -> Style { let mut style_spec = ColorSpec::new(); style_spec.set_italic(true); style(s, style_spec) } -pub fn italic_gray<'a>(s: impl fmt::Display + 'a) -> impl fmt::Display + 'a { +pub fn italic_gray<'a, S: fmt::Display + 'a>(s: S) -> Style { let mut style_spec = ColorSpec::new(); style_spec.set_fg(Some(Ansi256(8))).set_italic(true); style(s, style_spec) } -pub fn italic_bold<'a>(s: impl fmt::Display + 'a) -> impl fmt::Display + 'a { +pub fn italic_bold<'a, S: fmt::Display + 'a>(s: S) -> Style { let mut style_spec = ColorSpec::new(); style_spec.set_bold(true).set_italic(true); style(s, style_spec) } -pub fn white_on_red<'a>(s: impl fmt::Display + 'a) -> impl fmt::Display + 'a { +pub fn white_on_red<'a, S: fmt::Display + 'a>(s: S) -> Style { let mut style_spec = ColorSpec::new(); style_spec.set_bg(Some(Red)).set_fg(Some(White)); style(s, style_spec) } -pub fn black_on_green<'a>(s: impl fmt::Display + 'a) -> impl fmt::Display + 'a { +pub fn black_on_green<'a, S: fmt::Display + 'a>(s: S) -> Style { let mut style_spec = ColorSpec::new(); style_spec.set_bg(Some(Green)).set_fg(Some(Black)); style(s, style_spec) } -pub fn yellow<'a>(s: impl fmt::Display + 'a) -> impl fmt::Display + 'a { +pub fn yellow<'a, S: fmt::Display + 'a>(s: S) -> Style { let mut style_spec = ColorSpec::new(); style_spec.set_fg(Some(Yellow)); style(s, style_spec) } -pub fn cyan<'a>(s: impl fmt::Display + 'a) -> impl fmt::Display + 'a { +pub fn cyan<'a, S: fmt::Display + 'a>(s: S) -> Style { let mut style_spec = ColorSpec::new(); style_spec.set_fg(Some(Cyan)); style(s, style_spec) @@ -173,43 +176,43 @@ pub fn cyan_with_underline<'a>( style(s, style_spec) } -pub fn cyan_bold<'a>(s: impl fmt::Display + 'a) -> impl fmt::Display + 'a { +pub fn cyan_bold<'a, S: fmt::Display + 'a>(s: S) -> Style { let mut style_spec = ColorSpec::new(); style_spec.set_fg(Some(Cyan)).set_bold(true); style(s, style_spec) } -pub fn magenta<'a>(s: impl fmt::Display + 'a) -> impl fmt::Display + 'a { +pub fn magenta<'a, S: fmt::Display + 'a>(s: S) -> Style { let mut style_spec = ColorSpec::new(); style_spec.set_fg(Some(Magenta)); style(s, style_spec) } -pub fn red<'a>(s: impl fmt::Display + 'a) -> impl fmt::Display + 'a { +pub fn red<'a, S: fmt::Display + 'a>(s: S) -> Style { let mut style_spec = ColorSpec::new(); style_spec.set_fg(Some(Red)); style(s, style_spec) } -pub fn green<'a>(s: impl fmt::Display + 'a) -> impl fmt::Display + 'a { +pub fn green<'a, S: fmt::Display + 'a>(s: S) -> Style { let mut style_spec = ColorSpec::new(); style_spec.set_fg(Some(Green)); style(s, style_spec) } -pub fn bold<'a>(s: impl fmt::Display + 'a) -> impl fmt::Display + 'a { +pub fn bold<'a, S: fmt::Display + 'a>(s: S) -> Style { let mut style_spec = ColorSpec::new(); style_spec.set_bold(true); style(s, style_spec) } -pub fn gray<'a>(s: impl fmt::Display + 'a) -> impl fmt::Display + 'a { +pub fn gray<'a, S: fmt::Display + 'a>(s: S) -> Style { let mut style_spec = ColorSpec::new(); style_spec.set_fg(Some(Ansi256(245))); style(s, style_spec) } -pub fn intense_blue<'a>(s: impl fmt::Display + 'a) -> impl fmt::Display + 'a { +pub fn intense_blue<'a, S: fmt::Display + 'a>(s: S) -> Style { let mut style_spec = ColorSpec::new(); style_spec.set_fg(Some(Blue)).set_intense(true); style(s, style_spec)