Skip to content

some more lintcheck changes #6708

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Feb 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion clippy_dev/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ edition = "2018"
bytecount = "0.6"
clap = "2.33"
flate2 = { version = "1.0.19", optional = true }
fs_extra = { version = "1.2.0", optional = true }
itertools = "0.9"
opener = "0.4"
regex = "1"
Expand All @@ -21,5 +22,5 @@ ureq = { version = "2.0.0-rc3", optional = true }
walkdir = "2"

[features]
lintcheck = ["flate2", "serde_json", "tar", "toml", "ureq", "serde"]
lintcheck = ["flate2", "serde_json", "tar", "toml", "ureq", "serde", "fs_extra"]
deny-warnings = []
28 changes: 28 additions & 0 deletions clippy_dev/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Clippy Dev Tool

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.

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

You can set a custom sources.toml by adding `--crates-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`.

2 changes: 2 additions & 0 deletions clippy_dev/lintcheck_crates.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ cargo = {name = "cargo", versions = ['0.49.0']}
iron = {name = "iron", versions = ['0.6.1']}
ripgrep = {name = "ripgrep", versions = ['12.1.1']}
xsv = {name = "xsv", versions = ['0.13.0']}
# commented out because of 173K clippy::match_same_arms msgs in language_type.rs
#tokei = { name = "tokei", versions = ['12.0.4']}
rayon = {name = "rayon", versions = ['1.5.0']}
serde = {name = "serde", versions = ['1.0.118']}
# top 10 crates.io dls
bitflags = {name = "bitflags", versions = ['1.2.1']}
# crash = {name = "clippy_crash", path = "/tmp/clippy_crash"}
libc = {name = "libc", versions = ['0.2.81']}
log = {name = "log", versions = ['0.4.11']}
proc-macro2 = {name = "proc-macro2", versions = ['1.0.24']}
Expand Down
78 changes: 69 additions & 9 deletions clippy_dev/src/lintcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,15 @@ struct TomlCrate {
versions: Option<Vec<String>>,
git_url: Option<String>,
git_hash: Option<String>,
path: Option<String>,
}

// represents an archive we download from crates.io
// represents an archive we download from crates.io, or a git repo, or a local repo
#[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 },
}

// represents the extracted sourcecode of a crate
Expand All @@ -60,6 +62,7 @@ struct ClippyWarning {
column: String,
linttype: String,
message: String,
ice: bool,
}

impl std::fmt::Display for ClippyWarning {
Expand Down Expand Up @@ -111,7 +114,7 @@ impl CrateSource {
},
CrateSource::Git { name, url, commit } => {
let repo_path = {
let mut repo_path = PathBuf::from("target/lintcheck/downloads");
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
repo_path.push(format!("{}-git", name));
repo_path
Expand Down Expand Up @@ -139,6 +142,37 @@ impl CrateSource {
path: repo_path,
}
},
CrateSource::Path { name, path } => {
use fs_extra::dir;

// simply copy the entire directory into our target dir
let copy_dest = PathBuf::from("target/lintcheck/crates/");

// 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()).expect(&format!(
"Failed to copy from {}, to {}",
path.display(),
crate_root.display()
));
} else {
println!(
"Not copying {} to {}, destination already exists",
path.display(),
crate_root.display()
);
}

Crate {
version: String::from("local"),
name: name.clone(),
path: crate_root,
}
},
}
}
}
Expand Down Expand Up @@ -176,8 +210,8 @@ impl Crate {
let output_lines = stdout.lines();
let warnings: Vec<ClippyWarning> = output_lines
.into_iter()
// get all clippy warnings
.filter(|line| line.contains("clippy::"))
// get all clippy warnings and ICEs
.filter(|line| line.contains("clippy::") || line.contains("internal compiler error: "))
.map(|json_msg| parse_json_message(json_msg, &self))
.collect();
warnings
Expand All @@ -192,8 +226,10 @@ fn build_clippy() {
}

// get a list of CrateSources we want to check from a "lintcheck_crates.toml" file.
fn read_crates(toml_path: Option<&str>) -> Vec<CrateSource> {
fn read_crates(toml_path: Option<&str>) -> (String, Vec<CrateSource>) {
let toml_path = PathBuf::from(toml_path.unwrap_or("clippy_dev/lintcheck_crates.toml"));
// save it so that we can use the name of the sources.toml as name for the logfile later.
let toml_filename = toml_path.file_stem().unwrap().to_str().unwrap().to_string();
let toml_content: String =
std::fs::read_to_string(&toml_path).unwrap_or_else(|_| panic!("Failed to read {}", toml_path.display()));
let crate_list: CrateList =
Expand All @@ -209,6 +245,13 @@ fn read_crates(toml_path: Option<&str>) -> Vec<CrateSource> {
// multiple Cratesources)
let mut crate_sources = Vec::new();
tomlcrates.into_iter().for_each(|tk| {
if let Some(ref path) = tk.path {
crate_sources.push(CrateSource::Path {
name: tk.name.clone(),
path: PathBuf::from(path),
});
}

// if we have multiple versions, save each one
if let Some(ref versions) = tk.versions {
versions.iter().for_each(|ver| {
Expand All @@ -232,12 +275,15 @@ fn read_crates(toml_path: Option<&str>) -> Vec<CrateSource> {
{
eprintln!("tomlkrate: {:?}", tk);
if tk.git_hash.is_some() != tk.git_url.is_some() {
panic!("Encountered TomlCrate with only one of git_hash and git_url!")
panic!("Error: Encountered TomlCrate with only one of git_hash and git_url!");
}
if tk.path.is_some() && (tk.git_hash.is_some() || tk.versions.is_some()) {
panic!("Error: TomlCrate can only have one of 'git_.*', 'version' or 'path' fields");
}
unreachable!("Failed to translate TomlCrate into CrateSource!");
}
});
crate_sources
(toml_filename, crate_sources)
}

// extract interesting data from a json lint message
Expand All @@ -261,6 +307,7 @@ fn parse_json_message(json_message: &str, krate: &Crate) -> ClippyWarning {
.into(),
linttype: jmsg["message"]["code"]["code"].to_string().trim_matches('"').into(),
message: jmsg["message"]["message"].to_string().trim_matches('"').into(),
ice: json_message.contains("internal compiler error: "),
}
}

Expand Down Expand Up @@ -288,14 +335,15 @@ pub fn run(clap_config: &ArgMatches) {
// download and extract the crates, then run clippy on them and collect clippys warnings
// flatten into one big list of warnings

let crates = read_crates(clap_config.value_of("crates-toml"));
let (filename, crates) = read_crates(clap_config.value_of("crates-toml"));

let clippy_warnings: Vec<ClippyWarning> = if let Some(only_one_crate) = clap_config.value_of("only") {
// 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,
};
name == only_one_crate
}) {
Expand Down Expand Up @@ -326,6 +374,13 @@ pub fn run(clap_config: &ArgMatches) {

// generate some stats:

// grab crashes/ICEs, save the crate name and the ice message
let ices: Vec<(&String, &String)> = clippy_warnings
.iter()
.filter(|warning| warning.ice)
.map(|w| (&w.crate_name, &w.message))
.collect();

// count lint type occurrences
let mut counter: HashMap<&String, usize> = HashMap::new();
clippy_warnings
Expand All @@ -351,5 +406,10 @@ pub fn run(clap_config: &ArgMatches) {
// save the text into lintcheck-logs/logs.txt
let mut text = clippy_ver; // clippy version number on top
text.push_str(&format!("\n{}", all_msgs.join("")));
write("lintcheck-logs/logs.txt", text).unwrap();
text.push_str("ICEs:\n");
ices.iter()
.for_each(|(cratename, msg)| text.push_str(&format!("{}: '{}'", cratename, msg)));

let file = format!("lintcheck-logs/{}_logs.txt", filename);
write(file, text).unwrap();
}
File renamed without changes.