Auto merge of #6849 - flip1995:dogfood-fix, r=matthiaskrgr

Dogfood and CI fixes

The CI fix is practically #6829 rebased and squashed into one commit

Dogfood fix is a follow up of #6802

r? `@matthiaskrgr` for lintcheck changes

(best reviewed with whitespace changes hidden)

changelog: none
This commit is contained in:
bors 2021-03-05 14:09:24 +00:00
commit 9e54538355
10 changed files with 163 additions and 129 deletions

View file

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

View file

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

View file

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

View file

@ -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, &copy_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, &copy_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<String> = clippy_warnings.iter().map(|warning| warning.to_string()).collect();
let mut all_msgs: Vec<String> = 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<String, usize> {
},
};
let lines: Vec<String> = file_content.lines().map(|l| l.to_string()).collect();
let lines: Vec<String> = 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<String, usize>, 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 = [

View file

@ -17,7 +17,6 @@ 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,
@ -37,8 +36,7 @@ pub(super) fn check<'tcx>(
Applicability::Unspecified,
);
},
)
};
);
true
},
_ => false,

View file

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

View file

@ -18,8 +18,7 @@ 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 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();
@ -80,8 +79,6 @@ pub(super) fn check<'tcx>(
}
}
}
},
_ => {},
}
triggered

View file

@ -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)),

View file

@ -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<String> = old_args.collect();
if cargo_subcommand == "fix" && !clippy_args.iter().any(|arg| arg == "--no-deps") {
clippy_args.push("--no-deps".into());

View file

@ -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));