diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 4b975fa1d59..eb6c1d45ac7 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -1,3 +1,4 @@ +use std::borrow::Cow; use std::env; use std::fmt::{Debug, Display}; use std::io::{self, Write}; @@ -181,19 +182,11 @@ pub struct CheckDiffRunners { } pub trait CodeFormatter { - fn format_code>( - &self, - code: &str, - config: Option<&[T]>, - ) -> Result; - - fn format_code_from_path, P: AsRef>( - &self, - path: P, - config: Option<&[T]>, - ) -> Result { + fn format_code(&self, code: &str) -> Result; + + fn format_code_from_path>(&self, path: P) -> Result { let code = std::fs::read_to_string(path)?; - self.format_code(&code, config) + self.format_code(&code) } } @@ -202,6 +195,7 @@ pub struct RustfmtRunner { binary_path: PathBuf, edition: Edition, style_edition: StyleEdition, + config: Cow<'static, str>, } impl CheckDiffRunners { @@ -219,17 +213,9 @@ where S: CodeFormatter, { /// Creates a diff generated by running the source and feature binaries on the same file path - pub fn create_diff, P: AsRef>( - &self, - path: P, - additional_configs: Option<&[T]>, - ) -> Result { - let src_format = self - .src_runner - .format_code_from_path(&path, additional_configs); - let feature_format = self - .feature_runner - .format_code_from_path(&path, additional_configs); + pub fn create_diff>(&self, path: P) -> Result { + let src_format = self.src_runner.format_code_from_path(&path); + let feature_format = self.feature_runner.format_code_from_path(&path); match (src_format, feature_format) { (Ok(s), Ok(f)) => Ok(Diff { @@ -272,6 +258,10 @@ impl RustfmtRunner { Ok(buffer_into_utf8_lossy(command.stdout)) } + + fn command_line_configs(&self) -> &str { + &self.config + } } /// Convert a buffer of u8 into a String. @@ -303,12 +293,7 @@ impl CodeFormatter for RustfmtRunner { // rustfmt.toml `ignore` list. For example, this helps us skip files in r-l/rust that have // been explicitly skipped because trying to format them causes rustfmt to hang or rustfmt. // doesn't do a good job at formatting those files. - fn format_code_from_path, P: AsRef>( - &self, - path: P, - config: Option<&[T]>, - ) -> Result { - let config = create_config_arg(config); + fn format_code_from_path>(&self, path: P) -> Result { let command = Command::new(&self.binary_path) .env( dynamic_library_path_env_var_name(), @@ -322,7 +307,7 @@ impl CodeFormatter for RustfmtRunner { "--unstable-features", "--skip-children", "--emit=stdout", - config.as_str(), + self.command_line_configs(), ]) .arg(path.as_ref()) .stdin(Stdio::piped()) @@ -351,12 +336,7 @@ impl CodeFormatter for RustfmtRunner { // code: Code to run the binary on // config: Any additional configuration options to pass to rustfmt // - fn format_code>( - &self, - code: &str, - config: Option<&[T]>, - ) -> Result { - let config = create_config_arg(config); + fn format_code(&self, code: &str) -> Result { let mut command = Command::new(&self.binary_path) .env( dynamic_library_path_env_var_name(), @@ -370,7 +350,7 @@ impl CodeFormatter for RustfmtRunner { "--unstable-features", "--skip-children", "--emit=stdout", - config.as_str(), + self.command_line_configs(), ]) .stdin(Stdio::piped()) .stdout(Stdio::piped()) @@ -394,25 +374,42 @@ impl CodeFormatter for RustfmtRunner { } } +const DEFAULT_CONFIG: &str = "--config=error_on_line_overflow=false,error_on_unformatted=false"; + /// Creates a configuration in the following form: /// =, =, ... -fn create_config_arg>(config: Option<&[T]>) -> String { - let config_arg: String = match config { - Some(configs) => { - let mut result = String::new(); - for arg in configs.iter() { - result.push(','); - result.push_str(arg.as_ref()); - } - result - } - None => String::new(), +fn create_config_arg>(configs: Option<&[T]>) -> Cow<'static, str> { + let Some(configs) = configs else { + return Cow::Borrowed(DEFAULT_CONFIG); }; - let config = format!( - "--config=error_on_line_overflow=false,error_on_unformatted=false{}", - config_arg.as_str() - ); - config + + let mut configs_len = 0; + let mut num_configs = 0; + + // Determine how many non empty configs we've got + for c in configs.iter().map(AsRef::as_ref) { + if c.is_empty() { + continue; + } + + configs_len += c.len(); + num_configs += 1; + } + + if num_configs == 0 { + // All configs were empty so return the default. + return Cow::Borrowed(DEFAULT_CONFIG); + } + + // We need capacity for the default configs len + one ',' per config + total config element len + let mut result = String::with_capacity(DEFAULT_CONFIG.len() + num_configs + configs_len); + + for c in configs.iter().map(AsRef::as_ref) { + result.push(','); + result.push_str(c); + } + + Cow::Owned(result) } /// Clone a git repository /// @@ -537,11 +534,12 @@ pub fn get_cargo_version() -> Result { /// Obtains the ld_lib path and then builds rustfmt from source /// If that operation succeeds, the source is then copied to the output path specified -pub fn build_rustfmt_from_src( +pub fn build_rustfmt_from_src>( binary_path: PathBuf, dir: &Path, edition: Edition, style_edition: StyleEdition, + config: Option<&[T]>, ) -> Result { // Because we're building standalone binaries we need to set the dynamic library path // so each rustfmt binary can find it's runtime dependencies. @@ -565,6 +563,7 @@ pub fn build_rustfmt_from_src( binary_path, edition, style_edition, + config: create_config_arg(config), }) } @@ -572,13 +571,14 @@ pub fn build_rustfmt_from_src( // One for the current main branch, and another for the feature branch // Parameters: // dest: Directory where rustfmt will be cloned -pub fn compile_rustfmt( +pub fn compile_rustfmt>( dest: &Path, remote_repo_url: String, feature_branch: String, edition: Edition, style_edition: StyleEdition, commit_hash: Option, + config: Option<&[T]>, ) -> Result, CheckDiffError> { const RUSTFMT_REPO: &str = "https://github.com/rust-lang/rustfmt.git"; @@ -589,16 +589,26 @@ pub fn compile_rustfmt( let cargo_version = get_cargo_version()?; info!("Compiling with {}", cargo_version); - let src_runner = - build_rustfmt_from_src(dest.join("src_rustfmt"), dest, edition, style_edition)?; + let src_runner = build_rustfmt_from_src( + dest.join("src_rustfmt"), + dest, + edition, + style_edition, + config, + )?; let should_detach = commit_hash.is_some(); git_switch( commit_hash.as_ref().unwrap_or(&feature_branch), should_detach, )?; - let feature_runner = - build_rustfmt_from_src(dest.join("feature_rustfmt"), dest, edition, style_edition)?; + let feature_runner = build_rustfmt_from_src( + dest.join("feature_rustfmt"), + dest, + edition, + style_edition, + config, + )?; info!("RUSFMT_BIN {}", src_runner.get_binary_version()?); let dynamic_library_path_env_var = dynamic_library_path_env_var_name(); info!( @@ -633,8 +643,7 @@ pub fn search_for_rs_files(repo: &Path) -> impl Iterator { /// Calculates the number of errors when running the compiled binary and the feature binary on the /// repo specified with the specific configs. -pub fn check_diff, P: AsRef>( - config: Option<&[T]>, +pub fn check_diff>( runners: &CheckDiffRunners, repo: P, repo_url: &str, @@ -652,7 +661,7 @@ pub fn check_diff, P: AsRef>( relative_path.display() ); - match runners.create_diff(file.as_path(), config) { + match runners.create_diff(file.as_path()) { Ok(diff) => { if !diff.is_empty() { error!( diff --git a/check_diff/src/main.rs b/check_diff/src/main.rs index 50bda185398..b36ab18221e 100644 --- a/check_diff/src/main.rs +++ b/check_diff/src/main.rs @@ -77,6 +77,7 @@ fn main() -> Result { args.edition, args.style_edition, args.commit_hash, + args.rustfmt_config.as_deref(), ); let check_diff_runners = match compilation_result { @@ -88,13 +89,11 @@ fn main() -> Result { }; let errors = Arc::new(AtomicUsize::new(0)); - let rustfmt_config = Arc::new(args.rustfmt_config); let check_diff_runners = Arc::new(check_diff_runners); thread::scope(|s| { for url in REPOS { let errors = Arc::clone(&errors); - let rustfmt_config = Arc::clone(&rustfmt_config); let check_diff_runners = Arc::clone(&check_diff_runners); s.spawn(move || { let repo_name = get_repo_name(url); @@ -115,12 +114,7 @@ fn main() -> Result { return; }; - let error_count = check_diff( - rustfmt_config.as_deref(), - &check_diff_runners, - tmp_dir.path(), - url, - ); + let error_count = check_diff(&check_diff_runners, tmp_dir.path(), url); errors.fetch_add(error_count as usize, Ordering::Relaxed); }); diff --git a/check_diff/tests/check_diff.rs b/check_diff/tests/check_diff.rs index ddd39c0dfc8..6ec02b1f606 100644 --- a/check_diff/tests/check_diff.rs +++ b/check_diff/tests/check_diff.rs @@ -8,11 +8,7 @@ use tempfile::Builder; struct DoNothingFormatter; impl CodeFormatter for DoNothingFormatter { - fn format_code>( - &self, - _code: &str, - _config: Option<&[T]>, - ) -> Result { + fn format_code(&self, _code: &str) -> Result { Ok(String::new()) } } @@ -21,11 +17,7 @@ impl CodeFormatter for DoNothingFormatter { struct AddWhiteSpaceFormatter; impl CodeFormatter for AddWhiteSpaceFormatter { - fn format_code>( - &self, - code: &str, - _config: Option<&[T]>, - ) -> Result { + fn format_code(&self, code: &str) -> Result { let result = code.to_string() + " "; Ok(result) } @@ -80,7 +72,7 @@ fn check_diff_test_no_formatting_difference() -> Result<(), CheckDiffError> { let _tmp_file = File::create(file_path)?; let repo_url = "https://github.com/rust-lang/rustfmt.git"; - let errors = check_diff(None::<&[&str]>, &runners, dir.path(), repo_url); + let errors = check_diff(&runners, dir.path(), repo_url); assert_eq!(errors, 0); Ok(()) } @@ -93,7 +85,7 @@ fn check_diff_test_formatting_difference() -> Result<(), CheckDiffError> { let _tmp_file = File::create(file_path)?; let repo_url = "https://github.com/rust-lang/rustfmt.git"; - let errors = check_diff(None::<&[&str]>, &runners, dir.path(), repo_url); + let errors = check_diff(&runners, dir.path(), repo_url); assert_ne!(errors, 0); Ok(()) }