Merge commit '70c0f90453701e7d6d9b99aaa1fc6a765937b736' into clippyup

4 files changed, 197 insertions, 59 deletions

diff --git a/clippy_dev/src/lintcheck.rs b/clippy_dev/src/lintcheck.rs
index 785c692d3cb..749a791b280 100644
--- a/clippy_dev/src/lintcheck.rs
+++ b/clippy_dev/src/lintcheck.rs
@@ -20,24 +20,31 @@ use serde_json::Value;
 // use this to store the crates when interacting with the crates.toml file
 #[derive(Debug, Serialize, Deserialize)]
 struct CrateList {
-    crates: HashMap<String, Vec<String>>,
+    crates: HashMap<String, TomlCrate>,
 }
 
 // crate data we stored in the toml, can have multiple versions per crate
 // A single TomlCrate is laster mapped to several CrateSources in that case
+#[derive(Debug, Serialize, Deserialize)]
 struct TomlCrate {
     name: String,
-    versions: Vec<String>,
+    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)]
-struct CrateSource {
-    name: String,
-    version: String,
+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
+// we actually don't need to special-case git repos here because it does not matter for clippy, yay!
+// (clippy only needs a simple path)
 #[derive(Debug)]
 struct Crate {
     version: String,
@@ -55,6 +62,7 @@ struct ClippyWarning {
     column: String,
     linttype: String,
     message: String,
+    ice: bool,
 }
 
 impl std::fmt::Display for ClippyWarning {
@@ -69,40 +77,102 @@ impl std::fmt::Display for ClippyWarning {
 
 impl CrateSource {
     fn download_and_extract(&self) -> Crate {
-        let extract_dir = PathBuf::from("target/lintcheck/crates");
-        let krate_download_dir = PathBuf::from("target/lintcheck/downloads");
-
-        // url to download the crate from crates.io
-        let url = format!(
-            "https://crates.io/api/v1/crates/{}/{}/download",
-            self.name, self.version
-        );
-        println!("Downloading and extracting {} {} from {}", self.name, self.version, url);
-        let _ = std::fs::create_dir("target/lintcheck/");
-        let _ = std::fs::create_dir(&krate_download_dir);
-        let _ = std::fs::create_dir(&extract_dir);
-
-        let krate_file_path = krate_download_dir.join(format!("{}-{}.crate.tar.gz", &self.name, &self.version));
-        // don't download/extract if we already have done so
-        if !krate_file_path.is_file() {
-            // create a file path to download and write the crate data into
-            let mut krate_dest = std::fs::File::create(&krate_file_path).unwrap();
-            let mut krate_req = ureq::get(&url).call().unwrap().into_reader();
-            // copy the crate into the file
-            std::io::copy(&mut krate_req, &mut krate_dest).unwrap();
-
-            // unzip the tarball
-            let ungz_tar = flate2::read::GzDecoder::new(std::fs::File::open(&krate_file_path).unwrap());
-            // extract the tar archive
-            let mut archive = tar::Archive::new(ungz_tar);
-            archive.unpack(&extract_dir).expect("Failed to extract!");
-        }
-        // crate is extracted, return a new Krate object which contains the path to the extracted
-        // sources that clippy can check
-        Crate {
-            version: self.version.clone(),
-            name: self.name.clone(),
-            path: extract_dir.join(format!("{}-{}/", self.name, self.version)),
+        match self {
+            CrateSource::CratesIo { name, version } => {
+                let extract_dir = PathBuf::from("target/lintcheck/crates");
+                let krate_download_dir = PathBuf::from("target/lintcheck/downloads");
+
+                // 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);
+
+                let krate_file_path = krate_download_dir.join(format!("{}-{}.crate.tar.gz", name, version));
+                // don't download/extract if we already have done so
+                if !krate_file_path.is_file() {
+                    // create a file path to download and write the crate data into
+                    let mut krate_dest = std::fs::File::create(&krate_file_path).unwrap();
+                    let mut krate_req = ureq::get(&url).call().unwrap().into_reader();
+                    // copy the crate into the file
+                    std::io::copy(&mut krate_req, &mut krate_dest).unwrap();
+
+                    // unzip the tarball
+                    let ungz_tar = flate2::read::GzDecoder::new(std::fs::File::open(&krate_file_path).unwrap());
+                    // extract the tar archive
+                    let mut archive = tar::Archive::new(ungz_tar);
+                    archive.unpack(&extract_dir).expect("Failed to extract!");
+                }
+                // crate is extracted, return a new Krate object which contains the path to the extracted
+                // sources that clippy can check
+                Crate {
+                    version: version.clone(),
+                    name: name.clone(),
+                    path: extract_dir.join(format!("{}-{}/", name, version)),
+                }
+            },
+            CrateSource::Git { name, url, commit } => {
+                let repo_path = {
+                    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
+                };
+                // clone the repo if we have not done so
+                if !repo_path.is_dir() {
+                    println!("Cloning {} and checking out {}", url, commit);
+                    Command::new("git")
+                        .arg("clone")
+                        .arg(url)
+                        .arg(&repo_path)
+                        .output()
+                        .expect("Failed to clone git repo!");
+                }
+                // check out the commit/branch/whatever
+                Command::new("git")
+                    .arg("checkout")
+                    .arg(commit)
+                    .output()
+                    .expect("Failed to check out commit");
+
+                Crate {
+                    version: commit.clone(),
+                    name: name.clone(),
+                    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,
+                }
+            },
         }
     }
 }
@@ -114,7 +184,7 @@ impl Crate {
 
         let shared_target_dir = clippy_project_root().join("target/lintcheck/shared_target_dir/");
 
-        let all_output = std::process::Command::new(cargo_clippy_path)
+        let all_output = std::process::Command::new(&cargo_clippy_path)
             .env("CARGO_TARGET_DIR", shared_target_dir)
             // lint warnings will look like this:
             // src/cargo/ops/cargo_compile.rs:127:35: warning: usage of `FromIterator::from_iter`
@@ -128,14 +198,20 @@ impl Crate {
             ])
             .current_dir(&self.path)
             .output()
-            .unwrap();
+            .unwrap_or_else(|error| {
+                panic!(
+                    "Encountered error:\n{:?}\ncargo_clippy_path: {}\ncrate path:{}\n",
+                    error,
+                    &cargo_clippy_path.display(),
+                    &self.path.display()
+                );
+            });
         let stdout = String::from_utf8_lossy(&all_output.stdout);
         let output_lines = stdout.lines();
-        //dbg!(&output_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
@@ -150,8 +226,10 @@ fn build_clippy() {
 }
 
 // get a list of CrateSources we want to check from a "lintcheck_crates.toml" file.
-fn read_crates() -> Vec<CrateSource> {
-    let toml_path = PathBuf::from("clippy_dev/lintcheck_crates.toml");
+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 =
@@ -160,21 +238,52 @@ fn read_crates() -> Vec<CrateSource> {
     let tomlcrates: Vec<TomlCrate> = crate_list
         .crates
         .into_iter()
-        .map(|(name, versions)| TomlCrate { name, versions })
+        .map(|(_cratename, tomlcrate)| tomlcrate)
         .collect();
 
     // flatten TomlCrates into CrateSources (one TomlCrates may represent several versions of a crate =>
     // multiple Cratesources)
     let mut crate_sources = Vec::new();
     tomlcrates.into_iter().for_each(|tk| {
-        tk.versions.iter().for_each(|ver| {
-            crate_sources.push(CrateSource {
+        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| {
+                crate_sources.push(CrateSource::CratesIo {
+                    name: tk.name.clone(),
+                    version: ver.to_string(),
+                });
+            })
+        }
+        // otherwise, we should have a git source
+        if tk.git_url.is_some() && tk.git_hash.is_some() {
+            crate_sources.push(CrateSource::Git {
                 name: tk.name.clone(),
-                version: ver.to_string(),
+                url: tk.git_url.clone().unwrap(),
+                commit: tk.git_hash.clone().unwrap(),
             });
-        })
+        }
+        // if we have a version as well as a git data OR only one git data, something is funky
+        if tk.versions.is_some() && (tk.git_url.is_some() || tk.git_hash.is_some())
+            || tk.git_hash.is_some() != tk.git_url.is_some()
+        {
+            eprintln!("tomlkrate: {:?}", tk);
+            if tk.git_hash.is_some() != tk.git_url.is_some() {
+                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
@@ -198,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: "),
     }
 }
 
@@ -225,11 +335,18 @@ 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();
+    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 crated in the .toml, throw an error
-        if !crates.iter().any(|krate| krate.name == only_one_crate) {
+        // 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
+        }) {
             eprintln!(
                 "ERROR: could not find crate '{}' in clippy_dev/lintcheck_crates.toml",
                 only_one_crate
@@ -257,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
@@ -282,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();
 }
diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs
index e7a298a37e1..5dbd46935a5 100644
--- a/clippy_dev/src/main.rs
+++ b/clippy_dev/src/main.rs
@@ -62,6 +62,13 @@ fn get_clap_config<'a>() -> ArgMatches<'a> {
                 .value_name("CRATE")
                 .long("only")
                 .help("only process a single crate of the list"),
+        )
+        .arg(
+            Arg::with_name("crates-toml")
+                .takes_value(true)
+                .value_name("CRATES-SOURCES-TOML-PATH")
+                .long("crates-toml")
+                .help("set the path for a crates.toml where lintcheck should read the sources from"),
         );
 
     let app = App::new("Clippy developer tooling")
diff --git a/clippy_dev/src/ra_setup.rs b/clippy_dev/src/ra_setup.rs
index a3c329b578b..d0e2193ddc5 100644
--- a/clippy_dev/src/ra_setup.rs
+++ b/clippy_dev/src/ra_setup.rs
@@ -13,7 +13,9 @@ use std::path::{Path, PathBuf};
 /// Panics if `rustc_path` does not lead to a rustc repo or the files could not be read
 pub fn run(rustc_path: Option<&str>) {
     // we can unwrap here because the arg is required by clap
-    let rustc_path = PathBuf::from(rustc_path.unwrap());
+    let rustc_path = PathBuf::from(rustc_path.unwrap())
+        .canonicalize()
+        .expect("failed to get the absolute repo path");
     assert!(rustc_path.is_dir(), "path is not a directory");
     let rustc_source_basedir = rustc_path.join("compiler");
     assert!(
diff --git a/clippy_dev/src/serve.rs b/clippy_dev/src/serve.rs
index faa94859601..d13c27a1957 100644
--- a/clippy_dev/src/serve.rs
+++ b/clippy_dev/src/serve.rs
@@ -34,7 +34,7 @@ pub fn run(port: u16, lint: Option<&str>) -> ! {
                 // Give some time for python to start
                 thread::sleep(Duration::from_millis(500));
                 // Launch browser after first export.py has completed and http.server is up
-                let _ = opener::open(url);
+                let _result = opener::open(url);
             });
         }
         thread::sleep(Duration::from_millis(1000));