diff --git a/check_diff/Cargo.lock b/check_diff/Cargo.lock index 2abf5af2f98..95bdd32567b 100644 --- a/check_diff/Cargo.lock +++ b/check_diff/Cargo.lock @@ -77,9 +77,11 @@ name = "check_diff" version = "0.1.0" dependencies = [ "clap", + "diffy", "tempfile", "tracing", "tracing-subscriber", + "walkdir", ] [[package]] @@ -128,6 +130,15 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0b6a852b24ab71dffc585bcb46eaf7959d175cb865a7152e35b348d1b2960422" +[[package]] +name = "diffy" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5d3041965b7a63e70447ec818a46b1e5297f7fcae3058356d226c02750c4e6cb" +dependencies = [ + "nu-ansi-term 0.50.1", +] + [[package]] name = "errno" version = "0.3.9" @@ -205,6 +216,15 @@ dependencies = [ "winapi", ] +[[package]] +name = "nu-ansi-term" +version = "0.50.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d4a28e057d01f97e61255210fcff094d74ed0466038633e95017f5beb68e4399" +dependencies = [ + "windows-sys", +] + [[package]] name = "once_cell" version = "1.19.0" @@ -298,6 +318,15 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "same-file" +version = "1.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "93fc1dc3aaa9bfed95e02e6eadabb4baf7e3078b0bd1b4d7b6b0b68378900502" +dependencies = [ + "winapi-util", +] + [[package]] name = "sharded-slab" version = "0.1.7" @@ -402,7 +431,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ad0f048c97dbd9faa9b7df56362b8ebcaa52adb06b498c050d2f4e32f90a7a8b" dependencies = [ "matchers", - "nu-ansi-term", + "nu-ansi-term 0.46.0", "once_cell", "regex", "sharded-slab", @@ -431,6 +460,16 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "830b7e5d4d90034032940e4ace0d9a9a057e7a45cd94e6c007832e39edb82f6d" +[[package]] +name = "walkdir" +version = "2.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "29790946404f91d9c5d06f9874efddea1dc06c5efe94541a7d6863108e3a5e4b" +dependencies = [ + "same-file", + "winapi-util", +] + [[package]] name = "winapi" version = "0.3.9" @@ -447,6 +486,15 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" +[[package]] +name = "winapi-util" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cf221c93e13a30d793f7645a0e7762c55d169dbb0a49671918a2319d289b10bb" +dependencies = [ + "windows-sys", +] + [[package]] name = "winapi-x86_64-pc-windows-gnu" version = "0.4.0" diff --git a/check_diff/Cargo.toml b/check_diff/Cargo.toml index 6c277f4af64..877735e4e39 100644 --- a/check_diff/Cargo.toml +++ b/check_diff/Cargo.toml @@ -10,3 +10,5 @@ clap = { version = "4.4.2", features = ["derive"] } tracing = "0.1.37" tracing-subscriber = { version = "0.3.17", features = ["env-filter"] } tempfile = "3" +walkdir = "2.5.0" +diffy = "0.4.0" diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 072b2f5d5c1..1a01b9f5325 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -1,10 +1,14 @@ +use diffy; use std::env; -use std::io; +use std::fmt::{Debug, Display}; +use std::io::{self, Write}; use std::path::{Path, PathBuf}; -use std::process::Command; +use std::process::{Command, Stdio}; use std::str::Utf8Error; use tracing::info; +use walkdir::WalkDir; +#[derive(Debug)] pub enum CheckDiffError { /// Git related errors FailedGit(GitError), @@ -18,6 +22,9 @@ pub enum CheckDiffError { FailedBinaryVersioning(PathBuf), /// Error when obtaining cargo version FailedCargoVersion(&'static str), + /// Error when trying to find rust files + FailedFindingRustFiles(&'static str), + FailedWritingToFile(&'static str), IO(std::io::Error), } @@ -39,6 +46,7 @@ impl From for CheckDiffError { } } +#[derive(Debug)] pub enum GitError { FailedClone { stdout: Vec, stderr: Vec }, FailedRemoteAdd { stdout: Vec, stderr: Vec }, @@ -53,11 +61,37 @@ impl From for GitError { } } +pub struct Diff { + src_format: String, + feature_format: String, +} + +impl Display for Diff { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let patch = diffy::create_patch(self.src_format.as_str(), self.feature_format.as_str()); + write!(f, "{}", patch) + } +} + +impl Diff { + pub fn is_empty(&self) -> bool { + let patch = diffy::create_patch(self.src_format.as_str(), self.feature_format.as_str()); + patch.hunks().is_empty() + } +} + // will be used in future PRs, just added to make the compiler happy -#[allow(dead_code)] -pub struct CheckDiffRunners { - feature_runner: RustfmtRunner, - src_runner: RustfmtRunner, +pub struct CheckDiffRunners { + feature_runner: F, + src_runner: S, +} + +pub trait CodeFormatter { + fn format_code<'a>( + &self, + code: &'a str, + config: &Option>, + ) -> Result; } pub struct RustfmtRunner { @@ -65,6 +99,36 @@ pub struct RustfmtRunner { binary_path: PathBuf, } +impl CheckDiffRunners { + pub fn new(feature_runner: F, src_runner: S) -> Self { + Self { + feature_runner, + src_runner, + } + } +} + +impl CheckDiffRunners +where + F: CodeFormatter, + S: CodeFormatter, +{ + /// Creates a diff generated by running the source and feature binaries on the same file path + pub fn create_diff( + &self, + path: &Path, + additional_configs: &Option>, + ) -> 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, + }) + } +} + impl RustfmtRunner { fn get_binary_version(&self) -> Result { let Ok(command) = Command::new(&self.binary_path) @@ -82,6 +146,58 @@ impl RustfmtRunner { } } +impl CodeFormatter for RustfmtRunner { + // 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>( + &self, + code: &'a str, + config: &Option>, + ) -> Result { + let config = create_config_arg(config); + let mut command = Command::new(&self.binary_path) + .env("LD_LIBRARY_PATH", &self.ld_library_path) + .args([ + "--unstable-features", + "--skip-children", + "--emit=stdout", + config.as_str(), + ]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn()?; + + 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()) + } +} + +/// Creates a configuration in the following form: +/// =, =, ... +fn create_config_arg(config: &Option>) -> 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 + } + None => String::new(), + }; + let config = format!( + "--config=error_on_line_overflow=false,error_on_unformatted=false{}", + config_arg.as_str() + ); + config +} /// Clone a git repository /// /// Parameters: @@ -180,8 +296,12 @@ pub fn change_directory_to_path(dest: &Path) -> io::Result<()> { return Ok(()); } -pub fn get_ld_library_path() -> Result { - let Ok(command) = Command::new("rustc").args(["--print", "sysroot"]).output() else { +pub fn get_ld_library_path(dir: &Path) -> Result { + let Ok(command) = Command::new("rustc") + .current_dir(dir) + .args(["--print", "sysroot"]) + .output() + else { return Err(CheckDiffError::FailedCommand("Error getting sysroot")); }; let sysroot = std::str::from_utf8(&command.stdout)?.trim_end(); @@ -202,15 +322,19 @@ 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(binary_path: PathBuf) -> Result { +pub fn build_rustfmt_from_src( + binary_path: PathBuf, + dir: &Path, +) -> 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 master rustfmt binary - let ld_lib_path = get_ld_library_path()?; + let ld_lib_path = get_ld_library_path(&dir)?; info!("Building rustfmt from source"); let Ok(_) = Command::new("cargo") + .current_dir(dir) .args(["build", "-q", "--release", "--bin", "rustfmt"]) .output() else { @@ -219,7 +343,7 @@ pub fn build_rustfmt_from_src(binary_path: PathBuf) -> Result, -) -> Result { +) -> Result, CheckDiffError> { const RUSTFMT_REPO: &str = "https://github.com/rust-lang/rustfmt.git"; clone_git_repo(RUSTFMT_REPO, dest)?; @@ -246,14 +370,14 @@ 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"))?; + let src_runner = build_rustfmt_from_src(dest.join("src_rustfmt"), dest)?; 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"))?; + let feature_runner = build_rustfmt_from_src(dest.join("feature_rustfmt"), dest)?; info!("RUSFMT_BIN {}", src_runner.get_binary_version()?); info!( "Runtime dependencies for (src) rustfmt -- LD_LIBRARY_PATH: {}", @@ -270,3 +394,48 @@ pub fn compile_rustfmt( 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() { + Some(entry) => { + let path = entry.path(); + if path.is_file() && path.extension().map_or(false, |ext| ext == "rs") { + return Some(entry.into_path()); + } + return 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, + repo: &Path, +) -> i32 { + let mut errors = 0; + let iter = search_for_rs_files(repo); + for file in iter { + match runners.create_diff(file.as_path(), &config) { + Ok(diff) => { + if !diff.is_empty() { + eprint!("{diff}"); + errors += 1; + } + } + Err(e) => { + eprintln!( + "Error creating diff for {:?}: {:?}", + file.as_path().display(), + e + ); + errors += 1; + } + } + } + + return errors; +} diff --git a/check_diff/src/main.rs b/check_diff/src/main.rs index e70ae628da7..45aeaa0cc11 100644 --- a/check_diff/src/main.rs +++ b/check_diff/src/main.rs @@ -1,4 +1,4 @@ -use check_diff::compile_rustfmt; +use check_diff::{check_diff, compile_rustfmt, CheckDiffError}; use clap::Parser; use tempfile::Builder; use tracing::info; @@ -19,17 +19,22 @@ struct CliInputs { rustfmt_config: Option>, } -fn main() { +fn main() -> Result<(), CheckDiffError> { 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(); info!("Created tmp_dir {:?}", tmp_dir); - let _ = compile_rustfmt( + let check_diff_runners = compile_rustfmt( tmp_dir.path(), args.remote_repo_url, args.feature_branch, args.commit_hash, - ); + )?; + + // TODO: currently using same tmp dir path for sake of compilation + let _ = check_diff(args.rustfmt_config, check_diff_runners, tmp_dir.path()); + + Ok(()) } diff --git a/check_diff/tests/check_diff.rs b/check_diff/tests/check_diff.rs new file mode 100644 index 00000000000..0fe9219f65e --- /dev/null +++ b/check_diff/tests/check_diff.rs @@ -0,0 +1,82 @@ +use check_diff::{ + check_diff, compile_rustfmt, search_for_rs_files, CheckDiffError, CheckDiffRunners, +}; +use std::fs::File; +use tempfile::Builder; + +#[test] +fn search_for_files_correctly_non_nested() -> Result<(), Box> { + let dir = Builder::new().tempdir_in("").unwrap(); + let file_path = dir.path().join("test.rs"); + let _tmp_file = File::create(file_path)?; + + let iter = search_for_rs_files(dir.path()); + + let mut count = 0; + for _ in iter { + count += 1; + } + + assert_eq!(count, 1); + + Ok(()) +} + +#[test] +fn search_for_files_correctly_nested() -> Result<(), Box> { + let dir = Builder::new().tempdir_in("").unwrap(); + let file_path = dir.path().join("test.rs"); + let _tmp_file = File::create(file_path)?; + + let nested_dir = Builder::new().tempdir_in(dir.path()).unwrap(); + let nested_file_path = nested_dir.path().join("nested.rs"); + let _ = File::create(nested_file_path)?; + + let iter = search_for_rs_files(dir.path()); + + let mut count = 0; + for _ in iter { + count += 1; + } + + assert_eq!(count, 2); + + Ok(()) +} + +#[test] +fn check_diff_test() -> Result<(), CheckDiffError> { + let tmp_dir = Builder::new().tempdir_in("").unwrap(); + let runners = compile_rustfmt( + tmp_dir.path(), + "https://github.com/rust-lang/rustfmt".to_string(), + "rustfmt-1.4.32".to_string(), + None, + )?; + + let dir = Builder::new().tempdir_in("").unwrap(); + let file_path = dir.path().join("test.rs"); + let _tmp_file = File::create(file_path)?; + + let errors = check_diff(None, runners, dir.path()); + assert_eq!(errors, 0); + Ok(()) +} + +#[test] +fn format_simple_code() -> Result<(), CheckDiffError> { + let tmp_dir = Builder::new().tempdir_in("").unwrap(); + let runners = compile_rustfmt( + tmp_dir.path(), + "https://github.com/rust-lang/rustfmt".to_string(), + "rustfmt-1.4.32".to_string(), + None, + )?; + + //let output = runners + // .src_runner + // .format_code("fn main() {}", &None)?; + //assert_eq!(output, "fn main() {}\n".to_string()); + // + Ok(()) +}