diff --git a/.github/workflows/ty-ecosystem-analyzer.yaml b/.github/workflows/ty-ecosystem-analyzer.yaml index cfefcf6dc5..ef251600ce 100644 --- a/.github/workflows/ty-ecosystem-analyzer.yaml +++ b/.github/workflows/ty-ecosystem-analyzer.yaml @@ -64,7 +64,7 @@ jobs: cd .. - uv tool install "git+https://github.com/astral-sh/ecosystem-analyzer@f0eec0e549684d8e1d7b8bc3e351202124b63bda" + uv tool install "git+https://github.com/astral-sh/ecosystem-analyzer@27dd66d9e397d986ef9c631119ee09556eab8af9" ecosystem-analyzer \ --repository ruff \ diff --git a/.github/workflows/ty-ecosystem-report.yaml b/.github/workflows/ty-ecosystem-report.yaml index 997569f254..794c5c9591 100644 --- a/.github/workflows/ty-ecosystem-report.yaml +++ b/.github/workflows/ty-ecosystem-report.yaml @@ -49,7 +49,7 @@ jobs: cd .. - uv tool install "git+https://github.com/astral-sh/ecosystem-analyzer@f0eec0e549684d8e1d7b8bc3e351202124b63bda" + uv tool install "git+https://github.com/astral-sh/ecosystem-analyzer@27dd66d9e397d986ef9c631119ee09556eab8af9" ecosystem-analyzer \ --verbose \ diff --git a/crates/ruff/src/printer.rs b/crates/ruff/src/printer.rs index dc9f8c8538..186c481890 100644 --- a/crates/ruff/src/printer.rs +++ b/crates/ruff/src/printer.rs @@ -264,6 +264,7 @@ impl Printer { .with_show_fix_diff(self.flags.intersects(Flags::SHOW_FIX_DIFF)) .with_show_source(self.format == OutputFormat::Full) .with_unsafe_fixes(self.unsafe_fixes) + .with_preview(preview) .emit(writer, &diagnostics.inner, &context)?; if self.flags.intersects(Flags::SHOW_FIX_SUMMARY) { diff --git a/crates/ruff_db/src/diagnostic/mod.rs b/crates/ruff_db/src/diagnostic/mod.rs index 63707302eb..c5eb007b88 100644 --- a/crates/ruff_db/src/diagnostic/mod.rs +++ b/crates/ruff_db/src/diagnostic/mod.rs @@ -1,6 +1,6 @@ use std::{fmt::Formatter, path::Path, sync::Arc}; -use ruff_diagnostics::Fix; +use ruff_diagnostics::{Applicability, Fix}; use ruff_source_file::{LineColumn, SourceCode, SourceFile}; use ruff_annotate_snippets::Level as AnnotateLevel; @@ -1188,7 +1188,9 @@ impl Severity { /// Like [`Severity`] but exclusively for sub-diagnostics. /// -/// This supports an additional `Help` severity that may not be needed in main diagnostics. +/// This type only exists to add an additional `Help` severity that isn't present in `Severity` or +/// used for main diagnostics. If we want to add `Severity::Help` in the future, this type could be +/// deleted and the two combined again. #[derive(Debug, Clone, Copy, PartialEq, Eq, Ord, PartialOrd, get_size2::GetSize)] pub enum SubDiagnosticSeverity { Help, @@ -1236,6 +1238,15 @@ pub struct DisplayDiagnosticConfig { reason = "This is currently only used for JSON but will be needed soon for other formats" )] preview: bool, + /// Whether to hide the real `Severity` of diagnostics. + /// + /// This is intended for temporary use by Ruff, which only has a single `error` severity at the + /// moment. We should be able to remove this option when Ruff gets more severities. + hide_severity: bool, + /// Whether to show the availability of a fix in a diagnostic. + show_fix_status: bool, + /// The lowest applicability that should be shown when reporting diagnostics. + fix_applicability: Applicability, } impl DisplayDiagnosticConfig { @@ -1264,6 +1275,35 @@ impl DisplayDiagnosticConfig { ..self } } + + /// Whether to hide a diagnostic's severity or not. + pub fn hide_severity(self, yes: bool) -> DisplayDiagnosticConfig { + DisplayDiagnosticConfig { + hide_severity: yes, + ..self + } + } + + /// Whether to show a fix's availability or not. + pub fn show_fix_status(self, yes: bool) -> DisplayDiagnosticConfig { + DisplayDiagnosticConfig { + show_fix_status: yes, + ..self + } + } + + /// Set the lowest fix applicability that should be shown. + /// + /// In other words, an applicability of `Safe` (the default) would suppress showing fixes or fix + /// availability for unsafe or display-only fixes. + /// + /// Note that this option is currently ignored when `hide_severity` is false. + pub fn fix_applicability(self, applicability: Applicability) -> DisplayDiagnosticConfig { + DisplayDiagnosticConfig { + fix_applicability: applicability, + ..self + } + } } impl Default for DisplayDiagnosticConfig { @@ -1273,6 +1313,9 @@ impl Default for DisplayDiagnosticConfig { color: false, context: 2, preview: false, + hide_severity: false, + show_fix_status: false, + fix_applicability: Applicability::Safe, } } } diff --git a/crates/ruff_db/src/diagnostic/render.rs b/crates/ruff_db/src/diagnostic/render.rs index 70bb05f385..393272f39c 100644 --- a/crates/ruff_db/src/diagnostic/render.rs +++ b/crates/ruff_db/src/diagnostic/render.rs @@ -9,7 +9,7 @@ use ruff_notebook::{Notebook, NotebookIndex}; use ruff_source_file::{LineIndex, OneIndexed, SourceCode}; use ruff_text_size::{TextRange, TextSize}; -use crate::diagnostic::stylesheet::{DiagnosticStylesheet, fmt_styled}; +use crate::diagnostic::stylesheet::DiagnosticStylesheet; use crate::{ Db, files::File, @@ -18,14 +18,16 @@ use crate::{ }; use super::{ - Annotation, Diagnostic, DiagnosticFormat, DiagnosticSource, DisplayDiagnosticConfig, Severity, + Annotation, Diagnostic, DiagnosticFormat, DiagnosticSource, DisplayDiagnosticConfig, SubDiagnostic, UnifiedFile, }; use azure::AzureRenderer; +use concise::ConciseRenderer; use pylint::PylintRenderer; mod azure; +mod concise; mod full; #[cfg(feature = "serde")] mod json; @@ -105,48 +107,7 @@ 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, - "{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())?; - } + ConciseRenderer::new(self.resolver, self.config).render(f, self.diagnostics)?; } DiagnosticFormat::Full => { let stylesheet = if self.config.color { @@ -862,7 +823,7 @@ fn relativize_path<'p>(cwd: &SystemPath, path: &'p str) -> &'p str { #[cfg(test)] mod tests { - use ruff_diagnostics::{Edit, Fix}; + use ruff_diagnostics::{Applicability, Edit, Fix}; use crate::diagnostic::{ Annotation, DiagnosticId, IntoDiagnosticMessage, SecondaryCode, Severity, Span, @@ -2310,6 +2271,27 @@ watermelon self.config = config; } + /// Hide diagnostic severity when rendering. + pub(super) fn hide_severity(&mut self, yes: bool) { + let mut config = std::mem::take(&mut self.config); + config = config.hide_severity(yes); + self.config = config; + } + + /// Show fix availability when rendering. + pub(super) fn show_fix_status(&mut self, yes: bool) { + let mut config = std::mem::take(&mut self.config); + config = config.show_fix_status(yes); + self.config = config; + } + + /// The lowest fix applicability to show when rendering. + pub(super) fn fix_applicability(&mut self, applicability: Applicability) { + let mut config = std::mem::take(&mut self.config); + config = config.fix_applicability(applicability); + self.config = config; + } + /// Add a file with the given path and contents to this environment. pub(super) fn add(&mut self, path: &str, contents: &str) { let path = SystemPath::new(path); @@ -2675,6 +2657,25 @@ if call(foo } /// Create Ruff-style diagnostics for testing the various output formats for a notebook. + /// + /// The concatenated cells look like this: + /// + /// ```python + /// # cell 1 + /// import os + /// # cell 2 + /// import math + /// + /// print('hello world') + /// # cell 3 + /// def foo(): + /// print() + /// x = 1 + /// ``` + /// + /// The first diagnostic is on the unused `os` import with location cell 1, row 2, column 8 + /// (`cell 1:2:8`). The second diagnostic is the unused `math` import at `cell 2:2:8`, and the + /// third diagnostic is an unfixable unused variable at `cell 3:4:5`. #[allow( dead_code, reason = "This is currently only used for JSON but will be needed soon for other formats" diff --git a/crates/ruff_db/src/diagnostic/render/concise.rs b/crates/ruff_db/src/diagnostic/render/concise.rs new file mode 100644 index 0000000000..0973b10ddb --- /dev/null +++ b/crates/ruff_db/src/diagnostic/render/concise.rs @@ -0,0 +1,195 @@ +use crate::diagnostic::{ + Diagnostic, DisplayDiagnosticConfig, Severity, + stylesheet::{DiagnosticStylesheet, fmt_styled}, +}; + +use super::FileResolver; + +pub(super) struct ConciseRenderer<'a> { + resolver: &'a dyn FileResolver, + config: &'a DisplayDiagnosticConfig, +} + +impl<'a> ConciseRenderer<'a> { + pub(super) fn new(resolver: &'a dyn FileResolver, config: &'a DisplayDiagnosticConfig) -> Self { + Self { resolver, config } + } + + pub(super) fn render( + &self, + f: &mut std::fmt::Formatter, + diagnostics: &[Diagnostic], + ) -> std::fmt::Result { + let stylesheet = if self.config.color { + DiagnosticStylesheet::styled() + } else { + DiagnosticStylesheet::plain() + }; + + let sep = fmt_styled(":", stylesheet.separator); + for diag in diagnostics { + if let Some(span) = diag.primary_span() { + write!( + f, + "{path}", + path = fmt_styled( + span.file().relative_path(self.resolver).to_string_lossy(), + 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()); + + if let Some(notebook_index) = self.resolver.notebook_index(span.file()) { + write!( + f, + "{sep}cell {cell}{sep}{line}{sep}{col}", + cell = notebook_index.cell(start.line).unwrap_or_default(), + line = notebook_index.cell_row(start.line).unwrap_or_default(), + col = start.column, + )?; + } else { + write!( + f, + "{sep}{line}{sep}{col}", + line = start.line, + col = start.column, + )?; + } + } + write!(f, "{sep} ")?; + } + if self.config.hide_severity { + if let Some(code) = diag.secondary_code() { + write!( + f, + "{code} ", + code = fmt_styled(code, stylesheet.secondary_code) + )?; + } + if self.config.show_fix_status { + if let Some(fix) = diag.fix() { + // Do not display an indicator for inapplicable fixes + if fix.applies(self.config.fix_applicability) { + write!(f, "[{fix}] ", fix = fmt_styled("*", stylesheet.separator))?; + } + } + } + } else { + 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, + "{severity}[{id}] ", + severity = fmt_styled(severity, severity_style), + id = fmt_styled(diag.id(), stylesheet.emphasis) + )?; + } + + writeln!(f, "{message}", message = diag.concise_message())?; + } + + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use ruff_diagnostics::Applicability; + + 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::Concise); + insta::assert_snapshot!(env.render_diagnostics(&diagnostics), @r" + fib.py:1:8: error[unused-import] `os` imported but unused + fib.py:6:5: error[unused-variable] Local variable `x` is assigned to but never used + undef.py:1:4: error[undefined-name] Undefined name `a` + "); + } + + #[test] + fn show_fixes() { + let (mut env, diagnostics) = create_diagnostics(DiagnosticFormat::Concise); + env.hide_severity(true); + env.show_fix_status(true); + env.fix_applicability(Applicability::DisplayOnly); + insta::assert_snapshot!(env.render_diagnostics(&diagnostics), @r" + fib.py:1:8: F401 [*] `os` imported but unused + fib.py:6:5: F841 [*] Local variable `x` is assigned to but never used + undef.py:1:4: F821 Undefined name `a` + "); + } + + #[test] + fn show_fixes_preview() { + let (mut env, diagnostics) = create_diagnostics(DiagnosticFormat::Concise); + env.hide_severity(true); + env.show_fix_status(true); + env.fix_applicability(Applicability::DisplayOnly); + env.preview(true); + insta::assert_snapshot!(env.render_diagnostics(&diagnostics), @r" + fib.py:1:8: F401 [*] `os` imported but unused + fib.py:6:5: F841 [*] Local variable `x` is assigned to but never used + undef.py:1:4: F821 Undefined name `a` + "); + } + + #[test] + fn show_fixes_syntax_errors() { + let (mut env, diagnostics) = create_syntax_error_diagnostics(DiagnosticFormat::Concise); + env.hide_severity(true); + env.show_fix_status(true); + env.fix_applicability(Applicability::DisplayOnly); + insta::assert_snapshot!(env.render_diagnostics(&diagnostics), @r" + syntax_errors.py:1:15: SyntaxError: Expected one or more symbol names after import + syntax_errors.py:3:12: SyntaxError: Expected ')', found newline + "); + } + + #[test] + fn syntax_errors() { + let (env, diagnostics) = create_syntax_error_diagnostics(DiagnosticFormat::Concise); + insta::assert_snapshot!(env.render_diagnostics(&diagnostics), @r" + syntax_errors.py:1:15: error[invalid-syntax] SyntaxError: Expected one or more symbol names after import + syntax_errors.py:3:12: error[invalid-syntax] SyntaxError: Expected ')', found newline + "); + } + + #[test] + fn notebook_output() { + let (env, diagnostics) = create_notebook_diagnostics(DiagnosticFormat::Concise); + insta::assert_snapshot!(env.render_diagnostics(&diagnostics), @r" + notebook.ipynb:cell 1:2:8: error[unused-import] `os` imported but unused + notebook.ipynb:cell 2:2:8: error[unused-import] `math` imported but unused + notebook.ipynb:cell 3:4:5: error[unused-variable] Local variable `x` is assigned to but never used + "); + } + + #[test] + fn missing_file() { + let mut env = TestEnvironment::new(); + env.format(DiagnosticFormat::Concise); + + let diag = env.err().build(); + + insta::assert_snapshot!( + env.render(&diag), + @"error[test-diagnostic] main diagnostic message", + ); + } +} diff --git a/crates/ruff_db/src/diagnostic/stylesheet.rs b/crates/ruff_db/src/diagnostic/stylesheet.rs index f3d451030b..50be7ee41c 100644 --- a/crates/ruff_db/src/diagnostic/stylesheet.rs +++ b/crates/ruff_db/src/diagnostic/stylesheet.rs @@ -41,6 +41,8 @@ pub struct DiagnosticStylesheet { pub(crate) line_no: Style, pub(crate) emphasis: Style, pub(crate) none: Style, + pub(crate) separator: Style, + pub(crate) secondary_code: Style, } impl Default for DiagnosticStylesheet { @@ -62,6 +64,8 @@ impl DiagnosticStylesheet { line_no: bright_blue.effects(Effects::BOLD), emphasis: Style::new().effects(Effects::BOLD), none: Style::new(), + separator: AnsiColor::Cyan.on_default(), + secondary_code: AnsiColor::Red.on_default().effects(Effects::BOLD), } } @@ -75,6 +79,8 @@ impl DiagnosticStylesheet { line_no: Style::new(), emphasis: Style::new(), none: Style::new(), + separator: Style::new(), + secondary_code: Style::new(), } } } diff --git a/crates/ruff_linter/src/message/text.rs b/crates/ruff_linter/src/message/text.rs index 1cd6dd370d..5659306d81 100644 --- a/crates/ruff_linter/src/message/text.rs +++ b/crates/ruff_linter/src/message/text.rs @@ -6,13 +6,12 @@ use bitflags::bitflags; use colored::Colorize; use ruff_annotate_snippets::{Level, Renderer, Snippet}; -use ruff_db::diagnostic::{Diagnostic, SecondaryCode}; +use ruff_db::diagnostic::{Diagnostic, DiagnosticFormat, DisplayDiagnosticConfig, SecondaryCode}; use ruff_notebook::NotebookIndex; -use ruff_source_file::{LineColumn, OneIndexed}; +use ruff_source_file::OneIndexed; use ruff_text_size::{TextLen, TextRange, TextSize}; use crate::Locator; -use crate::fs::relativize_path; use crate::line_width::{IndentWidth, LineWidthBuilder}; use crate::message::diff::Diff; use crate::message::{Emitter, EmitterContext}; @@ -21,8 +20,6 @@ use crate::settings::types::UnsafeFixes; bitflags! { #[derive(Default)] struct EmitterFlags: u8 { - /// Whether to show the fix status of a diagnostic. - const SHOW_FIX_STATUS = 1 << 0; /// Whether to show the diff of a fix, for diagnostics that have a fix. const SHOW_FIX_DIFF = 1 << 1; /// Whether to show the source code of a diagnostic. @@ -30,17 +27,27 @@ bitflags! { } } -#[derive(Default)] pub struct TextEmitter { flags: EmitterFlags, - unsafe_fixes: UnsafeFixes, + config: DisplayDiagnosticConfig, +} + +impl Default for TextEmitter { + fn default() -> Self { + Self { + flags: EmitterFlags::default(), + config: DisplayDiagnosticConfig::default() + .format(DiagnosticFormat::Concise) + .hide_severity(true) + .color(!cfg!(test) && colored::control::SHOULD_COLORIZE.should_colorize()), + } + } } impl TextEmitter { #[must_use] pub fn with_show_fix_status(mut self, show_fix_status: bool) -> Self { - self.flags - .set(EmitterFlags::SHOW_FIX_STATUS, show_fix_status); + self.config = self.config.show_fix_status(show_fix_status); self } @@ -58,7 +65,15 @@ impl TextEmitter { #[must_use] pub fn with_unsafe_fixes(mut self, unsafe_fixes: UnsafeFixes) -> Self { - self.unsafe_fixes = unsafe_fixes; + self.config = self + .config + .fix_applicability(unsafe_fixes.required_applicability()); + self + } + + #[must_use] + pub fn with_preview(mut self, preview: bool) -> Self { + self.config = self.config.preview(preview); self } } @@ -71,51 +86,10 @@ impl Emitter for TextEmitter { context: &EmitterContext, ) -> anyhow::Result<()> { for message in diagnostics { + write!(writer, "{}", message.display(context, &self.config))?; + let filename = message.expect_ruff_filename(); - write!( - writer, - "{path}{sep}", - path = relativize_path(&filename).bold(), - sep = ":".cyan(), - )?; - - let start_location = message.expect_ruff_start_location(); let notebook_index = context.notebook_index(&filename); - - // Check if we're working on a jupyter notebook and translate positions with cell accordingly - let diagnostic_location = if let Some(notebook_index) = notebook_index { - write!( - writer, - "cell {cell}{sep}", - cell = notebook_index - .cell(start_location.line) - .unwrap_or(OneIndexed::MIN), - sep = ":".cyan(), - )?; - - LineColumn { - line: notebook_index - .cell_row(start_location.line) - .unwrap_or(OneIndexed::MIN), - column: start_location.column, - } - } else { - start_location - }; - - writeln!( - writer, - "{row}{sep}{col}{sep} {code_and_body}", - row = diagnostic_location.line, - col = diagnostic_location.column, - sep = ":".cyan(), - code_and_body = RuleCodeAndBody { - message, - show_fix_status: self.flags.intersects(EmitterFlags::SHOW_FIX_STATUS), - unsafe_fixes: self.unsafe_fixes, - } - )?; - if self.flags.intersects(EmitterFlags::SHOW_SOURCE) { // The `0..0` range is used to highlight file-level diagnostics. if message.expect_range() != TextRange::default() { diff --git a/crates/ty/tests/cli/file_selection.rs b/crates/ty/tests/cli/file_selection.rs index 4f84e2e7d5..96f1cf9caf 100644 --- a/crates/ty/tests/cli/file_selection.rs +++ b/crates/ty/tests/cli/file_selection.rs @@ -676,7 +676,7 @@ fn invalid_include_pattern_concise_output() -> anyhow::Result<()> { ----- stderr ----- WARN ty is pre-release software and not ready for production use. Expect to encounter bugs, missing features, and fatal errors. ty failed - Cause: error[invalid-glob] ty.toml:4:5: Invalid include pattern: Too many stars at position 5 + Cause: ty.toml:4:5: error[invalid-glob] Invalid include pattern: Too many stars at position 5 "); Ok(()) diff --git a/crates/ty/tests/cli/main.rs b/crates/ty/tests/cli/main.rs index 5d4152d0e1..82edcda8c7 100644 --- a/crates/ty/tests/cli/main.rs +++ b/crates/ty/tests/cli/main.rs @@ -592,8 +592,8 @@ fn concise_diagnostics() -> anyhow::Result<()> { success: false exit_code: 1 ----- stdout ----- - warning[unresolved-reference] test.py:2:7: Name `x` used when not defined - error[non-subscriptable] test.py:3:7: Cannot subscript object of type `Literal[4]` with no `__getitem__` method + test.py:2:7: warning[unresolved-reference] Name `x` used when not defined + test.py:3:7: error[non-subscriptable] Cannot subscript object of type `Literal[4]` with no `__getitem__` method Found 2 diagnostics ----- stderr ----- @@ -627,7 +627,7 @@ fn concise_revealed_type() -> anyhow::Result<()> { success: true exit_code: 0 ----- stdout ----- - info[revealed-type] test.py:5:13: Revealed type: `Literal["hello"]` + test.py:5:13: info[revealed-type] Revealed type: `Literal["hello"]` Found 1 diagnostic ----- stderr -----