diff --git a/crates/ruff_cli/resources/test/fixtures/formatted.py b/crates/ruff_cli/resources/test/fixtures/formatted.py new file mode 100644 index 0000000000..a205c715fb --- /dev/null +++ b/crates/ruff_cli/resources/test/fixtures/formatted.py @@ -0,0 +1 @@ +print("All formatted!") diff --git a/crates/ruff_cli/resources/test/fixtures/unformatted.ipynb b/crates/ruff_cli/resources/test/fixtures/unformatted.ipynb new file mode 100644 index 0000000000..7b8ebf87a0 --- /dev/null +++ b/crates/ruff_cli/resources/test/fixtures/unformatted.ipynb @@ -0,0 +1,37 @@ +{ + "cells": [ + { + "cell_type": "code", + "execution_count": null, + "id": "98e1dd71-14a2-454d-9be0-061dde560b07", + "metadata": {}, + "outputs": [], + "source": [ + "import numpy\n", + "maths = (numpy.arange(100)**2).sum()\n", + "stats= numpy.asarray([1,2,3,4]).median()" + ] + } + ], + "metadata": { + "kernelspec": { + "display_name": "Python 3 (ipykernel)", + "language": "python", + "name": "python3" + }, + "language_info": { + "codemirror_mode": { + "name": "ipython", + "version": 3 + }, + "file_extension": ".py", + "mimetype": "text/x-python", + "name": "python", + "nbconvert_exporter": "python", + "pygments_lexer": "ipython3", + "version": "3.10.12" + } + }, + "nbformat": 4, + "nbformat_minor": 5 +} diff --git a/crates/ruff_cli/resources/test/fixtures/unformatted.py b/crates/ruff_cli/resources/test/fixtures/unformatted.py new file mode 100644 index 0000000000..31bf5480ca --- /dev/null +++ b/crates/ruff_cli/resources/test/fixtures/unformatted.py @@ -0,0 +1,3 @@ +x = 1 +y=2 +z = 3 diff --git a/crates/ruff_cli/src/args.rs b/crates/ruff_cli/src/args.rs index f17014b6f9..8a1006a6e6 100644 --- a/crates/ruff_cli/src/args.rs +++ b/crates/ruff_cli/src/args.rs @@ -353,6 +353,10 @@ pub struct FormatCommand { /// files would have been modified, and zero otherwise. #[arg(long)] pub check: bool, + /// Avoid writing any formatted files back; instead, exit with a non-zero status code and the + /// difference between the current file and how the formatted file would look like. + #[arg(long)] + pub diff: bool, /// Path to the `pyproject.toml` or `ruff.toml` file to use for configuration. #[arg(long, conflicts_with = "isolated")] pub config: Option, @@ -520,6 +524,7 @@ impl FormatCommand { ( FormatArguments { check: self.check, + diff: self.diff, config: self.config, files: self.files, isolated: self.isolated, @@ -577,6 +582,7 @@ pub struct CheckArguments { #[allow(clippy::struct_excessive_bools)] pub struct FormatArguments { pub check: bool, + pub diff: bool, pub config: Option, pub files: Vec, pub isolated: bool, diff --git a/crates/ruff_cli/src/commands/format.rs b/crates/ruff_cli/src/commands/format.rs index 486d9d2bbc..21424ed421 100644 --- a/crates/ruff_cli/src/commands/format.rs +++ b/crates/ruff_cli/src/commands/format.rs @@ -1,5 +1,7 @@ use std::fmt::{Display, Formatter}; use std::fs::File; +use std::io; +use std::io::{stderr, stdout, Write}; use std::path::{Path, PathBuf}; use std::time::Instant; @@ -9,6 +11,7 @@ use itertools::Itertools; use log::error; use rayon::iter::Either::{Left, Right}; use rayon::iter::{IntoParallelIterator, ParallelIterator}; +use similar::TextDiff; use thiserror::Error; use tracing::debug; @@ -34,6 +37,20 @@ pub(crate) enum FormatMode { Write, /// Check if the file is formatted, but do not write the formatted contents back. Check, + /// Check if the file is formatted, show a diff if not. + Diff, +} + +impl FormatMode { + pub(crate) fn from_cli(cli: &FormatArguments) -> Self { + if cli.diff { + FormatMode::Diff + } else if cli.check { + FormatMode::Check + } else { + FormatMode::Write + } + } } /// Format a set of files, and return the exit status. @@ -48,11 +65,7 @@ pub(crate) fn format( overrides, cli.stdin_filename.as_deref(), )?; - let mode = if cli.check { - FormatMode::Check - } else { - FormatMode::Write - }; + let mode = FormatMode::from_cli(cli); let (paths, resolver) = python_files_in_path(&cli.files, &pyproject_config, overrides)?; if paths.is_empty() { @@ -61,7 +74,7 @@ pub(crate) fn format( } let start = Instant::now(); - let (mut results, errors): (Vec<_>, Vec<_>) = paths + let (mut results, mut errors): (Vec<_>, Vec<_>) = paths .into_par_iter() .filter_map(|entry| { match entry { @@ -85,12 +98,31 @@ pub(crate) fn format( return None; } + // Extract the sources from the file. + let unformatted = match SourceKind::from_path(path, source_type) { + Ok(Some(source_kind)) => source_kind, + Ok(None) => return None, + Err(err) => { + return Some(Err(FormatCommandError::Read( + Some(path.to_path_buf()), + err, + ))); + } + }; + Some( match catch_unwind(|| { - format_path(path, &resolved_settings.formatter, source_type, mode) + format_path( + path, + &resolved_settings.formatter, + &unformatted, + source_type, + mode, + ) }) { Ok(inner) => inner.map(|result| FormatPathResult { path: resolved_file.into_path(), + unformatted, result, }), Err(error) => Err(FormatCommandError::Panic( @@ -109,6 +141,27 @@ pub(crate) fn format( }); let duration = start.elapsed(); + // Make output deterministic, at least as long as we have a path + results.sort_unstable_by(|x, y| x.path.cmp(&y.path)); + errors.sort_by(|x, y| { + fn get_key(error: &FormatCommandError) -> Option<&PathBuf> { + match &error { + FormatCommandError::Ignore(ignore) => { + if let ignore::Error::WithPath { path, .. } = ignore { + Some(path) + } else { + None + } + } + FormatCommandError::Panic(path, _) + | FormatCommandError::Read(path, _) + | FormatCommandError::Format(path, _) + | FormatCommandError::Write(path, _) => path.as_ref(), + } + } + get_key(x).cmp(&get_key(y)) + }); + debug!( "Formatted {} files in {:.2?}", results.len() + errors.len(), @@ -121,14 +174,19 @@ pub(crate) fn format( } results.sort_unstable_by(|a, b| a.path.cmp(&b.path)); + let results = FormatResults::new(results.as_slice(), mode); - let summary = FormatSummary::new(results.as_slice(), mode); + if mode.is_diff() { + results.write_diff(&mut stdout().lock())?; + } // Report on the formatting changes. if log_level >= LogLevel::Default { - #[allow(clippy::print_stdout)] - { - println!("{summary}"); + 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 { + results.write_summary(&mut stdout().lock())?; } } @@ -140,9 +198,9 @@ pub(crate) fn format( Ok(ExitStatus::Error) } } - FormatMode::Check => { + FormatMode::Check | FormatMode::Diff => { if errors.is_empty() { - if summary.any_formatted() { + if results.any_formatted() { Ok(ExitStatus::Failure) } else { Ok(ExitStatus::Success) @@ -159,57 +217,43 @@ pub(crate) fn format( fn format_path( path: &Path, settings: &FormatterSettings, + unformatted: &SourceKind, source_type: PySourceType, mode: FormatMode, ) -> Result { - // Extract the sources from the file. - let source_kind = match SourceKind::from_path(path, source_type) { - Ok(Some(source_kind)) => source_kind, - Ok(None) => return Ok(FormatResult::Unchanged), - Err(err) => { - return Err(FormatCommandError::Read(Some(path.to_path_buf()), err)); - } - }; - // Format the source. - match format_source(source_kind, source_type, Some(path), settings)? { - FormattedSource::Formatted(formatted) => { - if mode.is_write() { + let format_result = match format_source(unformatted, source_type, Some(path), settings)? { + FormattedSource::Formatted(formatted) => match mode { + FormatMode::Write => { let mut writer = File::create(path).map_err(|err| { FormatCommandError::Write(Some(path.to_path_buf()), err.into()) })?; formatted .write(&mut writer) .map_err(|err| FormatCommandError::Write(Some(path.to_path_buf()), err))?; + FormatResult::Formatted } - Ok(FormatResult::Formatted) - } - FormattedSource::Unchanged(_) => Ok(FormatResult::Unchanged), - } + FormatMode::Check => FormatResult::Formatted, + FormatMode::Diff => FormatResult::Diff(formatted), + }, + FormattedSource::Unchanged => FormatResult::Unchanged, + }; + Ok(format_result) } #[derive(Debug)] pub(crate) enum FormattedSource { /// The source was formatted, and the [`SourceKind`] contains the transformed source code. Formatted(SourceKind), - /// The source was unchanged, and the [`SourceKind`] contains the original source code. - Unchanged(SourceKind), + /// The source was unchanged. + Unchanged, } impl From for FormatResult { fn from(value: FormattedSource) -> Self { match value { FormattedSource::Formatted(_) => FormatResult::Formatted, - FormattedSource::Unchanged(_) => FormatResult::Unchanged, - } - } -} - -impl FormattedSource { - pub(crate) fn source_kind(&self) -> &SourceKind { - match self { - FormattedSource::Formatted(source_kind) => source_kind, - FormattedSource::Unchanged(source_kind) => source_kind, + FormattedSource::Unchanged => FormatResult::Unchanged, } } } @@ -217,30 +261,28 @@ impl FormattedSource { /// Format a [`SourceKind`], returning the transformed [`SourceKind`], or `None` if the source was /// unchanged. pub(crate) fn format_source( - source_kind: SourceKind, + source_kind: &SourceKind, source_type: PySourceType, path: Option<&Path>, settings: &FormatterSettings, ) -> Result { match source_kind { SourceKind::Python(unformatted) => { - let options = settings.to_format_options(source_type, &unformatted); + let options = settings.to_format_options(source_type, unformatted); - let formatted = format_module_source(&unformatted, options) + let formatted = format_module_source(unformatted, options) .map_err(|err| FormatCommandError::Format(path.map(Path::to_path_buf), err))?; let formatted = formatted.into_code(); if formatted.len() == unformatted.len() && formatted == *unformatted { - Ok(FormattedSource::Unchanged(SourceKind::Python(unformatted))) + Ok(FormattedSource::Unchanged) } else { Ok(FormattedSource::Formatted(SourceKind::Python(formatted))) } } SourceKind::IpyNotebook(notebook) => { if !notebook.is_python_notebook() { - return Ok(FormattedSource::Unchanged(SourceKind::IpyNotebook( - notebook, - ))); + return Ok(FormattedSource::Unchanged); } let options = settings.to_format_options(source_type, notebook.source_code()); @@ -288,9 +330,7 @@ pub(crate) fn format_source( // If the file was unchanged, return `None`. let (Some(mut output), Some(last)) = (output, last) else { - return Ok(FormattedSource::Unchanged(SourceKind::IpyNotebook( - notebook, - ))); + return Ok(FormattedSource::Unchanged); }; // Add the remaining content. @@ -298,21 +338,23 @@ pub(crate) fn format_source( output.push_str(slice); // Update the notebook. - let mut notebook = notebook.clone(); - notebook.update(&source_map, output); + let mut formatted = notebook.clone(); + formatted.update(&source_map, output); Ok(FormattedSource::Formatted(SourceKind::IpyNotebook( - notebook, + formatted, ))) } } } /// The result of an individual formatting operation. -#[derive(Debug, Clone, Copy, is_macro::Is)] +#[derive(Debug, Clone, is_macro::Is)] pub(crate) enum FormatResult { /// The file was formatted. Formatted, + /// The file was formatted, [`SourceKind`] contains the formatted code + Diff(SourceKind), /// The file was unchanged, as the formatted contents matched the existing contents. Unchanged, } @@ -321,38 +363,55 @@ pub(crate) enum FormatResult { #[derive(Debug)] struct FormatPathResult { path: PathBuf, + unformatted: SourceKind, result: FormatResult, } -/// A summary of the formatting results. +/// The results of formatting a set of files #[derive(Debug)] -struct FormatSummary<'a> { +struct FormatResults<'a> { /// The individual formatting results. results: &'a [FormatPathResult], /// The format mode that was used. mode: FormatMode, } -impl<'a> FormatSummary<'a> { +impl<'a> FormatResults<'a> { fn new(results: &'a [FormatPathResult], mode: FormatMode) -> Self { Self { results, mode } } /// Returns `true` if any of the files require formatting. fn any_formatted(&self) -> bool { - self.results - .iter() - .any(|result| result.result.is_formatted()) + self.results.iter().any(|result| match result.result { + FormatResult::Formatted | FormatResult::Diff { .. } => true, + FormatResult::Unchanged => false, + }) } -} -impl Display for FormatSummary<'_> { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + fn write_diff(&self, f: &mut impl Write) -> io::Result<()> { + for result in self.results { + if let FormatResult::Diff(formatted) = &result.result { + let text_diff = + TextDiff::from_lines(result.unformatted.source_code(), formatted.source_code()); + let mut unified_diff = text_diff.unified_diff(); + unified_diff.header( + &fs::relativize_path(&result.path), + &fs::relativize_path(&result.path), + ); + unified_diff.to_writer(&mut *f)?; + } + } + + Ok(()) + } + + fn write_summary(&self, f: &mut impl Write) -> io::Result<()> { // Compute the number of changed and unchanged files. - let mut formatted = 0u32; + let mut changed = 0u32; let mut unchanged = 0u32; for result in self.results { - match result.result { + match &result.result { FormatResult::Formatted => { // If we're running in check mode, report on any files that would be formatted. if self.mode.is_check() { @@ -362,39 +421,42 @@ impl Display for FormatSummary<'_> { fs::relativize_path(&result.path).bold() )?; } - formatted += 1; + changed += 1; } FormatResult::Unchanged => unchanged += 1, + FormatResult::Diff(_) => { + changed += 1; + } } } // Write out a summary of the formatting results. - if formatted > 0 && unchanged > 0 { - write!( + if changed > 0 && unchanged > 0 { + writeln!( f, "{} file{} {}, {} file{} left unchanged", - formatted, - if formatted == 1 { "" } else { "s" }, + changed, + if changed == 1 { "" } else { "s" }, match self.mode { FormatMode::Write => "reformatted", - FormatMode::Check => "would be reformatted", + FormatMode::Check | FormatMode::Diff => "would be reformatted", }, unchanged, if unchanged == 1 { "" } else { "s" }, ) - } else if formatted > 0 { - write!( + } else if changed > 0 { + writeln!( f, "{} file{} {}", - formatted, - if formatted == 1 { "" } else { "s" }, + changed, + if changed == 1 { "" } else { "s" }, match self.mode { FormatMode::Write => "reformatted", - FormatMode::Check => "would be reformatted", + FormatMode::Check | FormatMode::Diff => "would be reformatted", } ) } else if unchanged > 0 { - write!( + writeln!( f, "{} file{} left unchanged", unchanged, diff --git a/crates/ruff_cli/src/commands/format_stdin.rs b/crates/ruff_cli/src/commands/format_stdin.rs index 1709998740..c483a4a8e9 100644 --- a/crates/ruff_cli/src/commands/format_stdin.rs +++ b/crates/ruff_cli/src/commands/format_stdin.rs @@ -3,6 +3,8 @@ use std::path::Path; use anyhow::Result; use log::error; +use ruff_linter::fs; +use similar::TextDiff; use ruff_linter::source_kind::SourceKind; use ruff_python_ast::{PySourceType, SourceType}; @@ -10,7 +12,9 @@ use ruff_workspace::resolver::{match_exclusion, python_file_at_path}; use ruff_workspace::FormatterSettings; use crate::args::{CliOverrides, FormatArguments}; -use crate::commands::format::{format_source, FormatCommandError, FormatMode, FormatResult}; +use crate::commands::format::{ + format_source, FormatCommandError, FormatMode, FormatResult, FormattedSource, +}; use crate::resolve::resolve; use crate::stdin::read_from_stdin; use crate::ExitStatus; @@ -23,11 +27,7 @@ pub(crate) fn format_stdin(cli: &FormatArguments, overrides: &CliOverrides) -> R overrides, cli.stdin_filename.as_deref(), )?; - let mode = if cli.check { - FormatMode::Check - } else { - FormatMode::Write - }; + let mode = FormatMode::from_cli(cli); if let Some(filename) = cli.stdin_filename.as_deref() { if !python_file_at_path(filename, &pyproject_config, overrides)? { @@ -58,7 +58,7 @@ pub(crate) fn format_stdin(cli: &FormatArguments, overrides: &CliOverrides) -> R ) { Ok(result) => match mode { FormatMode::Write => Ok(ExitStatus::Success), - FormatMode::Check => { + FormatMode::Check | FormatMode::Diff => { if result.is_formatted() { Ok(ExitStatus::Failure) } else { @@ -93,15 +93,37 @@ fn format_source_code( }; // Format the source. - let formatted = format_source(source_kind, source_type, path, settings)?; + let formatted = format_source(&source_kind, source_type, path, settings)?; - // Write to stdout regardless of whether the source was formatted. - if mode.is_write() { - let mut writer = stdout().lock(); - formatted - .source_kind() - .write(&mut writer) - .map_err(|err| FormatCommandError::Write(path.map(Path::to_path_buf), err))?; + match &formatted { + FormattedSource::Formatted(formatted) => match mode { + FormatMode::Write => { + let mut writer = stdout().lock(); + formatted + .write(&mut writer) + .map_err(|err| FormatCommandError::Write(path.map(Path::to_path_buf), err))?; + } + FormatMode::Check => {} + FormatMode::Diff => { + let mut writer = stdout().lock(); + let text_diff = + TextDiff::from_lines(source_kind.source_code(), formatted.source_code()); + let mut unified_diff = text_diff.unified_diff(); + if let Some(path) = path { + unified_diff.header(&fs::relativize_path(path), &fs::relativize_path(path)); + } + unified_diff.to_writer(&mut writer).unwrap(); + } + }, + FormattedSource::Unchanged => { + // Write to stdout regardless of whether the source was formatted + if mode.is_write() { + let mut writer = stdout().lock(); + source_kind + .write(&mut writer) + .map_err(|err| FormatCommandError::Write(path.map(Path::to_path_buf), err))?; + } + } } Ok(FormatResult::from(formatted)) diff --git a/crates/ruff_cli/tests/format.rs b/crates/ruff_cli/tests/format.rs index 83225ed57b..bc21404b3b 100644 --- a/crates/ruff_cli/tests/format.rs +++ b/crates/ruff_cli/tests/format.rs @@ -1,6 +1,7 @@ #![cfg(not(target_family = "wasm"))] use std::fs; +use std::path::Path; use std::process::Command; use std::str; @@ -284,3 +285,125 @@ format = "json" }); Ok(()) } + +#[test] +fn test_diff() { + let args = ["format", "--isolated", "--diff"]; + let fixtures = Path::new("resources").join("test").join("fixtures"); + let paths = [ + fixtures.join("unformatted.py"), + fixtures.join("formatted.py"), + 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).args(paths), + @r###" + success: false + exit_code: 1 + ----- stdout ----- + --- resources/test/fixtures/unformatted.ipynb + +++ resources/test/fixtures/unformatted.ipynb + @@ -1,3 +1,4 @@ + import numpy + -maths = (numpy.arange(100)**2).sum() + -stats= numpy.asarray([1,2,3,4]).median() + + + +maths = (numpy.arange(100) ** 2).sum() + +stats = numpy.asarray([1, 2, 3, 4]).median() + --- resources/test/fixtures/unformatted.py + +++ resources/test/fixtures/unformatted.py + @@ -1,3 +1,3 @@ + x = 1 + -y=2 + +y = 2 + z = 3 + + ----- stderr ----- + warning: `ruff format` is not yet stable, and subject to change in future versions. + 2 files would be reformatted, 1 file left unchanged + "###); + }); +} + +#[test] +fn test_diff_no_change() { + let args = ["format", "--isolated", "--diff"]; + let fixtures = Path::new("resources").join("test").join("fixtures"); + let paths = [fixtures.join("unformatted.py")]; + insta::with_settings!({filters => vec![ + // Replace windows paths + (r"\\", "/"), + ]}, { + assert_cmd_snapshot!( + Command::new(get_cargo_bin(BIN_NAME)).args(args).args(paths), + @r###" + success: false + exit_code: 1 + ----- stdout ----- + --- resources/test/fixtures/unformatted.py + +++ resources/test/fixtures/unformatted.py + @@ -1,3 +1,3 @@ + x = 1 + -y=2 + +y = 2 + z = 3 + + ----- stderr ----- + warning: `ruff format` is not yet stable, and subject to change in future versions. + 1 file would be reformatted + "### + ); + }); +} + +#[test] +fn test_diff_stdin_unformatted() { + let args = [ + "format", + "--isolated", + "--diff", + "-", + "--stdin-filename", + "unformatted.py", + ]; + let fixtures = Path::new("resources").join("test").join("fixtures"); + let unformatted = fs::read(fixtures.join("unformatted.py")).unwrap(); + assert_cmd_snapshot!( + Command::new(get_cargo_bin(BIN_NAME)).args(args).pass_stdin(unformatted), + @r###" + success: false + exit_code: 1 + ----- stdout ----- + --- unformatted.py + +++ unformatted.py + @@ -1,3 +1,3 @@ + x = 1 + -y=2 + +y = 2 + z = 3 + + ----- stderr ----- + warning: `ruff format` is not yet stable, and subject to change in future versions. + "###); +} + +#[test] +fn test_diff_stdin_formatted() { + let args = ["format", "--isolated", "--diff", "-"]; + let fixtures = Path::new("resources").join("test").join("fixtures"); + let unformatted = fs::read(fixtures.join("formatted.py")).unwrap(); + assert_cmd_snapshot!( + Command::new(get_cargo_bin(BIN_NAME)).args(args).pass_stdin(unformatted), + @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + warning: `ruff format` is not yet stable, and subject to change in future versions. + "###); +}