Check C++ files when running ./x fmt

Author: dianqk

clang-format.toml

@@ -0,0 +1,10 @@
+# Files to ignore. Each entry uses gitignore syntax, but `!` prefixes aren't allowed.
+ignore = [
+    "/build/",
+    "/*-build/",
+    "/build-*/",
+    "/vendor/",
+    "/tests/",
+    "library",
+    "src",
+]

src/bootstrap/Cargo.lock

@@ -53,6 +53,7 @@ dependencies = [
  "clap",
  "clap_complete",
  "cmake",
+ "diff",
  "fd-lock",
  "filetime",
  "home",

src/bootstrap/Cargo.toml

@@ -65,6 +65,7 @@ xz2 = "0.1"
 
 # Dependencies needed by the build-metrics feature
 sysinfo = { version = "0.30", default-features = false, optional = true }
+diff = "0.1.13"
 
 [target.'cfg(windows)'.dependencies.junction]
 version = "1.0.0"

src/bootstrap/src/core/build_steps/format.rs

@@ -6,6 +6,7 @@ use build_helper::ci::CiEnv;
 use build_helper::git::get_git_modified_files;
 use ignore::WalkBuilder;
 use std::collections::VecDeque;
+use std::io::Read;
 use std::path::{Path, PathBuf};
 use std::process::{Command, Stdio};
 use std::sync::mpsc::SyncSender;
@@ -96,11 +97,185 @@ fn get_modified_rs_files(build: &Builder<'_>) -> Result<Option<Vec<String>>, Str
     get_git_modified_files(&build.config.git_config(), Some(&build.config.src), &vec!["rs"])
 }
 
+fn get_modified_c_family_files(build: &Builder<'_>) -> Result<Option<Vec<String>>, String> {
+    get_git_modified_files(
+        &build.config.git_config(),
+        Some(&build.config.src),
+        &vec!["cpp", "c", "hpp", "h"],
+    )
+}
+
 #[derive(serde_derive::Deserialize)]
 struct RustfmtConfig {
     ignore: Vec<String>,
 }
 
+#[derive(Debug, PartialEq)]
+pub enum DiffLine {
+    Context(String),
+    Expected(String),
+    Resulting(String),
+}
+
+#[derive(Debug, PartialEq)]
+pub struct Mismatch {
+    pub line_number: u32,
+    pub lines: Vec<DiffLine>,
+}
+
+impl Mismatch {
+    fn new(line_number: u32) -> Mismatch {
+        Mismatch { line_number, lines: Vec::new() }
+    }
+}
+
+// Produces a diff between the expected output and actual output.
+pub fn make_diff(expected: &str, actual: &str, context_size: usize) -> Vec<Mismatch> {
+    // FIXME: Can we stop copying this code?
+    let mut line_number = 1;
+    let mut context_queue: VecDeque<&str> = VecDeque::with_capacity(context_size);
+    let mut lines_since_mismatch = context_size + 1;
+    let mut results = Vec::new();
+    let mut mismatch = Mismatch::new(0);
+
+    for result in diff::lines(expected, actual) {
+        match result {
+            diff::Result::Left(str) => {
+                if lines_since_mismatch >= context_size && lines_since_mismatch > 0 {
+                    results.push(mismatch);
+                    mismatch = Mismatch::new(line_number - context_queue.len() as u32);
+                }
+
+                while let Some(line) = context_queue.pop_front() {
+                    mismatch.lines.push(DiffLine::Context(line.to_owned()));
+                }
+
+                mismatch.lines.push(DiffLine::Expected(str.to_owned()));
+                line_number += 1;
+                lines_since_mismatch = 0;
+            }
+            diff::Result::Right(str) => {
+                if lines_since_mismatch >= context_size && lines_since_mismatch > 0 {
+                    results.push(mismatch);
+                    mismatch = Mismatch::new(line_number - context_queue.len() as u32);
+                }
+
+                while let Some(line) = context_queue.pop_front() {
+                    mismatch.lines.push(DiffLine::Context(line.to_owned()));
+                }
+
+                mismatch.lines.push(DiffLine::Resulting(str.to_owned()));
+                lines_since_mismatch = 0;
+            }
+            diff::Result::Both(str, _) => {
+                if context_queue.len() >= context_size {
+                    let _ = context_queue.pop_front();
+                }
+
+                if lines_since_mismatch < context_size {
+                    mismatch.lines.push(DiffLine::Context(str.to_owned()));
+                } else if context_size > 0 {
+                    context_queue.push_back(str);
+                }
+
+                line_number += 1;
+                lines_since_mismatch += 1;
+            }
+        }
+    }
+
+    results.push(mismatch);
+    results.remove(0);
+
+    results
+}
+
+#[derive(serde_derive::Deserialize)]
+struct ClangFormatConfig {
+    ignore: Vec<String>,
+}
+
+fn get_clang_format() -> Option<PathBuf> {
+    std::env::var_os("PATH").and_then(|paths| {
+        std::env::split_paths(&paths)
+            .filter_map(|dir| {
+                let full_path = dir.join("clang-format");
+                if full_path.is_file() { Some(full_path) } else { None }
+            })
+            .next()
+    })
+}
+
+fn clang_format(
+    src: &Path,
+    clang_format: &Path,
+    path: &Path,
+    check: bool,
+) -> impl FnMut(bool) -> bool {
+    let mut cmd = Command::new(clang_format);
+    let clang_form_config = src.join(".clang-format");
+    // avoid the submodule config paths from coming into play,
+    // we only allow a single global config for the workspace for now
+    cmd.arg("--style").arg(format!("file:{}", clang_form_config.display()));
+    if !check {
+        cmd.arg("-i");
+    }
+    cmd.arg(path);
+    let cmd_debug = format!("{cmd:?}");
+    let mut cmd = cmd.stdout(Stdio::piped()).spawn().expect("running clang-format");
+    let path = path.to_path_buf();
+    // poor man's async: return a closure that'll wait for clang-format's completion
+    move |block: bool| -> bool {
+        // let mut cmd = cmd; // Move `cmd` into the closure
+        if !block {
+            match cmd.try_wait() {
+                Ok(Some(_)) => {}
+                _ => return false,
+            }
+        }
+        let mut expected = String::new();
+        if check {
+            if let Some(stdout) = &mut cmd.stdout {
+                stdout.read_to_string(&mut expected).unwrap();
+            }
+        }
+        let status = cmd.wait().unwrap();
+        if !status.success() {
+            eprintln!(
+                "Running `{}` failed.\nIf you're running `tidy`, \
+                        try again with `--bless`. Or, if you just want to format \
+                        code, run `./x.py fmt` instead.",
+                cmd_debug,
+            );
+            crate::exit!(1);
+        }
+        if check {
+            let actual = std::fs::read_to_string(&path).expect("Failed");
+            if expected != actual {
+                for result in make_diff(&expected, &actual, 3) {
+                    println!("Diff in {} at line {}:", path.display(), result.line_number);
+                    for line in result.lines {
+                        match line {
+                            DiffLine::Expected(str) => println!("-{}", str),
+                            DiffLine::Context(str) => println!(" {}", str),
+                            DiffLine::Resulting(str) => println!("+{}", str),
+                        }
+                    }
+                }
+                let cmd_debug_diff = format!("{} | diff -u {} -", cmd_debug, path.display());
+                eprintln!(
+                    "Running `{}` failed.\nIf you're running `tidy`, \
+                        try again with `--bless`. Or, if you just want to format \
+                        code, run `./x.py fmt` instead.",
+                    cmd_debug_diff,
+                );
+                crate::exit!(1);
+            }
+        }
+        true
+    }
+}
+
 // Prints output describing a collection of paths, with lines such as "formatted modified file
 // foo/bar/baz" or "skipped 20 untracked files".
 fn print_paths(verb: &str, adjective: Option<&str>, paths: &[String]) {
@@ -134,6 +309,23 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
     let mut builder = ignore::types::TypesBuilder::new();
     builder.add_defaults();
     builder.select("rust");
+
+    let mut c_family_builder = ignore::types::TypesBuilder::new();
+    c_family_builder.add_defaults();
+    c_family_builder.select("c");
+    c_family_builder.select("cpp");
+
+    // TODO: Allow to configure the path of clang-format?
+    let clang_format_path = get_clang_format();
+    let clang_format_available = clang_format_path.is_some();
+    // TODO: Check the clang-format version.
+    if !clang_format_available && CiEnv::is_ci() {
+        eprintln!("fmt error: Can't find `clang-format`.");
+        crate::exit!(1);
+    }
+    let mut c_family_override_builder = ignore::overrides::OverrideBuilder::new(&build.src);
+    let mut run_clang_format = clang_format_available;
+
     let matcher = builder.build().unwrap();
     let rustfmt_config = build.src.join("rustfmt.toml");
     if !rustfmt_config.exists() {
@@ -156,8 +348,29 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
             crate::exit!(1);
         } else {
             override_builder.add(&format!("!{ignore}")).expect(&ignore);
+            c_family_override_builder.add(&format!("!{ignore}")).expect(&ignore);
         }
     }
+
+    let clang_format_config = build.src.join("clang-format.toml");
+    if !clang_format_config.exists() {
+        eprintln!("fmt error: Not running formatting checks; clang-format.toml does not exist.");
+        eprintln!("fmt error: This may happen in distributed tarballs.");
+        return;
+    }
+    let clang_format_config = t!(std::fs::read_to_string(&clang_format_config));
+    let clang_format_config: ClangFormatConfig = t!(toml::from_str(&clang_format_config));
+    for ignore in clang_format_config.ignore {
+        if ignore.starts_with('!') {
+            eprintln!(
+                "fmt error: `!`-prefixed entries are not supported in clang-format.toml, sorry"
+            );
+            crate::exit!(1);
+        } else {
+            c_family_override_builder.add(&format!("!{ignore}")).expect(&ignore);
+        }
+    }
+
     let git_available = match Command::new("git")
         .arg("--version")
         .stdout(Stdio::null())
@@ -208,6 +421,9 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
                 // against anything like `compiler/rustc_foo/src/foo.rs`,
                 // preventing the latter from being formatted.
                 override_builder.add(&format!("!/{untracked_path}")).expect(&untracked_path);
+                c_family_override_builder
+                    .add(&format!("!/{untracked_path}"))
+                    .expect(&untracked_path);
             }
             if !all {
                 adjective = Some("modified");
@@ -224,6 +440,28 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
                         eprintln!("fmt warning: Falling back to formatting all files.");
                     }
                 }
+                match get_modified_c_family_files(build) {
+                    Ok(Some(files)) => {
+                        if !files.is_empty() && !clang_format_available {
+                            eprintln!(
+                                "fmt warning: Modified C++ files but couldn't find the available clang-format."
+                            );
+                        }
+                        run_clang_format = run_clang_format && !files.is_empty();
+                        for file in files {
+                            c_family_override_builder.add(&format!("/{file}")).expect(&file);
+                        }
+                    }
+                    Ok(None) => {
+                        eprintln!("no check");
+                        run_clang_format = false;
+                    }
+                    Err(err) => {
+                        eprintln!("fmt warning: Something went wrong running git commands:");
+                        eprintln!("fmt warning: {err}");
+                        eprintln!("fmt warning: Falling back to formatting all files.");
+                    }
+                }
             }
         } else {
             eprintln!("fmt: warning: Not in git tree. Skipping git-aware format checks");
@@ -241,6 +479,7 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
     assert!(rustfmt_path.exists(), "{}", rustfmt_path.display());
     let src = build.src.clone();
     let (tx, rx): (SyncSender<PathBuf>, _) = std::sync::mpsc::sync_channel(128);
+    let (c_family_tx, c_family_rx): (SyncSender<PathBuf>, _) = std::sync::mpsc::sync_channel(128);
     let walker = WalkBuilder::new(src.clone()).types(matcher).overrides(override_).build_parallel();
 
     // There is a lot of blocking involved in spawning a child process and reading files to format.
@@ -303,6 +542,72 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
             ignore::WalkState::Continue
         })
     });
+
+    if run_clang_format {
+        if let Some(clang_format_path) = clang_format_path {
+            let src = build.src.clone();
+            let c_family_walker = {
+                let c_family_override = c_family_override_builder.build().unwrap();
+                let c_family_matcher = c_family_builder.build().unwrap();
+                WalkBuilder::new(src.clone())
+                    .types(c_family_matcher)
+                    .overrides(c_family_override)
+                    .build_parallel()
+            };
+
+            let c_family_thread = std::thread::spawn(move || {
+                let mut children = VecDeque::new();
+                while let Ok(path) = c_family_rx.recv() {
+                    let child = clang_format(&src, &clang_format_path, &path, check);
+                    children.push_back(child);
+
+                    // Poll completion before waiting.
+                    for i in (0..children.len()).rev() {
+                        if children[i](false) {
+                            children.swap_remove_back(i);
+                            break;
+                        }
+                    }
+
+                    if children.len() >= max_processes {
+                        // Await oldest child.
+                        children.pop_front().unwrap()(true);
+                    }
+                }
+
+                // Await remaining children.
+                for mut child in children {
+                    child(true);
+                }
+            });
+
+            c_family_walker.run(|| {
+                let tx = c_family_tx.clone();
+                Box::new(move |entry| {
+                    let cwd = std::env::current_dir();
+                    let entry = t!(entry);
+                    if entry.file_type().map_or(false, |t| t.is_file()) {
+                        formatted_paths_ref.lock().unwrap().push({
+                            // `into_path` produces an absolute path. Try to strip `cwd` to get a shorter
+                            // relative path.
+                            let mut path = entry.clone().into_path();
+                            if let Ok(cwd) = cwd {
+                                if let Ok(path2) = path.strip_prefix(cwd) {
+                                    path = path2.to_path_buf();
+                                }
+                            }
+                            path.display().to_string()
+                        });
+                        t!(tx.send(entry.into_path()));
+                    }
+                    ignore::WalkState::Continue
+                })
+            });
+            drop(c_family_tx);
+            c_family_thread.join().unwrap();
+        }
+    }
+
     let mut paths = formatted_paths.into_inner().unwrap();
     paths.sort();
     print_paths(if check { "checked" } else { "formatted" }, adjective, &paths);