diff --git a/Cargo.lock b/Cargo.lock index 0b99e928f7..268e69b8a9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2852,6 +2852,7 @@ dependencies = [ "salsa", "schemars", "serde", + "serde_json", "tempfile", "thiserror 2.0.12", "tracing", diff --git a/crates/ruff/src/lib.rs b/crates/ruff/src/lib.rs index bd5facff5c..af7e0bc065 100644 --- a/crates/ruff/src/lib.rs +++ b/crates/ruff/src/lib.rs @@ -439,7 +439,7 @@ pub fn check(args: CheckCommand, global_options: GlobalConfigArgs) -> Result Result<()> { if matches!(self.log_level, LogLevel::Silent) { return Ok(()); @@ -229,13 +231,21 @@ impl Printer { match self.format { OutputFormat::Json => { - JsonEmitter.emit(writer, &diagnostics.inner, &context)?; + let config = DisplayDiagnosticConfig::default() + .format(DiagnosticFormat::Json) + .preview(preview); + let value = DisplayDiagnostics::new(&context, &config, &diagnostics.inner); + write!(writer, "{value}")?; } OutputFormat::Rdjson => { RdjsonEmitter.emit(writer, &diagnostics.inner, &context)?; } OutputFormat::JsonLines => { - JsonLinesEmitter.emit(writer, &diagnostics.inner, &context)?; + let config = DisplayDiagnosticConfig::default() + .format(DiagnosticFormat::JsonLines) + .preview(preview); + let value = DisplayDiagnostics::new(&context, &config, &diagnostics.inner); + write!(writer, "{value}")?; } OutputFormat::Junit => { JunitEmitter.emit(writer, &diagnostics.inner, &context)?; @@ -283,7 +293,11 @@ impl Printer { PylintEmitter.emit(writer, &diagnostics.inner, &context)?; } OutputFormat::Azure => { - AzureEmitter.emit(writer, &diagnostics.inner, &context)?; + let config = DisplayDiagnosticConfig::default() + .format(DiagnosticFormat::Azure) + .preview(preview); + let value = DisplayDiagnostics::new(&context, &config, &diagnostics.inner); + write!(writer, "{value}")?; } OutputFormat::Sarif => { SarifEmitter.emit(writer, &diagnostics.inner, &context)?; diff --git a/crates/ruff_db/Cargo.toml b/crates/ruff_db/Cargo.toml index ac90e96214..07873cbfad 100644 --- a/crates/ruff_db/Cargo.toml +++ b/crates/ruff_db/Cargo.toml @@ -38,6 +38,7 @@ rustc-hash = { workspace = true } salsa = { workspace = true } schemars = { workspace = true, optional = true } serde = { workspace = true, optional = true } +serde_json = { workspace = true, optional = true } thiserror = { workspace = true } tracing = { workspace = true } tracing-subscriber = { workspace = true, optional = true } @@ -56,6 +57,6 @@ tempfile = { workspace = true } [features] cache = ["ruff_cache"] os = ["ignore", "dep:etcetera"] -serde = ["dep:serde", "camino/serde1"] +serde = ["camino/serde1", "dep:serde", "dep:serde_json", "ruff_diagnostics/serde"] # Exposes testing utilities. testing = ["tracing-subscriber"] diff --git a/crates/ruff_db/src/diagnostic/mod.rs b/crates/ruff_db/src/diagnostic/mod.rs index c7a7c080f6..ce9541d66b 100644 --- a/crates/ruff_db/src/diagnostic/mod.rs +++ b/crates/ruff_db/src/diagnostic/mod.rs @@ -1,13 +1,12 @@ use std::{fmt::Formatter, sync::Arc}; -use render::{FileResolver, Input}; use ruff_diagnostics::Fix; use ruff_source_file::{LineColumn, SourceCode, SourceFile}; use ruff_annotate_snippets::Level as AnnotateLevel; use ruff_text_size::{Ranged, TextRange, TextSize}; -pub use self::render::DisplayDiagnostic; +pub use self::render::{DisplayDiagnostic, DisplayDiagnostics, FileResolver, Input}; use crate::{Db, files::File}; mod render; @@ -380,7 +379,7 @@ impl Diagnostic { } /// Returns the URL for the rule documentation, if it exists. - pub fn to_url(&self) -> Option { + pub fn to_ruff_url(&self) -> Option { if self.is_invalid_syntax() { None } else { @@ -1175,6 +1174,12 @@ pub struct DisplayDiagnosticConfig { /// here for now as the most "sensible" place for it to live until /// we had more concrete use cases. ---AG context: usize, + /// Whether to use preview formatting for Ruff diagnostics. + #[allow( + dead_code, + reason = "This is currently only used for JSON but will be needed soon for other formats" + )] + preview: bool, } impl DisplayDiagnosticConfig { @@ -1195,6 +1200,14 @@ impl DisplayDiagnosticConfig { ..self } } + + /// Whether to enable preview behavior or not. + pub fn preview(self, yes: bool) -> DisplayDiagnosticConfig { + DisplayDiagnosticConfig { + preview: yes, + ..self + } + } } impl Default for DisplayDiagnosticConfig { @@ -1203,6 +1216,7 @@ impl Default for DisplayDiagnosticConfig { format: DiagnosticFormat::default(), color: false, context: 2, + preview: false, } } } @@ -1230,6 +1244,21 @@ pub enum DiagnosticFormat { /// /// This may use color when printing to a `tty`. Concise, + /// Print diagnostics in the [Azure Pipelines] format. + /// + /// [Azure Pipelines]: https://learn.microsoft.com/en-us/azure/devops/pipelines/scripts/logging-commands?view=azure-devops&tabs=bash#logissue-log-an-error-or-warning + Azure, + /// Print diagnostics in JSON format. + /// + /// Unlike `json-lines`, this prints all of the diagnostics as a JSON array. + #[cfg(feature = "serde")] + Json, + /// Print diagnostics in JSON format, one per line. + /// + /// This will print each diagnostic as a separate JSON object on its own line. See the `json` + /// format for an array of all diagnostics. See for more details. + #[cfg(feature = "serde")] + JsonLines, } /// A representation of the kinds of messages inside a diagnostic. diff --git a/crates/ruff_db/src/diagnostic/render.rs b/crates/ruff_db/src/diagnostic/render.rs index 59e762c568..6dc7752e3e 100644 --- a/crates/ruff_db/src/diagnostic/render.rs +++ b/crates/ruff_db/src/diagnostic/render.rs @@ -4,6 +4,7 @@ use ruff_annotate_snippets::{ Annotation as AnnotateAnnotation, Level as AnnotateLevel, Message as AnnotateMessage, Renderer as AnnotateRenderer, Snippet as AnnotateSnippet, }; +use ruff_notebook::{Notebook, NotebookIndex}; use ruff_source_file::{LineIndex, OneIndexed, SourceCode}; use ruff_text_size::{TextRange, TextSize}; @@ -17,9 +18,17 @@ use crate::{ use super::{ Annotation, Diagnostic, DiagnosticFormat, DiagnosticSource, DisplayDiagnosticConfig, Severity, - SubDiagnostic, + SubDiagnostic, UnifiedFile, }; +use azure::AzureRenderer; + +mod azure; +#[cfg(feature = "serde")] +mod json; +#[cfg(feature = "serde")] +mod json_lines; + /// A type that implements `std::fmt::Display` for diagnostic rendering. /// /// It is created via [`Diagnostic::display`]. @@ -34,7 +43,6 @@ use super::{ pub struct DisplayDiagnostic<'a> { config: &'a DisplayDiagnosticConfig, resolver: &'a dyn FileResolver, - annotate_renderer: AnnotateRenderer, diag: &'a Diagnostic, } @@ -44,16 +52,9 @@ impl<'a> DisplayDiagnostic<'a> { config: &'a DisplayDiagnosticConfig, diag: &'a Diagnostic, ) -> DisplayDiagnostic<'a> { - let annotate_renderer = if config.color { - AnnotateRenderer::styled() - } else { - AnnotateRenderer::plain() - }; - DisplayDiagnostic { config, resolver, - annotate_renderer, diag, } } @@ -61,68 +62,131 @@ impl<'a> DisplayDiagnostic<'a> { impl std::fmt::Display for DisplayDiagnostic<'_> { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - let stylesheet = if self.config.color { - DiagnosticStylesheet::styled() - } else { - DiagnosticStylesheet::plain() - }; + DisplayDiagnostics::new(self.resolver, self.config, std::slice::from_ref(self.diag)).fmt(f) + } +} - if matches!(self.config.format, DiagnosticFormat::Concise) { - let (severity, severity_style) = match self.diag.severity() { - Severity::Info => ("info", stylesheet.info), - Severity::Warning => ("warning", stylesheet.warning), - Severity::Error => ("error", stylesheet.error), - Severity::Fatal => ("fatal", stylesheet.error), - }; +/// A type that implements `std::fmt::Display` for rendering a collection of diagnostics. +/// +/// It is intended for collections of diagnostics that need to be serialized together, as is the +/// case for JSON, for example. +/// +/// See [`DisplayDiagnostic`] for rendering individual `Diagnostic`s and details about the lifetime +/// constraints. +pub struct DisplayDiagnostics<'a> { + config: &'a DisplayDiagnosticConfig, + resolver: &'a dyn FileResolver, + diagnostics: &'a [Diagnostic], +} - write!( - f, - "{severity}[{id}]", - severity = fmt_styled(severity, severity_style), - id = fmt_styled(self.diag.id(), stylesheet.emphasis) - )?; +impl<'a> DisplayDiagnostics<'a> { + pub fn new( + resolver: &'a dyn FileResolver, + config: &'a DisplayDiagnosticConfig, + diagnostics: &'a [Diagnostic], + ) -> DisplayDiagnostics<'a> { + DisplayDiagnostics { + config, + resolver, + diagnostics, + } + } +} - if let Some(span) = self.diag.primary_span() { - write!( - f, - " {path}", - path = fmt_styled(span.file().path(self.resolver), stylesheet.emphasis) - )?; - if let Some(range) = span.range() { - let diagnostic_source = span.file().diagnostic_source(self.resolver); - let start = diagnostic_source - .as_source_code() - .line_column(range.start()); +impl std::fmt::Display for DisplayDiagnostics<'_> { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + match self.config.format { + DiagnosticFormat::Concise => { + let stylesheet = if self.config.color { + DiagnosticStylesheet::styled() + } else { + DiagnosticStylesheet::plain() + }; + for diag in self.diagnostics { + let (severity, severity_style) = match diag.severity() { + Severity::Info => ("info", stylesheet.info), + Severity::Warning => ("warning", stylesheet.warning), + Severity::Error => ("error", stylesheet.error), + Severity::Fatal => ("fatal", stylesheet.error), + }; write!( f, - ":{line}:{col}", - line = fmt_styled(start.line, stylesheet.emphasis), - col = fmt_styled(start.column, stylesheet.emphasis), + "{severity}[{id}]", + severity = fmt_styled(severity, severity_style), + id = fmt_styled(diag.id(), stylesheet.emphasis) )?; + if let Some(span) = diag.primary_span() { + write!( + f, + " {path}", + path = fmt_styled(span.file().path(self.resolver), stylesheet.emphasis) + )?; + if let Some(range) = span.range() { + let diagnostic_source = span.file().diagnostic_source(self.resolver); + let start = diagnostic_source + .as_source_code() + .line_column(range.start()); + + write!( + f, + ":{line}:{col}", + line = fmt_styled(start.line, stylesheet.emphasis), + col = fmt_styled(start.column, stylesheet.emphasis), + )?; + } + write!(f, ":")?; + } + writeln!(f, " {message}", message = diag.concise_message())?; } - write!(f, ":")?; } - return writeln!(f, " {message}", message = self.diag.concise_message()); + DiagnosticFormat::Full => { + let stylesheet = if self.config.color { + DiagnosticStylesheet::styled() + } else { + DiagnosticStylesheet::plain() + }; + + let mut renderer = if self.config.color { + AnnotateRenderer::styled() + } else { + AnnotateRenderer::plain() + }; + + renderer = renderer + .error(stylesheet.error) + .warning(stylesheet.warning) + .info(stylesheet.info) + .note(stylesheet.note) + .help(stylesheet.help) + .line_no(stylesheet.line_no) + .emphasis(stylesheet.emphasis) + .none(stylesheet.none); + + for diag in self.diagnostics { + let resolved = Resolved::new(self.resolver, diag); + let renderable = resolved.to_renderable(self.config.context); + for diag in renderable.diagnostics.iter() { + writeln!(f, "{}", renderer.render(diag.to_annotate()))?; + } + writeln!(f)?; + } + } + DiagnosticFormat::Azure => { + AzureRenderer::new(self.resolver).render(f, self.diagnostics)?; + } + #[cfg(feature = "serde")] + DiagnosticFormat::Json => { + json::JsonRenderer::new(self.resolver, self.config).render(f, self.diagnostics)?; + } + #[cfg(feature = "serde")] + DiagnosticFormat::JsonLines => { + json_lines::JsonLinesRenderer::new(self.resolver, self.config) + .render(f, self.diagnostics)?; + } } - let mut renderer = self.annotate_renderer.clone(); - renderer = renderer - .error(stylesheet.error) - .warning(stylesheet.warning) - .info(stylesheet.info) - .note(stylesheet.note) - .help(stylesheet.help) - .line_no(stylesheet.line_no) - .emphasis(stylesheet.emphasis) - .none(stylesheet.none); - - let resolved = Resolved::new(self.resolver, self.diag); - let renderable = resolved.to_renderable(self.config.context); - for diag in renderable.diagnostics.iter() { - writeln!(f, "{}", renderer.render(diag.to_annotate()))?; - } - writeln!(f) + Ok(()) } } @@ -635,6 +699,12 @@ pub trait FileResolver { /// Returns the input contents associated with the file given. fn input(&self, file: File) -> Input; + + /// Returns the [`NotebookIndex`] associated with the file given, if it's a Jupyter notebook. + fn notebook_index(&self, file: &UnifiedFile) -> Option; + + /// Returns whether the file given is a Jupyter notebook. + fn is_notebook(&self, file: &UnifiedFile) -> bool; } impl FileResolver for T @@ -651,6 +721,25 @@ where line_index: line_index(self, file), } } + + fn notebook_index(&self, file: &UnifiedFile) -> Option { + match file { + UnifiedFile::Ty(file) => self + .input(*file) + .text + .as_notebook() + .map(Notebook::index) + .cloned(), + UnifiedFile::Ruff(_) => unimplemented!("Expected an interned ty file"), + } + } + + fn is_notebook(&self, file: &UnifiedFile) -> bool { + match file { + UnifiedFile::Ty(file) => self.input(*file).text.as_notebook().is_some(), + UnifiedFile::Ruff(_) => unimplemented!("Expected an interned ty file"), + } + } } impl FileResolver for &dyn Db { @@ -664,6 +753,25 @@ impl FileResolver for &dyn Db { line_index: line_index(*self, file), } } + + fn notebook_index(&self, file: &UnifiedFile) -> Option { + match file { + UnifiedFile::Ty(file) => self + .input(*file) + .text + .as_notebook() + .map(Notebook::index) + .cloned(), + UnifiedFile::Ruff(_) => unimplemented!("Expected an interned ty file"), + } + } + + fn is_notebook(&self, file: &UnifiedFile) -> bool { + match file { + UnifiedFile::Ty(file) => self.input(*file).text.as_notebook().is_some(), + UnifiedFile::Ruff(_) => unimplemented!("Expected an interned ty file"), + } + } } /// An abstraction over a unit of user input. @@ -724,7 +832,9 @@ fn relativize_path<'p>(cwd: &SystemPath, path: &'p str) -> &'p str { #[cfg(test)] mod tests { - use crate::diagnostic::{Annotation, DiagnosticId, Severity, Span}; + use ruff_diagnostics::{Edit, Fix}; + + use crate::diagnostic::{Annotation, DiagnosticId, SecondaryCode, Severity, Span}; use crate::files::system_path_to_file; use crate::system::{DbWithWritableSystem, SystemPath}; use crate::tests::TestDb; @@ -2121,7 +2231,7 @@ watermelon /// A small harness for setting up an environment specifically for testing /// diagnostic rendering. - struct TestEnvironment { + pub(super) struct TestEnvironment { db: TestDb, config: DisplayDiagnosticConfig, } @@ -2130,7 +2240,7 @@ watermelon /// Create a new test harness. /// /// This uses the default diagnostic rendering configuration. - fn new() -> TestEnvironment { + pub(super) fn new() -> TestEnvironment { TestEnvironment { db: TestDb::new(), config: DisplayDiagnosticConfig::default(), @@ -2149,8 +2259,26 @@ watermelon self.config = config; } + /// Set the output format to use in diagnostic rendering. + pub(super) fn format(&mut self, format: DiagnosticFormat) { + let mut config = std::mem::take(&mut self.config); + config = config.format(format); + self.config = config; + } + + /// Enable preview functionality for diagnostic rendering. + #[allow( + dead_code, + reason = "This is currently only used for JSON but will be needed soon for other formats" + )] + pub(super) fn preview(&mut self, yes: bool) { + let mut config = std::mem::take(&mut self.config); + config = config.preview(yes); + self.config = config; + } + /// Add a file with the given path and contents to this environment. - fn add(&mut self, path: &str, contents: &str) { + pub(super) fn add(&mut self, path: &str, contents: &str) { let path = SystemPath::new(path); self.db.write_file(path, contents).unwrap(); } @@ -2200,7 +2328,7 @@ watermelon /// A convenience function for returning a builder for a diagnostic /// with "error" severity and canned values for its identifier /// and message. - fn err(&mut self) -> DiagnosticBuilder<'_> { + pub(super) fn err(&mut self) -> DiagnosticBuilder<'_> { self.builder( "test-diagnostic", Severity::Error, @@ -2226,6 +2354,12 @@ watermelon DiagnosticBuilder { env: self, diag } } + /// A convenience function for returning a builder for an invalid syntax diagnostic. + fn invalid_syntax(&mut self, message: &str) -> DiagnosticBuilder<'_> { + let diag = Diagnostic::new(DiagnosticId::InvalidSyntax, Severity::Error, message); + DiagnosticBuilder { env: self, diag } + } + /// Returns a builder for tersely constructing sub-diagnostics. fn sub_builder(&mut self, severity: Severity, message: &str) -> SubDiagnosticBuilder<'_> { let subdiag = SubDiagnostic::new(severity, message); @@ -2235,9 +2369,18 @@ watermelon /// Render the given diagnostic into a `String`. /// /// (This will set the "printed" flag on `Diagnostic`.) - fn render(&self, diag: &Diagnostic) -> String { + pub(super) fn render(&self, diag: &Diagnostic) -> String { diag.display(&self.db, &self.config).to_string() } + + /// Render the given diagnostics into a `String`. + /// + /// See `render` for rendering a single diagnostic. + /// + /// (This will set the "printed" flag on `Diagnostic`.) + pub(super) fn render_diagnostics(&self, diagnostics: &[Diagnostic]) -> String { + DisplayDiagnostics::new(&self.db, &self.config, diagnostics).to_string() + } } /// A helper builder for tersely populating a `Diagnostic`. @@ -2246,14 +2389,14 @@ watermelon /// supported by this builder, and this only needs to be done /// infrequently, consider doing it more verbosely on `diag` /// itself. - struct DiagnosticBuilder<'e> { + pub(super) struct DiagnosticBuilder<'e> { env: &'e mut TestEnvironment, diag: Diagnostic, } impl<'e> DiagnosticBuilder<'e> { /// Return the built diagnostic. - fn build(self) -> Diagnostic { + pub(super) fn build(self) -> Diagnostic { self.diag } @@ -2302,6 +2445,25 @@ watermelon self.diag.annotate(ann); self } + + /// Set the secondary code on the diagnostic. + fn secondary_code(mut self, secondary_code: &str) -> DiagnosticBuilder<'e> { + self.diag + .set_secondary_code(SecondaryCode::new(secondary_code.to_string())); + self + } + + /// Set the fix on the diagnostic. + pub(super) fn fix(mut self, fix: Fix) -> DiagnosticBuilder<'e> { + self.diag.set_fix(fix); + self + } + + /// Set the noqa offset on the diagnostic. + fn noqa_offset(mut self, noqa_offset: TextSize) -> DiagnosticBuilder<'e> { + self.diag.set_noqa_offset(noqa_offset); + self + } } /// A helper builder for tersely populating a `SubDiagnostic`. @@ -2381,4 +2543,199 @@ watermelon let offset = TextSize::from(offset.parse::().unwrap()); (line_number, Some(offset)) } + + /// Create Ruff-style diagnostics for testing the various output formats. + pub(crate) fn create_diagnostics( + format: DiagnosticFormat, + ) -> (TestEnvironment, Vec) { + let mut env = TestEnvironment::new(); + env.add( + "fib.py", + r#"import os + + +def fibonacci(n): + """Compute the nth number in the Fibonacci sequence.""" + x = 1 + if n == 0: + return 0 + elif n == 1: + return 1 + else: + return fibonacci(n - 1) + fibonacci(n - 2) +"#, + ); + env.add("undef.py", r"if a == 1: pass"); + env.format(format); + + let diagnostics = vec![ + env.builder("unused-import", Severity::Error, "`os` imported but unused") + .primary("fib.py", "1:7", "1:9", "Remove unused import: `os`") + .secondary_code("F401") + .fix(Fix::unsafe_edit(Edit::range_deletion(TextRange::new( + TextSize::from(0), + TextSize::from(10), + )))) + .noqa_offset(TextSize::from(7)) + .build(), + env.builder( + "unused-variable", + Severity::Error, + "Local variable `x` is assigned to but never used", + ) + .primary( + "fib.py", + "6:4", + "6:5", + "Remove assignment to unused variable `x`", + ) + .secondary_code("F841") + .fix(Fix::unsafe_edit(Edit::deletion( + TextSize::from(94), + TextSize::from(99), + ))) + .noqa_offset(TextSize::from(94)) + .build(), + env.builder("undefined-name", Severity::Error, "Undefined name `a`") + .primary("undef.py", "1:3", "1:4", "") + .secondary_code("F821") + .noqa_offset(TextSize::from(3)) + .build(), + ]; + + (env, diagnostics) + } + + /// Create Ruff-style syntax error diagnostics for testing the various output formats. + pub(crate) fn create_syntax_error_diagnostics( + format: DiagnosticFormat, + ) -> (TestEnvironment, Vec) { + let mut env = TestEnvironment::new(); + env.add( + "syntax_errors.py", + r"from os import + +if call(foo + def bar(): + pass +", + ); + env.format(format); + + let diagnostics = vec![ + env.invalid_syntax("SyntaxError: Expected one or more symbol names after import") + .primary("syntax_errors.py", "1:14", "1:15", "") + .build(), + env.invalid_syntax("SyntaxError: Expected ')', found newline") + .primary("syntax_errors.py", "3:11", "3:12", "") + .build(), + ]; + + (env, diagnostics) + } + + /// Create Ruff-style diagnostics for testing the various output formats for a notebook. + #[allow( + dead_code, + reason = "This is currently only used for JSON but will be needed soon for other formats" + )] + pub(crate) fn create_notebook_diagnostics( + format: DiagnosticFormat, + ) -> (TestEnvironment, Vec) { + let mut env = TestEnvironment::new(); + env.add( + "notebook.ipynb", + r##" + { + "cells": [ + { + "cell_type": "code", + "metadata": {}, + "outputs": [], + "source": [ + "# cell 1\n", + "import os" + ] + }, + { + "cell_type": "code", + "metadata": {}, + "outputs": [], + "source": [ + "# cell 2\n", + "import math\n", + "\n", + "print('hello world')" + ] + }, + { + "cell_type": "code", + "metadata": {}, + "outputs": [], + "source": [ + "# cell 3\n", + "def foo():\n", + " print()\n", + " x = 1\n" + ] + } + ], + "metadata": {}, + "nbformat": 4, + "nbformat_minor": 5 +} +"##, + ); + env.format(format); + + let diagnostics = vec![ + env.builder("unused-import", Severity::Error, "`os` imported but unused") + .primary("notebook.ipynb", "2:7", "2:9", "Remove unused import: `os`") + .secondary_code("F401") + .fix(Fix::safe_edit(Edit::range_deletion(TextRange::new( + TextSize::from(9), + TextSize::from(19), + )))) + .noqa_offset(TextSize::from(16)) + .build(), + env.builder( + "unused-import", + Severity::Error, + "`math` imported but unused", + ) + .primary( + "notebook.ipynb", + "4:7", + "4:11", + "Remove unused import: `math`", + ) + .secondary_code("F401") + .fix(Fix::safe_edit(Edit::range_deletion(TextRange::new( + TextSize::from(28), + TextSize::from(40), + )))) + .noqa_offset(TextSize::from(35)) + .build(), + env.builder( + "unused-variable", + Severity::Error, + "Local variable `x` is assigned to but never used", + ) + .primary( + "notebook.ipynb", + "10:4", + "10:5", + "Remove assignment to unused variable `x`", + ) + .secondary_code("F841") + .fix(Fix::unsafe_edit(Edit::range_deletion(TextRange::new( + TextSize::from(94), + TextSize::from(104), + )))) + .noqa_offset(TextSize::from(98)) + .build(), + ]; + + (env, diagnostics) + } } diff --git a/crates/ruff_db/src/diagnostic/render/azure.rs b/crates/ruff_db/src/diagnostic/render/azure.rs new file mode 100644 index 0000000000..d607e08d86 --- /dev/null +++ b/crates/ruff_db/src/diagnostic/render/azure.rs @@ -0,0 +1,83 @@ +use ruff_source_file::LineColumn; + +use crate::diagnostic::{Diagnostic, Severity}; + +use super::FileResolver; + +pub(super) struct AzureRenderer<'a> { + resolver: &'a dyn FileResolver, +} + +impl<'a> AzureRenderer<'a> { + pub(super) fn new(resolver: &'a dyn FileResolver) -> Self { + Self { resolver } + } +} + +impl AzureRenderer<'_> { + pub(super) fn render( + &self, + f: &mut std::fmt::Formatter, + diagnostics: &[Diagnostic], + ) -> std::fmt::Result { + for diag in diagnostics { + let severity = match diag.severity() { + Severity::Info | Severity::Warning => "warning", + Severity::Error | Severity::Fatal => "error", + }; + write!(f, "##vso[task.logissue type={severity};")?; + if let Some(span) = diag.primary_span() { + let filename = span.file().path(self.resolver); + write!(f, "sourcepath={filename};")?; + if let Some(range) = span.range() { + let location = if self.resolver.notebook_index(span.file()).is_some() { + // We can't give a reasonable location for the structured formats, + // so we show one that's clearly a fallback + LineColumn::default() + } else { + span.file() + .diagnostic_source(self.resolver) + .as_source_code() + .line_column(range.start()) + }; + write!( + f, + "linenumber={line};columnnumber={col};", + line = location.line, + col = location.column, + )?; + } + } + writeln!( + f, + "{code}]{body}", + code = diag + .secondary_code() + .map_or_else(String::new, |code| format!("code={code};")), + body = diag.body(), + )?; + } + + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use crate::diagnostic::{ + DiagnosticFormat, + render::tests::{create_diagnostics, create_syntax_error_diagnostics}, + }; + + #[test] + fn output() { + let (env, diagnostics) = create_diagnostics(DiagnosticFormat::Azure); + insta::assert_snapshot!(env.render_diagnostics(&diagnostics)); + } + + #[test] + fn syntax_errors() { + let (env, diagnostics) = create_syntax_error_diagnostics(DiagnosticFormat::Azure); + insta::assert_snapshot!(env.render_diagnostics(&diagnostics)); + } +} diff --git a/crates/ruff_db/src/diagnostic/render/json.rs b/crates/ruff_db/src/diagnostic/render/json.rs new file mode 100644 index 0000000000..7419034ae6 --- /dev/null +++ b/crates/ruff_db/src/diagnostic/render/json.rs @@ -0,0 +1,393 @@ +use serde::{Serialize, Serializer, ser::SerializeSeq}; +use serde_json::{Value, json}; + +use ruff_diagnostics::{Applicability, Edit}; +use ruff_notebook::NotebookIndex; +use ruff_source_file::{LineColumn, OneIndexed}; +use ruff_text_size::Ranged; + +use crate::diagnostic::{Diagnostic, DiagnosticSource, DisplayDiagnosticConfig, SecondaryCode}; + +use super::FileResolver; + +pub(super) struct JsonRenderer<'a> { + resolver: &'a dyn FileResolver, + config: &'a DisplayDiagnosticConfig, +} + +impl<'a> JsonRenderer<'a> { + pub(super) fn new(resolver: &'a dyn FileResolver, config: &'a DisplayDiagnosticConfig) -> Self { + Self { resolver, config } + } +} + +impl JsonRenderer<'_> { + pub(super) fn render( + &self, + f: &mut std::fmt::Formatter, + diagnostics: &[Diagnostic], + ) -> std::fmt::Result { + write!( + f, + "{:#}", + diagnostics_to_json_value(diagnostics, self.resolver, self.config) + ) + } +} + +fn diagnostics_to_json_value<'a>( + diagnostics: impl IntoIterator, + resolver: &dyn FileResolver, + config: &DisplayDiagnosticConfig, +) -> Value { + let values: Vec<_> = diagnostics + .into_iter() + .map(|diag| diagnostic_to_json(diag, resolver, config)) + .collect(); + json!(values) +} + +pub(super) fn diagnostic_to_json<'a>( + diagnostic: &'a Diagnostic, + resolver: &'a dyn FileResolver, + config: &'a DisplayDiagnosticConfig, +) -> JsonDiagnostic<'a> { + let span = diagnostic.primary_span_ref(); + let filename = span.map(|span| span.file().path(resolver)); + let range = span.and_then(|span| span.range()); + let diagnostic_source = span.map(|span| span.file().diagnostic_source(resolver)); + let source_code = diagnostic_source + .as_ref() + .map(|diagnostic_source| diagnostic_source.as_source_code()); + let notebook_index = span.and_then(|span| resolver.notebook_index(span.file())); + + let mut start_location = None; + let mut end_location = None; + let mut noqa_location = None; + let mut notebook_cell_index = None; + if let Some(source_code) = source_code { + noqa_location = diagnostic + .noqa_offset() + .map(|offset| source_code.line_column(offset)); + if let Some(range) = range { + let mut start = source_code.line_column(range.start()); + let mut end = source_code.line_column(range.end()); + if let Some(notebook_index) = ¬ebook_index { + notebook_cell_index = + Some(notebook_index.cell(start.line).unwrap_or(OneIndexed::MIN)); + start = notebook_index.translate_line_column(&start); + end = notebook_index.translate_line_column(&end); + noqa_location = + noqa_location.map(|location| notebook_index.translate_line_column(&location)); + } + start_location = Some(start); + end_location = Some(end); + } + } + + let fix = diagnostic.fix().map(|fix| JsonFix { + applicability: fix.applicability(), + message: diagnostic.suggestion(), + edits: ExpandedEdits { + edits: fix.edits(), + notebook_index, + config, + diagnostic_source, + }, + }); + + // In preview, the locations and filename can be optional. + if config.preview { + JsonDiagnostic { + code: diagnostic.secondary_code(), + url: diagnostic.to_ruff_url(), + message: diagnostic.body(), + fix, + cell: notebook_cell_index, + location: start_location.map(JsonLocation::from), + end_location: end_location.map(JsonLocation::from), + filename, + noqa_row: noqa_location.map(|location| location.line), + } + } else { + JsonDiagnostic { + code: diagnostic.secondary_code(), + url: diagnostic.to_ruff_url(), + message: diagnostic.body(), + fix, + cell: notebook_cell_index, + location: Some(start_location.unwrap_or_default().into()), + end_location: Some(end_location.unwrap_or_default().into()), + filename: Some(filename.unwrap_or_default()), + noqa_row: noqa_location.map(|location| location.line), + } + } +} + +struct ExpandedEdits<'a> { + edits: &'a [Edit], + notebook_index: Option, + config: &'a DisplayDiagnosticConfig, + diagnostic_source: Option, +} + +impl Serialize for ExpandedEdits<'_> { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + let mut s = serializer.serialize_seq(Some(self.edits.len()))?; + + for edit in self.edits { + let (location, end_location) = if let Some(diagnostic_source) = &self.diagnostic_source + { + let source_code = diagnostic_source.as_source_code(); + let mut location = source_code.line_column(edit.start()); + let mut end_location = source_code.line_column(edit.end()); + + if let Some(notebook_index) = &self.notebook_index { + // There exists a newline between each cell's source code in the + // concatenated source code in Ruff. This newline doesn't actually + // exists in the JSON source field. + // + // Now, certain edits may try to remove this newline, which means + // the edit will spill over to the first character of the next cell. + // If it does, we need to translate the end location to the last + // character of the previous cell. + match ( + notebook_index.cell(location.line), + notebook_index.cell(end_location.line), + ) { + (Some(start_cell), Some(end_cell)) if start_cell != end_cell => { + debug_assert_eq!(end_location.column.get(), 1); + + let prev_row = end_location.line.saturating_sub(1); + end_location = LineColumn { + line: notebook_index.cell_row(prev_row).unwrap_or(OneIndexed::MIN), + column: source_code + .line_column(source_code.line_end_exclusive(prev_row)) + .column, + }; + } + (Some(_), None) => { + debug_assert_eq!(end_location.column.get(), 1); + + let prev_row = end_location.line.saturating_sub(1); + end_location = LineColumn { + line: notebook_index.cell_row(prev_row).unwrap_or(OneIndexed::MIN), + column: source_code + .line_column(source_code.line_end_exclusive(prev_row)) + .column, + }; + } + _ => { + end_location = notebook_index.translate_line_column(&end_location); + } + } + location = notebook_index.translate_line_column(&location); + } + + (Some(location), Some(end_location)) + } else { + (None, None) + }; + + // In preview, the locations can be optional. + let value = if self.config.preview { + JsonEdit { + content: edit.content().unwrap_or_default(), + location: location.map(JsonLocation::from), + end_location: end_location.map(JsonLocation::from), + } + } else { + JsonEdit { + content: edit.content().unwrap_or_default(), + location: Some(location.unwrap_or_default().into()), + end_location: Some(end_location.unwrap_or_default().into()), + } + }; + + s.serialize_element(&value)?; + } + + s.end() + } +} + +/// A serializable version of `Diagnostic`. +/// +/// The `Old` variant only exists to preserve backwards compatibility. Both this and `JsonEdit` +/// should become structs with the `New` definitions in a future Ruff release. +#[derive(Serialize)] +pub(crate) struct JsonDiagnostic<'a> { + cell: Option, + code: Option<&'a SecondaryCode>, + end_location: Option, + filename: Option<&'a str>, + fix: Option>, + location: Option, + message: &'a str, + noqa_row: Option, + url: Option, +} + +#[derive(Serialize)] +struct JsonFix<'a> { + applicability: Applicability, + edits: ExpandedEdits<'a>, + message: Option<&'a str>, +} + +#[derive(Serialize)] +struct JsonLocation { + column: OneIndexed, + row: OneIndexed, +} + +impl From for JsonLocation { + fn from(location: LineColumn) -> Self { + JsonLocation { + row: location.line, + column: location.column, + } + } +} + +#[derive(Serialize)] +struct JsonEdit<'a> { + content: &'a str, + end_location: Option, + location: Option, +} + +#[cfg(test)] +mod tests { + use ruff_diagnostics::{Edit, Fix}; + use ruff_text_size::TextSize; + + use crate::diagnostic::{ + DiagnosticFormat, + render::tests::{ + TestEnvironment, create_diagnostics, create_notebook_diagnostics, + create_syntax_error_diagnostics, + }, + }; + + #[test] + fn output() { + let (env, diagnostics) = create_diagnostics(DiagnosticFormat::Json); + insta::assert_snapshot!(env.render_diagnostics(&diagnostics)); + } + + #[test] + fn syntax_errors() { + let (env, diagnostics) = create_syntax_error_diagnostics(DiagnosticFormat::Json); + insta::assert_snapshot!(env.render_diagnostics(&diagnostics)); + } + + #[test] + fn notebook_output() { + let (env, diagnostics) = create_notebook_diagnostics(DiagnosticFormat::Json); + insta::assert_snapshot!(env.render_diagnostics(&diagnostics)); + } + + #[test] + fn missing_file_stable() { + let mut env = TestEnvironment::new(); + env.format(DiagnosticFormat::Json); + env.preview(false); + + let diag = env + .err() + .fix(Fix::safe_edit(Edit::insertion( + "edit".to_string(), + TextSize::from(0), + ))) + .build(); + + insta::assert_snapshot!( + env.render(&diag), + @r#" + [ + { + "cell": null, + "code": null, + "end_location": { + "column": 1, + "row": 1 + }, + "filename": "", + "fix": { + "applicability": "safe", + "edits": [ + { + "content": "edit", + "end_location": { + "column": 1, + "row": 1 + }, + "location": { + "column": 1, + "row": 1 + } + } + ], + "message": null + }, + "location": { + "column": 1, + "row": 1 + }, + "message": "main diagnostic message", + "noqa_row": null, + "url": "https://docs.astral.sh/ruff/rules/test-diagnostic" + } + ] + "#, + ); + } + + #[test] + fn missing_file_preview() { + let mut env = TestEnvironment::new(); + env.format(DiagnosticFormat::Json); + env.preview(true); + + let diag = env + .err() + .fix(Fix::safe_edit(Edit::insertion( + "edit".to_string(), + TextSize::from(0), + ))) + .build(); + + insta::assert_snapshot!( + env.render(&diag), + @r#" + [ + { + "cell": null, + "code": null, + "end_location": null, + "filename": null, + "fix": { + "applicability": "safe", + "edits": [ + { + "content": "edit", + "end_location": null, + "location": null + } + ], + "message": null + }, + "location": null, + "message": "main diagnostic message", + "noqa_row": null, + "url": "https://docs.astral.sh/ruff/rules/test-diagnostic" + } + ] + "#, + ); + } +} diff --git a/crates/ruff_db/src/diagnostic/render/json_lines.rs b/crates/ruff_db/src/diagnostic/render/json_lines.rs new file mode 100644 index 0000000000..44c270e564 --- /dev/null +++ b/crates/ruff_db/src/diagnostic/render/json_lines.rs @@ -0,0 +1,59 @@ +use crate::diagnostic::{Diagnostic, DisplayDiagnosticConfig, render::json::diagnostic_to_json}; + +use super::FileResolver; + +pub(super) struct JsonLinesRenderer<'a> { + resolver: &'a dyn FileResolver, + config: &'a DisplayDiagnosticConfig, +} + +impl<'a> JsonLinesRenderer<'a> { + pub(super) fn new(resolver: &'a dyn FileResolver, config: &'a DisplayDiagnosticConfig) -> Self { + Self { resolver, config } + } +} + +impl JsonLinesRenderer<'_> { + pub(super) fn render( + &self, + f: &mut std::fmt::Formatter, + diagnostics: &[Diagnostic], + ) -> std::fmt::Result { + for diag in diagnostics { + writeln!( + f, + "{}", + serde_json::json!(diagnostic_to_json(diag, self.resolver, self.config)) + )?; + } + + Ok(()) + } +} +#[cfg(test)] +mod tests { + use crate::diagnostic::{ + DiagnosticFormat, + render::tests::{ + create_diagnostics, create_notebook_diagnostics, create_syntax_error_diagnostics, + }, + }; + + #[test] + fn output() { + let (env, diagnostics) = create_diagnostics(DiagnosticFormat::JsonLines); + insta::assert_snapshot!(env.render_diagnostics(&diagnostics)); + } + + #[test] + fn syntax_errors() { + let (env, diagnostics) = create_syntax_error_diagnostics(DiagnosticFormat::JsonLines); + insta::assert_snapshot!(env.render_diagnostics(&diagnostics)); + } + + #[test] + fn notebook_output() { + let (env, diagnostics) = create_notebook_diagnostics(DiagnosticFormat::JsonLines); + insta::assert_snapshot!(env.render_diagnostics(&diagnostics)); + } +} diff --git a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__azure__tests__output.snap b/crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__azure__tests__output.snap similarity index 78% rename from crates/ruff_linter/src/message/snapshots/ruff_linter__message__azure__tests__output.snap rename to crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__azure__tests__output.snap index 263548e8ac..4ad7b67559 100644 --- a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__azure__tests__output.snap +++ b/crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__azure__tests__output.snap @@ -1,7 +1,6 @@ --- -source: crates/ruff_linter/src/message/azure.rs -expression: content -snapshot_kind: text +source: crates/ruff_db/src/diagnostic/render/azure.rs +expression: env.render_diagnostics(&diagnostics) --- ##vso[task.logissue type=error;sourcepath=fib.py;linenumber=1;columnnumber=8;code=F401;]`os` imported but unused ##vso[task.logissue type=error;sourcepath=fib.py;linenumber=6;columnnumber=5;code=F841;]Local variable `x` is assigned to but never used diff --git a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__azure__tests__syntax_errors.snap b/crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__azure__tests__syntax_errors.snap similarity index 73% rename from crates/ruff_linter/src/message/snapshots/ruff_linter__message__azure__tests__syntax_errors.snap rename to crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__azure__tests__syntax_errors.snap index 725467d98b..473567c7d1 100644 --- a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__azure__tests__syntax_errors.snap +++ b/crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__azure__tests__syntax_errors.snap @@ -1,7 +1,6 @@ --- -source: crates/ruff_linter/src/message/azure.rs -expression: content -snapshot_kind: text +source: crates/ruff_db/src/diagnostic/render/azure.rs +expression: env.render_diagnostics(&diagnostics) --- ##vso[task.logissue type=error;sourcepath=syntax_errors.py;linenumber=1;columnnumber=15;]SyntaxError: Expected one or more symbol names after import ##vso[task.logissue type=error;sourcepath=syntax_errors.py;linenumber=3;columnnumber=12;]SyntaxError: Expected ')', found newline diff --git a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__json__tests__notebook_output.snap b/crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__json__tests__notebook_output.snap similarity index 93% rename from crates/ruff_linter/src/message/snapshots/ruff_linter__message__json__tests__notebook_output.snap rename to crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__json__tests__notebook_output.snap index 544435ac5c..ccf202adb2 100644 --- a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__json__tests__notebook_output.snap +++ b/crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__json__tests__notebook_output.snap @@ -1,7 +1,6 @@ --- -source: crates/ruff_linter/src/message/json.rs -expression: content -snapshot_kind: text +source: crates/ruff_db/src/diagnostic/render/json.rs +expression: env.render_diagnostics(&diagnostics) --- [ { @@ -84,8 +83,8 @@ snapshot_kind: text { "content": "", "end_location": { - "column": 10, - "row": 4 + "column": 1, + "row": 5 }, "location": { "column": 1, diff --git a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__json__tests__output.snap b/crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__json__tests__output.snap similarity index 94% rename from crates/ruff_linter/src/message/snapshots/ruff_linter__message__json__tests__output.snap rename to crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__json__tests__output.snap index eaa9ae77bf..1012d4dca6 100644 --- a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__json__tests__output.snap +++ b/crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__json__tests__output.snap @@ -1,7 +1,6 @@ --- -source: crates/ruff_linter/src/message/json.rs -expression: content -snapshot_kind: text +source: crates/ruff_db/src/diagnostic/render/json.rs +expression: env.render_diagnostics(&diagnostics) --- [ { diff --git a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__json__tests__syntax_errors.snap b/crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__json__tests__syntax_errors.snap similarity index 86% rename from crates/ruff_linter/src/message/snapshots/ruff_linter__message__json__tests__syntax_errors.snap rename to crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__json__tests__syntax_errors.snap index e6cb8b9651..412debffab 100644 --- a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__json__tests__syntax_errors.snap +++ b/crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__json__tests__syntax_errors.snap @@ -1,7 +1,6 @@ --- -source: crates/ruff_linter/src/message/json.rs -expression: content -snapshot_kind: text +source: crates/ruff_db/src/diagnostic/render/json.rs +expression: env.render_diagnostics(&diagnostics) --- [ { diff --git a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__json_lines__tests__notebook_output.snap b/crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__json_lines__tests__notebook_output.snap similarity index 69% rename from crates/ruff_linter/src/message/snapshots/ruff_linter__message__json_lines__tests__notebook_output.snap rename to crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__json_lines__tests__notebook_output.snap index 4f1d092d1c..545a85f02d 100644 --- a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__json_lines__tests__notebook_output.snap +++ b/crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__json_lines__tests__notebook_output.snap @@ -1,8 +1,7 @@ --- -source: crates/ruff_linter/src/message/json_lines.rs -expression: content -snapshot_kind: text +source: crates/ruff_db/src/diagnostic/render/json_lines.rs +expression: env.render_diagnostics(&diagnostics) --- {"cell":1,"code":"F401","end_location":{"column":10,"row":2},"filename":"notebook.ipynb","fix":{"applicability":"safe","edits":[{"content":"","end_location":{"column":10,"row":2},"location":{"column":1,"row":2}}],"message":"Remove unused import: `os`"},"location":{"column":8,"row":2},"message":"`os` imported but unused","noqa_row":2,"url":"https://docs.astral.sh/ruff/rules/unused-import"} {"cell":2,"code":"F401","end_location":{"column":12,"row":2},"filename":"notebook.ipynb","fix":{"applicability":"safe","edits":[{"content":"","end_location":{"column":1,"row":3},"location":{"column":1,"row":2}}],"message":"Remove unused import: `math`"},"location":{"column":8,"row":2},"message":"`math` imported but unused","noqa_row":2,"url":"https://docs.astral.sh/ruff/rules/unused-import"} -{"cell":3,"code":"F841","end_location":{"column":6,"row":4},"filename":"notebook.ipynb","fix":{"applicability":"unsafe","edits":[{"content":"","end_location":{"column":10,"row":4},"location":{"column":1,"row":4}}],"message":"Remove assignment to unused variable `x`"},"location":{"column":5,"row":4},"message":"Local variable `x` is assigned to but never used","noqa_row":4,"url":"https://docs.astral.sh/ruff/rules/unused-variable"} +{"cell":3,"code":"F841","end_location":{"column":6,"row":4},"filename":"notebook.ipynb","fix":{"applicability":"unsafe","edits":[{"content":"","end_location":{"column":1,"row":5},"location":{"column":1,"row":4}}],"message":"Remove assignment to unused variable `x`"},"location":{"column":5,"row":4},"message":"Local variable `x` is assigned to but never used","noqa_row":4,"url":"https://docs.astral.sh/ruff/rules/unused-variable"} diff --git a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__json_lines__tests__output.snap b/crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__json_lines__tests__output.snap similarity index 90% rename from crates/ruff_linter/src/message/snapshots/ruff_linter__message__json_lines__tests__output.snap rename to crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__json_lines__tests__output.snap index 9248b0ffd5..6e46781395 100644 --- a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__json_lines__tests__output.snap +++ b/crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__json_lines__tests__output.snap @@ -1,7 +1,6 @@ --- -source: crates/ruff_linter/src/message/json_lines.rs -expression: content -snapshot_kind: text +source: crates/ruff_db/src/diagnostic/render/json_lines.rs +expression: env.render_diagnostics(&diagnostics) --- {"cell":null,"code":"F401","end_location":{"column":10,"row":1},"filename":"fib.py","fix":{"applicability":"unsafe","edits":[{"content":"","end_location":{"column":1,"row":2},"location":{"column":1,"row":1}}],"message":"Remove unused import: `os`"},"location":{"column":8,"row":1},"message":"`os` imported but unused","noqa_row":1,"url":"https://docs.astral.sh/ruff/rules/unused-import"} {"cell":null,"code":"F841","end_location":{"column":6,"row":6},"filename":"fib.py","fix":{"applicability":"unsafe","edits":[{"content":"","end_location":{"column":10,"row":6},"location":{"column":5,"row":6}}],"message":"Remove assignment to unused variable `x`"},"location":{"column":5,"row":6},"message":"Local variable `x` is assigned to but never used","noqa_row":6,"url":"https://docs.astral.sh/ruff/rules/unused-variable"} diff --git a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__json_lines__tests__syntax_errors.snap b/crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__json_lines__tests__syntax_errors.snap similarity index 80% rename from crates/ruff_linter/src/message/snapshots/ruff_linter__message__json_lines__tests__syntax_errors.snap rename to crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__json_lines__tests__syntax_errors.snap index 8466503638..b722248950 100644 --- a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__json_lines__tests__syntax_errors.snap +++ b/crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__json_lines__tests__syntax_errors.snap @@ -1,7 +1,6 @@ --- -source: crates/ruff_linter/src/message/json_lines.rs -expression: content -snapshot_kind: text +source: crates/ruff_db/src/diagnostic/render/json_lines.rs +expression: env.render_diagnostics(&diagnostics) --- {"cell":null,"code":null,"end_location":{"column":1,"row":2},"filename":"syntax_errors.py","fix":null,"location":{"column":15,"row":1},"message":"SyntaxError: Expected one or more symbol names after import","noqa_row":null,"url":null} {"cell":null,"code":null,"end_location":{"column":1,"row":4},"filename":"syntax_errors.py","fix":null,"location":{"column":12,"row":3},"message":"SyntaxError: Expected ')', found newline","noqa_row":null,"url":null} diff --git a/crates/ruff_linter/src/message/azure.rs b/crates/ruff_linter/src/message/azure.rs deleted file mode 100644 index 96433b61ef..0000000000 --- a/crates/ruff_linter/src/message/azure.rs +++ /dev/null @@ -1,71 +0,0 @@ -use std::io::Write; - -use ruff_db::diagnostic::Diagnostic; -use ruff_source_file::LineColumn; - -use crate::message::{Emitter, EmitterContext}; - -/// Generate error logging commands for Azure Pipelines format. -/// See [documentation](https://learn.microsoft.com/en-us/azure/devops/pipelines/scripts/logging-commands?view=azure-devops&tabs=bash#logissue-log-an-error-or-warning) -#[derive(Default)] -pub struct AzureEmitter; - -impl Emitter for AzureEmitter { - fn emit( - &mut self, - writer: &mut dyn Write, - diagnostics: &[Diagnostic], - context: &EmitterContext, - ) -> anyhow::Result<()> { - for diagnostic in diagnostics { - let filename = diagnostic.expect_ruff_filename(); - let location = if context.is_notebook(&filename) { - // We can't give a reasonable location for the structured formats, - // so we show one that's clearly a fallback - LineColumn::default() - } else { - diagnostic.expect_ruff_start_location() - }; - - writeln!( - writer, - "##vso[task.logissue type=error\ - ;sourcepath={filename};linenumber={line};columnnumber={col};{code}]{body}", - line = location.line, - col = location.column, - code = diagnostic - .secondary_code() - .map_or_else(String::new, |code| format!("code={code};")), - body = diagnostic.body(), - )?; - } - - Ok(()) - } -} - -#[cfg(test)] -mod tests { - use insta::assert_snapshot; - - use crate::message::AzureEmitter; - use crate::message::tests::{ - capture_emitter_output, create_diagnostics, create_syntax_error_diagnostics, - }; - - #[test] - fn output() { - let mut emitter = AzureEmitter; - let content = capture_emitter_output(&mut emitter, &create_diagnostics()); - - assert_snapshot!(content); - } - - #[test] - fn syntax_errors() { - let mut emitter = AzureEmitter; - let content = capture_emitter_output(&mut emitter, &create_syntax_error_diagnostics()); - - assert_snapshot!(content); - } -} diff --git a/crates/ruff_linter/src/message/json.rs b/crates/ruff_linter/src/message/json.rs deleted file mode 100644 index 8425975ea5..0000000000 --- a/crates/ruff_linter/src/message/json.rs +++ /dev/null @@ -1,260 +0,0 @@ -use std::io::Write; - -use ruff_diagnostics::Applicability; -use serde::ser::SerializeSeq; -use serde::{Serialize, Serializer}; - -use ruff_db::diagnostic::{Diagnostic, SecondaryCode}; -use ruff_notebook::NotebookIndex; -use ruff_source_file::{LineColumn, OneIndexed, SourceCode}; -use ruff_text_size::Ranged; - -use crate::Edit; -use crate::message::{Emitter, EmitterContext}; - -#[derive(Default)] -pub struct JsonEmitter; - -impl Emitter for JsonEmitter { - fn emit( - &mut self, - writer: &mut dyn Write, - diagnostics: &[Diagnostic], - context: &EmitterContext, - ) -> anyhow::Result<()> { - serde_json::to_writer_pretty( - writer, - &ExpandedMessages { - diagnostics, - context, - }, - )?; - - Ok(()) - } -} - -struct ExpandedMessages<'a> { - diagnostics: &'a [Diagnostic], - context: &'a EmitterContext<'a>, -} - -impl Serialize for ExpandedMessages<'_> { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - let mut s = serializer.serialize_seq(Some(self.diagnostics.len()))?; - - for message in self.diagnostics { - let value = message_to_json_value(message, self.context); - s.serialize_element(&value)?; - } - - s.end() - } -} - -pub(crate) fn message_to_json_value<'a>( - message: &'a Diagnostic, - context: &'a EmitterContext<'a>, -) -> JsonDiagnostic<'a> { - let source_file = message.expect_ruff_source_file(); - let source_code = source_file.to_source_code(); - let filename = message.expect_ruff_filename(); - let notebook_index = context.notebook_index(&filename); - - let mut start_location = source_code.line_column(message.expect_range().start()); - let mut end_location = source_code.line_column(message.expect_range().end()); - let mut noqa_location = message - .noqa_offset() - .map(|offset| source_code.line_column(offset)); - let mut notebook_cell_index = None; - - if let Some(notebook_index) = notebook_index { - notebook_cell_index = Some( - notebook_index - .cell(start_location.line) - .unwrap_or(OneIndexed::MIN), - ); - start_location = notebook_index.translate_line_column(&start_location); - end_location = notebook_index.translate_line_column(&end_location); - noqa_location = - noqa_location.map(|location| notebook_index.translate_line_column(&location)); - } - - let fix = message.fix().map(|fix| JsonFix { - applicability: fix.applicability(), - message: message.suggestion(), - edits: ExpandedEdits { - edits: fix.edits(), - source_code, - notebook_index, - }, - }); - - JsonDiagnostic { - code: message.secondary_code(), - url: message.to_url(), - message: message.body(), - fix, - cell: notebook_cell_index, - location: start_location.into(), - end_location: end_location.into(), - filename, - noqa_row: noqa_location.map(|location| location.line), - } -} - -struct ExpandedEdits<'a> { - edits: &'a [Edit], - source_code: SourceCode<'a, 'a>, - notebook_index: Option<&'a NotebookIndex>, -} - -impl Serialize for ExpandedEdits<'_> { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - let mut s = serializer.serialize_seq(Some(self.edits.len()))?; - - for edit in self.edits { - let mut location = self.source_code.line_column(edit.start()); - let mut end_location = self.source_code.line_column(edit.end()); - - if let Some(notebook_index) = self.notebook_index { - // There exists a newline between each cell's source code in the - // concatenated source code in Ruff. This newline doesn't actually - // exists in the JSON source field. - // - // Now, certain edits may try to remove this newline, which means - // the edit will spill over to the first character of the next cell. - // If it does, we need to translate the end location to the last - // character of the previous cell. - match ( - notebook_index.cell(location.line), - notebook_index.cell(end_location.line), - ) { - (Some(start_cell), Some(end_cell)) if start_cell != end_cell => { - debug_assert_eq!(end_location.column.get(), 1); - - let prev_row = end_location.line.saturating_sub(1); - end_location = LineColumn { - line: notebook_index.cell_row(prev_row).unwrap_or(OneIndexed::MIN), - column: self - .source_code - .line_column(self.source_code.line_end_exclusive(prev_row)) - .column, - }; - } - (Some(_), None) => { - debug_assert_eq!(end_location.column.get(), 1); - - let prev_row = end_location.line.saturating_sub(1); - end_location = LineColumn { - line: notebook_index.cell_row(prev_row).unwrap_or(OneIndexed::MIN), - column: self - .source_code - .line_column(self.source_code.line_end_exclusive(prev_row)) - .column, - }; - } - _ => { - end_location = notebook_index.translate_line_column(&end_location); - } - } - location = notebook_index.translate_line_column(&location); - } - - let value = JsonEdit { - content: edit.content().unwrap_or_default(), - location: location.into(), - end_location: end_location.into(), - }; - - s.serialize_element(&value)?; - } - - s.end() - } -} - -#[derive(Serialize)] -pub(crate) struct JsonDiagnostic<'a> { - cell: Option, - code: Option<&'a SecondaryCode>, - end_location: JsonLocation, - filename: String, - fix: Option>, - location: JsonLocation, - message: &'a str, - noqa_row: Option, - url: Option, -} - -#[derive(Serialize)] -struct JsonFix<'a> { - applicability: Applicability, - edits: ExpandedEdits<'a>, - message: Option<&'a str>, -} - -#[derive(Serialize)] -struct JsonLocation { - column: OneIndexed, - row: OneIndexed, -} - -impl From for JsonLocation { - fn from(location: LineColumn) -> Self { - JsonLocation { - row: location.line, - column: location.column, - } - } -} - -#[derive(Serialize)] -struct JsonEdit<'a> { - content: &'a str, - end_location: JsonLocation, - location: JsonLocation, -} - -#[cfg(test)] -mod tests { - use insta::assert_snapshot; - - use crate::message::JsonEmitter; - use crate::message::tests::{ - capture_emitter_notebook_output, capture_emitter_output, create_diagnostics, - create_notebook_diagnostics, create_syntax_error_diagnostics, - }; - - #[test] - fn output() { - let mut emitter = JsonEmitter; - let content = capture_emitter_output(&mut emitter, &create_diagnostics()); - - assert_snapshot!(content); - } - - #[test] - fn syntax_errors() { - let mut emitter = JsonEmitter; - let content = capture_emitter_output(&mut emitter, &create_syntax_error_diagnostics()); - - assert_snapshot!(content); - } - - #[test] - fn notebook_output() { - let mut emitter = JsonEmitter; - let (diagnostics, notebook_indexes) = create_notebook_diagnostics(); - let content = - capture_emitter_notebook_output(&mut emitter, &diagnostics, ¬ebook_indexes); - - assert_snapshot!(content); - } -} diff --git a/crates/ruff_linter/src/message/json_lines.rs b/crates/ruff_linter/src/message/json_lines.rs deleted file mode 100644 index fc9bd8625a..0000000000 --- a/crates/ruff_linter/src/message/json_lines.rs +++ /dev/null @@ -1,60 +0,0 @@ -use std::io::Write; - -use ruff_db::diagnostic::Diagnostic; - -use crate::message::json::message_to_json_value; -use crate::message::{Emitter, EmitterContext}; - -#[derive(Default)] -pub struct JsonLinesEmitter; - -impl Emitter for JsonLinesEmitter { - fn emit( - &mut self, - writer: &mut dyn Write, - diagnostics: &[Diagnostic], - context: &EmitterContext, - ) -> anyhow::Result<()> { - for diagnostic in diagnostics { - serde_json::to_writer(&mut *writer, &message_to_json_value(diagnostic, context))?; - writer.write_all(b"\n")?; - } - Ok(()) - } -} - -#[cfg(test)] -mod tests { - use insta::assert_snapshot; - - use crate::message::json_lines::JsonLinesEmitter; - use crate::message::tests::{ - capture_emitter_notebook_output, capture_emitter_output, create_diagnostics, - create_notebook_diagnostics, create_syntax_error_diagnostics, - }; - - #[test] - fn output() { - let mut emitter = JsonLinesEmitter; - let content = capture_emitter_output(&mut emitter, &create_diagnostics()); - - assert_snapshot!(content); - } - - #[test] - fn syntax_errors() { - let mut emitter = JsonLinesEmitter; - let content = capture_emitter_output(&mut emitter, &create_syntax_error_diagnostics()); - - assert_snapshot!(content); - } - - #[test] - fn notebook_output() { - let mut emitter = JsonLinesEmitter; - let (messages, notebook_indexes) = create_notebook_diagnostics(); - let content = capture_emitter_notebook_output(&mut emitter, &messages, ¬ebook_indexes); - - assert_snapshot!(content); - } -} diff --git a/crates/ruff_linter/src/message/mod.rs b/crates/ruff_linter/src/message/mod.rs index 2dc119e440..9240830123 100644 --- a/crates/ruff_linter/src/message/mod.rs +++ b/crates/ruff_linter/src/message/mod.rs @@ -3,17 +3,17 @@ use std::fmt::Display; use std::io::Write; use std::ops::Deref; -use ruff_db::diagnostic::{ - Annotation, Diagnostic, DiagnosticId, LintName, SecondaryCode, Severity, Span, -}; use rustc_hash::FxHashMap; -pub use azure::AzureEmitter; +use ruff_db::diagnostic::{ + Annotation, Diagnostic, DiagnosticId, FileResolver, Input, LintName, SecondaryCode, Severity, + Span, UnifiedFile, +}; +use ruff_db::files::File; + pub use github::GithubEmitter; pub use gitlab::GitlabEmitter; pub use grouped::GroupedEmitter; -pub use json::JsonEmitter; -pub use json_lines::JsonLinesEmitter; pub use junit::JunitEmitter; pub use pylint::PylintEmitter; pub use rdjson::RdjsonEmitter; @@ -26,13 +26,10 @@ pub use text::TextEmitter; use crate::Fix; use crate::registry::Rule; -mod azure; mod diff; mod github; mod gitlab; mod grouped; -mod json; -mod json_lines; mod junit; mod pylint; mod rdjson; @@ -107,6 +104,34 @@ where diagnostic } +impl FileResolver for EmitterContext<'_> { + fn path(&self, _file: File) -> &str { + unimplemented!("Expected a Ruff file for rendering a Ruff diagnostic"); + } + + fn input(&self, _file: File) -> Input { + unimplemented!("Expected a Ruff file for rendering a Ruff diagnostic"); + } + + fn notebook_index(&self, file: &UnifiedFile) -> Option { + match file { + UnifiedFile::Ty(_) => { + unimplemented!("Expected a Ruff file for rendering a Ruff diagnostic") + } + UnifiedFile::Ruff(file) => self.notebook_indexes.get(file.name()).cloned(), + } + } + + fn is_notebook(&self, file: &UnifiedFile) -> bool { + match file { + UnifiedFile::Ty(_) => { + unimplemented!("Expected a Ruff file for rendering a Ruff diagnostic") + } + UnifiedFile::Ruff(file) => self.notebook_indexes.get(file.name()).is_some(), + } + } +} + struct MessageWithLocation<'a> { message: &'a Diagnostic, start_location: LineColumn, diff --git a/crates/ruff_linter/src/message/rdjson.rs b/crates/ruff_linter/src/message/rdjson.rs index 8be2c56a54..dba61e64a5 100644 --- a/crates/ruff_linter/src/message/rdjson.rs +++ b/crates/ruff_linter/src/message/rdjson.rs @@ -73,7 +73,7 @@ fn message_to_rdjson_value(message: &Diagnostic) -> Value { }, "code": { "value": message.secondary_code(), - "url": message.to_url(), + "url": message.to_ruff_url(), }, "suggestions": rdjson_suggestions(fix.edits(), &source_code), }) @@ -86,7 +86,7 @@ fn message_to_rdjson_value(message: &Diagnostic) -> Value { }, "code": { "value": message.secondary_code(), - "url": message.to_url(), + "url": message.to_ruff_url(), }, }) } diff --git a/crates/ruff_server/src/lint.rs b/crates/ruff_server/src/lint.rs index 4283cde139..d1dfb20267 100644 --- a/crates/ruff_server/src/lint.rs +++ b/crates/ruff_server/src/lint.rs @@ -301,7 +301,7 @@ fn to_lsp_diagnostic( severity, tags, code, - code_description: diagnostic.to_url().and_then(|url| { + code_description: diagnostic.to_ruff_url().and_then(|url| { Some(lsp_types::CodeDescription { href: lsp_types::Url::parse(&url).ok()?, }) diff --git a/crates/ty/src/args.rs b/crates/ty/src/args.rs index c177c9d64b..243f596998 100644 --- a/crates/ty/src/args.rs +++ b/crates/ty/src/args.rs @@ -323,8 +323,8 @@ pub enum OutputFormat { Concise, } -impl From for ruff_db::diagnostic::DiagnosticFormat { - fn from(format: OutputFormat) -> ruff_db::diagnostic::DiagnosticFormat { +impl From for ty_project::metadata::options::OutputFormat { + fn from(format: OutputFormat) -> ty_project::metadata::options::OutputFormat { match format { OutputFormat::Full => Self::Full, OutputFormat::Concise => Self::Concise, diff --git a/crates/ty/src/lib.rs b/crates/ty/src/lib.rs index 1286bae8c8..25f5085567 100644 --- a/crates/ty/src/lib.rs +++ b/crates/ty/src/lib.rs @@ -290,7 +290,7 @@ impl MainLoop { } => { let terminal_settings = db.project().settings(db).terminal(); let display_config = DisplayDiagnosticConfig::default() - .format(terminal_settings.output_format) + .format(terminal_settings.output_format.into()) .color(colored::control::SHOULD_COLORIZE.should_colorize()); if check_revision == revision { diff --git a/crates/ty_project/src/metadata/options.rs b/crates/ty_project/src/metadata/options.rs index acb89e8cc1..5b2e824825 100644 --- a/crates/ty_project/src/metadata/options.rs +++ b/crates/ty_project/src/metadata/options.rs @@ -979,6 +979,39 @@ impl GlobFilterContext { } } +/// The diagnostic output format. +#[derive(Debug, Default, Clone, Copy, Eq, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case", deny_unknown_fields)] +#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] +pub enum OutputFormat { + /// The default full mode will print "pretty" diagnostics. + /// + /// That is, color will be used when printing to a `tty`. + /// Moreover, diagnostic messages may include additional + /// context and annotations on the input to help understand + /// the message. + #[default] + Full, + /// Print diagnostics in a concise mode. + /// + /// This will guarantee that each diagnostic is printed on + /// a single line. Only the most important or primary aspects + /// of the diagnostic are included. Contextual information is + /// dropped. + /// + /// This may use color when printing to a `tty`. + Concise, +} + +impl From for DiagnosticFormat { + fn from(value: OutputFormat) -> Self { + match value { + OutputFormat::Full => Self::Full, + OutputFormat::Concise => Self::Concise, + } + } +} + #[derive( Debug, Default, Clone, Eq, PartialEq, Combine, Serialize, Deserialize, OptionsMetadata, )] @@ -996,7 +1029,7 @@ pub struct TerminalOptions { output-format = "concise" "# )] - pub output_format: Option>, + pub output_format: Option>, /// Use exit code 1 if there are any warning-level diagnostics. /// /// Defaults to `false`. @@ -1295,7 +1328,7 @@ pub(super) struct InnerOverrideOptions { #[derive(Debug)] pub struct ToSettingsError { diagnostic: Box, - output_format: DiagnosticFormat, + output_format: OutputFormat, color: bool, } @@ -1309,7 +1342,7 @@ impl ToSettingsError { impl fmt::Display for DisplayPretty<'_> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let display_config = DisplayDiagnosticConfig::default() - .format(self.error.output_format) + .format(self.error.output_format.into()) .color(self.error.color); write!( diff --git a/crates/ty_project/src/metadata/settings.rs b/crates/ty_project/src/metadata/settings.rs index 98cef0f74c..1c5221e9cc 100644 --- a/crates/ty_project/src/metadata/settings.rs +++ b/crates/ty_project/src/metadata/settings.rs @@ -1,9 +1,9 @@ use std::sync::Arc; -use ruff_db::{diagnostic::DiagnosticFormat, files::File}; +use ruff_db::files::File; use ty_python_semantic::lint::RuleSelection; -use crate::metadata::options::InnerOverrideOptions; +use crate::metadata::options::{InnerOverrideOptions, OutputFormat}; use crate::{Db, combine::Combine, glob::IncludeExcludeFilter}; /// The resolved [`super::Options`] for the project. @@ -57,7 +57,7 @@ impl Settings { #[derive(Debug, Clone, PartialEq, Eq, Default)] pub struct TerminalSettings { - pub output_format: DiagnosticFormat, + pub output_format: OutputFormat, pub error_on_warning: bool, } diff --git a/ty.schema.json b/ty.schema.json index 7579b18235..0a70793b2c 100644 --- a/ty.schema.json +++ b/ty.schema.json @@ -58,25 +58,6 @@ }, "additionalProperties": false, "definitions": { - "DiagnosticFormat": { - "description": "The diagnostic output format.", - "oneOf": [ - { - "description": "The default full mode will print \"pretty\" diagnostics.\n\nThat is, color will be used when printing to a `tty`. Moreover, diagnostic messages may include additional context and annotations on the input to help understand the message.", - "type": "string", - "enum": [ - "full" - ] - }, - { - "description": "Print diagnostics in a concise mode.\n\nThis will guarantee that each diagnostic is printed on a single line. Only the most important or primary aspects of the diagnostic are included. Contextual information is dropped.\n\nThis may use color when printing to a `tty`.", - "type": "string", - "enum": [ - "concise" - ] - } - ] - }, "EnvironmentOptions": { "type": "object", "properties": { @@ -167,6 +148,25 @@ } ] }, + "OutputFormat": { + "description": "The diagnostic output format.", + "oneOf": [ + { + "description": "The default full mode will print \"pretty\" diagnostics.\n\nThat is, color will be used when printing to a `tty`. Moreover, diagnostic messages may include additional context and annotations on the input to help understand the message.", + "type": "string", + "enum": [ + "full" + ] + }, + { + "description": "Print diagnostics in a concise mode.\n\nThis will guarantee that each diagnostic is printed on a single line. Only the most important or primary aspects of the diagnostic are included. Contextual information is dropped.\n\nThis may use color when printing to a `tty`.", + "type": "string", + "enum": [ + "concise" + ] + } + ] + }, "OverrideOptions": { "type": "object", "properties": { @@ -991,7 +991,7 @@ "description": "The format to use for printing diagnostic messages.\n\nDefaults to `full`.", "anyOf": [ { - "$ref": "#/definitions/DiagnosticFormat" + "$ref": "#/definitions/OutputFormat" }, { "type": "null"