diff --git a/CHANGELOG.md b/CHANGELOG.md index 8af60f60dc6..12cc2db51af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,7 +39,7 @@ use std::num::{NonZeroU8, NonZeroU16, NonZeroU32, NonZeroU64}; ``` [style guide's version sorting algorithm]: https://doc.rust-lang.org/nightly/style-guide/#sorting -- When parsing rustfmt configurations fails, rustfmt will now include the path to the toml file in the erorr message [#6302](https://github.com/rust-lang/rustfmt/issues/6302) +- When parsing rustfmt configurations fails, rustfmt will now include the path to the toml file in the error message [#6302](https://github.com/rust-lang/rustfmt/issues/6302) ### Added - rustfmt now formats trailing where clauses in type aliases [#5887](https://github.com/rust-lang/rustfmt/pull/5887) @@ -133,7 +133,7 @@ ### Changed - `hide_parse_errors` has been soft deprecated and it's been renamed to `show_parse_errors` [#5961](https://github.com/rust-lang/rustfmt/pull/5961). -- The diff output produced by `rustfmt --check` is more compatable with editors that support navigating directly to line numbers [#5971](https://github.com/rust-lang/rustfmt/pull/5971) +- The diff output produced by `rustfmt --check` is more compatible with editors that support navigating directly to line numbers [#5971](https://github.com/rust-lang/rustfmt/pull/5971) - When using `version=Two`, the `trace!` macro from the [log crate] is now formatted similarly to `debug!`, `info!`, `warn!`, and `error!` [#5987](https://github.com/rust-lang/rustfmt/issues/5987). [log crate]: https://crates.io/crates/log diff --git a/Cargo.lock b/Cargo.lock index 0a69571afed..e3fc87feca4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -13,12 +13,12 @@ dependencies = [ [[package]] name = "annotate-snippets" -version = "0.9.1" +version = "0.11.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c3b9d411ecbaf79885c6df4d75fff75858d5995ff25385657a28af47e82f9c36" +checksum = "24e35ed54e5ea7997c14ed4c70ba043478db1112e98263b3b035907aa197d991" dependencies = [ + "anstyle", "unicode-width", - "yansi-term", ] [[package]] @@ -37,9 +37,9 @@ dependencies = [ [[package]] name = "anstyle" -version = "1.0.3" +version = "1.0.10" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b84bf0a05bbb2a83e5eb6fa36bb6e87baa08193c35ff52bbf6b38d8af2890e46" +checksum = "55cc3b69f167a1ef2e161439aa98aed94e6028e5f9a59be9a6ffb47aef1651f9" [[package]] name = "anstyle-parse" @@ -773,9 +773,9 @@ checksum = "1dd624098567895118886609431a7c3b8f516e41d30e0643f03d94592a147e36" [[package]] name = "unicode-width" -version = "0.1.10" +version = "0.1.14" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c0edd1e5b14653f783770bce4a4dabb4a5108a5370a5f5d8cfe8710c361f6c8b" +checksum = "7dd6e30e90baa6f72411720665d41d89b9a3d039dc45b8faea1ddd07f617f6af" [[package]] name = "utf8parse" @@ -911,12 +911,3 @@ checksum = "ca0ace3845f0d96209f0375e6d367e3eb87eb65d27d445bdc9f1843a26f39448" dependencies = [ "memchr", ] - -[[package]] -name = "yansi-term" -version = "0.1.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fe5c30ade05e61656247b2e334a031dfd0cc466fadef865bdcdea8d537951bf1" -dependencies = [ - "winapi", -] diff --git a/Cargo.toml b/Cargo.toml index 96746b5d119..07bc74a5127 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,7 +33,7 @@ rustfmt-format-diff = [] generic-simd = ["bytecount/generic-simd"] [dependencies] -annotate-snippets = { version = "0.9", features = ["color"] } +annotate-snippets = { version = "0.11" } anyhow = "1.0" bytecount = "0.6.8" cargo_metadata = "0.18" diff --git a/Configurations.md b/Configurations.md index 0f8b7ce0185..8e04ddd325e 100644 --- a/Configurations.md +++ b/Configurations.md @@ -1256,6 +1256,56 @@ Control the case of the letters in hexadecimal literal values - **Possible values**: `Preserve`, `Upper`, `Lower` - **Stable**: No (tracking issue: [#5081](https://github.com/rust-lang/rustfmt/issues/5081)) +## `float_literal_trailing_zero` + +Control the presence of trailing zero in floating-point literal values + +- **Default value**: `Preserve` +- **Possible values**: `Preserve`, `Always`, `IfNoPostfix`, `Never` +- **Stable**: No (tracking issue: [#6471](https://github.com/rust-lang/rustfmt/issues/6471)) + +#### `Preserve` (default): + +Leave the literal as-is. + +```rust +fn main() { + let values = [1.0, 2., 3.0e10, 4f32]; +} +``` + +#### `Always`: + +Add a trailing zero to the literal: + +```rust +fn main() { + let values = [1.0, 2.0, 3.0e10, 4.0f32]; +} +``` + +#### `IfNoPostfix`: + +Add a trailing zero by default. If the literal contains an exponent or a suffix, the zero +and the preceding period are removed: + +```rust +fn main() { + let values = [1.0, 2.0, 3e10, 4f32]; +} +``` + +#### `Never`: + +Remove the trailing zero. If the literal contains an exponent or a suffix, the preceding +period is also removed: + +```rust +fn main() { + let values = [1., 2., 3e10, 4f32]; +} +``` + ## `hide_parse_errors` This option is deprecated and has been renamed to `show_parse_errors` to avoid confusion around the double negative default of `hide_parse_errors=false`. @@ -3167,7 +3217,7 @@ Break comments to fit on the line Note that no wrapping will happen if: 1. The comment is the start of a markdown header doc comment -2. An URL was found in the comment +2. A URL was found in the comment - **Default value**: `false` - **Possible values**: `true`, `false` 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 4ae8a5f1f3a..877735e4e39 100644 --- a/check_diff/Cargo.toml +++ b/check_diff/Cargo.toml @@ -9,5 +9,6 @@ edition = "2021" clap = { version = "4.4.2", features = ["derive"] } tracing = "0.1.37" tracing-subscriber = { version = "0.3.17", features = ["env-filter"] } -[dev-dependencies] 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 b83d67c8b6e..90dbd5cc729 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -1,11 +1,54 @@ +use diffy; use std::env; -use std::io; -use std::path::Path; -use std::process::Command; +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 walkdir::WalkDir; +#[derive(Debug)] +pub enum CheckDiffError { + /// Git related errors + FailedGit(GitError), + /// Error for generic commands + FailedCommand(&'static str), + /// UTF8 related errors + FailedUtf8(Utf8Error), + /// Error for building rustfmt from source + FailedSourceBuild(&'static str), + /// Error when obtaining binary version + FailedBinaryVersioning(PathBuf), + /// Error when obtaining cargo version + FailedCargoVersion(&'static str), + IO(std::io::Error), +} + +impl From for CheckDiffError { + fn from(error: io::Error) -> Self { + CheckDiffError::IO(error) + } +} + +impl From for CheckDiffError { + fn from(error: GitError) -> Self { + CheckDiffError::FailedGit(error) + } +} + +impl From for CheckDiffError { + fn from(error: Utf8Error) -> Self { + CheckDiffError::FailedUtf8(error) + } +} + +#[derive(Debug)] pub enum GitError { FailedClone { stdout: Vec, stderr: Vec }, + FailedRemoteAdd { stdout: Vec, stderr: Vec }, + FailedFetch { stdout: Vec, stderr: Vec }, + FailedSwitch { stdout: Vec, stderr: Vec }, IO(std::io::Error), } @@ -15,6 +58,142 @@ 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() + } +} + +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 { + ld_library_path: String, + 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) + .env("LD_LIBRARY_PATH", &self.ld_library_path) + .args(["--version"]) + .output() + else { + return Err(CheckDiffError::FailedBinaryVersioning( + self.binary_path.clone(), + )); + }; + + let binary_version = std::str::from_utf8(&command.stdout)?.trim(); + return Ok(binary_version.to_string()); + } +} + +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: @@ -47,6 +226,62 @@ pub fn clone_git_repo(url: &str, dest: &Path) -> Result<(), GitError> { return Ok(()); } +pub fn git_remote_add(url: &str) -> Result<(), GitError> { + let git_cmd = Command::new("git") + .args(["remote", "add", "feature", url]) + .output()?; + + // if the git command does not return successfully, + // any command on the repo will fail. So fail fast. + if !git_cmd.status.success() { + let error = GitError::FailedRemoteAdd { + stdout: git_cmd.stdout, + stderr: git_cmd.stderr, + }; + return Err(error); + } + + info!("Successfully added remote: {url}"); + return Ok(()); +} + +pub fn git_fetch(branch_name: &str) -> Result<(), GitError> { + let git_cmd = Command::new("git") + .args(["fetch", "feature", branch_name]) + .output()?; + + // if the git command does not return successfully, + // any command on the repo will fail. So fail fast. + if !git_cmd.status.success() { + let error = GitError::FailedFetch { + stdout: git_cmd.stdout, + stderr: git_cmd.stderr, + }; + return Err(error); + } + + info!("Successfully fetched: {branch_name}"); + return Ok(()); +} + +pub fn git_switch(git_ref: &str, should_detach: bool) -> Result<(), GitError> { + let detach_arg = if should_detach { "--detach" } else { "" }; + let args = ["switch", git_ref, detach_arg]; + let output = Command::new("git") + .args(args.iter().filter(|arg| !arg.is_empty())) + .output()?; + if !output.status.success() { + tracing::error!("Git switch failed: {output:?}"); + let error = GitError::FailedSwitch { + stdout: output.stdout, + stderr: output.stderr, + }; + return Err(error); + } + info!("Successfully switched to {git_ref}"); + return Ok(()); +} + pub fn change_directory_to_path(dest: &Path) -> io::Result<()> { let dest_path = Path::new(&dest); env::set_current_dir(&dest_path)?; @@ -56,3 +291,147 @@ pub fn change_directory_to_path(dest: &Path) -> io::Result<()> { ); return Ok(()); } + +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(); + let ld_lib_path = format!("{}/lib", sysroot); + return Ok(ld_lib_path); +} + +pub fn get_cargo_version() -> Result { + let Ok(command) = Command::new("cargo").args(["--version"]).output() else { + return Err(CheckDiffError::FailedCargoVersion( + "Failed to obtain cargo version", + )); + }; + + let cargo_version = std::str::from_utf8(&command.stdout)?.trim_end(); + return Ok(cargo_version.to_string()); +} + +/// 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, + 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(&dir)?; + + info!("Building rustfmt from source"); + let Ok(_) = Command::new("cargo") + .current_dir(dir) + .args(["build", "-q", "--release", "--bin", "rustfmt"]) + .output() + else { + return Err(CheckDiffError::FailedSourceBuild( + "Error building rustfmt from source", + )); + }; + + std::fs::copy(dir.join("target/release/rustfmt"), &binary_path)?; + + return Ok(RustfmtRunner { + ld_library_path: ld_lib_path, + binary_path, + }); +} + +// Compiles and produces two rustfmt binaries. +// One for the current master, and another for the feature branch +// Parameters: +// dest: Directory where rustfmt will be cloned +pub fn compile_rustfmt( + dest: &Path, + remote_repo_url: String, + feature_branch: String, + commit_hash: Option, +) -> Result, CheckDiffError> { + const RUSTFMT_REPO: &str = "https://github.com/rust-lang/rustfmt.git"; + + clone_git_repo(RUSTFMT_REPO, dest)?; + change_directory_to_path(dest)?; + git_remote_add(remote_repo_url.as_str())?; + git_fetch(feature_branch.as_str())?; + + let cargo_version = get_cargo_version()?; + info!("Compiling with {}", cargo_version); + 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"), dest)?; + info!("RUSFMT_BIN {}", src_runner.get_binary_version()?); + info!( + "Runtime dependencies for (src) rustfmt -- LD_LIBRARY_PATH: {}", + src_runner.ld_library_path + ); + info!("FEATURE_BIN {}", feature_runner.get_binary_version()?); + info!( + "Runtime dependencies for (feature) rustfmt -- LD_LIBRARY_PATH: {}", + feature_runner.ld_library_path + ); + + return 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() { + 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 01c5926c490..f4ce3572faa 100644 --- a/check_diff/src/main.rs +++ b/check_diff/src/main.rs @@ -1,4 +1,7 @@ +use check_diff::{CheckDiffError, check_diff, compile_rustfmt}; use clap::Parser; +use tempfile::Builder; +use tracing::info; /// Inputs for the check_diff script #[derive(Parser)] @@ -16,6 +19,22 @@ struct CliInputs { rustfmt_config: Option>, } -fn main() { - let _args = CliInputs::parse(); +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 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..17297c13043 --- /dev/null +++ b/check_diff/tests/check_diff.rs @@ -0,0 +1,96 @@ +use check_diff::{ + CheckDiffError, CheckDiffRunners, CodeFormatter, check_diff, search_for_rs_files, +}; +use std::fs::File; +use tempfile::Builder; + +struct DoNothingFormatter; + +impl CodeFormatter for DoNothingFormatter { + fn format_code<'a>( + &self, + _code: &'a str, + _config: &Option>, + ) -> Result { + Ok(String::new()) + } +} + +/// Formatter that adds a white space to the end of the codd +struct AddWhiteSpaceFormatter; + +impl CodeFormatter for AddWhiteSpaceFormatter { + fn format_code<'a>( + &self, + code: &'a str, + _config: &Option>, + ) -> Result { + let result = code.to_string() + " "; + Ok(result) + } +} + +#[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_no_formatting_difference() -> Result<(), CheckDiffError> { + let runners = CheckDiffRunners::new(DoNothingFormatter, DoNothingFormatter); + + 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 check_diff_test_formatting_difference() -> Result<(), CheckDiffError> { + let runners = CheckDiffRunners::new(DoNothingFormatter, AddWhiteSpaceFormatter); + 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_ne!(errors, 0); + Ok(()) +} diff --git a/rust-toolchain b/rust-toolchain index 80723123274..348664bc168 100644 --- a/rust-toolchain +++ b/rust-toolchain @@ -1,3 +1,3 @@ [toolchain] -channel = "nightly-2024-09-10" +channel = "nightly-2025-01-02" components = ["llvm-tools", "rustc-dev"] diff --git a/src/attr.rs b/src/attr.rs index 5c2068b6a22..e2104617fdc 100644 --- a/src/attr.rs +++ b/src/attr.rs @@ -56,23 +56,22 @@ fn argument_shape( shape: Shape, context: &RewriteContext<'_>, ) -> Option { - match context.config.indent_style() { + let shape = match context.config.indent_style() { IndentStyle::Block => { if combine { - shape.offset_left(left) + shape.offset_left_opt(left)? } else { - Some( - shape - .block_indent(context.config.tab_spaces()) - .with_max_width(context.config), - ) + shape + .block_indent(context.config.tab_spaces()) + .with_max_width(context.config) } } IndentStyle::Visual => shape .visual_indent(0) - .shrink_left(left) - .and_then(|s| s.sub_width(right)), - } + .shrink_left_opt(left)? + .sub_width_opt(right)?, + }; + Some(shape) } fn format_derive( @@ -90,7 +89,7 @@ fn format_derive( let item_spans = attr.meta_item_list().map(|meta_item_list| { meta_item_list .into_iter() - .map(|nested_meta_item| nested_meta_item.span()) + .map(|meta_item_inner| meta_item_inner.span()) })?; let items = itemize_list( @@ -127,8 +126,8 @@ fn format_derive( context, )?; let one_line_shape = shape - .offset_left("[derive()]".len() + prefix.len())? - .sub_width("()]".len())?; + .offset_left_opt("[derive()]".len() + prefix.len())? + .sub_width_opt("()]".len())?; let one_line_budget = one_line_shape.width; let tactic = definitive_tactic( @@ -243,17 +242,15 @@ fn rewrite_initial_doc_comments( Ok((0, None)) } -impl Rewrite for ast::NestedMetaItem { +impl Rewrite for ast::MetaItemInner { fn rewrite(&self, context: &RewriteContext<'_>, shape: Shape) -> Option { self.rewrite_result(context, shape).ok() } fn rewrite_result(&self, context: &RewriteContext<'_>, shape: Shape) -> RewriteResult { match self { - ast::NestedMetaItem::MetaItem(ref meta_item) => { - meta_item.rewrite_result(context, shape) - } - ast::NestedMetaItem::Lit(ref l) => { + ast::MetaItemInner::MetaItem(ref meta_item) => meta_item.rewrite_result(context, shape), + ast::MetaItemInner::Lit(ref l) => { rewrite_literal(context, l.as_token_lit(), l.span, shape) } } @@ -297,7 +294,7 @@ impl Rewrite for ast::MetaItem { &path, list.iter(), // 1 = "]" - shape.sub_width(1).max_width_error(shape.width, self.span)?, + shape.sub_width(1, self.span)?, self.span, context.config.attr_fn_like_width(), Some(if has_trailing_comma { @@ -310,9 +307,7 @@ impl Rewrite for ast::MetaItem { ast::MetaItemKind::NameValue(ref lit) => { let path = rewrite_path(context, PathContext::Type, &None, &self.path, shape)?; // 3 = ` = ` - let lit_shape = shape - .shrink_left(path.len() + 3) - .max_width_error(shape.width, self.span)?; + let lit_shape = shape.shrink_left(path.len() + 3, self.span)?; // `rewrite_literal` returns `None` when `lit` exceeds max // width. Since a literal is basically unformattable unless it // is a string literal (and only if `format_strings` is set), @@ -369,9 +364,7 @@ impl Rewrite for ast::Attribute { } // 1 = `[` - let shape = shape - .offset_left(prefix.len() + 1) - .max_width_error(shape.width, self.span)?; + let shape = shape.offset_left(prefix.len() + 1, self.span)?; Ok(meta.rewrite_result(context, shape).map_or_else( |_| snippet.to_owned(), |rw| match &self.kind { @@ -535,10 +528,10 @@ pub(crate) trait MetaVisitor<'ast> { fn visit_meta_list( &mut self, _meta_item: &'ast ast::MetaItem, - list: &'ast [ast::NestedMetaItem], + list: &'ast [ast::MetaItemInner], ) { for nm in list { - self.visit_nested_meta_item(nm); + self.visit_meta_item_inner(nm); } } @@ -551,10 +544,10 @@ pub(crate) trait MetaVisitor<'ast> { ) { } - fn visit_nested_meta_item(&mut self, nm: &'ast ast::NestedMetaItem) { + fn visit_meta_item_inner(&mut self, nm: &'ast ast::MetaItemInner) { match nm { - ast::NestedMetaItem::MetaItem(ref meta_item) => self.visit_meta_item(meta_item), - ast::NestedMetaItem::Lit(ref lit) => self.visit_meta_item_lit(lit), + ast::MetaItemInner::MetaItem(ref meta_item) => self.visit_meta_item(meta_item), + ast::MetaItemInner::Lit(ref lit) => self.visit_meta_item_lit(lit), } } diff --git a/src/bin/main.rs b/src/bin/main.rs index fe24276311e..b72bad873d9 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -135,6 +135,12 @@ fn make_opts() -> Options { "Rust edition to use", "[2015|2018|2021|2024]", ); + opts.optopt( + "", + "style-edition", + "The edition of the Style Guide (unstable).", + "[2015|2018|2021|2024]", + ); opts.optopt( "", "color", @@ -186,12 +192,6 @@ fn make_opts() -> Options { "skip-children", "Don't reformat child modules (unstable).", ); - opts.optopt( - "", - "style-edition", - "The edition of the Style Guide (unstable).", - "[2015|2018|2021|2024]", - ); } opts.optflag("v", "verbose", "Print verbose output"); @@ -568,10 +568,6 @@ impl GetOptsOptions { if let Some(ref file_lines) = matches.opt_str("file-lines") { options.file_lines = file_lines.parse()?; } - if let Some(ref edition_str) = matches.opt_str("style-edition") { - options.style_edition = - Some(style_edition_from_style_edition_str(edition_str)?); - } } else { let mut unstable_options = vec![]; if matches.opt_present("skip-children") { @@ -583,9 +579,6 @@ impl GetOptsOptions { if matches.opt_present("file-lines") { unstable_options.push("`--file-lines`"); } - if matches.opt_present("style-edition") { - unstable_options.push("`--style-edition`"); - } if !unstable_options.is_empty() { let s = if unstable_options.len() == 1 { "" } else { "s" }; return Err(format_err!( @@ -635,6 +628,10 @@ impl GetOptsOptions { options.edition = Some(edition_from_edition_str(edition_str)?); } + if let Some(ref edition_str) = matches.opt_str("style-edition") { + options.style_edition = Some(style_edition_from_style_edition_str(edition_str)?); + } + if matches.opt_present("backup") { options.backup = true; } diff --git a/src/chains.rs b/src/chains.rs index fd2ef9cb1db..50a129b695f 100644 --- a/src/chains.rs +++ b/src/chains.rs @@ -67,7 +67,9 @@ use crate::config::{IndentStyle, StyleEdition}; use crate::expr::rewrite_call; use crate::lists::extract_pre_comment; use crate::macros::convert_try_mac; -use crate::rewrite::{Rewrite, RewriteContext, RewriteError, RewriteErrorExt, RewriteResult}; +use crate::rewrite::{ + ExceedsMaxWidthError, Rewrite, RewriteContext, RewriteError, RewriteErrorExt, RewriteResult, +}; use crate::shape::Shape; use crate::source_map::SpanUtils; use crate::utils::{ @@ -127,14 +129,15 @@ fn get_visual_style_child_shape( shape: Shape, offset: usize, parent_overflowing: bool, -) -> Option { + span: Span, +) -> Result { if !parent_overflowing { shape .with_max_width(context.config) - .offset_left(offset) + .offset_left(offset, span) .map(|s| s.visual_indent(0)) } else { - Some(shape.visual_indent(offset)) + Ok(shape.visual_indent(offset)) } } @@ -261,7 +264,7 @@ impl ChainItemKind { return ( ChainItemKind::Parent { expr: expr.clone(), - parens: is_method_call_receiver && should_add_parens(expr), + parens: is_method_call_receiver && should_add_parens(expr, context), }, expr.span, ); @@ -280,9 +283,7 @@ impl Rewrite for ChainItem { } fn rewrite_result(&self, context: &RewriteContext<'_>, shape: Shape) -> RewriteResult { - let shape = shape - .sub_width(self.tries) - .max_width_error(shape.width, self.span)?; + let shape = shape.sub_width(self.tries, self.span)?; let rewrite = match self.kind { ChainItemKind::Parent { ref expr, @@ -559,9 +560,7 @@ impl Rewrite for Chain { let full_span = self.parent.span.with_hi(children_span.hi()); // Decide how to layout the rest of the chain. - let child_shape = formatter - .child_shape(context, shape) - .max_width_error(shape.width, children_span)?; + let child_shape = formatter.child_shape(context, shape, children_span)?; formatter.format_children(context, child_shape)?; formatter.format_last_child(context, shape, child_shape)?; @@ -590,7 +589,12 @@ trait ChainFormatter { context: &RewriteContext<'_>, shape: Shape, ) -> Result<(), RewriteError>; - fn child_shape(&self, context: &RewriteContext<'_>, shape: Shape) -> Option; + fn child_shape( + &self, + context: &RewriteContext<'_>, + shape: Shape, + span: Span, + ) -> Result; fn format_children( &mut self, context: &RewriteContext<'_>, @@ -720,17 +724,11 @@ impl<'a> ChainFormatterShared<'a> { && self.rewrites.iter().all(|s| !s.contains('\n')) && one_line_budget > 0; let last_shape = if all_in_one_line { - shape - .sub_width(last.tries) - .max_width_error(shape.width, last.span)? + shape.sub_width(last.tries, last.span)? } else if extendable { - child_shape - .sub_width(last.tries) - .max_width_error(child_shape.width, last.span)? + child_shape.sub_width(last.tries, last.span)? } else { - child_shape - .sub_width(shape.rhs_overhead(context.config) + last.tries) - .max_width_error(child_shape.width, last.span)? + child_shape.sub_width(shape.rhs_overhead(context.config) + last.tries, last.span)? }; let mut last_subexpr_str = None; @@ -738,11 +736,11 @@ impl<'a> ChainFormatterShared<'a> { // First we try to 'overflow' the last child and see if it looks better than using // vertical layout. let one_line_shape = if context.use_block_indent() { - last_shape.offset_left(almost_total) + last_shape.offset_left_opt(almost_total) } else { last_shape .visual_indent(almost_total) - .sub_width(almost_total) + .sub_width_opt(almost_total) }; if let Some(one_line_shape) = one_line_shape { @@ -760,9 +758,10 @@ impl<'a> ChainFormatterShared<'a> { // layout, just by looking at the overflowed rewrite. Now we rewrite the // last child on its own line, and compare two rewrites to choose which is // better. - let last_shape = child_shape - .sub_width(shape.rhs_overhead(context.config) + last.tries) - .max_width_error(child_shape.width, last.span)?; + let last_shape = child_shape.sub_width( + shape.rhs_overhead(context.config) + last.tries, + last.span, + )?; match last.rewrite_result(context, last_shape) { Ok(ref new_rw) if !could_fit_single_line => { last_subexpr_str = Some(new_rw.clone()); @@ -787,9 +786,7 @@ impl<'a> ChainFormatterShared<'a> { let last_shape = if context.use_block_indent() { last_shape } else { - child_shape - .sub_width(shape.rhs_overhead(context.config) + last.tries) - .max_width_error(child_shape.width, last.span)? + child_shape.sub_width(shape.rhs_overhead(context.config) + last.tries, last.span)? }; let last_subexpr_str = @@ -863,9 +860,7 @@ impl<'a> ChainFormatter for ChainFormatterBlock<'a> { if let ChainItemKind::Comment(..) = item.kind { break; } - let shape = shape - .offset_left(root_rewrite.len()) - .max_width_error(shape.width, item.span)?; + let shape = shape.offset_left(root_rewrite.len(), item.span)?; match &item.rewrite_result(context, shape) { Ok(rewrite) => root_rewrite.push_str(rewrite), Err(_) => break, @@ -883,9 +878,14 @@ impl<'a> ChainFormatter for ChainFormatterBlock<'a> { Ok(()) } - fn child_shape(&self, context: &RewriteContext<'_>, shape: Shape) -> Option { + fn child_shape( + &self, + context: &RewriteContext<'_>, + shape: Shape, + _span: Span, + ) -> Result { let block_end = self.root_ends_with_block; - Some(get_block_child_shape(block_end, context, shape)) + Ok(get_block_child_shape(block_end, context, shape)) } fn format_children( @@ -955,8 +955,7 @@ impl<'a> ChainFormatter for ChainFormatterVisual<'a> { } let child_shape = parent_shape .visual_indent(self.offset) - .sub_width(self.offset) - .max_width_error(parent_shape.width, item.span)?; + .sub_width(self.offset, item.span)?; let rewrite = item.rewrite_result(context, child_shape)?; if filtered_str_fits(&rewrite, context.config.max_width(), shape) { root_rewrite.push_str(&rewrite); @@ -975,13 +974,19 @@ impl<'a> ChainFormatter for ChainFormatterVisual<'a> { Ok(()) } - fn child_shape(&self, context: &RewriteContext<'_>, shape: Shape) -> Option { + fn child_shape( + &self, + context: &RewriteContext<'_>, + shape: Shape, + span: Span, + ) -> Result { get_visual_style_child_shape( context, shape, self.offset, // TODO(calebcartwright): self.shared.permissibly_overflowing_parent, false, + span, ) } @@ -1044,12 +1049,12 @@ fn trim_tries(s: &str) -> String { /// 1. .method(); /// ``` /// Which all need parenthesis or a space before `.method()`. -fn should_add_parens(expr: &ast::Expr) -> bool { +fn should_add_parens(expr: &ast::Expr, context: &RewriteContext<'_>) -> bool { match expr.kind { - ast::ExprKind::Lit(ref lit) => crate::expr::lit_ends_in_dot(lit), + ast::ExprKind::Lit(ref lit) => crate::expr::lit_ends_in_dot(lit, context), ast::ExprKind::Closure(ref cl) => match cl.body.kind { ast::ExprKind::Range(_, _, ast::RangeLimits::HalfOpen) => true, - ast::ExprKind::Lit(ref lit) => crate::expr::lit_ends_in_dot(lit), + ast::ExprKind::Lit(ref lit) => crate::expr::lit_ends_in_dot(lit, context), _ => false, }, _ => false, diff --git a/src/closures.rs b/src/closures.rs index a37b47e3bc9..296b7407e40 100644 --- a/src/closures.rs +++ b/src/closures.rs @@ -1,4 +1,4 @@ -use rustc_ast::{ast, ptr}; +use rustc_ast::{Label, ast, ptr}; use rustc_span::Span; use thin_vec::thin_vec; use tracing::debug; @@ -53,9 +53,7 @@ pub(crate) fn rewrite_closure( shape, )?; // 1 = space between `|...|` and body. - let body_shape = shape - .offset_left(extra_offset) - .max_width_error(shape.width, span)?; + let body_shape = shape.offset_left(extra_offset, span)?; if let ast::ExprKind::Block(ref block, _) = body.kind { // The body of the closure is an empty block. @@ -74,7 +72,7 @@ pub(crate) fn rewrite_closure( result.or_else(|_| { // Either we require a block, or tried without and failed. - rewrite_closure_block(block, &prefix, context, body_shape) + rewrite_closure_block(body, &prefix, context, body_shape) }) } else { rewrite_closure_expr(body, &prefix, context, body_shape).or_else(|_| { @@ -106,8 +104,8 @@ fn get_inner_expr<'a>( prefix: &str, context: &RewriteContext<'_>, ) -> &'a ast::Expr { - if let ast::ExprKind::Block(ref block, _) = expr.kind { - if !needs_block(block, prefix, context) { + if let ast::ExprKind::Block(ref block, ref label) = expr.kind { + if !needs_block(block, label, prefix, context) { // block.stmts.len() == 1 except with `|| {{}}`; // https://github.com/rust-lang/rustfmt/issues/3844 if let Some(expr) = block.stmts.first().and_then(stmt_expr) { @@ -120,7 +118,12 @@ fn get_inner_expr<'a>( } // Figure out if a block is necessary. -fn needs_block(block: &ast::Block, prefix: &str, context: &RewriteContext<'_>) -> bool { +fn needs_block( + block: &ast::Block, + label: &Option