Skip to content

Commit de0ece2

Browse files
committed
Add clang-format to the external tool check
1 parent 9f1f6a3 commit de0ece2

File tree

3 files changed

+124
-12
lines changed

3 files changed

+124
-12
lines changed

Cargo.lock

+1
Original file line numberDiff line numberDiff line change
@@ -5627,6 +5627,7 @@ dependencies = [
56275627
"regex",
56285628
"rustc-hash",
56295629
"semver",
5630+
"similar",
56305631
"termcolor",
56315632
"walkdir",
56325633
]

src/tools/tidy/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ semver = "1.0"
1515
termcolor = "1.1.3"
1616
rustc-hash = "1.1.0"
1717
fluent-syntax = "0.11.1"
18+
similar = "2.5.0"
1819

1920
[[bin]]
2021
name = "rust-tidy"

src/tools/tidy/src/ext_tool_checks.rs

+122-12
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,8 @@ fn check_impl(
7373
let python_fmt = lint_args.contains(&"py:fmt") || python_all;
7474
let shell_all = lint_args.contains(&"shell");
7575
let shell_lint = lint_args.contains(&"shell:lint") || shell_all;
76+
let cpp_all = lint_args.contains(&"cpp");
77+
let cpp_fmt = lint_args.contains(&"cpp:fmt") || cpp_all;
7678

7779
let mut py_path = None;
7880

@@ -81,7 +83,7 @@ fn check_impl(
8183
.map(OsStr::new)
8284
.partition(|arg| arg.to_str().is_some_and(|s| s.starts_with('-')));
8385

84-
if python_lint || python_fmt {
86+
if python_lint || python_fmt || cpp_fmt {
8587
let venv_path = outdir.join("venv");
8688
let mut reqs_path = root_path.to_owned();
8789
reqs_path.extend(PIP_REQ_PATH);
@@ -111,13 +113,13 @@ fn check_impl(
111113

112114
let mut args = merge_args(&cfg_args_ruff, &file_args_ruff);
113115
args.insert(0, "check".as_ref());
114-
let res = py_runner(py_path.as_ref().unwrap(), "ruff", &args);
116+
let res = py_runner(py_path.as_ref().unwrap(), true, None, "ruff", &args);
115117

116118
if res.is_err() && show_diff {
117119
eprintln!("\npython linting failed! Printing diff suggestions:");
118120

119121
args.insert(1, "--diff".as_ref());
120-
let _ = py_runner(py_path.as_ref().unwrap(), "ruff", &args);
122+
let _ = py_runner(py_path.as_ref().unwrap(), true, None, "ruff", &args);
121123
}
122124
// Rethrow error
123125
let _ = res?;
@@ -144,13 +146,84 @@ fn check_impl(
144146
}
145147

146148
let mut args = merge_args(&cfg_args_black, &file_args_black);
147-
let res = py_runner(py_path.as_ref().unwrap(), "black", &args);
149+
let res = py_runner(py_path.as_ref().unwrap(), true, None, "black", &args);
148150

149151
if res.is_err() && show_diff {
150152
eprintln!("\npython formatting does not match! Printing diff:");
151153

152154
args.insert(0, "--diff".as_ref());
153-
let _ = py_runner(py_path.as_ref().unwrap(), "black", &args);
155+
let _ = py_runner(py_path.as_ref().unwrap(), true, None, "black", &args);
156+
}
157+
// Rethrow error
158+
let _ = res?;
159+
}
160+
161+
if cpp_fmt {
162+
let mut cfg_args_clang_format = cfg_args.clone();
163+
let mut file_args_clang_format = file_args.clone();
164+
let config_path = root_path.join(".clang-format");
165+
let config_file_arg = format!("file:{}", config_path.display());
166+
cfg_args_clang_format.extend(&["--style".as_ref(), config_file_arg.as_ref()]);
167+
if bless {
168+
eprintln!("formatting C++ files");
169+
cfg_args_clang_format.push("-i".as_ref());
170+
} else {
171+
eprintln!("checking C++ file formatting");
172+
cfg_args_clang_format.extend(&["--dry-run".as_ref(), "--Werror".as_ref()]);
173+
}
174+
let files;
175+
if file_args_clang_format.is_empty() {
176+
let llvm_wrapper = root_path.join("compiler/rustc_llvm/llvm-wrapper");
177+
files = find_with_extension(
178+
root_path,
179+
Some(llvm_wrapper.as_path()),
180+
&[OsStr::new("h"), OsStr::new("cpp")],
181+
)?;
182+
file_args_clang_format.extend(files.iter().map(|p| p.as_os_str()));
183+
}
184+
let args = merge_args(&cfg_args_clang_format, &file_args_clang_format);
185+
let res = py_runner(py_path.as_ref().unwrap(), false, None, "clang-format", &args);
186+
187+
if res.is_err() && show_diff {
188+
eprintln!("\nclang-format linting failed! Printing diff suggestions:");
189+
190+
let mut cfg_args_clang_format_diff = cfg_args.clone();
191+
cfg_args_clang_format_diff.extend(&["--style".as_ref(), config_file_arg.as_ref()]);
192+
for file in file_args_clang_format {
193+
let mut formatted = String::new();
194+
let mut diff_args = cfg_args_clang_format_diff.clone();
195+
diff_args.push(file);
196+
let _ = py_runner(
197+
py_path.as_ref().unwrap(),
198+
false,
199+
Some(&mut formatted),
200+
"clang-format",
201+
&diff_args,
202+
);
203+
if formatted.is_empty() {
204+
eprintln!(
205+
"failed to obtain the formatted content for '{}'",
206+
file.to_string_lossy()
207+
);
208+
continue;
209+
}
210+
let actual = std::fs::read_to_string(file).unwrap_or_else(|e| {
211+
panic!(
212+
"failed to read the C++ file at '{}' due to '{e}'",
213+
file.to_string_lossy()
214+
)
215+
});
216+
if formatted != actual {
217+
let diff = similar::TextDiff::from_lines(&actual, &formatted);
218+
eprintln!(
219+
"{}",
220+
diff.unified_diff().context_radius(4).header(
221+
&format!("{} (actual)", file.to_string_lossy()),
222+
&format!("{} (formatted)", file.to_string_lossy())
223+
)
224+
);
225+
}
226+
}
154227
}
155228
// Rethrow error
156229
let _ = res?;
@@ -162,7 +235,7 @@ fn check_impl(
162235
let mut file_args_shc = file_args.clone();
163236
let files;
164237
if file_args_shc.is_empty() {
165-
files = find_with_extension(root_path, "sh")?;
238+
files = find_with_extension(root_path, None, &[OsStr::new("sh")])?;
166239
file_args_shc.extend(files.iter().map(|p| p.as_os_str()));
167240
}
168241

@@ -181,8 +254,31 @@ fn merge_args<'a>(cfg_args: &[&'a OsStr], file_args: &[&'a OsStr]) -> Vec<&'a Os
181254
}
182255

183256
/// Run a python command with given arguments. `py_path` should be a virtualenv.
184-
fn py_runner(py_path: &Path, bin: &'static str, args: &[&OsStr]) -> Result<(), Error> {
185-
let status = Command::new(py_path).arg("-m").arg(bin).args(args).status()?;
257+
///
258+
/// Captures `stdout` to a string if provided, otherwise prints the output.
259+
fn py_runner(
260+
py_path: &Path,
261+
as_module: bool,
262+
stdout: Option<&mut String>,
263+
bin: &'static str,
264+
args: &[&OsStr],
265+
) -> Result<(), Error> {
266+
let mut cmd = Command::new(py_path);
267+
if as_module {
268+
cmd.arg("-m").arg(bin).args(args);
269+
} else {
270+
let bin_path = py_path.with_file_name(bin);
271+
cmd.arg(bin_path).args(args);
272+
}
273+
let status = if let Some(stdout) = stdout {
274+
let output = cmd.output()?;
275+
if let Ok(s) = std::str::from_utf8(&output.stdout) {
276+
stdout.push_str(s);
277+
}
278+
output.status
279+
} else {
280+
cmd.status()?
281+
};
186282
if status.success() { Ok(()) } else { Err(Error::FailedCheck(bin)) }
187283
}
188284

@@ -357,7 +453,11 @@ fn shellcheck_runner(args: &[&OsStr]) -> Result<(), Error> {
357453
}
358454

359455
/// Check git for tracked files matching an extension
360-
fn find_with_extension(root_path: &Path, extension: &str) -> Result<Vec<PathBuf>, Error> {
456+
fn find_with_extension(
457+
root_path: &Path,
458+
find_dir: Option<&Path>,
459+
extensions: &[&OsStr],
460+
) -> Result<Vec<PathBuf>, Error> {
361461
// Untracked files show up for short status and are indicated with a leading `?`
362462
// -C changes git to be as if run from that directory
363463
let stat_output =
@@ -368,15 +468,25 @@ fn find_with_extension(root_path: &Path, extension: &str) -> Result<Vec<PathBuf>
368468
}
369469

370470
let mut output = Vec::new();
371-
let binding = Command::new("git").arg("-C").arg(root_path).args(["ls-files"]).output()?;
471+
let binding = {
472+
let mut command = Command::new("git");
473+
command.arg("-C").arg(root_path).args(["ls-files"]);
474+
if let Some(find_dir) = find_dir {
475+
command.arg(find_dir);
476+
}
477+
command.output()?
478+
};
372479
let tracked = String::from_utf8_lossy(&binding.stdout);
373480

374481
for line in tracked.lines() {
375482
let line = line.trim();
376483
let path = Path::new(line);
377484

378-
if path.extension() == Some(OsStr::new(extension)) {
379-
output.push(path.to_owned());
485+
let Some(ref extension) = path.extension() else {
486+
continue;
487+
};
488+
if extensions.contains(extension) {
489+
output.push(root_path.join(path));
380490
}
381491
}
382492

0 commit comments

Comments
 (0)