Auto merge of #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.

clippy_dev/README.md

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

clippy_dev/src/lintcheck.rs

@@ -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
@@ -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`
@@ -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");
 
@@ -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
@@ -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
@@ -183,6 +205,7 @@ impl CrateSource {
                     version: String::from("local"),
                     name: name.clone(),
                     path: crate_root,
+                    options: options.clone(),
                 }
             },
         }
@@ -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| {
@@ -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
@@ -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(),
             });
         }
 
@@ -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(),
                 });
             })
         }
@@ -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
@@ -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(),
@@ -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();
 }