diff --git a/Cargo.toml b/Cargo.toml index 84253d2fb01..d1115b54002 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,6 +50,7 @@ regex = "1.7" serde = { version = "1.0.160", features = ["derive"] } serde_json = "1.0" term = "1.1" +tempfile = "3.23.0" thiserror = "1.0.40" toml = "1.1" tracing = { version = "0.1.37", default-features = false, features = ["std"] } @@ -62,7 +63,6 @@ rustfmt-config_proc_macro = { version = "0.3", path = "config_proc_macro" } semver = "1.0.21" [dev-dependencies] -tempfile = "3.23.0" insta = { version = "1.48.0", features = ["filters"] } # Rustc dependencies are loaded from the sysroot, Cargo doesn't know about them. diff --git a/README.md b/README.md index 77e3335cf2c..067b7861d3e 100644 --- a/README.md +++ b/README.md @@ -85,6 +85,11 @@ The easiest way to run rustfmt against a project is with `cargo fmt`. `cargo fmt single-crate projects and [cargo workspaces](https://doc.rust-lang.org/book/ch14-03-cargo-workspaces.html). Please see `cargo fmt --help` for usage information. +Rustfmt can read command-line arguments from a response file using `@path`. The response file must +be UTF-8 encoded and contain one argument per line. Response files are expanded once, so an +argument beginning with `@` inside a response file is treated literally. On the command line, use +`@@` to pass a literal argument beginning with `@`. + You can specify the path to your own `rustfmt` binary for cargo to use by setting the`RUSTFMT` environment variable. This was added in v1.4.22, so you must have this version or newer to leverage this feature (`cargo fmt --version`) diff --git a/src/bin/main.rs b/src/bin/main.rs index af2dfdfac5b..34d4966b4b3 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -10,7 +10,7 @@ use tracing_subscriber::EnvFilter; use std::collections::HashMap; use std::env; -use std::fs::File; +use std::fs::{self, File}; use std::io::{self, Read, Write, stdout}; use std::path::{Path, PathBuf}; use std::str::FromStr; @@ -222,7 +222,8 @@ fn is_nightly() -> bool { // Returned i32 is an exit code fn execute(opts: &Options) -> Result { - let matches = opts.parse(env::args().skip(1))?; + let args = expand_response_files(env::args().skip(1))?; + let matches = opts.parse(args)?; let options = GetOptsOptions::from_matches(&matches)?; match determine_operation(&matches)? { @@ -275,6 +276,22 @@ fn execute(opts: &Options) -> Result { } } +fn expand_response_files(args: impl IntoIterator) -> Result> { + let mut expanded = Vec::new(); + for arg in args { + if let Some(arg) = arg.strip_prefix("@@") { + expanded.push(format!("@{arg}")); + } else if let Some(path) = arg.strip_prefix('@') { + let contents = fs::read_to_string(path) + .map_err(|e| format_err!("failed to load argument file `{path}`: {e}"))?; + expanded.extend(contents.lines().map(str::to_owned)); + } else { + expanded.push(arg); + } + } + Ok(expanded) +} + fn format_string(input: String, options: GetOptsOptions) -> Result { // try to read config from local directory let (mut config, _) = load_config(Some(Path::new(".")), Some(options.clone()))?; @@ -807,11 +824,41 @@ fn emit_mode_from_emit_str(emit_str: &str) -> Result { mod test { use super::*; use rustfmt_config_proc_macro::nightly_only_test; + use tempfile::NamedTempFile; fn get_config(path: Option<&Path>, options: Option) -> Config { load_config(path, options).unwrap().0 } + #[test] + fn response_files_are_expanded_once() { + let mut response_file = NamedTempFile::new().unwrap(); + writeln!( + response_file, + "--edition\r\n2021\r\n\r\n@nested-response-file" + ) + .unwrap(); + + let args = expand_response_files([format!("@{}", response_file.path().display())]).unwrap(); + assert_eq!(args, ["--edition", "2021", "", "@nested-response-file"]); + } + + #[test] + fn doubled_at_sign_escapes_response_file_expansion() { + let args = expand_response_files(["@@source.rs".to_owned()]).unwrap(); + assert_eq!(args, ["@source.rs"]); + } + + #[test] + fn missing_response_file_is_an_error() { + let error = expand_response_files(["@missing-response-file".to_owned()]).unwrap_err(); + assert!( + error + .to_string() + .starts_with("failed to load argument file `missing-response-file`:") + ); + } + #[nightly_only_test] #[test] fn flag_sets_style_edition_override_correctly() { diff --git a/src/cargo-fmt/main.rs b/src/cargo-fmt/main.rs index 6627ada03fc..e4c4f17be3a 100644 --- a/src/cargo-fmt/main.rs +++ b/src/cargo-fmt/main.rs @@ -6,6 +6,7 @@ use std::cmp::Ordering; use std::collections::{BTreeMap, BTreeSet}; use std::env; +use std::ffi::OsString; use std::fs; use std::hash::{Hash, Hasher}; use std::io::{self, Write}; @@ -15,6 +16,12 @@ use std::str; use cargo_metadata::Edition; use clap::{CommandFactory, Parser}; +use tempfile::NamedTempFile; + +#[cfg(windows)] +use std::ffi::OsStr; +#[cfg(windows)] +use std::os::windows::ffi::OsStrExt; #[path = "test/mod.rs"] #[cfg(test)] @@ -165,15 +172,17 @@ fn execute() -> i32 { } } -fn rustfmt_command() -> Command { - let rustfmt = match env::var_os("RUSTFMT") { +fn rustfmt_path() -> PathBuf { + match env::var_os("RUSTFMT") { Some(rustfmt) => PathBuf::from(rustfmt), None => env::current_exe() .expect("current executable path invalid") .with_file_name("rustfmt"), - }; + } +} - Command::new(rustfmt) +fn rustfmt_command() -> Command { + Command::new(rustfmt_path()) } fn convert_message_format_to_rustfmt_args( @@ -494,6 +503,101 @@ fn add_targets(target_paths: &[cargo_metadata::Target], targets: &mut BTreeSet Result, io::Error> { + let mut expanded = Vec::new(); + for arg in args { + let arg = arg.to_str().ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + "rustfmt response-file arguments must be valid UTF-8", + ) + })?; + if let Some(arg) = arg.strip_prefix("@@") { + expanded.push(OsString::from(format!("@{arg}"))); + } else if let Some(path) = arg.strip_prefix('@') { + let contents = fs::read_to_string(path).map_err(|e| { + io::Error::new( + e.kind(), + format!("failed to load argument file `{path}`: {e}"), + ) + })?; + expanded.extend(contents.lines().map(OsString::from)); + } else { + expanded.push(OsString::from(arg)); + } + } + Ok(expanded) +} + +fn write_response_file(args: &[OsString]) -> Result { + let mut response_file = NamedTempFile::new()?; + for arg in expand_response_file_args(args)? { + let arg = arg.to_str().ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + "rustfmt response-file arguments must be valid UTF-8", + ) + })?; + if arg.contains('\n') || arg.contains('\r') { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "rustfmt response-file arguments cannot contain newlines", + )); + } + writeln!(response_file, "{arg}")?; + } + response_file.flush()?; + Ok(response_file) +} + +#[cfg(windows)] +fn command_line_arg_len(arg: &OsStr) -> usize { + let arg = arg.encode_wide().collect::>(); + let quoted = arg.is_empty() || arg.iter().any(|&c| c == b' ' as u16 || c == b'\t' as u16); + let mut len = arg.len().saturating_add(usize::from(quoted) * 2); + let mut backslashes = 0usize; + + for &c in &arg { + if c == b'\\' as u16 { + backslashes += 1; + } else { + if c == b'"' as u16 { + len = len.saturating_add(backslashes).saturating_add(1); + } + backslashes = 0; + } + } + if quoted { + len = len.saturating_add(backslashes); + } + + // Account for the separator before this argument. + len.saturating_add(1) +} + +#[cfg(windows)] +fn command_line_program_len(program: &Path) -> usize { + // Rust always surrounds argv[0] with quotes on Windows. + program.as_os_str().encode_wide().count().saturating_add(2) +} + +#[cfg(windows)] +fn should_use_response_file(program: &Path, args: &[OsString]) -> bool { + const WINDOWS_COMMAND_LINE_LIMIT: usize = 32_767; + + let command_line_len = command_line_program_len(program).saturating_add( + args.iter() + .map(|arg| command_line_arg_len(arg)) + .sum::(), + ); + command_line_len >= WINDOWS_COMMAND_LINE_LIMIT +} + +#[cfg(not(windows))] +fn should_use_response_file(_program: &Path, _args: &[OsString]) -> bool { + false +} + fn run_rustfmt( targets: &BTreeSet, fmt_args: &[String], @@ -527,19 +631,38 @@ fn run_rustfmt( println!(); } - let mut command = rustfmt_command() - .stdout(stdout) - .args(files) - .args(["--edition", edition.as_str()]) - .args(fmt_args) - .spawn() - .map_err(|e| match e.kind() { - io::ErrorKind::NotFound => io::Error::new( - io::ErrorKind::Other, - "Could not run rustfmt, please make sure it is in your PATH.", - ), - _ => e, - })?; + let rustfmt = rustfmt_path(); + let mut args = files + .iter() + .map(|file| file.as_os_str().to_owned()) + .collect::>(); + args.extend([ + OsString::from("--edition"), + OsString::from(edition.as_str()), + ]); + args.extend(fmt_args.iter().map(OsString::from)); + + let response_file = should_use_response_file(&rustfmt, &args) + .then(|| write_response_file(&args)) + .transpose()?; + + let mut command = Command::new(rustfmt); + command.stdout(stdout); + if let Some(response_file) = response_file.as_ref() { + let mut response_arg = OsString::from("@"); + response_arg.push(response_file.path()); + command.arg(response_arg); + } else { + command.args(&args); + } + + let mut command = command.spawn().map_err(|e| match e.kind() { + io::ErrorKind::NotFound => io::Error::new( + io::ErrorKind::Other, + "Could not run rustfmt, please make sure it is in your PATH.", + ), + _ => e, + })?; status.push(command.wait()?); } diff --git a/src/cargo-fmt/test/mod.rs b/src/cargo-fmt/test/mod.rs index 255e0d679d4..952f0a935a4 100644 --- a/src/cargo-fmt/test/mod.rs +++ b/src/cargo-fmt/test/mod.rs @@ -104,6 +104,35 @@ fn multiple_packages_grouped() { assert_eq!(4, o.packages.len()); } +#[cfg(windows)] +#[test] +fn windows_command_line_length_matches_rust_quoting() { + use std::ffi::OsStr; + + assert_eq!(command_line_arg_len(OsStr::new("plain")), 6); + assert_eq!(command_line_arg_len(OsStr::new("")), 3); + assert_eq!(command_line_arg_len(OsStr::new("has space")), 12); + assert_eq!(command_line_arg_len(OsStr::new(r#"a\"b"#)), 7); + assert_eq!(command_line_arg_len(OsStr::new(r#"a \"b"#)), 10); + assert_eq!(command_line_arg_len(OsStr::new(r#"a \"#)), 7); + assert_eq!(command_line_program_len(Path::new("rustfmt")), 9); +} + +#[test] +fn response_file_arguments_are_expanded_once() { + use std::io::Write; + + let mut response_file = tempfile::NamedTempFile::new().unwrap(); + writeln!(response_file, "--check\r\n@nested-response-file").unwrap(); + + let args = expand_response_file_args(&[ + OsString::from(format!("@{}", response_file.path().display())), + OsString::from("@@literal"), + ]) + .unwrap(); + assert_eq!(args, ["--check", "@nested-response-file", "@literal"]); +} + #[test] fn empty_packages_1() { assert!( diff --git a/tests/cargo-fmt/main.rs b/tests/cargo-fmt/main.rs index 63cc12521a8..85c7c46ac8c 100644 --- a/tests/cargo-fmt/main.rs +++ b/tests/cargo-fmt/main.rs @@ -1,6 +1,8 @@ // Integration tests for cargo-fmt. use std::env; +#[cfg(windows)] +use std::fs; use std::path::Path; use std::process::Command; @@ -39,6 +41,41 @@ fn cargo_fmt(args: &[&str]) -> (String, String) { } } +#[cfg(windows)] +#[rustfmt_only_ci_test] +#[test] +fn cargo_fmt_uses_response_file_for_long_command_lines() { + let temp_dir = tempfile::tempdir().unwrap(); + let source_dir = temp_dir.path().join("src"); + fs::create_dir(&source_dir).unwrap(); + + let mut manifest = String::from( + "[package]\nname = \"response-file-test\"\nversion = \"0.1.0\"\n\ + edition = \"2021\"\nautobins = false\n", + ); + for index in 0..500 { + let file_name = format!("response_file_target_with_a_long_name_{index:04}.rs"); + fs::write(source_dir.join(&file_name), "fn main() {}\n").unwrap(); + manifest.push_str(&format!( + "\n[[bin]]\nname = \"response-file-target-{index:04}\"\npath = \"src/{file_name}\"\n" + )); + } + let manifest_path = temp_dir.path().join("Cargo.toml"); + fs::write(&manifest_path, manifest).unwrap(); + let response_file = temp_dir.path().join("rustfmt.args"); + fs::write(&response_file, "--check\n").unwrap(); + let response_arg = format!("@{}", response_file.display()); + + let (stdout, stderr) = cargo_fmt(&[ + "--manifest-path", + manifest_path.to_str().unwrap(), + "--", + &response_arg, + ]); + assert_eq!(stdout, ""); + assert_eq!(stderr, ""); +} + macro_rules! assert_that { ($args:expr, $check:ident $check_args:tt) => { let (stdout, stderr) = cargo_fmt($args); diff --git a/tests/rustfmt/main.rs b/tests/rustfmt/main.rs index 9a46544b367..8d760107ca6 100644 --- a/tests/rustfmt/main.rs +++ b/tests/rustfmt/main.rs @@ -1,7 +1,8 @@ //! Integration tests for rustfmt. use std::env; -use std::fs::{File, remove_file}; +use std::fs::{self, File, remove_file}; +use std::io::Write; use std::path::Path; use std::process::Command; @@ -41,6 +42,49 @@ fn rustfmt(args: &[&str]) -> (String, String) { rustfmt_with_extra(args, None, &[]) } +#[test] +fn response_file() { + let temp_dir = tempfile::tempdir().unwrap(); + let source = temp_dir.path().join("response file.rs"); + fs::write(&source, "fn main () {}\n").unwrap(); + + let mut response_file = tempfile::NamedTempFile::new().unwrap(); + write!( + response_file, + "--quiet\r\n--emit\r\nstdout\r\n--edition\r\n2021\r\n{}\r\n", + source.display() + ) + .unwrap(); + + let response_arg = format!("@{}", response_file.path().display()); + let (stdout, stderr) = rustfmt(&[&response_arg]); + assert_eq!(stdout, "fn main() {}\n"); + assert_eq!(stderr, ""); +} + +#[test] +fn missing_response_file() { + let (stdout, stderr) = rustfmt(&["@missing-rustfmt-response-file"]); + assert!( + stdout.contains("failed to load argument file `missing-rustfmt-response-file`:") + || stderr.contains("failed to load argument file `missing-rustfmt-response-file`:") + ); +} + +#[test] +fn doubled_at_sign_formats_a_literal_at_path() { + let temp_dir = tempfile::tempdir().unwrap(); + fs::write(temp_dir.path().join("@source.rs"), "fn main () {}\n").unwrap(); + + let (stdout, stderr) = rustfmt_with_extra( + &["--quiet", "--emit", "stdout", "@@source.rs"], + temp_dir.path().to_str(), + &[], + ); + assert_eq!(stdout, "fn main() {}\n"); + assert_eq!(stderr, ""); +} + macro_rules! assert_that { ($args:expr, $($check:ident $check_args:tt)&&+) => { let (stdout, stderr) = rustfmt($args);