From c2413dcd2c991d3380b662438efe7e195e7eb85b Mon Sep 17 00:00:00 2001 From: konsti Date: Sun, 27 Aug 2023 21:12:18 +0200 Subject: [PATCH] Add prototype of `ruff format` for projects (#6871) **Summary** Add recursive formatting based on `ruff check` file discovery for `ruff format`, as a prototype for the formatter alpha. This allows e.g. `format ../projects/django/`. It's still lacking support for any settings except line length. Note just like the existing `ruff format` this will become part of the production build, i.e. you'll be able to use it - hidden by default and with a prominent warning - with `ruff format .` after the next release. Error handling works in my manual tests (the colors do also work): ``` $ target/debug/ruff format scripts/ warning: `ruff format` is a work-in-progress, subject to change at any time, and intended for internal use only. ``` (the above changes `add_rule.py` where we have the wrong bin op breaking) ``` $ target/debug/ruff format ../projects/django/ warning: `ruff format` is a work-in-progress, subject to change at any time, and intended for internal use only. Failed to format /home/konsti/projects/django/tests/test_runner_apps/tagged/tests_syntax_error.py: source contains syntax errors: ParseError { error: UnrecognizedToken(Name { name: "syntax_error" }, None), offset: 131, source_path: "" } ``` ``` $ target/debug/ruff format a warning: `ruff format` is a work-in-progress, subject to change at any time, and intended for internal use only. Failed to read /home/konsti/ruff/a/d.py: Permission denied (os error 13) ``` **Test Plan** Missing! I'm not sure if it's worth building tests at this stage or how they should look like. --- Cargo.lock | 3 + crates/ruff_cli/Cargo.toml | 3 + crates/ruff_cli/src/commands/format.rs | 107 ++++++++++++++++++++++++ crates/ruff_cli/src/commands/mod.rs | 1 + crates/ruff_cli/src/lib.rs | 31 +++---- crates/ruff_python_formatter/README.md | 9 ++ crates/ruff_python_formatter/src/lib.rs | 4 +- 7 files changed, 142 insertions(+), 16 deletions(-) create mode 100644 crates/ruff_cli/src/commands/format.rs diff --git a/Cargo.lock b/Cargo.lock index 28b9f9beb9..332400765f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2219,6 +2219,7 @@ dependencies = [ "ruff", "ruff_cache", "ruff_diagnostics", + "ruff_formatter", "ruff_macros", "ruff_python_ast", "ruff_python_formatter", @@ -2233,7 +2234,9 @@ dependencies = [ "similar", "strum", "tempfile", + "thiserror", "tikv-jemallocator", + "tracing", "ureq", "walkdir", "wild", diff --git a/crates/ruff_cli/Cargo.toml b/crates/ruff_cli/Cargo.toml index 6f17b684c7..a9796d72f2 100644 --- a/crates/ruff_cli/Cargo.toml +++ b/crates/ruff_cli/Cargo.toml @@ -24,6 +24,7 @@ doc = false ruff = { path = "../ruff", features = ["clap"] } ruff_cache = { path = "../ruff_cache" } ruff_diagnostics = { path = "../ruff_diagnostics" } +ruff_formatter = { path = "../ruff_formatter" } ruff_macros = { path = "../ruff_macros" } ruff_python_ast = { path = "../ruff_python_ast" } ruff_python_formatter = { path = "../ruff_python_formatter" } @@ -59,6 +60,8 @@ serde_json = { workspace = true } shellexpand = { workspace = true } similar = { workspace = true } strum = { workspace = true, features = [] } +thiserror = { workspace = true } +tracing = { workspace = true } walkdir = { version = "2.3.2" } wild = { version = "2" } diff --git a/crates/ruff_cli/src/commands/format.rs b/crates/ruff_cli/src/commands/format.rs new file mode 100644 index 0000000000..dc63c795a8 --- /dev/null +++ b/crates/ruff_cli/src/commands/format.rs @@ -0,0 +1,107 @@ +use std::io; +use std::path::{Path, PathBuf}; + +use anyhow::Result; +use colored::Colorize; +use rayon::iter::{IntoParallelIterator, ParallelIterator}; +use thiserror::Error; +use tracing::{span, Level}; + +use ruff::resolver::python_files_in_path; +use ruff::warn_user_once; +use ruff_formatter::LineWidth; +use ruff_python_ast::PySourceType; +use ruff_python_formatter::{format_module, FormatModuleError, PyFormatOptions}; + +use crate::args::{Arguments, Overrides}; +use crate::resolve::resolve; +use crate::ExitStatus; + +/// Format a set of files, and return the exit status. +pub(crate) fn format(cli: &Arguments, overrides: &Overrides) -> Result { + let pyproject_config = resolve( + cli.isolated, + cli.config.as_deref(), + overrides, + cli.stdin_filename.as_deref(), + )?; + let (paths, resolver) = python_files_in_path(&cli.files, &pyproject_config, overrides)?; + + if paths.is_empty() { + warn_user_once!("No Python files found under the given path(s)"); + return Ok(ExitStatus::Success); + } + + let all_success = paths + .into_par_iter() + .map(|dir_entry| { + let dir_entry = dir_entry?; + let path = dir_entry.path(); + let source_type = PySourceType::from(path); + if !(source_type.is_python() || source_type.is_stub()) + || path + .extension() + .is_some_and(|extension| extension == "toml") + { + return Ok(()); + } + + let line_length = resolver.resolve(path, &pyproject_config).line_length; + // TODO(konstin): Unify `LineWidth` and `LineLength` + let line_width = LineWidth::try_from( + u16::try_from(line_length.get()).expect("Line shouldn't be larger than 2**16"), + ) + .expect("Configured line length is too large for the formatter"); + let options = PyFormatOptions::from_extension(path).with_line_width(line_width); + + format_path(path, options) + }) + .map(|result| { + match result { + Ok(()) => true, + Err(err) => { + // The inner errors are all flat, i.e., none of them has a source. + #[allow(clippy::print_stderr)] + { + eprintln!("{}", err.to_string().red().bold()); + } + false + } + } + }) + .all(|success| success); + + if all_success { + Ok(ExitStatus::Success) + } else { + Ok(ExitStatus::Error) + } +} + +/// An error that can occur while formatting a set of files. +#[derive(Error, Debug)] +enum FormatterIterationError { + #[error("Failed to traverse the inputs paths: {0}")] + Ignore(#[from] ignore::Error), + #[error("Failed to read {0}: {1}")] + Read(PathBuf, io::Error), + #[error("Failed to write {0}: {1}")] + Write(PathBuf, io::Error), + #[error("Failed to format {0}: {1}")] + FormatModule(PathBuf, FormatModuleError), +} + +#[tracing::instrument(skip_all, fields(path = %path.display()))] +fn format_path(path: &Path, options: PyFormatOptions) -> Result<(), FormatterIterationError> { + let unformatted = std::fs::read_to_string(path) + .map_err(|err| FormatterIterationError::Read(path.to_path_buf(), err))?; + let formatted = { + let span = span!(Level::TRACE, "format_path_without_io", path = %path.display()); + let _enter = span.enter(); + format_module(&unformatted, options) + .map_err(|err| FormatterIterationError::FormatModule(path.to_path_buf(), err))? + }; + std::fs::write(path, formatted.as_code().as_bytes()) + .map_err(|err| FormatterIterationError::Write(path.to_path_buf(), err))?; + Ok(()) +} diff --git a/crates/ruff_cli/src/commands/mod.rs b/crates/ruff_cli/src/commands/mod.rs index e989f433e0..30c6fabf8a 100644 --- a/crates/ruff_cli/src/commands/mod.rs +++ b/crates/ruff_cli/src/commands/mod.rs @@ -1,6 +1,7 @@ pub(crate) mod add_noqa; pub(crate) mod clean; pub(crate) mod config; +pub(crate) mod format; pub(crate) mod linter; pub(crate) mod rule; pub(crate) mod run; diff --git a/crates/ruff_cli/src/lib.rs b/crates/ruff_cli/src/lib.rs index a6d8a6f378..0f104b3f28 100644 --- a/crates/ruff_cli/src/lib.rs +++ b/crates/ruff_cli/src/lib.rs @@ -4,8 +4,9 @@ use std::path::{Path, PathBuf}; use std::process::ExitCode; use std::sync::mpsc::channel; -use anyhow::{Context, Result}; +use anyhow::Result; use clap::CommandFactory; +use clap::FromArgMatches; use log::warn; use notify::{recommended_watcher, RecursiveMode, Watcher}; @@ -154,32 +155,34 @@ quoting the executed command, along with the relevant file contents and `pyproje Ok(ExitStatus::Success) } -fn format(files: &[PathBuf]) -> Result { +fn format(paths: &[PathBuf]) -> Result { warn_user_once!( - "`ruff format` is a work-in-progress, subject to change at any time, and intended for \ - internal use only." + "`ruff format` is a work-in-progress, subject to change at any time, and intended only for \ + experimentation." ); - match &files { + match &paths { // Check if we should read from stdin [path] if path == Path::new("-") => { let unformatted = read_from_stdin()?; let options = PyFormatOptions::from_extension(Path::new("stdin.py")); let formatted = format_module(&unformatted, options)?; stdout().lock().write_all(formatted.as_code().as_bytes())?; + Ok(ExitStatus::Success) } _ => { - for file in files { - let unformatted = std::fs::read_to_string(file) - .with_context(|| format!("Could not read {}: ", file.display()))?; - let options = PyFormatOptions::from_extension(file); - let formatted = format_module(&unformatted, options)?; - std::fs::write(file, formatted.as_code().as_bytes()) - .with_context(|| format!("Could not write to {}, exiting", file.display()))?; - } + // We want to use the same as `ruff check `, but we don't actually want to allow + // any of the linter settings. + // TODO(@konstin): Refactor this to allow getting config and resolver without going + // though clap. + let args_matches = CheckArgs::command() + .no_binary_name(true) + .get_matches_from(paths); + let check_args: CheckArgs = CheckArgs::from_arg_matches(&args_matches)?; + let (cli, overrides) = check_args.partition(); + commands::format::format(&cli, &overrides) } } - Ok(ExitStatus::Success) } pub fn check(args: CheckArgs, log_level: LogLevel) -> Result { diff --git a/crates/ruff_python_formatter/README.md b/crates/ruff_python_formatter/README.md index 11f306bf62..42f4fd7693 100644 --- a/crates/ruff_python_formatter/README.md +++ b/crates/ruff_python_formatter/README.md @@ -3,6 +3,15 @@ The goal of our formatter is to be compatible with Black except for rare edge cases (mostly involving comment placement). +You can try an experimental version of the formatter on your project with: + +```shell +cargo run --bin ruff -- format path/to/your/project +``` + +Note that currently the only supported option is `line-length` and that both the CLI and the +formatting are a work-in-progress and will change before the stable release. + ## Dev tools **Testing your changes** You can use the `ruff_python_formatter` binary to format individual files diff --git a/crates/ruff_python_formatter/src/lib.rs b/crates/ruff_python_formatter/src/lib.rs index a6253f1088..8c261fe3f0 100644 --- a/crates/ruff_python_formatter/src/lib.rs +++ b/crates/ruff_python_formatter/src/lib.rs @@ -99,9 +99,9 @@ where #[derive(Error, Debug)] pub enum FormatModuleError { - #[error("source contains syntax errors (lexer error): {0:?}")] + #[error("source contains syntax errors: {0:?}")] LexError(LexicalError), - #[error("source contains syntax errors (parser error): {0:?}")] + #[error("source contains syntax errors: {0:?}")] ParseError(ParseError), #[error(transparent)] FormatError(#[from] FormatError),