diff --git a/Cargo.lock b/Cargo.lock index 8886b0c5ea..0a3feac2e9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3022,7 +3022,6 @@ dependencies = [ "pep440_rs", "pyproject-toml", "regex", - "ruff_annotate_snippets", "ruff_cache", "ruff_db", "ruff_diagnostics", diff --git a/crates/ruff_linter/Cargo.toml b/crates/ruff_linter/Cargo.toml index 8c314df19d..156bba22e7 100644 --- a/crates/ruff_linter/Cargo.toml +++ b/crates/ruff_linter/Cargo.toml @@ -13,7 +13,6 @@ license = { workspace = true } [lib] [dependencies] -ruff_annotate_snippets = { workspace = true } ruff_cache = { workspace = true } ruff_db = { workspace = true, features = ["junit", "serde"] } ruff_diagnostics = { workspace = true, features = ["serde"] } diff --git a/crates/ruff_linter/src/message/grouped.rs b/crates/ruff_linter/src/message/grouped.rs index 7e75134891..0a7de26b2d 100644 --- a/crates/ruff_linter/src/message/grouped.rs +++ b/crates/ruff_linter/src/message/grouped.rs @@ -1,3 +1,4 @@ +use std::collections::BTreeMap; use std::fmt::{Display, Formatter}; use std::io::Write; use std::num::NonZeroUsize; @@ -6,18 +7,16 @@ use colored::Colorize; use ruff_db::diagnostic::Diagnostic; use ruff_notebook::NotebookIndex; -use ruff_source_file::OneIndexed; +use ruff_source_file::{LineColumn, OneIndexed}; use crate::fs::relativize_path; use crate::message::diff::calculate_print_width; -use crate::message::text::{MessageCodeFrame, RuleCodeAndBody}; -use crate::message::{Emitter, EmitterContext, MessageWithLocation, group_diagnostics_by_filename}; +use crate::message::{Emitter, EmitterContext}; use crate::settings::types::UnsafeFixes; #[derive(Default)] pub struct GroupedEmitter { show_fix_status: bool, - show_source: bool, unsafe_fixes: UnsafeFixes, } @@ -28,12 +27,6 @@ impl GroupedEmitter { self } - #[must_use] - pub fn with_show_source(mut self, show_source: bool) -> Self { - self.show_source = show_source; - self - } - #[must_use] pub fn with_unsafe_fixes(mut self, unsafe_fixes: UnsafeFixes) -> Self { self.unsafe_fixes = unsafe_fixes; @@ -76,29 +69,53 @@ impl Emitter for GroupedEmitter { message, show_fix_status: self.show_fix_status, unsafe_fixes: self.unsafe_fixes, - show_source: self.show_source, row_length, column_length, } )?; } - // Print a blank line between files, unless we're showing the source, in which case - // we'll have already printed a blank line between messages. - if !self.show_source { - writeln!(writer)?; - } + // Print a blank line between files. + writeln!(writer)?; } Ok(()) } } +struct MessageWithLocation<'a> { + message: &'a Diagnostic, + start_location: LineColumn, +} + +impl std::ops::Deref for MessageWithLocation<'_> { + type Target = Diagnostic; + + fn deref(&self) -> &Self::Target { + self.message + } +} + +fn group_diagnostics_by_filename( + diagnostics: &[Diagnostic], +) -> BTreeMap>> { + let mut grouped_messages = BTreeMap::default(); + for diagnostic in diagnostics { + grouped_messages + .entry(diagnostic.expect_ruff_filename()) + .or_insert_with(Vec::new) + .push(MessageWithLocation { + message: diagnostic, + start_location: diagnostic.expect_ruff_start_location(), + }); + } + grouped_messages +} + struct DisplayGroupedMessage<'a> { message: MessageWithLocation<'a>, show_fix_status: bool, unsafe_fixes: UnsafeFixes, - show_source: bool, row_length: NonZeroUsize, column_length: NonZeroUsize, notebook_index: Option<&'a NotebookIndex>, @@ -152,51 +169,50 @@ impl Display for DisplayGroupedMessage<'_> { }, )?; - if self.show_source { - use std::fmt::Write; - let mut padded = PadAdapter::new(f); - writeln!( - padded, - "{}", - MessageCodeFrame { - message, - notebook_index: self.notebook_index + Ok(()) + } +} + +pub(super) struct RuleCodeAndBody<'a> { + pub(crate) message: &'a Diagnostic, + pub(crate) show_fix_status: bool, + pub(crate) unsafe_fixes: UnsafeFixes, +} + +impl Display for RuleCodeAndBody<'_> { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + if self.show_fix_status { + if let Some(fix) = self.message.fix() { + // Do not display an indicator for inapplicable fixes + if fix.applies(self.unsafe_fixes.required_applicability()) { + if let Some(code) = self.message.secondary_code() { + write!(f, "{} ", code.red().bold())?; + } + return write!( + f, + "{fix}{body}", + fix = format_args!("[{}] ", "*".cyan()), + body = self.message.body(), + ); } - )?; - } - - Ok(()) - } -} - -/// Adapter that adds a ' ' at the start of every line without the need to copy the string. -/// Inspired by Rust's `debug_struct()` internal implementation that also uses a `PadAdapter`. -struct PadAdapter<'buf> { - buf: &'buf mut (dyn std::fmt::Write + 'buf), - on_newline: bool, -} - -impl<'buf> PadAdapter<'buf> { - fn new(buf: &'buf mut (dyn std::fmt::Write + 'buf)) -> Self { - Self { - buf, - on_newline: true, - } - } -} - -impl std::fmt::Write for PadAdapter<'_> { - fn write_str(&mut self, s: &str) -> std::fmt::Result { - for s in s.split_inclusive('\n') { - if self.on_newline { - self.buf.write_str(" ")?; } - - self.on_newline = s.ends_with('\n'); - self.buf.write_str(s)?; } - Ok(()) + if let Some(code) = self.message.secondary_code() { + write!( + f, + "{code} {body}", + code = code.red().bold(), + body = self.message.body(), + ) + } else { + write!( + f, + "{code}: {body}", + code = self.message.id().as_str().red().bold(), + body = self.message.body(), + ) + } } } @@ -226,19 +242,9 @@ mod tests { assert_snapshot!(content); } - #[test] - fn show_source() { - let mut emitter = GroupedEmitter::default().with_show_source(true); - let content = capture_emitter_output(&mut emitter, &create_diagnostics()); - - assert_snapshot!(content); - } - #[test] fn fix_status() { - let mut emitter = GroupedEmitter::default() - .with_show_fix_status(true) - .with_show_source(true); + let mut emitter = GroupedEmitter::default().with_show_fix_status(true); let content = capture_emitter_output(&mut emitter, &create_diagnostics()); assert_snapshot!(content); @@ -248,7 +254,6 @@ mod tests { fn fix_status_unsafe() { let mut emitter = GroupedEmitter::default() .with_show_fix_status(true) - .with_show_source(true) .with_unsafe_fixes(UnsafeFixes::Enabled); let content = capture_emitter_output(&mut emitter, &create_diagnostics()); diff --git a/crates/ruff_linter/src/message/mod.rs b/crates/ruff_linter/src/message/mod.rs index df56a8ff59..b6328c686b 100644 --- a/crates/ruff_linter/src/message/mod.rs +++ b/crates/ruff_linter/src/message/mod.rs @@ -1,7 +1,5 @@ -use std::collections::BTreeMap; use std::fmt::Display; use std::io::Write; -use std::ops::Deref; use rustc_hash::FxHashMap; @@ -15,7 +13,7 @@ pub use github::GithubEmitter; pub use gitlab::GitlabEmitter; pub use grouped::GroupedEmitter; use ruff_notebook::NotebookIndex; -use ruff_source_file::{LineColumn, SourceFile}; +use ruff_source_file::SourceFile; use ruff_text_size::{Ranged, TextRange, TextSize}; pub use sarif::SarifEmitter; pub use text::TextEmitter; @@ -134,35 +132,6 @@ impl FileResolver for EmitterContext<'_> { } } -struct MessageWithLocation<'a> { - message: &'a Diagnostic, - start_location: LineColumn, -} - -impl Deref for MessageWithLocation<'_> { - type Target = Diagnostic; - - fn deref(&self) -> &Self::Target { - self.message - } -} - -fn group_diagnostics_by_filename( - diagnostics: &[Diagnostic], -) -> BTreeMap>> { - let mut grouped_messages = BTreeMap::default(); - for diagnostic in diagnostics { - grouped_messages - .entry(diagnostic.expect_ruff_filename()) - .or_insert_with(Vec::new) - .push(MessageWithLocation { - message: diagnostic, - start_location: diagnostic.expect_ruff_start_location(), - }); - } - grouped_messages -} - /// Display format for [`Diagnostic`]s. /// /// The emitter serializes a slice of [`Diagnostic`]s and writes them to a [`Write`]. diff --git a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__grouped__tests__fix_status.snap b/crates/ruff_linter/src/message/snapshots/ruff_linter__message__grouped__tests__fix_status.snap index 50b46b41e2..7c9d9d5bfc 100644 --- a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__grouped__tests__fix_status.snap +++ b/crates/ruff_linter/src/message/snapshots/ruff_linter__message__grouped__tests__fix_status.snap @@ -1,30 +1,10 @@ --- source: crates/ruff_linter/src/message/grouped.rs expression: content -snapshot_kind: text --- fib.py: 1:8 F401 `os` imported but unused - | - 1 | import os - | ^^ F401 - | - = help: Remove unused import: `os` - 6:5 F841 Local variable `x` is assigned to but never used - | - 4 | def fibonacci(n): - 5 | """Compute the nth number in the Fibonacci sequence.""" - 6 | x = 1 - | ^ F841 - 7 | if n == 0: - 8 | return 0 - | - = help: Remove assignment to unused variable `x` - + undef.py: 1:4 F821 Undefined name `a` - | - 1 | if a == 1: pass - | ^ F821 - | diff --git a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__grouped__tests__fix_status_unsafe.snap b/crates/ruff_linter/src/message/snapshots/ruff_linter__message__grouped__tests__fix_status_unsafe.snap index 0ed5b6efcb..d812b15604 100644 --- a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__grouped__tests__fix_status_unsafe.snap +++ b/crates/ruff_linter/src/message/snapshots/ruff_linter__message__grouped__tests__fix_status_unsafe.snap @@ -1,30 +1,10 @@ --- source: crates/ruff_linter/src/message/grouped.rs expression: content -snapshot_kind: text --- fib.py: 1:8 F401 [*] `os` imported but unused - | - 1 | import os - | ^^ F401 - | - = help: Remove unused import: `os` - 6:5 F841 [*] Local variable `x` is assigned to but never used - | - 4 | def fibonacci(n): - 5 | """Compute the nth number in the Fibonacci sequence.""" - 6 | x = 1 - | ^ F841 - 7 | if n == 0: - 8 | return 0 - | - = help: Remove assignment to unused variable `x` - + undef.py: 1:4 F821 Undefined name `a` - | - 1 | if a == 1: pass - | ^ F821 - | diff --git a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__grouped__tests__show_source.snap b/crates/ruff_linter/src/message/snapshots/ruff_linter__message__grouped__tests__show_source.snap deleted file mode 100644 index 50b46b41e2..0000000000 --- a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__grouped__tests__show_source.snap +++ /dev/null @@ -1,30 +0,0 @@ ---- -source: crates/ruff_linter/src/message/grouped.rs -expression: content -snapshot_kind: text ---- -fib.py: - 1:8 F401 `os` imported but unused - | - 1 | import os - | ^^ F401 - | - = help: Remove unused import: `os` - - 6:5 F841 Local variable `x` is assigned to but never used - | - 4 | def fibonacci(n): - 5 | """Compute the nth number in the Fibonacci sequence.""" - 6 | x = 1 - | ^ F841 - 7 | if n == 0: - 8 | return 0 - | - = help: Remove assignment to unused variable `x` - -undef.py: - 1:4 F821 Undefined name `a` - | - 1 | if a == 1: pass - | ^ F821 - | diff --git a/crates/ruff_linter/src/message/text.rs b/crates/ruff_linter/src/message/text.rs index b20e4f32b0..610a7a750c 100644 --- a/crates/ruff_linter/src/message/text.rs +++ b/crates/ruff_linter/src/message/text.rs @@ -1,16 +1,6 @@ -use std::borrow::Cow; -use std::fmt::{Display, Formatter}; use std::io::Write; -use colored::Colorize; -use ruff_annotate_snippets::{Level, Renderer, Snippet}; - -use ruff_db::diagnostic::{ - Diagnostic, DiagnosticFormat, DisplayDiagnosticConfig, SecondaryCode, ceil_char_boundary, -}; -use ruff_notebook::NotebookIndex; -use ruff_source_file::OneIndexed; -use ruff_text_size::{TextLen, TextRange, TextSize}; +use ruff_db::diagnostic::{Diagnostic, DiagnosticFormat, DisplayDiagnosticConfig}; use crate::message::diff::Diff; use crate::message::{Emitter, EmitterContext}; @@ -101,267 +91,6 @@ impl Emitter for TextEmitter { } } -pub(super) struct RuleCodeAndBody<'a> { - pub(crate) message: &'a Diagnostic, - pub(crate) show_fix_status: bool, - pub(crate) unsafe_fixes: UnsafeFixes, -} - -impl Display for RuleCodeAndBody<'_> { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - if self.show_fix_status { - if let Some(fix) = self.message.fix() { - // Do not display an indicator for inapplicable fixes - if fix.applies(self.unsafe_fixes.required_applicability()) { - if let Some(code) = self.message.secondary_code() { - write!(f, "{} ", code.red().bold())?; - } - return write!( - f, - "{fix}{body}", - fix = format_args!("[{}] ", "*".cyan()), - body = self.message.body(), - ); - } - } - } - - if let Some(code) = self.message.secondary_code() { - write!( - f, - "{code} {body}", - code = code.red().bold(), - body = self.message.body(), - ) - } else { - write!( - f, - "{code}: {body}", - code = self.message.id().as_str().red().bold(), - body = self.message.body(), - ) - } - } -} - -pub(super) struct MessageCodeFrame<'a> { - pub(crate) message: &'a Diagnostic, - pub(crate) notebook_index: Option<&'a NotebookIndex>, -} - -impl Display for MessageCodeFrame<'_> { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - let suggestion = self.message.first_help_text(); - let footers = if let Some(suggestion) = suggestion { - vec![Level::Help.title(suggestion)] - } else { - Vec::new() - }; - - let source_file = self.message.expect_ruff_source_file(); - let source_code = source_file.to_source_code(); - - let content_start_index = source_code.line_index(self.message.expect_range().start()); - let mut start_index = content_start_index.saturating_sub(2); - - // If we're working with a Jupyter Notebook, skip the lines which are - // outside of the cell containing the diagnostic. - if let Some(index) = self.notebook_index { - let content_start_cell = index.cell(content_start_index).unwrap_or(OneIndexed::MIN); - while start_index < content_start_index { - if index.cell(start_index).unwrap_or(OneIndexed::MIN) == content_start_cell { - break; - } - start_index = start_index.saturating_add(1); - } - } - - // Trim leading empty lines. - while start_index < content_start_index { - if !source_code.line_text(start_index).trim().is_empty() { - break; - } - start_index = start_index.saturating_add(1); - } - - let content_end_index = source_code.line_index(self.message.expect_range().end()); - let mut end_index = content_end_index - .saturating_add(2) - .min(OneIndexed::from_zero_indexed(source_code.line_count())); - - // If we're working with a Jupyter Notebook, skip the lines which are - // outside of the cell containing the diagnostic. - if let Some(index) = self.notebook_index { - let content_end_cell = index.cell(content_end_index).unwrap_or(OneIndexed::MIN); - while end_index > content_end_index { - if index.cell(end_index).unwrap_or(OneIndexed::MIN) == content_end_cell { - break; - } - end_index = end_index.saturating_sub(1); - } - } - - // Trim trailing empty lines. - while end_index > content_end_index { - if !source_code.line_text(end_index).trim().is_empty() { - break; - } - - end_index = end_index.saturating_sub(1); - } - - let start_offset = source_code.line_start(start_index); - let end_offset = source_code.line_end(end_index); - - let source = replace_unprintable( - source_code.slice(TextRange::new(start_offset, end_offset)), - self.message.expect_range() - start_offset, - ) - .fix_up_empty_spans_after_line_terminator(); - - let label = self - .message - .secondary_code() - .map(SecondaryCode::as_str) - .unwrap_or_default(); - - let line_start = self.notebook_index.map_or_else( - || start_index.get(), - |notebook_index| { - notebook_index - .cell_row(start_index) - .unwrap_or(OneIndexed::MIN) - .get() - }, - ); - - let span = usize::from(source.annotation_range.start()) - ..usize::from(source.annotation_range.end()); - let annotation = Level::Error.span(span).label(label); - let snippet = Snippet::source(&source.text) - .line_start(line_start) - .annotation(annotation) - .fold(false); - let message = Level::None.title("").snippet(snippet).footers(footers); - - let renderer = if !cfg!(test) && colored::control::SHOULD_COLORIZE.should_colorize() { - Renderer::styled() - } else { - Renderer::plain() - } - .cut_indicator("…"); - let rendered = renderer.render(message); - writeln!(f, "{rendered}") - } -} - -/// Given some source code and an annotation range, this routine replaces -/// unprintable characters with printable representations of them. -/// -/// The source code returned has an annotation that is updated to reflect -/// changes made to the source code (if any). -/// -/// We don't need to normalize whitespace, such as converting tabs to spaces, -/// because `annotate-snippets` handles that internally. Similarly, it's safe to -/// modify the annotation ranges by inserting 3-byte Unicode replacements -/// because `annotate-snippets` will account for their actual width when -/// rendering and displaying the column to the user. -fn replace_unprintable(source: &str, annotation_range: TextRange) -> SourceCode<'_> { - let mut result = String::new(); - let mut last_end = 0; - let mut range = annotation_range; - - // Updates the range given by the caller whenever a single byte (at - // `index` in `source`) is replaced with `len` bytes. - // - // When the index occurs before the start of the range, the range is - // offset by `len`. When the range occurs after or at the start but before - // the end, then the end of the range only is offset by `len`. - let mut update_range = |index, len| { - if index < usize::from(annotation_range.start()) { - range += TextSize::new(len - 1); - } else if index < usize::from(annotation_range.end()) { - range = range.add_end(TextSize::new(len - 1)); - } - }; - - // If `c` is an unprintable character, then this returns a printable - // representation of it (using a fancier Unicode codepoint). - let unprintable_replacement = |c: char| -> Option { - match c { - '\x07' => Some('␇'), - '\x08' => Some('␈'), - '\x1b' => Some('␛'), - '\x7f' => Some('␡'), - _ => None, - } - }; - - for (index, c) in source.char_indices() { - if let Some(printable) = unprintable_replacement(c) { - result.push_str(&source[last_end..index]); - result.push(printable); - last_end = index + 1; - - let len = printable.text_len().to_u32(); - update_range(index, len); - } - } - - // No tabs or unprintable chars - if result.is_empty() { - SourceCode { - annotation_range, - text: Cow::Borrowed(source), - } - } else { - result.push_str(&source[last_end..]); - SourceCode { - annotation_range: range, - text: Cow::Owned(result), - } - } -} - -struct SourceCode<'a> { - text: Cow<'a, str>, - annotation_range: TextRange, -} - -impl<'a> SourceCode<'a> { - /// This attempts to "fix up" the span on `SourceCode` in the case where - /// it's an empty span immediately following a line terminator. - /// - /// At present, `annotate-snippets` (both upstream and our vendored copy) - /// will render annotations of such spans to point to the space immediately - /// following the previous line. But ideally, this should point to the space - /// immediately preceding the next line. - /// - /// After attempting to fix `annotate-snippets` and giving up after a couple - /// hours, this routine takes a different tact: it adjusts the span to be - /// non-empty and it will cover the first codepoint of the following line. - /// This forces `annotate-snippets` to point to the right place. - /// - /// See also: - fn fix_up_empty_spans_after_line_terminator(self) -> SourceCode<'a> { - if !self.annotation_range.is_empty() - || self.annotation_range.start() == TextSize::from(0) - || self.annotation_range.start() >= self.text.text_len() - { - return self; - } - if self.text.as_bytes()[self.annotation_range.start().to_usize() - 1] != b'\n' { - return self; - } - let start = self.annotation_range.start(); - let end = ceil_char_boundary(&self.text, start + TextSize::from(1)); - SourceCode { - annotation_range: TextRange::new(start, end), - ..self - } - } -} - #[cfg(test)] mod tests { use insta::assert_snapshot;