Skip to content

Commit ddeea97

Browse files
committed
Auto merge of rust-lang#6750 - flip1995:lintcheck_options, r=matthiaskrgr
Lintcheck and an options for command line options Make it possible to add command line options to the clippy invocation of the lintcheck-tool changelog: none r? `@matthiaskrgr` I found that this will be really helpful if we use a separate repository and want to maintain a all-lints-passing list of crates. See my early experimentation here: https://github.com/flip1995/clippy-lintcheck ``` git submodule update --init cargo run -- --mode=all ``` Will run the lintcheck tool on all the specified crates in `config/` in that repository.
2 parents 877be18 + 79d7f4c commit ddeea97

File tree

2 files changed

+112
-51
lines changed

2 files changed

+112
-51
lines changed

clippy_dev/README.md

+59-34
Original file line numberDiff line numberDiff line change
@@ -1,52 +1,77 @@
1-
# Clippy Dev Tool
1+
# Clippy Dev Tool
22

3-
The Clippy Dev Tool is a tool to ease Clippy development, similar to `rustc`s `x.py`.
3+
The Clippy Dev Tool is a tool to ease Clippy development, similar to `rustc`s
4+
`x.py`.
45

56
Functionalities (incomplete):
67

78
## `lintcheck`
8-
Runs clippy on a fixed set of crates read from `clippy_dev/lintcheck_crates.toml`
9-
and saves logs of the lint warnings into the repo.
10-
We can then check the diff and spot new or disappearing warnings.
9+
10+
Runs clippy on a fixed set of crates read from
11+
`clippy_dev/lintcheck_crates.toml` and saves logs of the lint warnings into the
12+
repo. We can then check the diff and spot new or disappearing warnings.
1113

1214
From the repo root, run:
13-
````
15+
16+
```
1417
cargo run --target-dir clippy_dev/target --package clippy_dev \
1518
--bin clippy_dev --manifest-path clippy_dev/Cargo.toml --features lintcheck -- lintcheck
16-
````
19+
```
20+
1721
or
18-
````
22+
23+
```
1924
cargo dev-lintcheck
20-
````
25+
```
2126

22-
By default the logs will be saved into `lintcheck-logs/lintcheck_crates_logs.txt`.
27+
By default the logs will be saved into
28+
`lintcheck-logs/lintcheck_crates_logs.txt`.
2329

24-
You can set a custom sources.toml by adding `--crates-toml custom.toml` or using `LINTCHECK_TOML="custom.toml"`
25-
where `custom.toml` must be a relative path from the repo root.
30+
You can set a custom sources.toml by adding `--crates-toml custom.toml` or using
31+
`LINTCHECK_TOML="custom.toml"` where `custom.toml` must be a relative path from
32+
the repo root.
2633

2734
The results will then be saved to `lintcheck-logs/custom_logs.toml`.
2835

2936
### Configuring the Crate Sources
3037

31-
The sources to check are saved in a `toml` file.
32-
There are three types of sources.
33-
A crates-io source:
34-
````toml
35-
bitflags = {name = "bitflags", versions = ['1.2.1']}
36-
````
37-
Requires a "name" and one or multiple "versions" to be checked.
38-
39-
A git source:
40-
````toml
41-
puffin = {name = "puffin", git_url = "https://github.com/EmbarkStudios/puffin", git_hash = "02dd4a3"}
42-
````
43-
Requires a name, the url to the repo and unique identifier of a commit,
44-
branch or tag which is checked out before linting.
45-
There is no way to always check `HEAD` because that would lead to changing lint-results as the repo would get updated.
46-
If `git_url` or `git_hash` is missing, an error will be thrown.
47-
48-
A local dependency:
49-
````toml
50-
clippy = {name = "clippy", path = "/home/user/clippy"}
51-
````
52-
For when you want to add a repository that is not published yet.
38+
The sources to check are saved in a `toml` file. There are three types of
39+
sources.
40+
41+
1. Crates-io Source
42+
43+
```toml
44+
bitflags = {name = "bitflags", versions = ['1.2.1']}
45+
```
46+
Requires a "name" and one or multiple "versions" to be checked.
47+
48+
2. `git` Source
49+
````toml
50+
puffin = {name = "puffin", git_url = "https://github.com/EmbarkStudios/puffin", git_hash = "02dd4a3"}
51+
````
52+
Requires a name, the url to the repo and unique identifier of a commit,
53+
branch or tag which is checked out before linting. There is no way to always
54+
check `HEAD` because that would lead to changing lint-results as the repo
55+
would get updated. If `git_url` or `git_hash` is missing, an error will be
56+
thrown.
57+
58+
3. Local Dependency
59+
```toml
60+
clippy = {name = "clippy", path = "/home/user/clippy"}
61+
```
62+
For when you want to add a repository that is not published yet.
63+
64+
#### Command Line Options (optional)
65+
66+
```toml
67+
bitflags = {name = "bitflags", versions = ['1.2.1'], options = ['-Wclippy::pedantic', '-Wclippy::cargo']}
68+
```
69+
70+
It is possible to specify command line options for each crate. This makes it
71+
possible to only check a crate for certain lint groups. If no options are
72+
specified, the lint groups `clippy::all`, `clippy::pedantic`, and
73+
`clippy::cargo` are checked. If an empty array is specified only `clippy::all`
74+
is checked.
75+
76+
**Note:** `-Wclippy::all` is always enabled by default, unless `-Aclippy::all`
77+
is explicitly specified in the options.

clippy_dev/src/lintcheck.rs

+53-17
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,29 @@ struct TomlCrate {
3232
git_url: Option<String>,
3333
git_hash: Option<String>,
3434
path: Option<String>,
35+
options: Option<Vec<String>>,
3536
}
3637

3738
/// Represents an archive we download from crates.io, or a git repo, or a local repo/folder
3839
/// Once processed (downloaded/extracted/cloned/copied...), this will be translated into a `Crate`
3940
#[derive(Debug, Serialize, Deserialize, Eq, Hash, PartialEq)]
4041
enum CrateSource {
41-
CratesIo { name: String, version: String },
42-
Git { name: String, url: String, commit: String },
43-
Path { name: String, path: PathBuf },
42+
CratesIo {
43+
name: String,
44+
version: String,
45+
options: Option<Vec<String>>,
46+
},
47+
Git {
48+
name: String,
49+
url: String,
50+
commit: String,
51+
options: Option<Vec<String>>,
52+
},
53+
Path {
54+
name: String,
55+
path: PathBuf,
56+
options: Option<Vec<String>>,
57+
},
4458
}
4559

4660
/// Represents the actual source code of a crate that we ran "cargo clippy" on
@@ -50,6 +64,7 @@ struct Crate {
5064
name: String,
5165
// path to the extracted sources that clippy can check
5266
path: PathBuf,
67+
options: Option<Vec<String>>,
5368
}
5469

5570
/// A single warning that clippy issued while checking a `Crate`
@@ -81,7 +96,7 @@ impl CrateSource {
8196
/// copies a local folder
8297
fn download_and_extract(&self) -> Crate {
8398
match self {
84-
CrateSource::CratesIo { name, version } => {
99+
CrateSource::CratesIo { name, version, options } => {
85100
let extract_dir = PathBuf::from("target/lintcheck/crates");
86101
let krate_download_dir = PathBuf::from("target/lintcheck/downloads");
87102

@@ -113,9 +128,15 @@ impl CrateSource {
113128
version: version.clone(),
114129
name: name.clone(),
115130
path: extract_dir.join(format!("{}-{}/", name, version)),
131+
options: options.clone(),
116132
}
117133
},
118-
CrateSource::Git { name, url, commit } => {
134+
CrateSource::Git {
135+
name,
136+
url,
137+
commit,
138+
options,
139+
} => {
119140
let repo_path = {
120141
let mut repo_path = PathBuf::from("target/lintcheck/crates");
121142
// add a -git suffix in case we have the same crate from crates.io and a git repo
@@ -152,9 +173,10 @@ impl CrateSource {
152173
version: commit.clone(),
153174
name: name.clone(),
154175
path: repo_path,
176+
options: options.clone(),
155177
}
156178
},
157-
CrateSource::Path { name, path } => {
179+
CrateSource::Path { name, path, options } => {
158180
use fs_extra::dir;
159181

160182
// simply copy the entire directory into our target dir
@@ -183,6 +205,7 @@ impl CrateSource {
183205
version: String::from("local"),
184206
name: name.clone(),
185207
path: crate_root,
208+
options: options.clone(),
186209
}
187210
},
188211
}
@@ -198,18 +221,21 @@ impl Crate {
198221

199222
let shared_target_dir = clippy_project_root().join("target/lintcheck/shared_target_dir/");
200223

224+
let mut args = vec!["--", "--message-format=json", "--", "--cap-lints=warn"];
225+
226+
if let Some(options) = &self.options {
227+
for opt in options {
228+
args.push(opt);
229+
}
230+
} else {
231+
args.extend(&["-Wclippy::pedantic", "-Wclippy::cargo"])
232+
}
233+
201234
let all_output = std::process::Command::new(&cargo_clippy_path)
202235
.env("CARGO_TARGET_DIR", shared_target_dir)
203236
// lint warnings will look like this:
204237
// src/cargo/ops/cargo_compile.rs:127:35: warning: usage of `FromIterator::from_iter`
205-
.args(&[
206-
"--",
207-
"--message-format=json",
208-
"--",
209-
"--cap-lints=warn",
210-
"-Wclippy::pedantic",
211-
"-Wclippy::cargo",
212-
])
238+
.args(&args)
213239
.current_dir(&self.path)
214240
.output()
215241
.unwrap_or_else(|error| {
@@ -257,10 +283,14 @@ fn filter_clippy_warnings(line: &str) -> bool {
257283

258284
/// Builds clippy inside the repo to make sure we have a clippy executable we can use.
259285
fn build_clippy() {
260-
Command::new("cargo")
286+
let output = Command::new("cargo")
261287
.arg("build")
262288
.output()
263289
.expect("Failed to build clippy!");
290+
if !output.status.success() {
291+
eprintln!("Failed to compile Clippy");
292+
eprintln!("stderr: {}", String::from_utf8_lossy(&output.stderr))
293+
}
264294
}
265295

266296
/// Read a `toml` file and return a list of `CrateSources` that we want to check with clippy
@@ -289,6 +319,7 @@ fn read_crates(toml_path: Option<&str>) -> (String, Vec<CrateSource>) {
289319
crate_sources.push(CrateSource::Path {
290320
name: tk.name.clone(),
291321
path: PathBuf::from(path),
322+
options: tk.options.clone(),
292323
});
293324
}
294325

@@ -298,6 +329,7 @@ fn read_crates(toml_path: Option<&str>) -> (String, Vec<CrateSource>) {
298329
crate_sources.push(CrateSource::CratesIo {
299330
name: tk.name.clone(),
300331
version: ver.to_string(),
332+
options: tk.options.clone(),
301333
});
302334
})
303335
}
@@ -307,6 +339,7 @@ fn read_crates(toml_path: Option<&str>) -> (String, Vec<CrateSource>) {
307339
name: tk.name.clone(),
308340
url: tk.git_url.clone().unwrap(),
309341
commit: tk.git_hash.clone().unwrap(),
342+
options: tk.options.clone(),
310343
});
311344
}
312345
// if we have a version as well as a git data OR only one git data, something is funky
@@ -373,12 +406,14 @@ fn gather_stats(clippy_warnings: &[ClippyWarning]) -> String {
373406

374407
/// lintchecks `main()` function
375408
pub fn run(clap_config: &ArgMatches) {
376-
let cargo_clippy_path: PathBuf = PathBuf::from("target/debug/cargo-clippy");
377-
378409
println!("Compiling clippy...");
379410
build_clippy();
380411
println!("Done compiling");
381412

413+
let cargo_clippy_path: PathBuf = PathBuf::from("target/debug/cargo-clippy")
414+
.canonicalize()
415+
.expect("failed to canonicalize path to clippy binary");
416+
382417
// assert that clippy is found
383418
assert!(
384419
cargo_clippy_path.is_file(),
@@ -455,5 +490,6 @@ pub fn run(clap_config: &ArgMatches) {
455490
.for_each(|(cratename, msg)| text.push_str(&format!("{}: '{}'", cratename, msg)));
456491

457492
let file = format!("lintcheck-logs/{}_logs.txt", filename);
493+
println!("Writing logs to {}", file);
458494
write(file, text).unwrap();
459495
}

0 commit comments

Comments
 (0)