diff --git a/.github/workflows/clippy.yml b/.github/workflows/clippy.yml index 9d5e12aac5f..32103f59d8b 100644 --- a/.github/workflows/clippy.yml +++ b/.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: @@ -50,9 +51,6 @@ 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 @@ -72,6 +70,10 @@ jobs: run: ../target/debug/cargo-clippy working-directory: clippy_workspace_tests + - name: Test cargo-clippy --fix + run: ../target/debug/cargo-clippy clippy --fix -Zunstable-options + working-directory: clippy_workspace_tests + - name: Test clippy-driver run: bash .github/driver.sh env: diff --git a/.github/workflows/clippy_bors.yml b/.github/workflows/clippy_bors.yml index 5d846eb64c7..47253eecc4c 100644 --- a/.github/workflows/clippy_bors.yml +++ b/.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 @@ -131,11 +132,22 @@ jobs: run: ../target/debug/cargo-clippy working-directory: clippy_workspace_tests + - name: Test cargo-clippy --fix + run: ../target/debug/cargo-clippy clippy --fix -Zunstable-options + working-directory: clippy_workspace_tests + - name: Test clippy-driver run: bash .github/driver.sh 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 diff --git a/clippy_dev/src/lib.rs b/clippy_dev/src/lib.rs index 01d1fc9211a..0244ff2b6c2 100644 --- a/clippy_dev/src/lib.rs +++ b/clippy_dev/src/lib.rs @@ -530,7 +530,7 @@ fn test_gen_deprecated() { #[should_panic] fn test_gen_deprecated_fail() { let lints = vec![Lint::new("should_assert_eq2", "group2", "abc", None, "module_name")]; - let _ = gen_deprecated(lints.iter()); + let _deprecated_lints = gen_deprecated(lints.iter()); } #[test] diff --git a/clippy_dev/src/lintcheck.rs b/clippy_dev/src/lintcheck.rs index 01f3c9a5bd8..f01f14eb458 100644 --- a/clippy_dev/src/lintcheck.rs +++ b/clippy_dev/src/lintcheck.rs @@ -6,13 +6,12 @@ #![cfg(feature = "lintcheck")] #![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::{collections::HashMap, io::ErrorKind}; use std::{ env, fmt, fs::write, @@ -116,9 +115,7 @@ impl CrateSource { // url to download the crate from crates.io let url = format!("https://crates.io/api/v1/crates/{}/{}/download", name, version); println!("Downloading and extracting {} {} from {}", name, version, url); - let _ = std::fs::create_dir("target/lintcheck/"); - let _ = std::fs::create_dir(&krate_download_dir); - let _ = std::fs::create_dir(&extract_dir); + create_dirs(&krate_download_dir, &extract_dir); let krate_file_path = krate_download_dir.join(format!("{}-{}.crate.tar.gz", name, version)); // don't download/extract if we already have done so @@ -198,18 +195,18 @@ impl CrateSource { // the source path of the crate we copied, ${copy_dest}/crate_name let crate_root = copy_dest.join(name); // .../crates/local_crate - if !crate_root.exists() { - println!("Copying {} to {}", path.display(), copy_dest.display()); - - dir::copy(path, ©_dest, &dir::CopyOptions::new()).unwrap_or_else(|_| { - panic!("Failed to copy from {}, to {}", path.display(), crate_root.display()) - }); - } else { + if crate_root.exists() { println!( "Not copying {} to {}, destination already exists", path.display(), crate_root.display() ); + } else { + println!("Copying {} to {}", path.display(), copy_dest.display()); + + dir::copy(path, ©_dest, &dir::CopyOptions::new()).unwrap_or_else(|_| { + panic!("Failed to copy from {}, to {}", path.display(), crate_root.display()) + }); } Crate { @@ -236,8 +233,8 @@ impl Crate { // advance the atomic index by one let index = target_dir_index.fetch_add(1, Ordering::SeqCst); // "loop" the index within 0..thread_limit - let target_dir_index = index % thread_limit; - let perc = ((index * 100) as f32 / total_crates_to_lint as f32) as u8; + let thread_index = index % thread_limit; + let perc = (index * 100) / total_crates_to_lint; if thread_limit == 1 { println!( @@ -247,7 +244,7 @@ impl Crate { } else { println!( "{}/{} {}% Linting {} {} in target dir {:?}", - index, total_crates_to_lint, perc, &self.name, &self.version, target_dir_index + index, total_crates_to_lint, perc, &self.name, &self.version, thread_index ); } @@ -269,7 +266,7 @@ impl Crate { // use the looping index to create individual target dirs .env( "CARGO_TARGET_DIR", - shared_target_dir.join(format!("_{:?}", target_dir_index)), + shared_target_dir.join(format!("_{:?}", thread_index)), ) // lint warnings will look like this: // src/cargo/ops/cargo_compile.rs:127:35: warning: usage of `FromIterator::from_iter` @@ -529,6 +526,10 @@ fn lintcheck_needs_rerun(lintcheck_logs_path: &Path) -> bool { } /// lintchecks `main()` function +/// +/// # Panics +/// +/// This function panics if the clippy binaries don't exist. pub fn run(clap_config: &ArgMatches) { let config = LintcheckConfig::from_clap(clap_config); @@ -579,9 +580,9 @@ pub fn run(clap_config: &ArgMatches) { // if we don't have the specified crate in the .toml, throw an error if !crates.iter().any(|krate| { let name = match krate { - CrateSource::CratesIo { name, .. } => name, - CrateSource::Git { name, .. } => name, - CrateSource::Path { name, .. } => name, + CrateSource::CratesIo { name, .. } | CrateSource::Git { name, .. } | CrateSource::Path { name, .. } => { + name + }, }; name == only_one_crate }) { @@ -597,8 +598,7 @@ pub fn run(clap_config: &ArgMatches) { .into_iter() .map(|krate| krate.download_and_extract()) .filter(|krate| krate.name == only_one_crate) - .map(|krate| krate.run_clippy_lints(&cargo_clippy_path, &AtomicUsize::new(0), 1, 1)) - .flatten() + .flat_map(|krate| krate.run_clippy_lints(&cargo_clippy_path, &AtomicUsize::new(0), 1, 1)) .collect() } else { if config.max_jobs > 1 { @@ -621,8 +621,7 @@ pub fn run(clap_config: &ArgMatches) { crates .into_par_iter() .map(|krate| krate.download_and_extract()) - .map(|krate| krate.run_clippy_lints(&cargo_clippy_path, &counter, num_cpus, num_crates)) - .flatten() + .flat_map(|krate| krate.run_clippy_lints(&cargo_clippy_path, &counter, num_cpus, num_crates)) .collect() } else { // run sequential @@ -630,8 +629,7 @@ pub fn run(clap_config: &ArgMatches) { crates .into_iter() .map(|krate| krate.download_and_extract()) - .map(|krate| krate.run_clippy_lints(&cargo_clippy_path, &counter, 1, num_crates)) - .flatten() + .flat_map(|krate| krate.run_clippy_lints(&cargo_clippy_path, &counter, 1, num_crates)) .collect() } }; @@ -646,7 +644,7 @@ pub fn run(clap_config: &ArgMatches) { .map(|w| (&w.crate_name, &w.message)) .collect(); - let mut all_msgs: Vec = clippy_warnings.iter().map(|warning| warning.to_string()).collect(); + let mut all_msgs: Vec = clippy_warnings.iter().map(ToString::to_string).collect(); all_msgs.sort(); all_msgs.push("\n\n\n\nStats:\n".into()); all_msgs.push(stats_formatted); @@ -673,13 +671,13 @@ fn read_stats_from_file(file_path: &Path) -> HashMap { }, }; - let lines: Vec = file_content.lines().map(|l| l.to_string()).collect(); + let lines: Vec = file_content.lines().map(ToString::to_string).collect(); // search for the beginning "Stats:" and the end "ICEs:" of the section we want let start = lines.iter().position(|line| line == "Stats:").unwrap(); let end = lines.iter().position(|line| line == "ICEs:").unwrap(); - let stats_lines = &lines[start + 1..=end - 1]; + let stats_lines = &lines[start + 1..end]; stats_lines .iter() @@ -738,6 +736,29 @@ fn print_stats(old_stats: HashMap, new_stats: HashMap<&String, us }); } +/// Create necessary directories to run the lintcheck tool. +/// +/// # Panics +/// +/// This function panics if creating one of the dirs fails. +fn create_dirs(krate_download_dir: &Path, extract_dir: &Path) { + std::fs::create_dir("target/lintcheck/").unwrap_or_else(|err| { + if err.kind() != ErrorKind::AlreadyExists { + panic!("cannot create lintcheck target dir"); + } + }); + std::fs::create_dir(&krate_download_dir).unwrap_or_else(|err| { + if err.kind() != ErrorKind::AlreadyExists { + panic!("cannot create crate download dir"); + } + }); + std::fs::create_dir(&extract_dir).unwrap_or_else(|err| { + if err.kind() != ErrorKind::AlreadyExists { + panic!("cannot create crate extraction dir"); + } + }); +} + #[test] fn lintcheck_test() { let args = [ diff --git a/clippy_lints/src/transmute/transmute_int_to_char.rs b/clippy_lints/src/transmute/transmute_int_to_char.rs index 48473e0d799..29d2450618a 100644 --- a/clippy_lints/src/transmute/transmute_int_to_char.rs +++ b/clippy_lints/src/transmute/transmute_int_to_char.rs @@ -17,28 +17,26 @@ pub(super) fn check<'tcx>( ) -> bool { match (&from_ty.kind(), &to_ty.kind()) { (ty::Int(ty::IntTy::I32) | ty::Uint(ty::UintTy::U32), &ty::Char) => { - { - span_lint_and_then( - cx, - TRANSMUTE_INT_TO_CHAR, - e.span, - &format!("transmute from a `{}` to a `char`", from_ty), - |diag| { - let arg = sugg::Sugg::hir(cx, &args[0], ".."); - let arg = if let ty::Int(_) = from_ty.kind() { - arg.as_ty(ast::UintTy::U32.name_str()) - } else { - arg - }; - diag.span_suggestion( - e.span, - "consider using", - format!("std::char::from_u32({}).unwrap()", arg.to_string()), - Applicability::Unspecified, - ); - }, - ) - }; + span_lint_and_then( + cx, + TRANSMUTE_INT_TO_CHAR, + e.span, + &format!("transmute from a `{}` to a `char`", from_ty), + |diag| { + let arg = sugg::Sugg::hir(cx, &args[0], ".."); + let arg = if let ty::Int(_) = from_ty.kind() { + arg.as_ty(ast::UintTy::U32.name_str()) + } else { + arg + }; + diag.span_suggestion( + e.span, + "consider using", + format!("std::char::from_u32({}).unwrap()", arg.to_string()), + Applicability::Unspecified, + ); + }, + ); true }, _ => false, diff --git a/clippy_lints/src/transmute/transmute_ptr_to_ref.rs b/clippy_lints/src/transmute/transmute_ptr_to_ref.rs index a6719b68098..f5dbbbe33bc 100644 --- a/clippy_lints/src/transmute/transmute_ptr_to_ref.rs +++ b/clippy_lints/src/transmute/transmute_ptr_to_ref.rs @@ -17,7 +17,7 @@ pub(super) fn check<'tcx>( qpath: &'tcx QPath<'_>, ) -> bool { match (&from_ty.kind(), &to_ty.kind()) { - (ty::RawPtr(from_pty), ty::Ref(_, to_ref_ty, mutbl)) => { + (ty::RawPtr(from_ptr_ty), ty::Ref(_, to_ref_ty, mutbl)) => { span_lint_and_then( cx, TRANSMUTE_PTR_TO_REF, @@ -34,7 +34,7 @@ pub(super) fn check<'tcx>( ("&*", "*const") }; - let arg = if from_pty.ty == *to_ref_ty { + let arg = if from_ptr_ty.ty == *to_ref_ty { arg } else { arg.as_ty(&format!("{} {}", cast, get_type_snippet(cx, qpath, to_ref_ty))) diff --git a/clippy_lints/src/transmute/transmute_ref_to_ref.rs b/clippy_lints/src/transmute/transmute_ref_to_ref.rs index ccfcb03e87a..01b00bb0a22 100644 --- a/clippy_lints/src/transmute/transmute_ref_to_ref.rs +++ b/clippy_lints/src/transmute/transmute_ref_to_ref.rs @@ -18,70 +18,67 @@ pub(super) fn check<'tcx>( ) -> bool { let mut triggered = false; - match (&from_ty.kind(), &to_ty.kind()) { - (ty::Ref(_, ty_from, from_mutbl), ty::Ref(_, ty_to, to_mutbl)) => { - if_chain! { - if let (&ty::Slice(slice_ty), &ty::Str) = (&ty_from.kind(), &ty_to.kind()); - if let ty::Uint(ty::UintTy::U8) = slice_ty.kind(); - if from_mutbl == to_mutbl; - then { - let postfix = if *from_mutbl == Mutability::Mut { - "_mut" - } else { - "" - }; - - span_lint_and_sugg( - cx, - TRANSMUTE_BYTES_TO_STR, - e.span, - &format!("transmute from a `{}` to a `{}`", from_ty, to_ty), - "consider using", - format!( - "std::str::from_utf8{}({}).unwrap()", - postfix, - snippet(cx, args[0].span, ".."), - ), - Applicability::Unspecified, - ); - triggered = true; + if let (ty::Ref(_, ty_from, from_mutbl), ty::Ref(_, ty_to, to_mutbl)) = (&from_ty.kind(), &to_ty.kind()) { + if_chain! { + if let (&ty::Slice(slice_ty), &ty::Str) = (&ty_from.kind(), &ty_to.kind()); + if let ty::Uint(ty::UintTy::U8) = slice_ty.kind(); + if from_mutbl == to_mutbl; + then { + let postfix = if *from_mutbl == Mutability::Mut { + "_mut" } else { - if (cx.tcx.erase_regions(from_ty) != cx.tcx.erase_regions(to_ty)) - && !const_context { - span_lint_and_then( - cx, - TRANSMUTE_PTR_TO_PTR, - e.span, - "transmute from a reference to a reference", - |diag| if let Some(arg) = sugg::Sugg::hir_opt(cx, &args[0]) { - let ty_from_and_mut = ty::TypeAndMut { - ty: ty_from, - mutbl: *from_mutbl - }; - let ty_to_and_mut = ty::TypeAndMut { ty: ty_to, mutbl: *to_mutbl }; - let sugg_paren = arg - .as_ty(cx.tcx.mk_ptr(ty_from_and_mut)) - .as_ty(cx.tcx.mk_ptr(ty_to_and_mut)); - let sugg = if *to_mutbl == Mutability::Mut { - sugg_paren.mut_addr_deref() - } else { - sugg_paren.addr_deref() - }; - diag.span_suggestion( - e.span, - "try", - sugg.to_string(), - Applicability::Unspecified, - ); - }, - ); + "" + }; - triggered = true; - } + span_lint_and_sugg( + cx, + TRANSMUTE_BYTES_TO_STR, + e.span, + &format!("transmute from a `{}` to a `{}`", from_ty, to_ty), + "consider using", + format!( + "std::str::from_utf8{}({}).unwrap()", + postfix, + snippet(cx, args[0].span, ".."), + ), + Applicability::Unspecified, + ); + triggered = true; + } else { + if (cx.tcx.erase_regions(from_ty) != cx.tcx.erase_regions(to_ty)) + && !const_context { + span_lint_and_then( + cx, + TRANSMUTE_PTR_TO_PTR, + e.span, + "transmute from a reference to a reference", + |diag| if let Some(arg) = sugg::Sugg::hir_opt(cx, &args[0]) { + let ty_from_and_mut = ty::TypeAndMut { + ty: ty_from, + mutbl: *from_mutbl + }; + let ty_to_and_mut = ty::TypeAndMut { ty: ty_to, mutbl: *to_mutbl }; + let sugg_paren = arg + .as_ty(cx.tcx.mk_ptr(ty_from_and_mut)) + .as_ty(cx.tcx.mk_ptr(ty_to_and_mut)); + let sugg = if *to_mutbl == Mutability::Mut { + sugg_paren.mut_addr_deref() + } else { + sugg_paren.addr_deref() + }; + diag.span_suggestion( + e.span, + "try", + sugg.to_string(), + Applicability::Unspecified, + ); + }, + ); + + triggered = true; } } - }, - _ => {}, + } } triggered diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs index c262ec993b1..f0523cec621 100644 --- a/clippy_lints/src/use_self.rs +++ b/clippy_lints/src/use_self.rs @@ -265,7 +265,7 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf { let hir = cx.tcx.hir(); let id = hir.get_parent_node(hir_ty.hir_id); - if !hir.opt_span(id).map(in_macro).unwrap_or(false) { + if !hir.opt_span(id).map_or(false, in_macro) { match hir.find(id) { Some(Node::Expr(Expr { kind: ExprKind::Path(QPath::TypeRelative(_, segment)), diff --git a/src/main.rs b/src/main.rs index d13a831f5ff..7bb80b1196e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -92,12 +92,6 @@ impl ClippyCmd { panic!("Usage of `--fix` requires `-Z unstable-options`"); } - // Run the dogfood tests directly on nightly cargo. This is required due - // to a bug in rustup.rs when running cargo on custom toolchains. See issue #3118. - if env::var_os("CLIPPY_DOGFOOD").is_some() && cfg!(windows) { - args.insert(0, "+nightly".to_string()); - } - let mut clippy_args: Vec = old_args.collect(); if cargo_subcommand == "fix" && !clippy_args.iter().any(|arg| arg == "--no-deps") { clippy_args.push("--no-deps".into()); diff --git a/tests/dogfood.rs b/tests/dogfood.rs index 8fe48a67beb..89526648d2e 100644 --- a/tests/dogfood.rs +++ b/tests/dogfood.rs @@ -23,10 +23,9 @@ fn dogfood_clippy() { .current_dir(root_dir) .env("CLIPPY_DOGFOOD", "1") .env("CARGO_INCREMENTAL", "0") - .arg("clippy-preview") + .arg("clippy") .arg("--all-targets") .arg("--all-features") - .args(&["-p", "clippy_lints", "-p", "clippy_utils", "-p", "rustc_tools_util"]) .arg("--") .args(&["-D", "clippy::all"]) .args(&["-D", "clippy::pedantic"]) @@ -125,19 +124,30 @@ fn dogfood_subprojects() { "clippy_workspace_tests/subcrate", "clippy_workspace_tests/subcrate/src", "clippy_dev", + "clippy_lints", + "clippy_utils", "rustc_tools_util", ] { - let output = Command::new(&*CLIPPY_PATH) + let mut command = Command::new(&*CLIPPY_PATH); + command .current_dir(root_dir.join(d)) .env("CLIPPY_DOGFOOD", "1") .env("CARGO_INCREMENTAL", "0") .arg("clippy") + .arg("--all-targets") + .arg("--all-features") .arg("--") .args(&["-D", "clippy::all"]) .args(&["-D", "clippy::pedantic"]) - .arg("-Cdebuginfo=0") // disable debuginfo to generate less data in the target dir - .output() - .unwrap(); + .arg("-Cdebuginfo=0"); // disable debuginfo to generate less data in the target dir + + // internal lints only exist if we build with the internal-lints feature + if cfg!(feature = "internal-lints") { + command.args(&["-D", "clippy::internal"]); + } + + let output = command.output().unwrap(); + println!("status: {}", output.status); println!("stdout: {}", String::from_utf8_lossy(&output.stdout)); println!("stderr: {}", String::from_utf8_lossy(&output.stderr));