diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 239cab77984..9d84091a4df 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -1,13 +1,97 @@ -use diffy; use std::env; use std::fmt::{Debug, Display}; use std::io::{self, Write}; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; -use std::str::Utf8Error; -use tracing::info; +use std::str::{FromStr, Utf8Error}; +use tracing::{debug, error, info, trace}; use walkdir::WalkDir; +#[derive(Debug, Clone, Copy)] +pub enum Edition { + /// rust edition 2015 + Edition2015, + /// rust edition 2018 + Edition2018, + /// rust edition 2021 + Edition2021, + /// rust edition 2024 + Edition2024, +} + +impl Edition { + fn as_str(&self) -> &str { + match self { + Edition::Edition2015 => "2015", + Edition::Edition2018 => "2018", + Edition::Edition2021 => "2021", + Edition::Edition2024 => "2024", + } + } +} + +impl FromStr for Edition { + type Err = String; + + fn from_str(s: &str) -> Result { + match s { + "2015" => Ok(Edition::Edition2015), + "2018" => Ok(Edition::Edition2018), + "2021" => Ok(Edition::Edition2021), + "2024" => Ok(Edition::Edition2024), + _ => Err(format!("Invalid rust edition {s}")), + } + } +} + +#[derive(Debug, Clone, Copy)] +pub enum StyleEdition { + // rustfmt style_edition 2021. Also equivaluent to 2015 and 2018. + Edition2021, + // rustfmt style_edition 2024 + Edition2024, +} + +impl StyleEdition { + fn as_str(&self) -> &str { + match self { + StyleEdition::Edition2021 => "2021", + StyleEdition::Edition2024 => "2024", + } + } +} + +impl FromStr for StyleEdition { + type Err = String; + + fn from_str(s: &str) -> Result { + match s { + "2015" => Ok(StyleEdition::Edition2021), + "2018" => Ok(StyleEdition::Edition2021), + "2021" => Ok(StyleEdition::Edition2021), + "2024" => Ok(StyleEdition::Edition2024), + _ => Err(format!("Invalid rustfmt style_edition {s}")), + } + } +} + +#[derive(Debug)] +pub struct FormatCodeError { + error: String, +} + +#[derive(Debug)] +pub enum CreateDiffError { + /// Only the rustfmt binary compiled from the `main` branch failed + MainRustfmtFailed(String), + /// Only the rustfmt binary compiled form the `feature` branch failed + FeatureRustfmtFailed(String), + /// Both rustfmt binaries failed to format the input + BothRustfmtFailed { src: String, feature: String }, + /// Some Error unrelated to formatting + GeneralError, +} + #[derive(Debug)] pub enum CheckDiffError { /// Git related errors @@ -23,6 +107,10 @@ pub enum CheckDiffError { /// Error when obtaining cargo version FailedCargoVersion(&'static str), IO(std::io::Error), + /// Error formatting code + FormatCodeError(FormatCodeError), + /// Error generating diff + CreateDiffError(CreateDiffError), } impl From for CheckDiffError { @@ -83,16 +171,27 @@ pub struct CheckDiffRunners { } pub trait CodeFormatter { - fn format_code<'a>( + fn format_code_from_path>( &self, - code: &'a str, - config: &Option>, + path: &Path, + config: Option<&[T]>, + ) -> Result { + let code = std::fs::read_to_string(path)?; + self.format_code(&code, config) + } + + fn format_code>( + &self, + code: &str, + config: Option<&[T]>, ) -> Result; } pub struct RustfmtRunner { ld_library_path: String, binary_path: PathBuf, + edition: Edition, + style_edition: StyleEdition, } impl CheckDiffRunners { @@ -110,25 +209,60 @@ where S: CodeFormatter, { /// Creates a diff generated by running the source and feature binaries on the same file path - pub fn create_diff( + pub fn create_diff>( &self, path: &Path, - additional_configs: &Option>, + additional_configs: Option<&[T]>, ) -> Result { - let code = std::fs::read_to_string(path)?; - let src_format = self.src_runner.format_code(&code, additional_configs)?; - let feature_format = self.feature_runner.format_code(&code, additional_configs)?; - Ok(Diff { - src_format, - feature_format, - }) + 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); + + match (src_format, feature_format) { + (Ok(s), Ok(f)) => Ok(Diff { + src_format: s, + feature_format: f, + }), + (Err(CheckDiffError::FormatCodeError(FormatCodeError { error })), Ok(_)) => { + // src formatting failed. + Err(CheckDiffError::CreateDiffError( + CreateDiffError::MainRustfmtFailed(error), + )) + } + (Ok(_), Err(CheckDiffError::FormatCodeError(FormatCodeError { error }))) => { + // feature formatting failed + Err(CheckDiffError::CreateDiffError( + CreateDiffError::FeatureRustfmtFailed(error), + )) + } + ( + Err(CheckDiffError::FormatCodeError(FormatCodeError { error: src_error })), + Err(CheckDiffError::FormatCodeError(FormatCodeError { + error: feature_error, + })), + ) => { + // Both src formatting and feature formatting failed + Err(CheckDiffError::CreateDiffError( + CreateDiffError::BothRustfmtFailed { + src: src_error, + feature: feature_error, + }, + )) + } + _ => Err(CheckDiffError::CreateDiffError( + CreateDiffError::GeneralError, + )), + } } } impl RustfmtRunner { fn get_binary_version(&self) -> Result { let Ok(command) = Command::new(&self.binary_path) - .env("LD_LIBRARY_PATH", &self.ld_library_path) + .env(dynamic_library_path_env_var_name(), &self.ld_library_path) .args(["--version"]) .output() else { @@ -138,26 +272,86 @@ impl RustfmtRunner { }; let binary_version = std::str::from_utf8(&command.stdout)?.trim(); - return Ok(binary_version.to_string()); + Ok(binary_version.to_string()) + } +} + +// This is the same logic that cargo uses when setting thewse env vars +/// Returns the name of the environment variable used to search for dynamic libraries. +fn dynamic_library_path_env_var_name() -> &'static str { + if cfg!(windows) { + "PATH" + } else if cfg!(target_os = "macos") { + "DYLD_FALLBACK_LIBRARY_PATH" + } else if cfg!(target_os = "aix") { + "LIBPATH" + } else { + "LD_LIBRARY_PATH" } } impl CodeFormatter for RustfmtRunner { + fn format_code_from_path>( + &self, + path: &Path, + config: Option<&[T]>, + ) -> Result { + let config = create_config_arg(config); + let command = Command::new(&self.binary_path) + .env(dynamic_library_path_env_var_name(), &self.ld_library_path) + .args([ + "--edition", + self.edition.as_str(), + "--style-edition", + self.style_edition.as_str(), + "--unstable-features", + "--skip-children", + "--emit=stdout", + config.as_str(), + ]) + .arg(path) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn()?; + + let output = command.wait_with_output()?; + let errors = std::str::from_utf8(&output.stderr)?.to_string(); + match output.status.code() { + Some(0) => Ok(std::str::from_utf8(&output.stdout)?.to_string()), + Some(_) | None => { + let formatted_code = std::str::from_utf8(&output.stdout)?.to_string(); + if !formatted_code.is_empty() { + Ok(formatted_code) + } else { + debug!("{} {errors}", path.display()); + Err(CheckDiffError::FormatCodeError(FormatCodeError { + error: errors, + })) + } + } + } + } + // Run rusfmt to see if a diff is produced. Runs on the code specified // // Parameters: // code: Code to run the binary on // config: Any additional configuration options to pass to rustfmt // - fn format_code<'a>( + fn format_code>( &self, - code: &'a str, - config: &Option>, + code: &str, + config: Option<&[T]>, ) -> Result { let config = create_config_arg(config); let mut command = Command::new(&self.binary_path) - .env("LD_LIBRARY_PATH", &self.ld_library_path) + .env(dynamic_library_path_env_var_name(), &self.ld_library_path) .args([ + "--edition", + self.edition.as_str(), + "--style-edition", + self.style_edition.as_str(), "--unstable-features", "--skip-children", "--emit=stdout", @@ -170,19 +364,33 @@ impl CodeFormatter for RustfmtRunner { command.stdin.as_mut().unwrap().write_all(code.as_bytes())?; let output = command.wait_with_output()?; - Ok(std::str::from_utf8(&output.stdout)?.to_string()) + let errors = std::str::from_utf8(&output.stderr)?.to_string(); + match output.status.code() { + Some(0) => Ok(std::str::from_utf8(&output.stdout)?.to_string()), + Some(_) | None => { + let formatted_code = std::str::from_utf8(&output.stdout)?.to_string(); + if !formatted_code.is_empty() { + Ok(formatted_code) + } else { + debug!("{errors}"); + Err(CheckDiffError::FormatCodeError(FormatCodeError { + error: errors, + })) + } + } + } } } /// Creates a configuration in the following form: /// =, =, ... -fn create_config_arg(config: &Option>) -> String { +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_str()); + result.push_str(arg.as_ref()); } result } @@ -222,8 +430,8 @@ pub fn clone_git_repo(url: &str, dest: &Path) -> Result<(), GitError> { return Err(error); } - info!("Successfully clone repository."); - return Ok(()); + info!("Successfully cloned repository {url} to {}", dest.display()); + Ok(()) } pub fn git_remote_add(url: &str) -> Result<(), GitError> { @@ -242,7 +450,7 @@ pub fn git_remote_add(url: &str) -> Result<(), GitError> { } info!("Successfully added remote: {url}"); - return Ok(()); + Ok(()) } pub fn git_fetch(branch_name: &str) -> Result<(), GitError> { @@ -261,7 +469,7 @@ pub fn git_fetch(branch_name: &str) -> Result<(), GitError> { } info!("Successfully fetched: {branch_name}"); - return Ok(()); + Ok(()) } pub fn git_switch(git_ref: &str, should_detach: bool) -> Result<(), GitError> { @@ -279,17 +487,17 @@ pub fn git_switch(git_ref: &str, should_detach: bool) -> Result<(), GitError> { return Err(error); } info!("Successfully switched to {git_ref}"); - return Ok(()); + Ok(()) } pub fn change_directory_to_path(dest: &Path) -> io::Result<()> { let dest_path = Path::new(&dest); - env::set_current_dir(&dest_path)?; + env::set_current_dir(dest_path)?; info!( - "Current directory: {}", + "Setting current directory to: {}", env::current_dir().unwrap().display() ); - return Ok(()); + Ok(()) } pub fn get_ld_library_path(dir: &Path) -> Result { @@ -302,7 +510,7 @@ pub fn get_ld_library_path(dir: &Path) -> Result { }; let sysroot = std::str::from_utf8(&command.stdout)?.trim_end(); let ld_lib_path = format!("{}/lib", sysroot); - return Ok(ld_lib_path); + Ok(ld_lib_path) } pub fn get_cargo_version() -> Result { @@ -313,7 +521,7 @@ pub fn get_cargo_version() -> Result { }; let cargo_version = std::str::from_utf8(&command.stdout)?.trim_end(); - return Ok(cargo_version.to_string()); + Ok(cargo_version.to_string()) } /// Obtains the ld_lib path and then builds rustfmt from source @@ -321,16 +529,17 @@ pub fn get_cargo_version() -> Result { pub fn build_rustfmt_from_src( binary_path: PathBuf, dir: &Path, + edition: Edition, + style_edition: StyleEdition, ) -> Result { //Because we're building standalone binaries we need to set `LD_LIBRARY_PATH` so each // binary can find it's runtime dependencies. // See https://github.com/rust-lang/rustfmt/issues/5675 // This will prepend the `LD_LIBRARY_PATH` for the main rustfmt binary - let ld_lib_path = get_ld_library_path(&dir)?; + let ld_lib_path = get_ld_library_path(dir)?; info!("Building rustfmt from source"); - let Ok(_) = Command::new("cargo") - .current_dir(dir) + let Ok(_output) = Command::new("cargo") .args(["build", "-q", "--release", "--bin", "rustfmt"]) .output() else { @@ -341,10 +550,12 @@ pub fn build_rustfmt_from_src( std::fs::copy(dir.join("target/release/rustfmt"), &binary_path)?; - return Ok(RustfmtRunner { + Ok(RustfmtRunner { ld_library_path: ld_lib_path, binary_path, - }); + edition, + style_edition, + }) } // Compiles and produces two rustfmt binaries. @@ -355,6 +566,8 @@ pub fn compile_rustfmt( dest: &Path, remote_repo_url: String, feature_branch: String, + edition: Edition, + style_edition: StyleEdition, commit_hash: Option, ) -> Result, CheckDiffError> { const RUSTFMT_REPO: &str = "https://github.com/rust-lang/rustfmt.git"; @@ -366,14 +579,16 @@ 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)?; + let src_runner = + build_rustfmt_from_src(dest.join("src_rustfmt"), dest, edition, style_edition)?; let should_detach = commit_hash.is_some(); git_switch( commit_hash.unwrap_or(feature_branch).as_str(), should_detach, )?; - let feature_runner = build_rustfmt_from_src(dest.join("feature_rustfmt"), dest)?; + let feature_runner = + build_rustfmt_from_src(dest.join("feature_rustfmt"), dest, edition, style_edition)?; info!("RUSFMT_BIN {}", src_runner.get_binary_version()?); info!( "Runtime dependencies for (src) rustfmt -- LD_LIBRARY_PATH: {}", @@ -385,53 +600,104 @@ pub fn compile_rustfmt( feature_runner.ld_library_path ); - return Ok(CheckDiffRunners { + Ok(CheckDiffRunners { src_runner, feature_runner, - }); + }) } /// Searches for rust files in the particular path and returns an iterator to them. pub fn search_for_rs_files(repo: &Path) -> impl Iterator { - return WalkDir::new(repo).into_iter().filter_map(|e| match e.ok() { + // TODO implement rustfmt ignore logic so that we skip ignored files. + // There are a lot of ignored files in the r-l/rust repo so this should speed things up + WalkDir::new(repo).into_iter().filter_map(|e| match e.ok() { Some(entry) => { let path = entry.path(); - if path.is_file() && path.extension().map_or(false, |ext| ext == "rs") { + if path.is_file() && path.extension().is_some_and(|ext| ext == "rs") { return Some(entry.into_path()); } - return None; + None } None => None, - }); + }) } /// 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( - config: Option>, - runners: CheckDiffRunners, +pub fn check_diff>( + config: Option<&[T]>, + runners: &CheckDiffRunners, repo: &Path, -) -> i32 { - let mut errors = 0; + repo_url: &str, +) -> u8 { + let mut errors: u8 = 0; let iter = search_for_rs_files(repo); for file in iter { - match runners.create_diff(file.as_path(), &config) { + let relative_path = file.strip_prefix(repo).unwrap_or(&file); + let repo_name = get_repo_name(repo_url); + trace!( + "Formatting '{repo_name}' file {repo_name}/{}", + relative_path.display() + ); + match runners.create_diff(file.as_path(), config) { Ok(diff) => { if !diff.is_empty() { - eprint!("{diff}"); - errors += 1; + error!( + "Diff found in '{0}' when formatting {0}/{1}\n{2}", + repo_name, + relative_path.display(), + diff, + ); + errors = errors.saturating_add(1); + } else { + trace!( + "No diff found in '{0}' when formatting {0}/{1}", + repo_name, + relative_path.display() + ) } } + Err(CheckDiffError::CreateDiffError(CreateDiffError::MainRustfmtFailed(_))) => { + debug!( + "Couldn't create diff because `main` rustfmt failed to format {}/{}", + repo_name, + relative_path.display() + ); + continue; + } + Err(CheckDiffError::CreateDiffError(CreateDiffError::FeatureRustfmtFailed(_))) => { + debug!( + "Couldn't create diff because `feature` rustfmt failed to format {}/{}", + repo_name, + relative_path.display() + ); + continue; + } + Err(CheckDiffError::CreateDiffError(CreateDiffError::BothRustfmtFailed { .. })) => { + debug!( + "Couldn't create diff because both rustfmt binaries failed to format {}/{}", + repo_name, + relative_path.display() + ); + continue; + } Err(e) => { - eprintln!( - "Error creating diff for {:?}: {:?}", - file.as_path().display(), + error!( + "Error creating diff for {}/{:?}: {:?}", + repo_name, + relative_path.display(), e ); - errors += 1; } } } - return errors; + errors +} + +/// parse out the repository name from a GitHub Repository name. +pub fn get_repo_name(git_url: &str) -> &str { + let strip_git_prefix = &git_url[0..git_url.len() - 4]; + let (_, repo_name) = strip_git_prefix.rsplit_once('/').unwrap_or(("", git_url)); + repo_name } diff --git a/check_diff/src/main.rs b/check_diff/src/main.rs index 9c14c5f08cd..e1496cb50f4 100644 --- a/check_diff/src/main.rs +++ b/check_diff/src/main.rs @@ -1,7 +1,15 @@ -use check_diff::{CheckDiffError, check_diff, compile_rustfmt}; +use std::io::Error; +use std::process::ExitCode; +use std::sync::Arc; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::thread; + +use check_diff::{ + Edition, StyleEdition, check_diff, clone_git_repo, compile_rustfmt, get_repo_name, +}; use clap::Parser; -use tempfile::Builder; -use tracing::info; +use tempfile::tempdir; +use tracing::{error, info, warn}; /// Inputs for the check_diff script #[derive(Parser)] @@ -10,6 +18,12 @@ struct CliInputs { remote_repo_url: String, /// Name of the feature branch on the forked repo feature_branch: String, + /// Rust `edition`` used to parse code. Possible values {2015, 2018, 2021, 2024} + #[arg(short, long, default_value = "2015")] + edition: Edition, + /// rustfmt `style_edition` used when formatting code. Possible vales {2021, 2024} + #[arg(short, long, default_value = "2021")] + style_edition: StyleEdition, /// Optional commit hash from the feature branch #[arg(short, long)] commit_hash: Option, @@ -19,22 +33,91 @@ struct CliInputs { rustfmt_config: Option>, } -fn main() -> Result<(), CheckDiffError> { +const REPOS: &[&str] = &[ + "https://github.com/rust-lang/rust.git", + "https://github.com/rust-lang/cargo.git", + "https://github.com/rust-lang/miri.git", + "https://github.com/rust-lang/rust-analyzer.git", + "https://github.com/bitflags/bitflags.git", + "https://github.com/rust-lang/log.git", + "https://github.com/rust-lang/mdBook.git", + "https://github.com/rust-lang/packed_simd.git", + "https://github.com/rust-lang/rust-semverver.git", + "https://github.com/Stebalien/tempfile.git", + "https://github.com/rust-lang/futures-rs.git", + "https://github.com/dtolnay/anyhow.git", + "https://github.com/dtolnay/thiserror.git", + "https://github.com/dtolnay/syn.git", + "https://github.com/serde-rs/serde.git", + "https://github.com/rust-lang/rustlings.git", + "https://github.com/rust-lang/rustup.git", + "https://github.com/SergioBenitez/Rocket.git", + "https://github.com/rustls/rustls.git", + "https://github.com/rust-lang/rust-bindgen.git", + "https://github.com/hyperium/hyper.git", + "https://github.com/actix/actix.git", + "https://github.com/denoland/deno.git", +]; + +fn main() -> Result { tracing_subscriber::fmt() .with_env_filter(tracing_subscriber::EnvFilter::from_env("CHECK_DIFF_LOG")) .init(); let args = CliInputs::parse(); - let tmp_dir = Builder::new().tempdir_in("").unwrap(); + let tmp_dir = tempdir()?; info!("Created tmp_dir {:?}", tmp_dir); - let check_diff_runners = compile_rustfmt( + let Ok(check_diff_runners) = compile_rustfmt( tmp_dir.path(), args.remote_repo_url, args.feature_branch, + args.edition, + args.style_edition, args.commit_hash, - )?; + ) else { + error!("Failed to compile rustfmt"); + return Ok(ExitCode::FAILURE); + }; + + let errors = Arc::new(AtomicUsize::new(0)); + let rustfmt_args = Arc::new(args.rustfmt_config); + let check_diff_runners = Arc::new(check_diff_runners); + + thread::scope(|s| { + for url in REPOS { + let cloned_error = Arc::clone(&errors); + let cloned_rustfmt_configs = Arc::clone(&rustfmt_args); + let cloned_check_diff_runner = Arc::clone(&check_diff_runners); + s.spawn(move || { + let repo_name = get_repo_name(url); + info!("Processing repo: {repo_name}"); + let Ok(tmp_dir) = tempdir() else { + warn!("Failed to create a tempdir for {}", &url); + return; + }; + + let Ok(_) = clone_git_repo(url, tmp_dir.path()) else { + warn!("Failed to clone repo {}", &url); + return; + }; + + let error_count = check_diff( + cloned_rustfmt_configs.as_deref(), + &cloned_check_diff_runner, + tmp_dir.path(), + url, + ); - // TODO: currently using same tmp dir path for sake of compilation - let _ = check_diff(args.rustfmt_config, check_diff_runners, tmp_dir.path()); + cloned_error.fetch_add(error_count as usize, Ordering::Relaxed); + }); + } + }); - Ok(()) + let error_count = Arc::into_inner(errors).expect("All other threads are done").load(Ordering::Relaxed); + if error_count > 0 { + error!("Formatting diff found 💔"); + Ok(ExitCode::from(u8::try_from(error_count).unwrap_or(u8::MAX))) + } else { + info!("No diff found 😊"); + Ok(ExitCode::SUCCESS) + } } diff --git a/check_diff/tests/check_diff.rs b/check_diff/tests/check_diff.rs index 17297c13043..124358c2ef3 100644 --- a/check_diff/tests/check_diff.rs +++ b/check_diff/tests/check_diff.rs @@ -7,10 +7,10 @@ use tempfile::Builder; struct DoNothingFormatter; impl CodeFormatter for DoNothingFormatter { - fn format_code<'a>( + fn format_code>( &self, - _code: &'a str, - _config: &Option>, + _code: &str, + _config: Option<&[T]>, ) -> Result { Ok(String::new()) } @@ -20,10 +20,10 @@ impl CodeFormatter for DoNothingFormatter { struct AddWhiteSpaceFormatter; impl CodeFormatter for AddWhiteSpaceFormatter { - fn format_code<'a>( + fn format_code>( &self, - code: &'a str, - _config: &Option>, + code: &str, + _config: Option<&[T]>, ) -> Result { let result = code.to_string() + " "; Ok(result) @@ -77,8 +77,9 @@ fn check_diff_test_no_formatting_difference() -> Result<(), CheckDiffError> { let dir = Builder::new().tempdir_in("").unwrap(); let file_path = dir.path().join("test.rs"); let _tmp_file = File::create(file_path)?; + let repo_name = "https://github.com/rust-lang/rustfmt.git"; - let errors = check_diff(None, runners, dir.path()); + let errors = check_diff(None::<&[&str]>, &runners, dir.path(), repo_name); assert_eq!(errors, 0); Ok(()) } @@ -89,8 +90,9 @@ fn check_diff_test_formatting_difference() -> Result<(), CheckDiffError> { let dir = Builder::new().tempdir_in("").unwrap(); let file_path = dir.path().join("test.rs"); let _tmp_file = File::create(file_path)?; + let repo_name = "https://github.com/rust-lang/rustfmt.git"; - let errors = check_diff(None, runners, dir.path()); + let errors = check_diff(None::<&[&str]>, &runners, dir.path(), repo_name); assert_ne!(errors, 0); Ok(()) }