Skip to content
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

Lintcheck and an options for command line options #6750

Merged
merged 4 commits into from
Feb 17, 2021
Merged
Show file tree
Hide file tree
Changes from 2 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
58 changes: 36 additions & 22 deletions clippy_dev/README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Clippy Dev Tool
# Clippy Dev Tool

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

Expand Down Expand Up @@ -28,25 +28,39 @@ 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.
The sources to check are saved in a `toml` file.
There are three types of sources.

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.
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.

flip1995 marked this conversation as resolved.
Show resolved Hide resolved
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.
flip1995 marked this conversation as resolved.
Show resolved Hide resolved
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 {
flip1995 marked this conversation as resolved.
Show resolved Hide resolved
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();
}