Add clang-format to the external tool check

Author: dianqk

Cargo.lock

@@ -5621,6 +5621,7 @@ version = "0.1.0"
 dependencies = [
  "build_helper",
  "cargo_metadata 0.15.4",
+ "diff",
  "fluent-syntax",
  "ignore",
  "miropt-test-tools",

src/tools/tidy/Cargo.toml

@@ -15,6 +15,7 @@ semver = "1.0"
 termcolor = "1.1.3"
 rustc-hash = "1.1.0"
 fluent-syntax = "0.11.1"
+diff = "0.1.13"
 
 [[bin]]
 name = "rust-tidy"

src/tools/tidy/src/ext_tool_checks.rs

@@ -17,6 +17,7 @@
 //! 3. Print the output of the given command. If it fails and `TIDY_PRINT_DIFF`
 //!    is set, rerun the tool to print a suggestion diff (for e.g. CI)
 
+use std::collections::VecDeque;
 use std::ffi::OsStr;
 use std::fmt;
 use std::fs;
@@ -39,6 +40,85 @@ const BLACK_CONFIG_PATH: &[&str] = &["src", "tools", "tidy", "config", "black.to
 const RUFF_CACH_PATH: &[&str] = &["cache", "ruff_cache"];
 const PIP_REQ_PATH: &[&str] = &["src", "tools", "tidy", "config", "requirements.txt"];
 
+#[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> {
+    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
+}
+
 pub fn check(
     root_path: &Path,
     outdir: &Path,
@@ -73,6 +153,8 @@ fn check_impl(
     let python_fmt = lint_args.contains(&"py:fmt") || python_all;
     let shell_all = lint_args.contains(&"shell");
     let shell_lint = lint_args.contains(&"shell:lint") || shell_all;
+    let clang_all = lint_args.contains(&"clang");
+    let clang_fmt = lint_args.contains(&"clang:fmt") || clang_all;
 
     let mut py_path = None;
 
@@ -81,7 +163,7 @@ fn check_impl(
         .map(OsStr::new)
         .partition(|arg| arg.to_str().is_some_and(|s| s.starts_with('-')));
 
-    if python_lint || python_fmt {
+    if python_lint || python_fmt || clang_fmt {
         let venv_path = outdir.join("venv");
         let mut reqs_path = root_path.to_owned();
         reqs_path.extend(PIP_REQ_PATH);
@@ -111,13 +193,13 @@ fn check_impl(
 
         let mut args = merge_args(&cfg_args_ruff, &file_args_ruff);
         args.insert(0, "check".as_ref());
-        let res = py_runner(py_path.as_ref().unwrap(), "ruff", &args);
+        let res = py_runner(py_path.as_ref().unwrap(), true, None, "ruff", &args);
 
         if res.is_err() && show_diff {
             eprintln!("\npython linting failed! Printing diff suggestions:");
 
             args.insert(1, "--diff".as_ref());
-            let _ = py_runner(py_path.as_ref().unwrap(), "ruff", &args);
+            let _ = py_runner(py_path.as_ref().unwrap(), true, None, "ruff", &args);
         }
         // Rethrow error
         let _ = res?;
@@ -144,13 +226,78 @@ fn check_impl(
         }
 
         let mut args = merge_args(&cfg_args_black, &file_args_black);
-        let res = py_runner(py_path.as_ref().unwrap(), "black", &args);
+        let res = py_runner(py_path.as_ref().unwrap(), true, None, "black", &args);
 
         if res.is_err() && show_diff {
             eprintln!("\npython formatting does not match! Printing diff:");
 
             args.insert(0, "--diff".as_ref());
-            let _ = py_runner(py_path.as_ref().unwrap(), "black", &args);
+            let _ = py_runner(py_path.as_ref().unwrap(), true, None, "black", &args);
+        }
+        // Rethrow error
+        let _ = res?;
+    }
+
+    if clang_fmt {
+        let mut cfg_args_clang_format = cfg_args.clone();
+        let mut file_args_clang_format = file_args.clone();
+        let config_path = root_path.join(".clang-format");
+        let config_file_arg = format!("file:{}", config_path.display());
+        cfg_args_clang_format.extend(&["--style".as_ref(), config_file_arg.as_ref()]);
+        if bless {
+            eprintln!("formatting C++ files");
+            cfg_args_clang_format.push("-i".as_ref());
+        } else {
+            eprintln!("checking C++ file formatting");
+            cfg_args_clang_format.extend(&["--dry-run".as_ref(), "--Werror".as_ref()]);
+        }
+        let files;
+        if file_args_clang_format.is_empty() {
+            let llvm_wrapper = root_path.join("compiler/rustc_llvm/llvm-wrapper");
+            files = find_with_extension(
+                root_path,
+                Some(llvm_wrapper.as_path()),
+                &[OsStr::new("h"), OsStr::new("cpp")],
+            )?;
+            file_args_clang_format.extend(files.iter().map(|p| p.as_os_str()));
+        }
+        let args = merge_args(&cfg_args_clang_format, &file_args_clang_format);
+        let res = py_runner(py_path.as_ref().unwrap(), false, None, "clang-format", &args);
+
+        if res.is_err() && show_diff {
+            eprintln!("\nclang-format linting failed! Printing diff suggestions:");
+
+            let mut cfg_args_clang_format_diff = cfg_args.clone();
+            cfg_args_clang_format_diff.extend(&["--style".as_ref(), config_file_arg.as_ref()]);
+            for file in file_args_clang_format {
+                let mut expected = String::new();
+                let mut diff_args = cfg_args_clang_format_diff.clone();
+                diff_args.push(file);
+                let _ = py_runner(
+                    py_path.as_ref().unwrap(),
+                    false,
+                    Some(&mut expected),
+                    "clang-format",
+                    &diff_args,
+                );
+                if expected.is_empty() {
+                    eprintln!("failed to obtain the expected content");
+                    continue;
+                }
+                let actual = std::fs::read_to_string(file).expect("failed to read the C++ file");
+                if expected != actual {
+                    for result in make_diff(&expected, &actual, 3) {
+                        eprintln!("Diff in {:?} at line {}:", file, result.line_number);
+                        for line in result.lines {
+                            match line {
+                                DiffLine::Expected(str) => eprintln!("-{}", str),
+                                DiffLine::Context(str) => eprintln!(" {}", str),
+                                DiffLine::Resulting(str) => eprintln!("+{}", str),
+                            }
+                        }
+                    }
+                }
+            }
         }
         // Rethrow error
         let _ = res?;
@@ -162,7 +309,7 @@ fn check_impl(
         let mut file_args_shc = file_args.clone();
         let files;
         if file_args_shc.is_empty() {
-            files = find_with_extension(root_path, "sh")?;
+            files = find_with_extension(root_path, None, &[OsStr::new("sh")])?;
             file_args_shc.extend(files.iter().map(|p| p.as_os_str()));
         }
 
@@ -181,8 +328,29 @@ fn merge_args<'a>(cfg_args: &[&'a OsStr], file_args: &[&'a OsStr]) -> Vec<&'a Os
 }
 
 /// Run a python command with given arguments. `py_path` should be a virtualenv.
-fn py_runner(py_path: &Path, bin: &'static str, args: &[&OsStr]) -> Result<(), Error> {
-    let status = Command::new(py_path).arg("-m").arg(bin).args(args).status()?;
+fn py_runner(
+    py_path: &Path,
+    as_module: bool,
+    stdout: Option<&mut String>,
+    bin: &'static str,
+    args: &[&OsStr],
+) -> Result<(), Error> {
+    let mut cmd = Command::new(py_path);
+    if as_module {
+        cmd.arg("-m").arg(bin).args(args);
+    } else {
+        let bin_path = py_path.with_file_name(bin);
+        cmd.arg(bin_path).args(args);
+    }
+    let status = if let Some(stdout) = stdout {
+        let output = cmd.output()?;
+        if let Ok(s) = std::str::from_utf8(&output.stdout) {
+            stdout.push_str(s);
+        }
+        output.status
+    } else {
+        cmd.status()?
+    };
     if status.success() { Ok(()) } else { Err(Error::FailedCheck(bin)) }
 }
 
@@ -357,7 +525,11 @@ fn shellcheck_runner(args: &[&OsStr]) -> Result<(), Error> {
 }
 
 /// Check git for tracked files matching an extension
-fn find_with_extension(root_path: &Path, extension: &str) -> Result<Vec<PathBuf>, Error> {
+fn find_with_extension(
+    root_path: &Path,
+    find_dir: Option<&Path>,
+    extensions: &[&OsStr],
+) -> Result<Vec<PathBuf>, Error> {
     // Untracked files show up for short status and are indicated with a leading `?`
     // -C changes git to be as if run from that directory
     let stat_output =
@@ -368,14 +540,24 @@ fn find_with_extension(root_path: &Path, extension: &str) -> Result<Vec<PathBuf>
     }
 
     let mut output = Vec::new();
-    let binding = Command::new("git").arg("-C").arg(root_path).args(["ls-files"]).output()?;
+    let binding = {
+        let mut command = Command::new("git");
+        command.arg("-C").arg(root_path).args(["ls-files"]);
+        if let Some(find_dir) = find_dir {
+            command.arg(find_dir);
+        }
+        command.output()?
+    };
     let tracked = String::from_utf8_lossy(&binding.stdout);
 
     for line in tracked.lines() {
         let line = line.trim();
         let path = Path::new(line);
 
-        if path.extension() == Some(OsStr::new(extension)) {
+        let Some(ref extension) = path.extension() else {
+            continue;
+        };
+        if extensions.contains(extension) {
             output.push(path.to_owned());
         }
     }