Skip to content

Commit

Permalink
Auto merge of #6849 - flip1995:dogfood-fix, r=matthiaskrgr
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bors committed Mar 5, 2021
2 parents f0e6ce8 + 74eb448 commit e368355
Show file tree
Hide file tree
Showing 10 changed files with 160 additions and 126 deletions.
8 changes: 5 additions & 3 deletions .github/workflows/clippy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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

Expand All @@ -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:
Expand Down
12 changes: 12 additions & 0 deletions .github/workflows/clippy_bors.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion clippy_dev/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
77 changes: 49 additions & 28 deletions clippy_dev/src/lintcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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!(
Expand All @@ -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
);
}

Expand All @@ -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`
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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
}) {
Expand All @@ -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 {
Expand All @@ -621,17 +621,15 @@ 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
let num_crates = crates.len();
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()
}
};
Expand All @@ -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);
Expand All @@ -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()
Expand Down Expand Up @@ -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 = [
Expand Down
42 changes: 20 additions & 22 deletions clippy_lints/src/transmute/transmute_int_to_char.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/transmute/transmute_ptr_to_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)))
Expand Down
Loading

0 comments on commit e368355

Please sign in to comment.