Skip to content

Lintcheck and an options for command line options #6750

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 59 additions & 34 deletions clippy_dev/README.md
Original file line number Diff line number Diff line change
@@ -1,52 +1,77 @@
# Clippy Dev Tool
# Clippy Dev Tool

The Clippy Dev Tool is a tool to ease Clippy development, similar to `rustc`s `x.py`.
The Clippy Dev Tool is a tool to ease Clippy development, similar to `rustc`s
`x.py`.

Functionalities (incomplete):

## `lintcheck`
Runs clippy on a fixed set of crates read from `clippy_dev/lintcheck_crates.toml`
and saves logs of the lint warnings into the repo.
We can then check the diff and spot new or disappearing warnings.

Runs clippy on a fixed set of crates read from
`clippy_dev/lintcheck_crates.toml` and saves logs of the lint warnings into the
repo. We can then check the diff and spot new or disappearing warnings.

From the repo root, run:
````

```
cargo run --target-dir clippy_dev/target --package clippy_dev \
--bin clippy_dev --manifest-path clippy_dev/Cargo.toml --features lintcheck -- lintcheck
````
```

or
````

```
cargo dev-lintcheck
````
```

By default the logs will be saved into `lintcheck-logs/lintcheck_crates_logs.txt`.
By default the logs will be saved into
`lintcheck-logs/lintcheck_crates_logs.txt`.

You can set a custom sources.toml by adding `--crates-toml custom.toml` or using `LINTCHECK_TOML="custom.toml"`
where `custom.toml` must be a relative path from the repo root.
You can set a custom sources.toml by adding `--crates-toml custom.toml` or using
`LINTCHECK_TOML="custom.toml"` where `custom.toml` must be a relative path from
the repo root.

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

### Configuring the Crate Sources

The sources to check are saved in a `toml` file.
There are three types of sources.
A crates-io source:
````toml
bitflags = {name = "bitflags", versions = ['1.2.1']}
````
Requires a "name" and one or multiple "versions" to be checked.

A git source:
````toml
puffin = {name = "puffin", git_url = "https://github.com/EmbarkStudios/puffin", git_hash = "02dd4a3"}
````
Requires a name, the url to the repo and unique identifier of a commit,
branch or tag which is checked out before linting.
There is no way to always check `HEAD` because that would lead to changing lint-results as the repo would get updated.
If `git_url` or `git_hash` is missing, an error will be thrown.

A local dependency:
````toml
clippy = {name = "clippy", path = "/home/user/clippy"}
````
For when you want to add a repository that is not published yet.
The sources to check are saved in a `toml` file. There are three types of
sources.

1. Crates-io Source

```toml
bitflags = {name = "bitflags", versions = ['1.2.1']}
```
Requires a "name" and one or multiple "versions" to be checked.

2. `git` Source
````toml
puffin = {name = "puffin", git_url = "https://github.com/EmbarkStudios/puffin", git_hash = "02dd4a3"}
````
Requires a name, the url to the repo and unique identifier of a commit,
branch or tag which is checked out before linting. There is no way to always
check `HEAD` because that would lead to changing lint-results as the repo
would get updated. If `git_url` or `git_hash` is missing, an error will be
thrown.

3. Local Dependency
```toml
clippy = {name = "clippy", path = "/home/user/clippy"}
```
For when you want to add a repository that is not published yet.

#### Command Line Options (optional)

```toml
bitflags = {name = "bitflags", versions = ['1.2.1'], options = ['-Wclippy::pedantic', '-Wclippy::cargo']}
```

It is possible to specify command line options for each crate. This makes it
possible to only check a crate for certain lint groups. If no options are
specified, the lint groups `clippy::all`, `clippy::pedantic`, and
`clippy::cargo` are checked. If an empty array is specified only `clippy::all`
is checked.

**Note:** `-Wclippy::all` is always enabled by default, unless `-Aclippy::all`
is explicitly specified in the options.
70 changes: 53 additions & 17 deletions clippy_dev/src/lintcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,29 @@ struct TomlCrate {
git_url: Option<String>,
git_hash: Option<String>,
path: Option<String>,
options: Option<Vec<String>>,
}

/// Represents an archive we download from crates.io, or a git repo, or a local repo/folder
/// Once processed (downloaded/extracted/cloned/copied...), this will be translated into a `Crate`
#[derive(Debug, Serialize, Deserialize, Eq, Hash, PartialEq)]
enum CrateSource {
CratesIo { name: String, version: String },
Git { name: String, url: String, commit: String },
Path { name: String, path: PathBuf },
CratesIo {
name: String,
version: String,
options: Option<Vec<String>>,
},
Git {
name: String,
url: String,
commit: String,
options: Option<Vec<String>>,
},
Path {
name: String,
path: PathBuf,
options: Option<Vec<String>>,
},
}

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

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

Expand Down Expand Up @@ -113,9 +128,15 @@ impl CrateSource {
version: version.clone(),
name: name.clone(),
path: extract_dir.join(format!("{}-{}/", name, version)),
options: options.clone(),
}
},
CrateSource::Git { name, url, commit } => {
CrateSource::Git {
name,
url,
commit,
options,
} => {
let repo_path = {
let mut repo_path = PathBuf::from("target/lintcheck/crates");
// add a -git suffix in case we have the same crate from crates.io and a git repo
Expand Down Expand Up @@ -152,9 +173,10 @@ impl CrateSource {
version: commit.clone(),
name: name.clone(),
path: repo_path,
options: options.clone(),
}
},
CrateSource::Path { name, path } => {
CrateSource::Path { name, path, options } => {
use fs_extra::dir;

// simply copy the entire directory into our target dir
Expand Down Expand Up @@ -183,6 +205,7 @@ impl CrateSource {
version: String::from("local"),
name: name.clone(),
path: crate_root,
options: options.clone(),
}
},
}
Expand All @@ -198,18 +221,21 @@ impl Crate {

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

let mut args = vec!["--", "--message-format=json", "--", "--cap-lints=warn"];

if let Some(options) = &self.options {
for opt in options {
args.push(opt);
}
} else {
args.extend(&["-Wclippy::pedantic", "-Wclippy::cargo"])
}

let all_output = std::process::Command::new(&cargo_clippy_path)
.env("CARGO_TARGET_DIR", shared_target_dir)
// lint warnings will look like this:
// src/cargo/ops/cargo_compile.rs:127:35: warning: usage of `FromIterator::from_iter`
.args(&[
"--",
"--message-format=json",
"--",
"--cap-lints=warn",
"-Wclippy::pedantic",
"-Wclippy::cargo",
])
.args(&args)
.current_dir(&self.path)
.output()
.unwrap_or_else(|error| {
Expand Down Expand Up @@ -257,10 +283,14 @@ fn filter_clippy_warnings(line: &str) -> bool {

/// Builds clippy inside the repo to make sure we have a clippy executable we can use.
fn build_clippy() {
Command::new("cargo")
let output = Command::new("cargo")
.arg("build")
.output()
.expect("Failed to build clippy!");
if !output.status.success() {
eprintln!("Failed to compile Clippy");
eprintln!("stderr: {}", String::from_utf8_lossy(&output.stderr))
}
}

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

Expand All @@ -298,6 +329,7 @@ fn read_crates(toml_path: Option<&str>) -> (String, Vec<CrateSource>) {
crate_sources.push(CrateSource::CratesIo {
name: tk.name.clone(),
version: ver.to_string(),
options: tk.options.clone(),
});
})
}
Expand All @@ -307,6 +339,7 @@ fn read_crates(toml_path: Option<&str>) -> (String, Vec<CrateSource>) {
name: tk.name.clone(),
url: tk.git_url.clone().unwrap(),
commit: tk.git_hash.clone().unwrap(),
options: tk.options.clone(),
});
}
// if we have a version as well as a git data OR only one git data, something is funky
Expand Down Expand Up @@ -373,12 +406,14 @@ fn gather_stats(clippy_warnings: &[ClippyWarning]) -> String {

/// lintchecks `main()` function
pub fn run(clap_config: &ArgMatches) {
let cargo_clippy_path: PathBuf = PathBuf::from("target/debug/cargo-clippy");

println!("Compiling clippy...");
build_clippy();
println!("Done compiling");

let cargo_clippy_path: PathBuf = PathBuf::from("target/debug/cargo-clippy")
.canonicalize()
.expect("failed to canonicalize path to clippy binary");

// assert that clippy is found
assert!(
cargo_clippy_path.is_file(),
Expand Down Expand Up @@ -455,5 +490,6 @@ pub fn run(clap_config: &ArgMatches) {
.for_each(|(cratename, msg)| text.push_str(&format!("{}: '{}'", cratename, msg)));

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