diff --git a/crates/ruff/src/args.rs b/crates/ruff/src/args.rs index 180b251816..eb4bcd0a92 100644 --- a/crates/ruff/src/args.rs +++ b/crates/ruff/src/args.rs @@ -538,6 +538,14 @@ pub struct FormatCommand { /// Exit with a non-zero status code if any files were modified via format, even if all files were formatted successfully. #[arg(long, help_heading = "Miscellaneous", alias = "exit-non-zero-on-fix")] pub exit_non_zero_on_format: bool, + + /// Output serialization format for violations, when used with `--check`. + /// The default serialization format is "full". + /// + /// Note that this option is currently only respected in preview mode. A warning will be emitted + /// if this flag is used on stable. + #[arg(long, value_enum, env = "RUFF_OUTPUT_FORMAT")] + pub output_format: Option, } #[derive(Copy, Clone, Debug, clap::Parser)] @@ -785,6 +793,7 @@ impl FormatCommand { target_version: self.target_version.map(ast::PythonVersion::from), cache_dir: self.cache_dir, extension: self.extension, + output_format: self.output_format, ..ExplicitConfigOverrides::default() }; diff --git a/crates/ruff/src/commands/check.rs b/crates/ruff/src/commands/check.rs index e8b5817db4..dcdd0f9b18 100644 --- a/crates/ruff/src/commands/check.rs +++ b/crates/ruff/src/commands/check.rs @@ -9,11 +9,10 @@ use ignore::Error; use log::{debug, warn}; #[cfg(not(target_family = "wasm"))] use rayon::prelude::*; +use ruff_linter::message::create_panic_diagnostic; use rustc_hash::FxHashMap; -use ruff_db::diagnostic::{ - Annotation, Diagnostic, DiagnosticId, Span, SubDiagnostic, SubDiagnosticSeverity, -}; +use ruff_db::diagnostic::Diagnostic; use ruff_db::panic::catch_unwind; use ruff_linter::package::PackageRoot; use ruff_linter::registry::Rule; @@ -195,23 +194,7 @@ fn lint_path( match result { Ok(inner) => inner, Err(error) => { - let message = match error.payload.as_str() { - Some(summary) => format!("Fatal error while linting: {summary}"), - _ => "Fatal error while linting".to_owned(), - }; - let mut diagnostic = Diagnostic::new( - DiagnosticId::Panic, - ruff_db::diagnostic::Severity::Fatal, - message, - ); - let span = Span::from(SourceFileBuilder::new(path.to_string_lossy(), "").finish()); - let mut annotation = Annotation::primary(span); - annotation.set_file_level(true); - diagnostic.annotate(annotation); - diagnostic.sub(SubDiagnostic::new( - SubDiagnosticSeverity::Info, - format!("{error}"), - )); + let diagnostic = create_panic_diagnostic(&error, Some(path)); Ok(Diagnostics::new(vec![diagnostic], FxHashMap::default())) } } diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index 1006346008..20c000a89d 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -11,13 +11,19 @@ use itertools::Itertools; use log::{error, warn}; use rayon::iter::Either::{Left, Right}; use rayon::iter::{IntoParallelRefIterator, ParallelIterator}; +use ruff_db::diagnostic::{ + Annotation, Diagnostic, DiagnosticId, DisplayDiagnosticConfig, Severity, Span, +}; +use ruff_linter::message::{EmitterContext, create_panic_diagnostic, render_diagnostics}; +use ruff_linter::settings::types::OutputFormat; +use ruff_notebook::NotebookIndex; use ruff_python_parser::ParseError; -use rustc_hash::FxHashSet; +use rustc_hash::{FxHashMap, FxHashSet}; use thiserror::Error; use tracing::debug; use ruff_db::panic::{PanicError, catch_unwind}; -use ruff_diagnostics::SourceMap; +use ruff_diagnostics::{Edit, Fix, SourceMap}; use ruff_linter::fs; use ruff_linter::logging::{DisplayParseError, LogLevel}; use ruff_linter::package::PackageRoot; @@ -27,14 +33,15 @@ use ruff_linter::source_kind::{SourceError, SourceKind}; use ruff_linter::warn_user_once; use ruff_python_ast::{PySourceType, SourceType}; use ruff_python_formatter::{FormatModuleError, QuoteStyle, format_module_source, format_range}; -use ruff_source_file::LineIndex; +use ruff_source_file::{LineIndex, LineRanges, OneIndexed, SourceFileBuilder}; use ruff_text_size::{TextLen, TextRange, TextSize}; use ruff_workspace::FormatterSettings; -use ruff_workspace::resolver::{ResolvedFile, Resolver, match_exclusion, python_files_in_path}; +use ruff_workspace::resolver::{ + PyprojectConfig, ResolvedFile, Resolver, match_exclusion, python_files_in_path, +}; use crate::args::{ConfigArguments, FormatArguments, FormatRange}; use crate::cache::{Cache, FileCacheKey, PackageCacheMap, PackageCaches}; -use crate::resolve::resolve; use crate::{ExitStatus, resolve_default_files}; #[derive(Debug, Copy, Clone, is_macro::Is)] @@ -63,11 +70,14 @@ impl FormatMode { pub(crate) fn format( cli: FormatArguments, config_arguments: &ConfigArguments, + pyproject_config: &PyprojectConfig, ) -> Result { - let pyproject_config = resolve(config_arguments, cli.stdin_filename.as_deref())?; let mode = FormatMode::from_cli(&cli); let files = resolve_default_files(cli.files, false); - let (paths, resolver) = python_files_in_path(&files, &pyproject_config, config_arguments)?; + let (paths, resolver) = python_files_in_path(&files, pyproject_config, config_arguments)?; + + let output_format = pyproject_config.settings.output_format; + let preview = pyproject_config.settings.formatter.preview; if paths.is_empty() { warn_user_once!("No Python files found under the given path(s)"); @@ -184,17 +194,26 @@ pub(crate) fn format( caches.persist()?; // Report on any errors. - errors.sort_unstable_by(|a, b| a.path().cmp(&b.path())); + // + // We only convert errors to `Diagnostic`s in `Check` mode with preview enabled, otherwise we + // fall back on printing simple messages. + if !(preview.is_enabled() && mode.is_check()) { + errors.sort_unstable_by(|a, b| a.path().cmp(&b.path())); - for error in &errors { - error!("{error}"); + for error in &errors { + error!("{error}"); + } } let results = FormatResults::new(results.as_slice(), mode); match mode { FormatMode::Write => {} FormatMode::Check => { - results.write_changed(&mut stdout().lock())?; + if preview.is_enabled() { + results.write_changed_preview(&mut stdout().lock(), output_format, &errors)?; + } else { + results.write_changed(&mut stdout().lock())?; + } } FormatMode::Diff => { results.write_diff(&mut stdout().lock())?; @@ -206,7 +225,7 @@ pub(crate) fn format( if mode.is_diff() { // Allow piping the diff to e.g. a file by writing the summary to stderr results.write_summary(&mut stderr().lock())?; - } else { + } else if !preview.is_enabled() || output_format.is_human_readable() { results.write_summary(&mut stdout().lock())?; } } @@ -295,8 +314,7 @@ pub(crate) fn format_path( FormatResult::Formatted } - FormatMode::Check => FormatResult::Formatted, - FormatMode::Diff => FormatResult::Diff { + FormatMode::Check | FormatMode::Diff => FormatResult::Diff { unformatted, formatted, }, @@ -329,7 +347,7 @@ pub(crate) enum FormattedSource { impl From for FormatResult { fn from(value: FormattedSource) -> Self { match value { - FormattedSource::Formatted(_) => FormatResult::Formatted, + FormattedSource::Formatted { .. } => FormatResult::Formatted, FormattedSource::Unchanged => FormatResult::Unchanged, } } @@ -477,10 +495,10 @@ pub(crate) fn format_source( /// The result of an individual formatting operation. #[derive(Debug, Clone, is_macro::Is)] pub(crate) enum FormatResult { - /// The file was formatted. + /// The file was formatted and written back to disk. Formatted, - /// The file was formatted, [`SourceKind`] contains the formatted code + /// The file needs to be formatted, as the `formatted` and `unformatted` contents differ. Diff { unformatted: SourceKind, formatted: SourceKind, @@ -552,7 +570,7 @@ impl<'a> FormatResults<'a> { .results .iter() .filter_map(|result| { - if result.result.is_formatted() { + if result.result.is_diff() { Some(result.path.as_path()) } else { None @@ -566,6 +584,30 @@ impl<'a> FormatResults<'a> { Ok(()) } + /// Write a list of the files that would be changed and any errors to the given writer. + fn write_changed_preview( + &self, + f: &mut impl Write, + output_format: OutputFormat, + errors: &[FormatCommandError], + ) -> io::Result<()> { + let mut notebook_index = FxHashMap::default(); + let diagnostics: Vec<_> = errors + .iter() + .map(Diagnostic::from) + .chain(self.to_diagnostics(&mut notebook_index)) + .sorted_unstable_by(Diagnostic::ruff_start_ordering) + .collect(); + + let context = EmitterContext::new(¬ebook_index); + let config = DisplayDiagnosticConfig::default() + .hide_severity(true) + .show_fix_diff(true) + .color(!cfg!(test) && colored::control::SHOULD_COLORIZE.should_colorize()); + + render_diagnostics(f, output_format, config, &context, &diagnostics) + } + /// Write a summary of the formatting results to the given writer. fn write_summary(&self, f: &mut impl Write) -> io::Result<()> { // Compute the number of changed and unchanged files. @@ -628,6 +670,155 @@ impl<'a> FormatResults<'a> { Ok(()) } } + + /// Convert formatted files into [`Diagnostic`]s. + fn to_diagnostics( + &self, + notebook_index: &mut FxHashMap, + ) -> impl Iterator { + /// The number of unmodified context lines rendered in diffs. + /// + /// Note that this should be kept in sync with the argument to `TextDiff::grouped_ops` in + /// the diff rendering in `ruff_db` (currently 3). The `similar` crate uses two times that + /// argument as a cutoff for rendering unmodified lines. + const CONTEXT_LINES: u32 = 6; + + self.results.iter().filter_map(|result| { + let (unformatted, formatted) = match &result.result { + FormatResult::Skipped | FormatResult::Unchanged => return None, + FormatResult::Diff { + unformatted, + formatted, + } => (unformatted, formatted), + FormatResult::Formatted => { + debug_assert!( + false, + "Expected `FormatResult::Diff` for changed files in check mode" + ); + return None; + } + }; + + let mut diagnostic = Diagnostic::new( + DiagnosticId::Unformatted, + Severity::Error, + "File would be reformatted", + ); + + // Locate the first and last characters that differ to use as the diagnostic + // range and to narrow the `Edit` range. + let modified_range = ModifiedRange::new(unformatted, formatted); + + let path = result.path.to_string_lossy(); + // For scripts, this is a single `Edit` using the `ModifiedRange` above, but notebook + // edits must be split by cell in order to render them as diffs. + // + // We also attempt to estimate the line number width for aligning the + // annotate-snippets header. This is only an estimate because we don't actually know + // if the maximum line number present in the document will be rendered as part of + // the diff, either as a changed line or as an unchanged context line. For + // notebooks, we refine our estimate by checking the number of lines in each cell + // individually, otherwise we could use `formatted.source_code().count_lines(...)` + // in both cases. + let (fix, line_count) = if let SourceKind::IpyNotebook(formatted) = formatted + && let SourceKind::IpyNotebook(unformatted) = unformatted + { + notebook_index.insert(path.to_string(), unformatted.index().clone()); + + let mut edits = formatted + .cell_offsets() + .ranges() + .zip(unformatted.cell_offsets().ranges()) + .filter_map(|(formatted_range, unformatted_range)| { + // Filter out cells that weren't modified. We use `intersect` instead of + // `contains_range` because the full modified range might start or end in + // the middle of a cell: + // + // ``` + // | cell 1 | cell 2 | cell 3 | + // |----------------| modified range + // ``` + // + // The intersection will be `Some` for all three cells in this case. + if modified_range + .unformatted + .intersect(unformatted_range) + .is_some() + { + let formatted = &formatted.source_code()[formatted_range]; + let edit = if formatted.is_empty() { + Edit::range_deletion(unformatted_range) + } else { + Edit::range_replacement(formatted.to_string(), unformatted_range) + }; + Some(edit) + } else { + None + } + }); + + let fix = Fix::safe_edits( + edits + .next() + .expect("Formatted files must have at least one edit"), + edits, + ); + let source = formatted.source_code(); + let line_count = formatted + .cell_offsets() + .ranges() + .filter_map(|range| { + if modified_range.formatted.contains_range(range) { + Some(source.count_lines(range)) + } else { + None + } + }) + .max() + .unwrap_or_default(); + (fix, line_count) + } else { + let formatted_code = &formatted.source_code()[modified_range.formatted]; + let edit = if formatted_code.is_empty() { + Edit::range_deletion(modified_range.unformatted) + } else { + Edit::range_replacement(formatted_code.to_string(), modified_range.unformatted) + }; + let fix = Fix::safe_edit(edit); + let line_count = formatted + .source_code() + .count_lines(TextRange::up_to(modified_range.formatted.end())); + (fix, line_count) + }; + + let source_file = SourceFileBuilder::new(path, unformatted.source_code()).finish(); + let span = Span::from(source_file).with_range(modified_range.unformatted); + let mut annotation = Annotation::primary(span); + annotation.hide_snippet(true); + diagnostic.annotate(annotation); + diagnostic.set_fix(fix); + + // TODO(brent) this offset is a hack to get the header of the diagnostic message, which + // is rendered by our fork of `annotate-snippets`, to align with our manually-rendered + // diff. `annotate-snippets` computes the alignment of the arrow in the header based on + // the maximum line number width in its rendered snippet. However, we don't have a + // reasonable range to underline in an annotation, so we don't send `annotate-snippets` + // a snippet to measure. If we commit to staying on our fork, a more robust way of + // handling this would be to move the diff rendering in + // `ruff_db::diagnostic::render::full` into `annotate-snippets`, likely as another + // `DisplayLine` variant and update the `lineno_width` calculation in + // `DisplayList::fmt`. That would handle this offset "automatically." + let line_count = (line_count + CONTEXT_LINES).min( + formatted + .source_code() + .count_lines(TextRange::up_to(formatted.source_code().text_len())), + ); + let lines = OneIndexed::new(line_count as usize).unwrap_or_default(); + diagnostic.set_header_offset(lines.digits().get()); + + Some(diagnostic) + }) + } } /// An error that can occur while formatting a set of files. @@ -639,7 +830,6 @@ pub(crate) enum FormatCommandError { Read(Option, SourceError), Format(Option, FormatModuleError), Write(Option, SourceError), - Diff(Option, io::Error), RangeFormatNotebook(Option), } @@ -658,12 +848,65 @@ impl FormatCommandError { | Self::Read(path, _) | Self::Format(path, _) | Self::Write(path, _) - | Self::Diff(path, _) | Self::RangeFormatNotebook(path) => path.as_deref(), } } } +impl From<&FormatCommandError> for Diagnostic { + fn from(error: &FormatCommandError) -> Self { + let annotation = error.path().map(|path| { + let file = SourceFileBuilder::new(path.to_string_lossy(), "").finish(); + let span = Span::from(file); + let mut annotation = Annotation::primary(span); + annotation.hide_snippet(true); + annotation + }); + + let mut diagnostic = match error { + FormatCommandError::Ignore(error) => { + Diagnostic::new(DiagnosticId::Io, Severity::Error, error) + } + FormatCommandError::Parse(display_parse_error) => Diagnostic::new( + DiagnosticId::InvalidSyntax, + Severity::Error, + &display_parse_error.error().error, + ), + FormatCommandError::Panic(path, panic_error) => { + return create_panic_diagnostic(panic_error, path.as_deref()); + } + FormatCommandError::Read(_, source_error) + | FormatCommandError::Write(_, source_error) => { + Diagnostic::new(DiagnosticId::Io, Severity::Error, source_error) + } + FormatCommandError::Format(_, format_module_error) => match format_module_error { + FormatModuleError::ParseError(parse_error) => Diagnostic::new( + DiagnosticId::InternalError, + Severity::Error, + &parse_error.error, + ), + FormatModuleError::FormatError(format_error) => { + Diagnostic::new(DiagnosticId::InternalError, Severity::Error, format_error) + } + FormatModuleError::PrintError(print_error) => { + Diagnostic::new(DiagnosticId::InternalError, Severity::Error, print_error) + } + }, + FormatCommandError::RangeFormatNotebook(_) => Diagnostic::new( + DiagnosticId::InvalidCliOption, + Severity::Error, + "Range formatting isn't supported for notebooks.", + ), + }; + + if let Some(annotation) = annotation { + diagnostic.annotate(annotation); + } + + diagnostic + } +} + impl Display for FormatCommandError { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { @@ -731,23 +974,6 @@ impl Display for FormatCommandError { write!(f, "{header} {err}", header = "Failed to format:".bold()) } } - Self::Diff(path, err) => { - if let Some(path) = path { - write!( - f, - "{}{}{} {err}", - "Failed to generate diff for ".bold(), - fs::relativize_path(path).bold(), - ":".bold() - ) - } else { - write!( - f, - "{header} {err}", - header = "Failed to generate diff:".bold(), - ) - } - } Self::RangeFormatNotebook(path) => { if let Some(path) = path { write!( @@ -792,6 +1018,54 @@ impl Display for FormatCommandError { } } +#[derive(Debug)] +struct ModifiedRange { + unformatted: TextRange, + formatted: TextRange, +} + +impl ModifiedRange { + /// Determine the range that differs between `unformatted` and `formatted`. + /// + /// If the two inputs are equal, the returned ranges will be empty. + fn new(unformatted: &SourceKind, formatted: &SourceKind) -> Self { + let unformatted = unformatted.source_code(); + let formatted = formatted.source_code(); + + let mut prefix_length = TextSize::ZERO; + for (unformatted, formatted) in unformatted.chars().zip(formatted.chars()) { + if unformatted != formatted { + break; + } + prefix_length += unformatted.text_len(); + } + + // For the ends of the ranges, track the length of the common suffix and then subtract that + // from each total text length. Unlike for `start`, the character offsets are very unlikely + // to be equal, so they need to be treated separately. + let mut suffix_length = TextSize::ZERO; + for (old, new) in unformatted[prefix_length.to_usize()..] + .chars() + .rev() + .zip(formatted[prefix_length.to_usize()..].chars().rev()) + { + if old != new { + break; + } + suffix_length += old.text_len(); + } + + let unformatted_range = + TextRange::new(prefix_length, unformatted.text_len() - suffix_length); + let formatted_range = TextRange::new(prefix_length, formatted.text_len() - suffix_length); + + Self { + unformatted: unformatted_range, + formatted: formatted_range, + } + } +} + pub(super) fn warn_incompatible_formatter_settings(resolver: &Resolver) { // First, collect all rules that are incompatible regardless of the linter-specific settings. let mut incompatible_rules = FxHashSet::default(); @@ -963,3 +1237,144 @@ pub(super) fn warn_incompatible_formatter_settings(resolver: &Resolver) { } } } + +#[cfg(test)] +mod tests { + use std::io; + use std::ops::Range; + use std::path::PathBuf; + + use ignore::Error; + use insta::assert_snapshot; + + use ruff_db::panic::catch_unwind; + use ruff_linter::logging::DisplayParseError; + use ruff_linter::source_kind::{SourceError, SourceKind}; + use ruff_python_formatter::FormatModuleError; + use ruff_python_parser::{ParseError, ParseErrorType}; + use ruff_text_size::{TextRange, TextSize}; + use test_case::test_case; + + use crate::commands::format::{FormatCommandError, FormatMode, FormatResults, ModifiedRange}; + + #[test] + fn error_diagnostics() -> anyhow::Result<()> { + let path = PathBuf::from("test.py"); + let source_kind = SourceKind::Python("1".to_string()); + + let panic_error = catch_unwind(|| { + panic!("Test panic for FormatCommandError"); + }) + .unwrap_err(); + + let errors = [ + FormatCommandError::Ignore(Error::WithPath { + path: path.clone(), + err: Box::new(Error::Io(io::Error::new( + io::ErrorKind::PermissionDenied, + "Permission denied", + ))), + }), + FormatCommandError::Parse(DisplayParseError::from_source_kind( + ParseError { + error: ParseErrorType::UnexpectedIndentation, + location: TextRange::default(), + }, + Some(path.clone()), + &source_kind, + )), + FormatCommandError::Panic(Some(path.clone()), Box::new(panic_error)), + FormatCommandError::Read( + Some(path.clone()), + SourceError::Io(io::Error::new(io::ErrorKind::NotFound, "File not found")), + ), + FormatCommandError::Format( + Some(path.clone()), + FormatModuleError::ParseError(ParseError { + error: ParseErrorType::EmptySlice, + location: TextRange::default(), + }), + ), + FormatCommandError::Write( + Some(path.clone()), + SourceError::Io(io::Error::new( + io::ErrorKind::PermissionDenied, + "Cannot write to file", + )), + ), + FormatCommandError::RangeFormatNotebook(Some(path)), + ]; + + let results = FormatResults::new(&[], FormatMode::Check); + let mut buf = Vec::new(); + results.write_changed_preview( + &mut buf, + ruff_linter::settings::types::OutputFormat::Full, + &errors, + )?; + + let mut settings = insta::Settings::clone_current(); + settings.add_filter(r"(Panicked at) [^:]+:\d+:\d+", "$1 "); + let _s = settings.bind_to_scope(); + + assert_snapshot!(str::from_utf8(&buf)?, @r" + io: test.py: Permission denied + --> test.py:1:1 + + invalid-syntax: Unexpected indentation + --> test.py:1:1 + + io: File not found + --> test.py:1:1 + + internal-error: Expected index or slice expression + --> test.py:1:1 + + io: Cannot write to file + --> test.py:1:1 + + invalid-cli-option: Range formatting isn't supported for notebooks. + --> test.py:1:1 + + panic: Panicked at when checking `test.py`: `Test panic for FormatCommandError` + --> test.py:1:1 + info: This indicates a bug in Ruff. + info: If you could open an issue at https://github.com/astral-sh/ruff/issues/new?title=%5Bpanic%5D, we'd be very appreciative! + info: run with `RUST_BACKTRACE=1` environment variable to show the full backtrace information + "); + + Ok(()) + } + + #[test_case("abcdef", "abcXYdef", 3..3, 3..5; "insertion")] + #[test_case("abcXYdef", "abcdef", 3..5, 3..3; "deletion")] + #[test_case("abcXdef", "abcYdef", 3..4, 3..4; "modification")] + #[test_case("abc", "abcX", 3..3, 3..4; "strict_prefix")] + #[test_case("", "", 0..0, 0..0; "empty")] + #[test_case("abc", "abc", 3..3, 3..3; "equal")] + fn modified_range( + unformatted: &str, + formatted: &str, + expect_unformatted: Range, + expect_formatted: Range, + ) { + let mr = ModifiedRange::new( + &SourceKind::Python(unformatted.to_string()), + &SourceKind::Python(formatted.to_string()), + ); + assert_eq!( + mr.unformatted, + TextRange::new( + TextSize::new(expect_unformatted.start), + TextSize::new(expect_unformatted.end) + ) + ); + assert_eq!( + mr.formatted, + TextRange::new( + TextSize::new(expect_formatted.start), + TextSize::new(expect_formatted.end) + ) + ); + } +} diff --git a/crates/ruff/src/commands/format_stdin.rs b/crates/ruff/src/commands/format_stdin.rs index f5b8ea6c60..015f32fba9 100644 --- a/crates/ruff/src/commands/format_stdin.rs +++ b/crates/ruff/src/commands/format_stdin.rs @@ -4,10 +4,10 @@ use std::path::Path; use anyhow::Result; use log::error; -use ruff_linter::source_kind::SourceKind; +use ruff_linter::source_kind::{SourceError, SourceKind}; use ruff_python_ast::{PySourceType, SourceType}; use ruff_workspace::FormatterSettings; -use ruff_workspace::resolver::{Resolver, match_exclusion, python_file_at_path}; +use ruff_workspace::resolver::{PyprojectConfig, Resolver, match_exclusion, python_file_at_path}; use crate::ExitStatus; use crate::args::{ConfigArguments, FormatArguments, FormatRange}; @@ -15,17 +15,15 @@ use crate::commands::format::{ FormatCommandError, FormatMode, FormatResult, FormattedSource, format_source, warn_incompatible_formatter_settings, }; -use crate::resolve::resolve; use crate::stdin::{parrot_stdin, read_from_stdin}; /// Run the formatter over a single file, read from `stdin`. pub(crate) fn format_stdin( cli: &FormatArguments, config_arguments: &ConfigArguments, + pyproject_config: &PyprojectConfig, ) -> Result { - let pyproject_config = resolve(config_arguments, cli.stdin_filename.as_deref())?; - - let mut resolver = Resolver::new(&pyproject_config); + let mut resolver = Resolver::new(pyproject_config); warn_incompatible_formatter_settings(&resolver); let mode = FormatMode::from_cli(cli); @@ -124,7 +122,9 @@ fn format_source_code( "{}", source_kind.diff(formatted, path).unwrap() ) - .map_err(|err| FormatCommandError::Diff(path.map(Path::to_path_buf), err))?; + .map_err(|err| { + FormatCommandError::Write(path.map(Path::to_path_buf), SourceError::Io(err)) + })?; } }, FormattedSource::Unchanged => { diff --git a/crates/ruff/src/lib.rs b/crates/ruff/src/lib.rs index 07b33fb71c..3bd457de8c 100644 --- a/crates/ruff/src/lib.rs +++ b/crates/ruff/src/lib.rs @@ -205,12 +205,18 @@ pub fn run( } fn format(args: FormatCommand, global_options: GlobalConfigArgs) -> Result { + let cli_output_format_set = args.output_format.is_some(); let (cli, config_arguments) = args.partition(global_options)?; - + let pyproject_config = resolve::resolve(&config_arguments, cli.stdin_filename.as_deref())?; + if cli_output_format_set && !pyproject_config.settings.formatter.preview.is_enabled() { + warn_user_once!( + "The --output-format flag for the formatter is unstable and requires preview mode to use." + ); + } if is_stdin(&cli.files, cli.stdin_filename.as_deref()) { - commands::format_stdin::format_stdin(&cli, &config_arguments) + commands::format_stdin::format_stdin(&cli, &config_arguments, &pyproject_config) } else { - commands::format::format(cli, &config_arguments) + commands::format::format(cli, &config_arguments, &pyproject_config) } } diff --git a/crates/ruff/tests/format.rs b/crates/ruff/tests/format.rs index f1369048f7..6205fa98e8 100644 --- a/crates/ruff/tests/format.rs +++ b/crates/ruff/tests/format.rs @@ -12,8 +12,8 @@ use tempfile::TempDir; const BIN_NAME: &str = "ruff"; -fn tempdir_filter(tempdir: &TempDir) -> String { - format!(r"{}\\?/?", escape(tempdir.path().to_str().unwrap())) +fn tempdir_filter(path: impl AsRef) -> String { + format!(r"{}\\?/?", escape(path.as_ref().to_str().unwrap())) } #[test] @@ -609,6 +609,112 @@ if __name__ == "__main__": Ok(()) } +#[test_case::test_case("concise")] +#[test_case::test_case("full")] +#[test_case::test_case("json")] +#[test_case::test_case("json-lines")] +#[test_case::test_case("junit")] +#[test_case::test_case("grouped")] +#[test_case::test_case("github")] +#[test_case::test_case("gitlab")] +#[test_case::test_case("pylint")] +#[test_case::test_case("rdjson")] +#[test_case::test_case("azure")] +#[test_case::test_case("sarif")] +fn output_format(output_format: &str) -> Result<()> { + const CONTENT: &str = r#" +from test import say_hy + +if __name__ == "__main__": + say_hy("dear Ruff contributor") +"#; + + let tempdir = TempDir::new()?; + let input = tempdir.path().join("input.py"); + fs::write(&input, CONTENT)?; + + let snapshot = format!("output_format_{output_format}"); + + let project_dir = dunce::canonicalize(tempdir.path())?; + + insta::with_settings!({ + filters => vec![ + (tempdir_filter(&project_dir).as_str(), "[TMP]/"), + (tempdir_filter(&tempdir).as_str(), "[TMP]/"), + (r#""[^"]+\\?/?input.py"#, r#""[TMP]/input.py"#), + (ruff_linter::VERSION, "[VERSION]"), + ] + }, { + assert_cmd_snapshot!( + snapshot, + Command::new(get_cargo_bin(BIN_NAME)) + .args([ + "format", + "--no-cache", + "--output-format", + output_format, + "--preview", + "--check", + "input.py", + ]) + .current_dir(&tempdir), + ); + }); + + Ok(()) +} + +#[test] +fn output_format_notebook() { + let args = ["format", "--no-cache", "--isolated", "--preview", "--check"]; + let fixtures = Path::new("resources").join("test").join("fixtures"); + let path = fixtures.join("unformatted.ipynb"); + insta::with_settings!({filters => vec![ + // Replace windows paths + (r"\\", "/"), + ]}, { + assert_cmd_snapshot!( + Command::new(get_cargo_bin(BIN_NAME)).args(args).arg(path), + @r" + success: false + exit_code: 1 + ----- stdout ----- + unformatted: File would be reformatted + --> resources/test/fixtures/unformatted.ipynb:cell 1:1:1 + ::: cell 1 + 1 | import numpy + - maths = (numpy.arange(100)**2).sum() + - stats= numpy.asarray([1,2,3,4]).median() + 2 + + 3 + maths = (numpy.arange(100) ** 2).sum() + 4 + stats = numpy.asarray([1, 2, 3, 4]).median() + ::: cell 3 + 1 | # A cell with IPython escape command + 2 | def some_function(foo, bar): + 3 | pass + 4 + + 5 + + 6 | %matplotlib inline + ::: cell 4 + 1 | foo = %pwd + - def some_function(foo,bar,): + 2 + + 3 + + 4 + def some_function( + 5 + foo, + 6 + bar, + 7 + ): + 8 | # Another cell with IPython escape command + 9 | foo = %pwd + 10 | print(foo) + + 1 file would be reformatted + + ----- stderr ----- + "); + }); +} + #[test] fn exit_non_zero_on_format() -> Result<()> { let tempdir = TempDir::new()?; @@ -2355,3 +2461,21 @@ fn cookiecutter_globbing() -> Result<()> { Ok(()) } + +#[test] +fn stable_output_format_warning() { + assert_cmd_snapshot!( + Command::new(get_cargo_bin(BIN_NAME)) + .args(["format", "--output-format=full", "-"]) + .pass_stdin("1"), + @r" + success: true + exit_code: 0 + ----- stdout ----- + 1 + + ----- stderr ----- + warning: The --output-format flag for the formatter is unstable and requires preview mode to use. + ", + ); +} diff --git a/crates/ruff/tests/lint.rs b/crates/ruff/tests/lint.rs index c44f7b9ecd..1c19a40c24 100644 --- a/crates/ruff/tests/lint.rs +++ b/crates/ruff/tests/lint.rs @@ -6302,6 +6302,7 @@ fn rule_panic_mixed_results_concise() -> Result<()> { filters => vec![ (tempdir_filter(&tempdir).as_str(), "[TMP]/"), (r"\\", r"/"), + (r"(Panicked at) [^:]+:\d+:\d+", "$1 ") ] }, { assert_cmd_snapshot!( @@ -6318,7 +6319,7 @@ fn rule_panic_mixed_results_concise() -> Result<()> { [TMP]/normal.py:1:1: RUF903 Hey this is a stable test rule with a display only fix. [TMP]/normal.py:1:1: RUF911 Hey this is a preview test rule. [TMP]/normal.py:1:1: RUF950 Hey this is a test rule that was redirected from another. - [TMP]/panic.py: panic: Fatal error while linting: This is a fake panic for testing. + [TMP]/panic.py: panic: Panicked at when checking `[TMP]/panic.py`: `This is a fake panic for testing.` Found 7 errors. [*] 1 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option). @@ -6347,6 +6348,7 @@ fn rule_panic_mixed_results_full() -> Result<()> { filters => vec![ (tempdir_filter(&tempdir).as_str(), "[TMP]/"), (r"\\", r"/"), + (r"(Panicked at) [^:]+:\d+:\d+", "$1 "), ] }, { assert_cmd_snapshot!( @@ -6377,12 +6379,11 @@ fn rule_panic_mixed_results_full() -> Result<()> { RUF950 Hey this is a test rule that was redirected from another. --> [TMP]/normal.py:1:1 - panic: Fatal error while linting: This is a fake panic for testing. + panic: Panicked at when checking `[TMP]/panic.py`: `This is a fake panic for testing.` --> [TMP]/panic.py:1:1 - info: panicked at crates/ruff_linter/src/rules/ruff/rules/test_rules.rs:511:9: - This is a fake panic for testing. - run with `RUST_BACKTRACE=1` environment variable to display a backtrace - + info: This indicates a bug in Ruff. + info: If you could open an issue at https://github.com/astral-sh/ruff/issues/new?title=%5Bpanic%5D, we'd be very appreciative! + info: run with `RUST_BACKTRACE=1` environment variable to show the full backtrace information Found 7 errors. [*] 1 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option). diff --git a/crates/ruff/tests/snapshots/format__output_format_azure.snap b/crates/ruff/tests/snapshots/format__output_format_azure.snap new file mode 100644 index 0000000000..f27a327727 --- /dev/null +++ b/crates/ruff/tests/snapshots/format__output_format_azure.snap @@ -0,0 +1,19 @@ +--- +source: crates/ruff/tests/format.rs +info: + program: ruff + args: + - format + - "--no-cache" + - "--output-format" + - azure + - "--preview" + - "--check" + - input.py +--- +success: false +exit_code: 1 +----- stdout ----- +##vso[task.logissue type=error;sourcepath=[TMP]/input.py;linenumber=1;columnnumber=1;code=unformatted;]File would be reformatted + +----- stderr ----- diff --git a/crates/ruff/tests/snapshots/format__output_format_concise.snap b/crates/ruff/tests/snapshots/format__output_format_concise.snap new file mode 100644 index 0000000000..8a24633768 --- /dev/null +++ b/crates/ruff/tests/snapshots/format__output_format_concise.snap @@ -0,0 +1,20 @@ +--- +source: crates/ruff/tests/format.rs +info: + program: ruff + args: + - format + - "--no-cache" + - "--output-format" + - concise + - "--preview" + - "--check" + - input.py +--- +success: false +exit_code: 1 +----- stdout ----- +input.py:1:1: unformatted: File would be reformatted +1 file would be reformatted + +----- stderr ----- diff --git a/crates/ruff/tests/snapshots/format__output_format_full.snap b/crates/ruff/tests/snapshots/format__output_format_full.snap new file mode 100644 index 0000000000..30eb3a0a51 --- /dev/null +++ b/crates/ruff/tests/snapshots/format__output_format_full.snap @@ -0,0 +1,26 @@ +--- +source: crates/ruff/tests/format.rs +info: + program: ruff + args: + - format + - "--no-cache" + - "--output-format" + - full + - "--preview" + - "--check" + - input.py +--- +success: false +exit_code: 1 +----- stdout ----- +unformatted: File would be reformatted + --> input.py:1:1 + - +1 | from test import say_hy +2 | +3 | if __name__ == "__main__": + +1 file would be reformatted + +----- stderr ----- diff --git a/crates/ruff/tests/snapshots/format__output_format_github.snap b/crates/ruff/tests/snapshots/format__output_format_github.snap new file mode 100644 index 0000000000..e279ad2b87 --- /dev/null +++ b/crates/ruff/tests/snapshots/format__output_format_github.snap @@ -0,0 +1,19 @@ +--- +source: crates/ruff/tests/format.rs +info: + program: ruff + args: + - format + - "--no-cache" + - "--output-format" + - github + - "--preview" + - "--check" + - input.py +--- +success: false +exit_code: 1 +----- stdout ----- +::error title=Ruff (unformatted),file=[TMP]/input.py,line=1,col=1,endLine=2,endColumn=1::input.py:1:1: unformatted: File would be reformatted + +----- stderr ----- diff --git a/crates/ruff/tests/snapshots/format__output_format_gitlab.snap b/crates/ruff/tests/snapshots/format__output_format_gitlab.snap new file mode 100644 index 0000000000..fbf68871d3 --- /dev/null +++ b/crates/ruff/tests/snapshots/format__output_format_gitlab.snap @@ -0,0 +1,38 @@ +--- +source: crates/ruff/tests/format.rs +info: + program: ruff + args: + - format + - "--no-cache" + - "--output-format" + - gitlab + - "--preview" + - "--check" + - input.py +--- +success: false +exit_code: 1 +----- stdout ----- +[ + { + "check_name": "unformatted", + "description": "unformatted: File would be reformatted", + "severity": "major", + "fingerprint": "d868d7da11a65fcf", + "location": { + "path": "input.py", + "positions": { + "begin": { + "line": 1, + "column": 1 + }, + "end": { + "line": 2, + "column": 1 + } + } + } + } +] +----- stderr ----- diff --git a/crates/ruff/tests/snapshots/format__output_format_grouped.snap b/crates/ruff/tests/snapshots/format__output_format_grouped.snap new file mode 100644 index 0000000000..bd9ec0e7e4 --- /dev/null +++ b/crates/ruff/tests/snapshots/format__output_format_grouped.snap @@ -0,0 +1,21 @@ +--- +source: crates/ruff/tests/format.rs +info: + program: ruff + args: + - format + - "--no-cache" + - "--output-format" + - grouped + - "--check" + - input.py +--- +success: false +exit_code: 1 +----- stdout ----- +input.py: + 1:1 unformatted: File would be reformatted + +1 file would be reformatted + +----- stderr ----- diff --git a/crates/ruff/tests/snapshots/format__output_format_json-lines.snap b/crates/ruff/tests/snapshots/format__output_format_json-lines.snap new file mode 100644 index 0000000000..dadde72435 --- /dev/null +++ b/crates/ruff/tests/snapshots/format__output_format_json-lines.snap @@ -0,0 +1,19 @@ +--- +source: crates/ruff/tests/format.rs +info: + program: ruff + args: + - format + - "--no-cache" + - "--output-format" + - json-lines + - "--preview" + - "--check" + - input.py +--- +success: false +exit_code: 1 +----- stdout ----- +{"cell":null,"code":"unformatted","end_location":{"column":1,"row":2},"filename":"[TMP]/input.py","fix":{"applicability":"safe","edits":[{"content":"","end_location":{"column":1,"row":2},"location":{"column":1,"row":1}}],"message":null},"location":{"column":1,"row":1},"message":"File would be reformatted","noqa_row":null,"url":null} + +----- stderr ----- diff --git a/crates/ruff/tests/snapshots/format__output_format_json.snap b/crates/ruff/tests/snapshots/format__output_format_json.snap new file mode 100644 index 0000000000..1bbff05aec --- /dev/null +++ b/crates/ruff/tests/snapshots/format__output_format_json.snap @@ -0,0 +1,52 @@ +--- +source: crates/ruff/tests/format.rs +info: + program: ruff + args: + - format + - "--no-cache" + - "--output-format" + - json + - "--preview" + - "--check" + - input.py +--- +success: false +exit_code: 1 +----- stdout ----- +[ + { + "cell": null, + "code": "unformatted", + "end_location": { + "column": 1, + "row": 2 + }, + "filename": "[TMP]/input.py", + "fix": { + "applicability": "safe", + "edits": [ + { + "content": "", + "end_location": { + "column": 1, + "row": 2 + }, + "location": { + "column": 1, + "row": 1 + } + } + ], + "message": null + }, + "location": { + "column": 1, + "row": 1 + }, + "message": "File would be reformatted", + "noqa_row": null, + "url": null + } +] +----- stderr ----- diff --git a/crates/ruff/tests/snapshots/format__output_format_junit.snap b/crates/ruff/tests/snapshots/format__output_format_junit.snap new file mode 100644 index 0000000000..518c836729 --- /dev/null +++ b/crates/ruff/tests/snapshots/format__output_format_junit.snap @@ -0,0 +1,26 @@ +--- +source: crates/ruff/tests/format.rs +info: + program: ruff + args: + - format + - "--no-cache" + - "--output-format" + - junit + - "--preview" + - "--check" + - input.py +--- +success: false +exit_code: 1 +----- stdout ----- + + + + + line 1, col 1, File would be reformatted + + + + +----- stderr ----- diff --git a/crates/ruff/tests/snapshots/format__output_format_pylint.snap b/crates/ruff/tests/snapshots/format__output_format_pylint.snap new file mode 100644 index 0000000000..7d5f80fed4 --- /dev/null +++ b/crates/ruff/tests/snapshots/format__output_format_pylint.snap @@ -0,0 +1,18 @@ +--- +source: crates/ruff/tests/format.rs +info: + program: ruff + args: + - format + - "--no-cache" + - "--output-format" + - pylint + - "--check" + - input.py +--- +success: false +exit_code: 1 +----- stdout ----- +input.py:1: [unformatted] File would be reformatted + +----- stderr ----- diff --git a/crates/ruff/tests/snapshots/format__output_format_rdjson.snap b/crates/ruff/tests/snapshots/format__output_format_rdjson.snap new file mode 100644 index 0000000000..dcdb5bda8c --- /dev/null +++ b/crates/ruff/tests/snapshots/format__output_format_rdjson.snap @@ -0,0 +1,60 @@ +--- +source: crates/ruff/tests/format.rs +info: + program: ruff + args: + - format + - "--no-cache" + - "--output-format" + - rdjson + - "--preview" + - "--check" + - input.py +--- +success: false +exit_code: 1 +----- stdout ----- +{ + "diagnostics": [ + { + "code": { + "value": "unformatted" + }, + "location": { + "path": "[TMP]/input.py", + "range": { + "end": { + "column": 1, + "line": 2 + }, + "start": { + "column": 1, + "line": 1 + } + } + }, + "message": "File would be reformatted", + "suggestions": [ + { + "range": { + "end": { + "column": 1, + "line": 2 + }, + "start": { + "column": 1, + "line": 1 + } + }, + "text": "" + } + ] + } + ], + "severity": "WARNING", + "source": { + "name": "ruff", + "url": "https://docs.astral.sh/ruff" + } +} +----- stderr ----- diff --git a/crates/ruff/tests/snapshots/format__output_format_sarif.snap b/crates/ruff/tests/snapshots/format__output_format_sarif.snap new file mode 100644 index 0000000000..1f31ab0374 --- /dev/null +++ b/crates/ruff/tests/snapshots/format__output_format_sarif.snap @@ -0,0 +1,81 @@ +--- +source: crates/ruff/tests/format.rs +info: + program: ruff + args: + - format + - "--no-cache" + - "--output-format" + - sarif + - "--preview" + - "--check" + - input.py +--- +success: false +exit_code: 1 +----- stdout ----- +{ + "$schema": "https://json.schemastore.org/sarif-2.1.0.json", + "runs": [ + { + "results": [ + { + "fixes": [ + { + "artifactChanges": [ + { + "artifactLocation": { + "uri": "[TMP]/input.py" + }, + "replacements": [ + { + "deletedRegion": { + "endColumn": 1, + "endLine": 2, + "startColumn": 1, + "startLine": 1 + } + } + ] + } + ], + "description": { + "text": null + } + } + ], + "level": "error", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "[TMP]/input.py" + }, + "region": { + "endColumn": 1, + "endLine": 2, + "startColumn": 1, + "startLine": 1 + } + } + } + ], + "message": { + "text": "File would be reformatted" + }, + "ruleId": "unformatted" + } + ], + "tool": { + "driver": { + "informationUri": "https://github.com/astral-sh/ruff", + "name": "ruff", + "rules": [], + "version": "[VERSION]" + } + } + } + ], + "version": "2.1.0" +} +----- stderr ----- diff --git a/crates/ruff_annotate_snippets/src/renderer/display_list.rs b/crates/ruff_annotate_snippets/src/renderer/display_list.rs index 2728cfba7b..fb88a26223 100644 --- a/crates/ruff_annotate_snippets/src/renderer/display_list.rs +++ b/crates/ruff_annotate_snippets/src/renderer/display_list.rs @@ -56,6 +56,7 @@ pub(crate) struct DisplayList<'a> { pub(crate) stylesheet: &'a Stylesheet, pub(crate) anonymized_line_numbers: bool, pub(crate) cut_indicator: &'static str, + pub(crate) lineno_offset: usize, } impl PartialEq for DisplayList<'_> { @@ -81,13 +82,14 @@ impl Display for DisplayList<'_> { _ => max, }) }); - let lineno_width = if lineno_width == 0 { - lineno_width - } else if self.anonymized_line_numbers { - ANONYMIZED_LINE_NUM.len() - } else { - ((lineno_width as f64).log10().floor() as usize) + 1 - }; + let lineno_width = self.lineno_offset + + if lineno_width == 0 { + lineno_width + } else if self.anonymized_line_numbers { + ANONYMIZED_LINE_NUM.len() + } else { + ((lineno_width as f64).log10().floor() as usize) + 1 + }; let multiline_depth = self.body.iter().fold(0, |max, set| { set.display_lines.iter().fold(max, |max2, line| match line { @@ -124,6 +126,7 @@ impl<'a> DisplayList<'a> { term_width: usize, cut_indicator: &'static str, ) -> DisplayList<'a> { + let lineno_offset = message.lineno_offset; let body = format_message( message, term_width, @@ -137,6 +140,7 @@ impl<'a> DisplayList<'a> { stylesheet, anonymized_line_numbers, cut_indicator, + lineno_offset, } } @@ -1088,6 +1092,7 @@ fn format_message<'m>( footer, snippets, is_fixable, + lineno_offset: _, } = message; let mut sets = vec![]; diff --git a/crates/ruff_annotate_snippets/src/snippet.rs b/crates/ruff_annotate_snippets/src/snippet.rs index 339cd3a850..76d615189d 100644 --- a/crates/ruff_annotate_snippets/src/snippet.rs +++ b/crates/ruff_annotate_snippets/src/snippet.rs @@ -23,6 +23,7 @@ pub struct Message<'a> { pub(crate) snippets: Vec>, pub(crate) footer: Vec>, pub(crate) is_fixable: bool, + pub(crate) lineno_offset: usize, } impl<'a> Message<'a> { @@ -59,6 +60,16 @@ impl<'a> Message<'a> { self.is_fixable = yes; self } + + /// Add an offset used for aligning the header sigil (`-->`) with the line number separators. + /// + /// For normal diagnostics this is computed automatically based on the lines to be rendered. + /// This is intended only for use in the formatter, where we don't render a snippet directly but + /// still want the header to align with the diff. + pub fn lineno_offset(mut self, offset: usize) -> Self { + self.lineno_offset = offset; + self + } } /// Structure containing the slice of text to be annotated and @@ -144,7 +155,7 @@ impl<'a> Annotation<'a> { self } - pub fn is_file_level(mut self, yes: bool) -> Self { + pub fn hide_snippet(mut self, yes: bool) -> Self { self.is_file_level = yes; self } @@ -173,6 +184,7 @@ impl Level { snippets: vec![], footer: vec![], is_fixable: false, + lineno_offset: 0, } } diff --git a/crates/ruff_db/src/diagnostic/mod.rs b/crates/ruff_db/src/diagnostic/mod.rs index 413e08cbe2..20ff72f14e 100644 --- a/crates/ruff_db/src/diagnostic/mod.rs +++ b/crates/ruff_db/src/diagnostic/mod.rs @@ -69,6 +69,7 @@ impl Diagnostic { parent: None, noqa_offset: None, secondary_code: None, + header_offset: 0, }); Diagnostic { inner } } @@ -432,14 +433,23 @@ impl Diagnostic { /// Returns the URL for the rule documentation, if it exists. pub fn to_ruff_url(&self) -> Option { - if self.is_invalid_syntax() { - None - } else { - Some(format!( - "{}/rules/{}", - env!("CARGO_PKG_HOMEPAGE"), - self.name() - )) + match self.id() { + DiagnosticId::Panic + | DiagnosticId::Io + | DiagnosticId::InvalidSyntax + | DiagnosticId::RevealedType + | DiagnosticId::UnknownRule + | DiagnosticId::InvalidGlob + | DiagnosticId::EmptyInclude + | DiagnosticId::UnnecessaryOverridesSection + | DiagnosticId::UselessOverridesSection + | DiagnosticId::DeprecatedSetting + | DiagnosticId::Unformatted + | DiagnosticId::InvalidCliOption + | DiagnosticId::InternalError => None, + DiagnosticId::Lint(lint_name) => { + Some(format!("{}/rules/{lint_name}", env!("CARGO_PKG_HOMEPAGE"))) + } } } @@ -512,6 +522,11 @@ impl Diagnostic { a.cmp(&b) } + + /// Add an offset for aligning the header sigil with the line number separators in a diff. + pub fn set_header_offset(&mut self, offset: usize) { + Arc::make_mut(&mut self.inner).header_offset = offset; + } } #[derive(Debug, Clone, Eq, PartialEq, Hash, get_size2::GetSize)] @@ -525,6 +540,7 @@ struct DiagnosticInner { parent: Option, noqa_offset: Option, secondary_code: Option, + header_offset: usize, } struct RenderingSortKey<'a> { @@ -742,11 +758,11 @@ pub struct Annotation { is_primary: bool, /// The diagnostic tags associated with this annotation. tags: Vec, - /// Whether this annotation is a file-level or full-file annotation. + /// Whether the snippet for this annotation should be hidden. /// /// When set, rendering will only include the file's name and (optional) range. Everything else /// is omitted, including any file snippet or message. - is_file_level: bool, + hide_snippet: bool, } impl Annotation { @@ -765,7 +781,7 @@ impl Annotation { message: None, is_primary: true, tags: Vec::new(), - is_file_level: false, + hide_snippet: false, } } @@ -782,7 +798,7 @@ impl Annotation { message: None, is_primary: false, tags: Vec::new(), - is_file_level: false, + hide_snippet: false, } } @@ -849,19 +865,20 @@ impl Annotation { self.tags.push(tag); } - /// Set whether or not this annotation is file-level. + /// Set whether or not the snippet on this annotation should be suppressed when rendering. /// - /// File-level annotations are only rendered with their file name and range, if available. This - /// is intended for backwards compatibility with Ruff diagnostics, which historically used + /// Such annotations are only rendered with their file name and range, if available. This is + /// intended for backwards compatibility with Ruff diagnostics, which historically used /// `TextRange::default` to indicate a file-level diagnostic. In the new diagnostic model, a /// [`Span`] with a range of `None` should be used instead, as mentioned in the `Span` /// documentation. /// /// TODO(brent) update this usage in Ruff and remove `is_file_level` entirely. See /// , especially my first comment, for more - /// details. - pub fn set_file_level(&mut self, yes: bool) { - self.is_file_level = yes; + /// details. As of 2025-09-26 we also use this to suppress snippet rendering for formatter + /// diagnostics, which also need to have a range, so we probably can't eliminate this entirely. + pub fn hide_snippet(&mut self, yes: bool) { + self.hide_snippet = yes; } } @@ -1016,6 +1033,17 @@ pub enum DiagnosticId { /// Use of a deprecated setting. DeprecatedSetting, + + /// The code needs to be formatted. + Unformatted, + + /// Use of an invalid command-line option. + InvalidCliOption, + + /// An internal assumption was violated. + /// + /// This indicates a bug in the program rather than a user error. + InternalError, } impl DiagnosticId { @@ -1055,6 +1083,9 @@ impl DiagnosticId { DiagnosticId::UnnecessaryOverridesSection => "unnecessary-overrides-section", DiagnosticId::UselessOverridesSection => "useless-overrides-section", DiagnosticId::DeprecatedSetting => "deprecated-setting", + DiagnosticId::Unformatted => "unformatted", + DiagnosticId::InvalidCliOption => "invalid-cli-option", + DiagnosticId::InternalError => "internal-error", } } diff --git a/crates/ruff_db/src/diagnostic/render.rs b/crates/ruff_db/src/diagnostic/render.rs index 8f1b9184a8..96fbb565b4 100644 --- a/crates/ruff_db/src/diagnostic/render.rs +++ b/crates/ruff_db/src/diagnostic/render.rs @@ -208,6 +208,7 @@ struct ResolvedDiagnostic<'a> { message: String, annotations: Vec>, is_fixable: bool, + header_offset: usize, } impl<'a> ResolvedDiagnostic<'a> { @@ -258,7 +259,8 @@ impl<'a> ResolvedDiagnostic<'a> { id, message: diag.inner.message.as_str().to_string(), annotations, - is_fixable: diag.has_applicable_fix(config), + is_fixable: config.show_fix_status && diag.has_applicable_fix(config), + header_offset: diag.inner.header_offset, } } @@ -288,6 +290,7 @@ impl<'a> ResolvedDiagnostic<'a> { message: diag.inner.message.as_str().to_string(), annotations, is_fixable: false, + header_offset: 0, } } @@ -385,6 +388,7 @@ impl<'a> ResolvedDiagnostic<'a> { message: &self.message, snippets_by_input, is_fixable: self.is_fixable, + header_offset: self.header_offset, } } } @@ -404,7 +408,7 @@ struct ResolvedAnnotation<'a> { line_end: OneIndexed, message: Option<&'a str>, is_primary: bool, - is_file_level: bool, + hide_snippet: bool, notebook_index: Option, } @@ -452,7 +456,7 @@ impl<'a> ResolvedAnnotation<'a> { line_end, message: ann.get_message(), is_primary: ann.is_primary, - is_file_level: ann.is_file_level, + hide_snippet: ann.hide_snippet, notebook_index: resolver.notebook_index(&ann.span.file), }) } @@ -492,6 +496,11 @@ struct RenderableDiagnostic<'r> { /// /// This is rendered as a `[*]` indicator after the diagnostic ID. is_fixable: bool, + /// Offset to align the header sigil (`-->`) with the subsequent line number separators. + /// + /// This is only needed for formatter diagnostics where we don't render a snippet via + /// `annotate-snippets` and thus the alignment isn't computed automatically. + header_offset: usize, } impl RenderableDiagnostic<'_> { @@ -504,7 +513,11 @@ impl RenderableDiagnostic<'_> { .iter() .map(|snippet| snippet.to_annotate(path)) }); - let mut message = self.level.title(self.message).is_fixable(self.is_fixable); + let mut message = self + .level + .title(self.message) + .is_fixable(self.is_fixable) + .lineno_offset(self.header_offset); if let Some(id) = self.id { message = message.id(id); } @@ -709,8 +722,8 @@ struct RenderableAnnotation<'r> { message: Option<&'r str>, /// Whether this annotation is considered "primary" or not. is_primary: bool, - /// Whether this annotation applies to an entire file, rather than a snippet within it. - is_file_level: bool, + /// Whether the snippet for this annotation should be hidden instead of rendered. + hide_snippet: bool, } impl<'r> RenderableAnnotation<'r> { @@ -732,7 +745,7 @@ impl<'r> RenderableAnnotation<'r> { range, message: ann.message, is_primary: ann.is_primary, - is_file_level: ann.is_file_level, + hide_snippet: ann.hide_snippet, } } @@ -758,7 +771,7 @@ impl<'r> RenderableAnnotation<'r> { if let Some(message) = self.message { ann = ann.label(message); } - ann.is_file_level(self.is_file_level) + ann.hide_snippet(self.hide_snippet) } } diff --git a/crates/ruff_db/src/diagnostic/render/full.rs b/crates/ruff_db/src/diagnostic/render/full.rs index c6a5bc13fa..c87413a84e 100644 --- a/crates/ruff_db/src/diagnostic/render/full.rs +++ b/crates/ruff_db/src/diagnostic/render/full.rs @@ -366,6 +366,7 @@ mod tests { fn hide_severity_output() { let (mut env, diagnostics) = create_diagnostics(DiagnosticFormat::Full); env.hide_severity(true); + env.show_fix_status(true); env.fix_applicability(Applicability::DisplayOnly); insta::assert_snapshot!(env.render_diagnostics(&diagnostics), @r#" @@ -572,7 +573,7 @@ print() let mut diagnostic = env.err().build(); let span = env.path("example.py").with_range(TextRange::default()); let mut annotation = Annotation::primary(span); - annotation.set_file_level(true); + annotation.hide_snippet(true); diagnostic.annotate(annotation); insta::assert_snapshot!(env.render(&diagnostic), @r" @@ -584,7 +585,8 @@ print() /// Check that ranges in notebooks are remapped relative to the cells. #[test] fn notebook_output() { - let (env, diagnostics) = create_notebook_diagnostics(DiagnosticFormat::Full); + let (mut env, diagnostics) = create_notebook_diagnostics(DiagnosticFormat::Full); + env.show_fix_status(true); insta::assert_snapshot!(env.render_diagnostics(&diagnostics), @r" error[unused-import][*]: `os` imported but unused --> notebook.ipynb:cell 1:2:8 @@ -698,6 +700,7 @@ print() fn notebook_output_with_diff() { let (mut env, diagnostics) = create_notebook_diagnostics(DiagnosticFormat::Full); env.show_fix_diff(true); + env.show_fix_status(true); env.fix_applicability(Applicability::DisplayOnly); insta::assert_snapshot!(env.render_diagnostics(&diagnostics), @r" @@ -752,6 +755,7 @@ print() fn notebook_output_with_diff_spanning_cells() { let (mut env, mut diagnostics) = create_notebook_diagnostics(DiagnosticFormat::Full); env.show_fix_diff(true); + env.show_fix_status(true); env.fix_applicability(Applicability::DisplayOnly); // Move all of the edits from the later diagnostics to the first diagnostic to simulate a @@ -928,6 +932,7 @@ line 10 env.add("example.py", contents); env.format(DiagnosticFormat::Full); env.show_fix_diff(true); + env.show_fix_status(true); env.fix_applicability(Applicability::DisplayOnly); let mut diagnostic = env.err().primary("example.py", "3", "3", "label").build(); diff --git a/crates/ruff_db/src/panic.rs b/crates/ruff_db/src/panic.rs index a6b67f1f1d..d91c1f7fe7 100644 --- a/crates/ruff_db/src/panic.rs +++ b/crates/ruff_db/src/panic.rs @@ -31,6 +31,29 @@ impl Payload { } } +impl PanicError { + pub fn to_diagnostic_message(&self, path: Option) -> String { + use std::fmt::Write; + + let mut message = String::new(); + message.push_str("Panicked"); + + if let Some(location) = &self.location { + let _ = write!(&mut message, " at {location}"); + } + + if let Some(path) = path { + let _ = write!(&mut message, " when checking `{path}`"); + } + + if let Some(payload) = self.payload.as_str() { + let _ = write!(&mut message, ": `{payload}`"); + } + + message + } +} + impl std::fmt::Display for PanicError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "panicked at")?; diff --git a/crates/ruff_formatter/src/diagnostics.rs b/crates/ruff_formatter/src/diagnostics.rs index eb2f1435b3..08964496ad 100644 --- a/crates/ruff_formatter/src/diagnostics.rs +++ b/crates/ruff_formatter/src/diagnostics.rs @@ -38,12 +38,12 @@ impl std::fmt::Display for FormatError { ), FormatError::InvalidDocument(error) => std::write!( fmt, - "Invalid document: {error}\n\n This is an internal Rome error. Please report if necessary." + "Invalid document: {error}\n\n This is an internal Ruff error. Please report if necessary." ), FormatError::PoorLayout => { std::write!( fmt, - "Poor layout: The formatter wasn't able to pick a good layout for your document. This is an internal Rome error. Please report if necessary." + "Poor layout: The formatter wasn't able to pick a good layout for your document. This is an internal Ruff error. Please report if necessary." ) } } diff --git a/crates/ruff_linter/src/logging.rs b/crates/ruff_linter/src/logging.rs index 5b293c797b..9dba2228b0 100644 --- a/crates/ruff_linter/src/logging.rs +++ b/crates/ruff_linter/src/logging.rs @@ -223,6 +223,11 @@ impl DisplayParseError { pub fn path(&self) -> Option<&Path> { self.path.as_deref() } + + /// Return the underlying [`ParseError`]. + pub fn error(&self) -> &ParseError { + &self.error + } } impl std::error::Error for DisplayParseError {} diff --git a/crates/ruff_linter/src/message/mod.rs b/crates/ruff_linter/src/message/mod.rs index 5ac4712a61..f552d0df51 100644 --- a/crates/ruff_linter/src/message/mod.rs +++ b/crates/ruff_linter/src/message/mod.rs @@ -1,18 +1,21 @@ +use std::backtrace::BacktraceStatus; use std::fmt::Display; use std::io::Write; +use std::path::Path; +use ruff_db::panic::PanicError; use rustc_hash::FxHashMap; use ruff_db::diagnostic::{ Annotation, Diagnostic, DiagnosticFormat, DiagnosticId, DisplayDiagnosticConfig, DisplayDiagnostics, DisplayGithubDiagnostics, FileResolver, GithubRenderer, Input, LintName, - SecondaryCode, Severity, Span, UnifiedFile, + SecondaryCode, Severity, Span, SubDiagnostic, SubDiagnosticSeverity, UnifiedFile, }; use ruff_db::files::File; pub use grouped::GroupedEmitter; use ruff_notebook::NotebookIndex; -use ruff_source_file::SourceFile; +use ruff_source_file::{SourceFile, SourceFileBuilder}; use ruff_text_size::{Ranged, TextRange, TextSize}; pub use sarif::SarifEmitter; @@ -41,6 +44,55 @@ pub fn create_syntax_error_diagnostic( diag } +/// Create a `Diagnostic` from a panic. +pub fn create_panic_diagnostic(error: &PanicError, path: Option<&Path>) -> Diagnostic { + let mut diagnostic = Diagnostic::new( + DiagnosticId::Panic, + Severity::Fatal, + error.to_diagnostic_message(path.as_ref().map(|path| path.display())), + ); + + diagnostic.sub(SubDiagnostic::new( + SubDiagnosticSeverity::Info, + "This indicates a bug in Ruff.", + )); + let report_message = "If you could open an issue at \ + https://github.com/astral-sh/ruff/issues/new?title=%5Bpanic%5D, \ + we'd be very appreciative!"; + diagnostic.sub(SubDiagnostic::new( + SubDiagnosticSeverity::Info, + report_message, + )); + + if let Some(backtrace) = &error.backtrace { + match backtrace.status() { + BacktraceStatus::Disabled => { + diagnostic.sub(SubDiagnostic::new( + SubDiagnosticSeverity::Info, + "run with `RUST_BACKTRACE=1` environment variable to show the full backtrace information", + )); + } + BacktraceStatus::Captured => { + diagnostic.sub(SubDiagnostic::new( + SubDiagnosticSeverity::Info, + format!("Backtrace:\n{backtrace}"), + )); + } + _ => {} + } + } + + if let Some(path) = path { + let file = SourceFileBuilder::new(path.to_string_lossy(), "").finish(); + let span = Span::from(file); + let mut annotation = Annotation::primary(span); + annotation.hide_snippet(true); + diagnostic.annotate(annotation); + } + + diagnostic +} + #[expect(clippy::too_many_arguments)] pub fn create_lint_diagnostic( body: B, @@ -70,7 +122,7 @@ where // actually need it, but we need to be able to cache the new diagnostic model first. See // https://github.com/astral-sh/ruff/issues/19688. if range == TextRange::default() { - annotation.set_file_level(true); + annotation.hide_snippet(true); } diagnostic.annotate(annotation); diff --git a/crates/ruff_linter/src/settings/types.rs b/crates/ruff_linter/src/settings/types.rs index 760422a0ce..1331035345 100644 --- a/crates/ruff_linter/src/settings/types.rs +++ b/crates/ruff_linter/src/settings/types.rs @@ -535,6 +535,20 @@ pub enum OutputFormat { Sarif, } +impl OutputFormat { + /// Returns `true` if this format is intended for users to read directly, in contrast to + /// machine-readable or structured formats. + /// + /// This can be used to check whether information beyond the diagnostics, such as a header or + /// `Found N diagnostics` footer, should be included. + pub const fn is_human_readable(&self) -> bool { + matches!( + self, + OutputFormat::Full | OutputFormat::Concise | OutputFormat::Grouped + ) + } +} + impl Display for OutputFormat { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { diff --git a/crates/ruff_notebook/src/cell.rs b/crates/ruff_notebook/src/cell.rs index 56c1822341..0bcd613370 100644 --- a/crates/ruff_notebook/src/cell.rs +++ b/crates/ruff_notebook/src/cell.rs @@ -308,6 +308,13 @@ impl CellOffsets { }) .is_ok() } + + /// Returns an iterator over [`TextRange`]s covered by each cell. + pub fn ranges(&self) -> impl Iterator { + self.iter() + .tuple_windows() + .map(|(start, end)| TextRange::new(*start, *end)) + } } impl Deref for CellOffsets { diff --git a/crates/ty_project/src/lib.rs b/crates/ty_project/src/lib.rs index b98917dbf5..f5ef2201da 100644 --- a/crates/ty_project/src/lib.rs +++ b/crates/ty_project/src/lib.rs @@ -666,24 +666,7 @@ where }) { Ok(result) => Ok(result), Err(error) => { - use std::fmt::Write; - let mut message = String::new(); - message.push_str("Panicked"); - - if let Some(location) = error.location { - let _ = write!(&mut message, " at {location}"); - } - - let _ = write!( - &mut message, - " when checking `{file}`", - file = file.path(db) - ); - - if let Some(payload) = error.payload.as_str() { - let _ = write!(&mut message, ": `{payload}`"); - } - + let message = error.to_diagnostic_message(Some(file.path(db))); let mut diagnostic = Diagnostic::new(DiagnosticId::Panic, Severity::Fatal, message); diagnostic.sub(SubDiagnostic::new( SubDiagnosticSeverity::Info, diff --git a/docs/configuration.md b/docs/configuration.md index 0b79a20de9..8d3297fbca 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -727,6 +727,11 @@ Options: --preview Enable preview mode; enables unstable formatting. Use `--no-preview` to disable + --output-format + Output serialization format for violations, when used with `--check`. + The default serialization format is "full" [env: RUFF_OUTPUT_FORMAT=] + [possible values: concise, full, json, json-lines, junit, grouped, + github, gitlab, pylint, rdjson, azure, sarif] -h, --help Print help (see more with '--help')