Auto merge of #6829 - matthiaskrgr:lintcheck_unclippy, r=<try>

lintcheck: clippy fixes, test on ci

fixes clippy warnings in lintcheck and adds a small test (run on 3 small crates) to gha ci

changelog: none

Author: bors

.github/workflows/clippy.yml

@@ -27,6 +27,7 @@ env:
 
 jobs:
   base:
+    # NOTE: If you modify this job, make sure you copy the changes to clippy_bors.yml
     runs-on: ubuntu-latest
 
     steps:
@@ -64,8 +65,8 @@ jobs:
       run: cargo test --features deny-warnings
       working-directory: rustc_tools_util
 
-    - name: Test clippy_dev
-      run: cargo test --features deny-warnings
+    - name: Test clippy_dev and lintcheck
+      run: cargo test --features lintcheck,deny-warnings
       working-directory: clippy_dev
 
     - name: Test cargo-clippy

.github/workflows/clippy_bors.yml

@@ -72,6 +72,7 @@ jobs:
 
     runs-on: ${{ matrix.os }}
 
+    # NOTE: If you modify this job, make sure you copy the changes to clippy.yml
     steps:
     # Setup
     - uses: rust-lang/simpleinfra/github-actions/cancel-outdated-builds@master
@@ -112,6 +113,9 @@ jobs:
     - name: Build
       run: cargo build --features deny-warnings,internal-lints
 
+    - name: Test "--fix -Zunstable-options"
+      run: cargo run --features deny-warnings,internal-lints --bin cargo-clippy -- clippy --fix -Zunstable-options
+
     - name: Test
       run: cargo test --features deny-warnings,internal-lints
 
@@ -123,8 +127,8 @@ jobs:
       run: cargo test --features deny-warnings
       working-directory: rustc_tools_util
 
-    - name: Test clippy_dev
-      run: cargo test --features deny-warnings
+    - name: Test clippy_dev and lintcheck
+      run: cargo test --features lintcheck,deny-warnings
       working-directory: clippy_dev
 
     - name: Test cargo-clippy
@@ -136,6 +140,13 @@ jobs:
       env:
         OS: ${{ runner.os }}
 
+    - name: Test cargo dev new lint
+      run: |
+        cargo dev new_lint --name new_early_pass --pass early
+        cargo dev new_lint --name new_late_pass --pass late
+        cargo check
+        git reset --hard HEAD
+
   integration_build:
     needs: changelog
     runs-on: ubuntu-latest

clippy_dev/ci_test_sources.toml

@@ -0,0 +1,4 @@
+[crates]
+cc = {name = "cc", versions = ['1.0.67']}
+home = {name = "home", git_url = "https://github.com/brson/home", git_hash = "32044e53dfbdcd32bafad3109d1fbab805fc0f40"}
+rustc_tools_util = {name = "rustc_tools_util", versions = ['0.2.0']}

clippy_dev/src/lintcheck.rs

@@ -5,14 +5,19 @@
 // positives.
 
 #![cfg(feature = "lintcheck")]
-#![allow(clippy::filter_map)]
+#![allow(clippy::filter_map, clippy::collapsible_else_if)]
+#![allow(clippy::blocks_in_if_conditions)] // FP on `if x.iter().any(|x| ...)`
 
 use crate::clippy_project_root;
 
 use std::collections::HashMap;
 use std::process::Command;
 use std::sync::atomic::{AtomicUsize, Ordering};
-use std::{env, fmt, fs::write, path::PathBuf};
+use std::{
+    env, fmt,
+    fs::write,
+    path::{Path, PathBuf},
+};
 
 use clap::ArgMatches;
 use rayon::prelude::*;
@@ -196,11 +201,9 @@ impl CrateSource {
                 if !crate_root.exists() {
                     println!("Copying {} to {}", path.display(), copy_dest.display());
 
-                    dir::copy(path, &copy_dest, &dir::CopyOptions::new()).expect(&format!(
-                        "Failed to copy from {}, to  {}",
-                        path.display(),
-                        crate_root.display()
-                    ));
+                    dir::copy(path, &copy_dest, &dir::CopyOptions::new()).unwrap_or_else(|_| {
+                        panic!("Failed to copy from {}, to  {}", path.display(), crate_root.display())
+                    });
                 } else {
                     println!(
                         "Not copying {} to {}, destination already exists",
@@ -225,7 +228,7 @@ impl Crate {
     /// issued
     fn run_clippy_lints(
         &self,
-        cargo_clippy_path: &PathBuf,
+        cargo_clippy_path: &Path,
         target_dir_index: &AtomicUsize,
         thread_limit: usize,
         total_crates_to_lint: usize,
@@ -308,13 +311,13 @@ impl LintcheckConfig {
         // first, check if we got anything passed via the LINTCHECK_TOML env var,
         // if not, ask clap if we got any value for --crates-toml  <foo>
         // if not, use the default "clippy_dev/lintcheck_crates.toml"
-        let sources_toml = env::var("LINTCHECK_TOML").unwrap_or(
+        let sources_toml = env::var("LINTCHECK_TOML").unwrap_or_else(|_| {
             clap_config
                 .value_of("crates-toml")
                 .clone()
                 .unwrap_or("clippy_dev/lintcheck_crates.toml")
-                .to_string(),
-        );
+                .to_string()
+        });
 
         let sources_toml_path = PathBuf::from(sources_toml);
 
@@ -330,7 +333,7 @@ impl LintcheckConfig {
             Some(threads) => {
                 let threads: usize = threads
                     .parse()
-                    .expect(&format!("Failed to parse '{}' to a digit", threads));
+                    .unwrap_or_else(|_| panic!("Failed to parse '{}' to a digit", threads));
                 if threads == 0 {
                     // automatic choice
                     // Rayon seems to return thread count so half that for core count
@@ -387,7 +390,7 @@ fn build_clippy() {
 }
 
 /// Read a `toml` file and return a list of `CrateSources` that we want to check with clippy
-fn read_crates(toml_path: &PathBuf) -> Vec<CrateSource> {
+fn read_crates(toml_path: &Path) -> Vec<CrateSource> {
     let toml_content: String =
         std::fs::read_to_string(&toml_path).unwrap_or_else(|_| panic!("Failed to read {}", toml_path.display()));
     let crate_list: SourceList =
@@ -499,7 +502,10 @@ fn gather_stats(clippy_warnings: &[ClippyWarning]) -> (String, HashMap<&String,
 
 /// check if the latest modification of the logfile is older than the modification date of the
 /// clippy binary, if this is true, we should clean the lintchec shared target directory and recheck
-fn lintcheck_needs_rerun(lintcheck_logs_path: &PathBuf) -> bool {
+fn lintcheck_needs_rerun(lintcheck_logs_path: &Path) -> bool {
+    if !lintcheck_logs_path.exists() {
+        return true;
+    }
     let clippy_modified: std::time::SystemTime = {
         let mut times = [CLIPPY_DRIVER_PATH, CARGO_CLIPPY_PATH].iter().map(|p| {
             std::fs::metadata(p)
@@ -533,15 +539,13 @@ pub fn run(clap_config: &ArgMatches) {
     // refresh the logs
     if lintcheck_needs_rerun(&config.lintcheck_results_path) {
         let shared_target_dir = "target/lintcheck/shared_target_dir";
-        match std::fs::metadata(&shared_target_dir) {
-            Ok(metadata) => {
-                if metadata.is_dir() {
-                    println!("Clippy is newer than lint check logs, clearing lintcheck shared target dir...");
-                    std::fs::remove_dir_all(&shared_target_dir)
-                        .expect("failed to remove target/lintcheck/shared_target_dir");
-                }
-            },
-            Err(_) => { /*  dir probably does not exist, don't remove anything  */ },
+        // if we get an Err here, the shared target dir probably does simply not exist
+        if let Ok(metadata) = std::fs::metadata(&shared_target_dir) {
+            if metadata.is_dir() {
+                println!("Clippy is newer than lint check logs, clearing lintcheck shared target dir...");
+                std::fs::remove_dir_all(&shared_target_dir)
+                    .expect("failed to remove target/lintcheck/shared_target_dir");
+            }
         }
     }
 
@@ -660,11 +664,10 @@ pub fn run(clap_config: &ArgMatches) {
 }
 
 /// read the previous stats from the lintcheck-log file
-fn read_stats_from_file(file_path: &PathBuf) -> HashMap<String, usize> {
+fn read_stats_from_file(file_path: &Path) -> HashMap<String, usize> {
     let file_content: String = match std::fs::read_to_string(file_path).ok() {
         Some(content) => content,
         None => {
-            eprintln!("RETURND");
             return HashMap::new();
         },
     };
@@ -678,9 +681,9 @@ fn read_stats_from_file(file_path: &PathBuf) -> HashMap<String, usize> {
     let stats_lines = &lines[start + 1..=end - 1];
 
     stats_lines
-        .into_iter()
+        .iter()
         .map(|line| {
-            let mut spl = line.split(" ").into_iter();
+            let mut spl = line.split(' ');
             (
                 spl.next().unwrap().to_string(),
                 spl.next().unwrap().parse::<usize>().unwrap(),
@@ -733,3 +736,30 @@ fn print_stats(old_stats: HashMap<String, usize>, new_stats: HashMap<&String, us
             println!("{} {} => 0", old_key, old_value);
         });
 }
+
+#[test]
+fn lintcheck_test() {
+    let args = [
+        "run",
+        "--target-dir",
+        "clippy_dev/target",
+        "--package",
+        "clippy_dev",
+        "--bin",
+        "clippy_dev",
+        "--manifest-path",
+        "clippy_dev/Cargo.toml",
+        "--features",
+        "lintcheck",
+        "--",
+        "lintcheck",
+        "--crates-toml",
+        "clippy_dev/ci_test_sources.toml",
+    ];
+    let status = std::process::Command::new("cargo")
+        .args(&args)
+        .current_dir("../" /* repo root */)
+        .status();
+
+    assert!(status.unwrap().success());
+}

lintcheck-logs/lintcheck_crates_logs.txt

@@ -1,4 +1,4 @@
-clippy 0.1.52 (70d952e75 2021-02-28)
+clippy 0.1.52 (3c72f0b53 2021-03-02)
 
 target/lintcheck/sources/anyhow-1.0.38/build.rs:1:null clippy::cargo_common_metadata "package `anyhow` is missing `package.keywords` metadata"
 target/lintcheck/sources/anyhow-1.0.38/src/error.rs:350:5 clippy::missing_panics_doc "docs for function which may panic missing `# Panics` section"
@@ -99,6 +99,7 @@ target/lintcheck/sources/cargo-0.49.0/src/bin/cargo/main.rs:1:null clippy::multi
 target/lintcheck/sources/cargo-0.49.0/src/bin/cargo/main.rs:1:null clippy::multiple_crate_versions "multiple versions for dependency `hex`: 0.3.2, 0.4.0"
 target/lintcheck/sources/cargo-0.49.0/src/bin/cargo/main.rs:1:null clippy::multiple_crate_versions "multiple versions for dependency `humantime`: 1.3.0, 2.0.0"
 target/lintcheck/sources/cargo-0.49.0/src/bin/cargo/main.rs:72:22 clippy::redundant_closure_for_method_calls "redundant closure found"
+target/lintcheck/sources/cargo-0.49.0/src/bin/cargo/main.rs:79:40 clippy::manual_map "manual implementation of `Option::map`"
 target/lintcheck/sources/cargo-0.49.0/src/bin/cargo/main.rs:94:13 clippy::match_wildcard_for_single_variants "wildcard match will miss any future added variants"
 target/lintcheck/sources/cargo-0.49.0/src/bin/cargo/main.rs:96:41 clippy::redundant_closure_for_method_calls "redundant closure found"
 target/lintcheck/sources/cargo-0.49.0/src/bin/cargo/main.rs:98:60 clippy::redundant_closure_for_method_calls "redundant closure found"
@@ -1208,6 +1209,7 @@ target/lintcheck/sources/cargo-0.49.0/src/cargo/util/config/mod.rs:689:20 clippy
 target/lintcheck/sources/cargo-0.49.0/src/cargo/util/config/mod.rs:699:5 clippy::fn_params_excessive_bools "more than 3 bools in function parameters"
 target/lintcheck/sources/cargo-0.49.0/src/cargo/util/config/mod.rs:699:5 clippy::missing_errors_doc "docs for function returning `Result` missing `# Errors` section"
 target/lintcheck/sources/cargo-0.49.0/src/cargo/util/config/mod.rs:719:58 clippy::redundant_closure_for_method_calls "redundant closure found"
+target/lintcheck/sources/cargo-0.49.0/src/cargo/util/config/mod.rs:748:30 clippy::manual_map "manual implementation of `Option::map`"
 target/lintcheck/sources/cargo-0.49.0/src/cargo/util/config/mod.rs:816:5 clippy::missing_errors_doc "docs for function returning `Result` missing `# Errors` section"
 target/lintcheck/sources/cargo-0.49.0/src/cargo/util/config/path.rs:10:1 clippy::module_name_repetitions "item name ends with its containing module's name"
 target/lintcheck/sources/cargo-0.49.0/src/cargo/util/config/path.rs:14:5 clippy::must_use_candidate "this method could have a `#[must_use]` attribute"
@@ -1463,6 +1465,7 @@ target/lintcheck/sources/cfg-expr-0.7.1/src/expr/mod.rs:464:5 clippy::missing_pa
 target/lintcheck/sources/cfg-expr-0.7.1/src/expr/mod.rs:57:13 clippy::enum_glob_use "usage of wildcard import for enum variants"
 target/lintcheck/sources/cfg-expr-0.7.1/src/expr/mod.rs:586:33 clippy::match_same_arms "this `match` has identical arm bodies"
 target/lintcheck/sources/cfg-expr-0.7.1/src/expr/mod.rs:599:32 clippy::match_same_arms "this `match` has identical arm bodies"
+target/lintcheck/sources/cfg-expr-0.7.1/src/expr/mod.rs:609:9 clippy::manual_map "manual implementation of `Option::map`"
 target/lintcheck/sources/cfg-expr-0.7.1/src/expr/parser.rs:116:31 clippy::similar_names "binding's name is too similar to existing binding"
 target/lintcheck/sources/cfg-expr-0.7.1/src/expr/parser.rs:124:36 clippy::similar_names "binding's name is too similar to existing binding"
 target/lintcheck/sources/cfg-expr-0.7.1/src/expr/parser.rs:17:5 clippy::missing_errors_doc "docs for function returning `Result` missing `# Errors` section"
@@ -3509,6 +3512,7 @@ clippy::filter_map_next 3
 clippy::fn_params_excessive_bools 3
 clippy::if_same_then_else 3
 clippy::inconsistent_struct_constructor 3
+clippy::manual_map 3
 clippy::mut_mut 3
 clippy::ptr_arg 3
 clippy::zero_ptr 3