Skip to content

Commit 3267e05

Browse files
committed
Auto merge of #4232 - mikerite:dev-fmt-4, r=flip1995
Add dev fmt subcommand changelog: none
2 parents 10b915f + 76d66e6 commit 3267e05

File tree

8 files changed

+244
-53
lines changed

8 files changed

+244
-53
lines changed

appveyor.yml

+1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ install:
2222
- del rust-toolchain
2323
- cargo install rustup-toolchain-install-master --debug || echo "rustup-toolchain-install-master already installed"
2424
- rustup-toolchain-install-master %RUSTC_HASH% -f -n master
25+
- rustup component add rustfmt --toolchain nightly
2526
- rustup default master
2627
- set PATH=%PATH%;C:\Users\appveyor\.rustup\toolchains\master\bin
2728
- rustc -V

ci/base-tests.sh

-30
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ export CARGO_TARGET_DIR=`pwd`/target/
2424
# Perform various checks for lint registration
2525
./util/dev update_lints --check
2626
./util/dev --limit-stderr-length
27-
cargo +nightly fmt --all -- --check
2827

2928
# Check running clippy-driver without cargo
3029
(
@@ -50,32 +49,3 @@ cargo +nightly fmt --all -- --check
5049

5150
# TODO: CLIPPY_CONF_DIR / CARGO_MANIFEST_DIR
5251
)
53-
54-
# make sure tests are formatted
55-
56-
# some lints are sensitive to formatting, exclude some files
57-
tests_need_reformatting="false"
58-
# switch to nightly
59-
rustup override set nightly
60-
# avoid loop spam and allow cmds with exit status != 0
61-
set +ex
62-
63-
# Excluding `ice-3891.rs` because the code triggers a rustc parse error which
64-
# makes rustfmt fail.
65-
for file in `find tests -not -path "tests/ui/crashes/ice-3891.rs" | grep "\.rs$"` ; do
66-
rustfmt ${file} --check
67-
if [ $? -ne 0 ]; then
68-
echo "${file} needs reformatting!"
69-
tests_need_reformatting="true"
70-
fi
71-
done
72-
73-
set -ex # reset
74-
75-
if [ "${tests_need_reformatting}" == "true" ] ; then
76-
echo "Tests need reformatting!"
77-
exit 2
78-
fi
79-
80-
# switch back to master
81-
rustup override set master

clippy_dev/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,5 @@ clap = "2.33"
99
itertools = "0.8"
1010
regex = "1"
1111
lazy_static = "1.0"
12+
shell-escape = "0.1"
1213
walkdir = "2"

clippy_dev/src/fmt.rs

+171
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
use shell_escape::escape;
2+
use std::ffi::OsStr;
3+
use std::io;
4+
use std::path::{Path, PathBuf};
5+
use std::process::{self, Command};
6+
use walkdir::WalkDir;
7+
8+
#[derive(Debug)]
9+
pub enum CliError {
10+
CommandFailed(String),
11+
IoError(io::Error),
12+
ProjectRootNotFound,
13+
WalkDirError(walkdir::Error),
14+
}
15+
16+
impl From<io::Error> for CliError {
17+
fn from(error: io::Error) -> Self {
18+
CliError::IoError(error)
19+
}
20+
}
21+
22+
impl From<walkdir::Error> for CliError {
23+
fn from(error: walkdir::Error) -> Self {
24+
CliError::WalkDirError(error)
25+
}
26+
}
27+
28+
struct FmtContext {
29+
check: bool,
30+
verbose: bool,
31+
}
32+
33+
pub fn run(check: bool, verbose: bool) {
34+
fn try_run(context: &FmtContext) -> Result<bool, CliError> {
35+
let mut success = true;
36+
37+
let project_root = project_root()?;
38+
39+
success &= cargo_fmt(context, project_root.as_path())?;
40+
success &= cargo_fmt(context, &project_root.join("clippy_dev"))?;
41+
success &= cargo_fmt(context, &project_root.join("rustc_tools_util"))?;
42+
43+
for entry in WalkDir::new(project_root.join("tests")) {
44+
let entry = entry?;
45+
let path = entry.path();
46+
47+
if path.extension() != Some("rs".as_ref())
48+
|| entry.file_name() == "ice-3891.rs"
49+
// Avoid rustfmt bug rust-lang/rustfmt#1873
50+
|| cfg!(windows) && entry.file_name() == "implicit_hasher.rs"
51+
{
52+
continue;
53+
}
54+
55+
success &= rustfmt(context, &path)?;
56+
}
57+
58+
Ok(success)
59+
}
60+
61+
fn output_err(err: CliError) {
62+
match err {
63+
CliError::CommandFailed(command) => {
64+
eprintln!("error: A command failed! `{}`", command);
65+
},
66+
CliError::IoError(err) => {
67+
eprintln!("error: {}", err);
68+
},
69+
CliError::ProjectRootNotFound => {
70+
eprintln!("error: Can't determine root of project. Please run inside a Clippy working dir.");
71+
},
72+
CliError::WalkDirError(err) => {
73+
eprintln!("error: {}", err);
74+
},
75+
}
76+
}
77+
78+
let context = FmtContext { check, verbose };
79+
let result = try_run(&context);
80+
let code = match result {
81+
Ok(true) => 0,
82+
Ok(false) => {
83+
eprintln!();
84+
eprintln!("Formatting check failed.");
85+
eprintln!("Run `./util/dev fmt` to update formatting.");
86+
1
87+
},
88+
Err(err) => {
89+
output_err(err);
90+
1
91+
},
92+
};
93+
process::exit(code);
94+
}
95+
96+
fn format_command(program: impl AsRef<OsStr>, dir: impl AsRef<Path>, args: &[impl AsRef<OsStr>]) -> String {
97+
let arg_display: Vec<_> = args
98+
.iter()
99+
.map(|a| escape(a.as_ref().to_string_lossy()).to_owned())
100+
.collect();
101+
102+
format!(
103+
"cd {} && {} {}",
104+
escape(dir.as_ref().to_string_lossy()),
105+
escape(program.as_ref().to_string_lossy()),
106+
arg_display.join(" ")
107+
)
108+
}
109+
110+
fn exec(
111+
context: &FmtContext,
112+
program: impl AsRef<OsStr>,
113+
dir: impl AsRef<Path>,
114+
args: &[impl AsRef<OsStr>],
115+
) -> Result<bool, CliError> {
116+
if context.verbose {
117+
println!("{}", format_command(&program, &dir, args));
118+
}
119+
120+
let mut child = Command::new(&program).current_dir(&dir).args(args.iter()).spawn()?;
121+
let code = child.wait()?;
122+
let success = code.success();
123+
124+
if !context.check && !success {
125+
return Err(CliError::CommandFailed(format_command(&program, &dir, args)));
126+
}
127+
128+
Ok(success)
129+
}
130+
131+
fn cargo_fmt(context: &FmtContext, path: &Path) -> Result<bool, CliError> {
132+
let mut args = vec!["+nightly", "fmt", "--all"];
133+
if context.check {
134+
args.push("--");
135+
args.push("--check");
136+
}
137+
let success = exec(context, "cargo", path, &args)?;
138+
139+
Ok(success)
140+
}
141+
142+
fn rustfmt(context: &FmtContext, path: &Path) -> Result<bool, CliError> {
143+
let mut args = vec!["+nightly".as_ref(), path.as_os_str()];
144+
if context.check {
145+
args.push("--check".as_ref());
146+
}
147+
let success = exec(context, "rustfmt", std::env::current_dir()?, &args)?;
148+
if !success {
149+
eprintln!("rustfmt failed on {}", path.display());
150+
}
151+
Ok(success)
152+
}
153+
154+
fn project_root() -> Result<PathBuf, CliError> {
155+
let current_dir = std::env::current_dir()?;
156+
for path in current_dir.ancestors() {
157+
let result = std::fs::read_to_string(path.join("Cargo.toml"));
158+
if let Err(err) = &result {
159+
if err.kind() == io::ErrorKind::NotFound {
160+
continue;
161+
}
162+
}
163+
164+
let content = result?;
165+
if content.contains("[package]\nname = \"clippy\"") {
166+
return Ok(path.to_path_buf());
167+
}
168+
}
169+
170+
Err(CliError::ProjectRootNotFound)
171+
}

clippy_dev/src/main.rs

+32-8
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ extern crate regex;
44

55
use clap::{App, Arg, SubCommand};
66
use clippy_dev::*;
7+
8+
mod fmt;
79
mod stderr_length_check;
810

911
#[derive(PartialEq)]
@@ -14,6 +16,21 @@ enum UpdateMode {
1416

1517
fn main() {
1618
let matches = App::new("Clippy developer tooling")
19+
.subcommand(
20+
SubCommand::with_name("fmt")
21+
.about("Run rustfmt on all projects and tests")
22+
.arg(
23+
Arg::with_name("check")
24+
.long("check")
25+
.help("Use the rustfmt --check option"),
26+
)
27+
.arg(
28+
Arg::with_name("verbose")
29+
.short("v")
30+
.long("verbose")
31+
.help("Echo commands run"),
32+
),
33+
)
1734
.subcommand(
1835
SubCommand::with_name("update_lints")
1936
.about("Updates lint registration and information from the source code")
@@ -46,14 +63,21 @@ fn main() {
4663
if matches.is_present("limit-stderr-length") {
4764
stderr_length_check::check();
4865
}
49-
if let Some(matches) = matches.subcommand_matches("update_lints") {
50-
if matches.is_present("print-only") {
51-
print_lints();
52-
} else if matches.is_present("check") {
53-
update_lints(&UpdateMode::Check);
54-
} else {
55-
update_lints(&UpdateMode::Change);
56-
}
66+
67+
match matches.subcommand() {
68+
("fmt", Some(matches)) => {
69+
fmt::run(matches.is_present("check"), matches.is_present("verbose"));
70+
},
71+
("update_lints", Some(matches)) => {
72+
if matches.is_present("print-only") {
73+
print_lints();
74+
} else if matches.is_present("check") {
75+
update_lints(&UpdateMode::Check);
76+
} else {
77+
update_lints(&UpdateMode::Change);
78+
}
79+
},
80+
_ => {},
5781
}
5882
}
5983

clippy_dev/src/stderr_length_check.rs

+9-10
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,15 @@ pub fn check() {
2323
}
2424

2525
fn exceeding_stderr_files(files: impl Iterator<Item = walkdir::DirEntry>) -> impl Iterator<Item = String> {
26-
files
27-
.filter_map(|file| {
28-
let path = file.path().to_str().expect("Could not convert path to str").to_string();
29-
let linecount = count_linenumbers(&path);
30-
if linecount > LIMIT {
31-
Some(path)
32-
} else {
33-
None
34-
}
35-
})
26+
files.filter_map(|file| {
27+
let path = file.path().to_str().expect("Could not convert path to str").to_string();
28+
let linecount = count_linenumbers(&path);
29+
if linecount > LIMIT {
30+
Some(path)
31+
} else {
32+
None
33+
}
34+
})
3635
}
3736

3837
fn stderr_files() -> impl Iterator<Item = walkdir::DirEntry> {

doc/adding_lints.md

+7-5
Original file line numberDiff line numberDiff line change
@@ -345,16 +345,18 @@ list][lint_list].
345345

346346
### Running rustfmt
347347

348-
[Rustfmt](https://github.com/rust-lang/rustfmt) is a tool for formatting Rust code according
349-
to style guidelines. Your code has to be formatted by `rustfmt` before a PR can be merged.
348+
[Rustfmt](https://github.com/rust-lang/rustfmt) is a tool for formatting Rust
349+
code according to style guidelines. Your code has to be formatted by `rustfmt`
350+
before a PR can be merged. Clippy uses nightly `rustfmt` in the CI.
350351

351352
It can be installed via `rustup`:
352353

353354
```bash
354-
rustup component add rustfmt
355+
rustup component add rustfmt --toolchain=nightly
355356
```
356357

357-
Use `cargo fmt --all` to format the whole codebase.
358+
Use `./util/dev fmt` to format the whole codebase. Make sure that `rustfmt` is
359+
installed for the nightly toolchain.
358360

359361
### Debugging
360362

@@ -371,7 +373,7 @@ Before submitting your PR make sure you followed all of the basic requirements:
371373
- [ ] `cargo test` passes locally
372374
- [ ] Executed `util/dev update_lints`
373375
- [ ] Added lint documentation
374-
- [ ] Run `cargo fmt`
376+
- [ ] Run `./util/dev fmt`
375377

376378
### Cheatsheet
377379

tests/fmt.rs

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
#[test]
2+
fn fmt() {
3+
if option_env!("RUSTC_TEST_SUITE").is_some() {
4+
return;
5+
}
6+
7+
let root_dir = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR"));
8+
let dev_dir = root_dir.join("clippy_dev");
9+
let output = std::process::Command::new("cargo")
10+
.current_dir(dev_dir)
11+
.args(&["+nightly", "run", "--", "fmt", "--check"])
12+
.output()
13+
.unwrap();
14+
15+
println!("status: {}", output.status);
16+
println!("stdout: {}", String::from_utf8_lossy(&output.stdout));
17+
println!("stderr: {}", String::from_utf8_lossy(&output.stderr));
18+
19+
assert!(
20+
output.status.success(),
21+
"Formatting check failed. Run `./util/dev fmt` to update formatting."
22+
);
23+
}

0 commit comments

Comments
 (0)