From ebbd00b1abb2d6d2f189288a10f23556ee997424 Mon Sep 17 00:00:00 2001 From: Arnavion Date: Sat, 22 Oct 2016 18:15:42 -0700 Subject: [PATCH 1/3] Don't assume the first package in the result of `cargo metadata` is the current crate. Instead find the one with the manifest path that matches the --manifest-path argument or the current directory. Fixes #1247 --- clippy_lints/src/utils/cargo.rs | 6 +++--- src/main.rs | 26 +++++++++++++++++++++++--- tests/versioncheck.rs | 4 ++-- 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/clippy_lints/src/utils/cargo.rs b/clippy_lints/src/utils/cargo.rs index 48a97f2f1..0722cef96 100644 --- a/clippy_lints/src/utils/cargo.rs +++ b/clippy_lints/src/utils/cargo.rs @@ -20,7 +20,7 @@ pub struct Package { pub dependencies: Vec, pub targets: Vec, features: HashMap>, - manifest_path: String, + pub manifest_path: String, } #[derive(RustcDecodable, Debug)] @@ -65,10 +65,10 @@ impl From for Error { } } -pub fn metadata(manifest_path: Option) -> Result { +pub fn metadata(manifest_path_arg: &Option) -> Result { let mut cmd = Command::new("cargo"); cmd.arg("metadata").arg("--no-deps"); - if let Some(ref mani) = manifest_path { + if let Some(ref mani) = *manifest_path_arg { cmd.arg(mani); } let output = cmd.output()?; diff --git a/src/main.rs b/src/main.rs index ed1c30d56..29713a183 100644 --- a/src/main.rs +++ b/src/main.rs @@ -138,10 +138,30 @@ pub fn main() { if let Some("clippy") = std::env::args().nth(1).as_ref().map(AsRef::as_ref) { // this arm is executed on the initial call to `cargo clippy` - let manifest_path = std::env::args().skip(2).find(|val| val.starts_with("--manifest-path=")); - let mut metadata = cargo::metadata(manifest_path).expect("could not obtain cargo metadata"); + let manifest_path_arg = std::env::args().skip(2).find(|val| val.starts_with("--manifest-path=")); + + let mut metadata = cargo::metadata(&manifest_path_arg).expect("could not obtain cargo metadata"); assert_eq!(metadata.version, 1); - for target in metadata.packages.remove(0).targets { + + let manifest_path = manifest_path_arg.map(|arg| PathBuf::from(Path::new(&arg["--manifest-path=".len()..]))); + + let current_dir = std::env::current_dir(); + + let package_index = metadata.packages.iter() + .position(|package| { + let package_manifest_path = Path::new(&package.manifest_path); + if let Some(ref manifest_path) = manifest_path { + package_manifest_path == manifest_path + } else if let Ok(ref current_dir) = current_dir { + let package_manifest_directory = package_manifest_path.parent().expect("could not find parent directory of package manifest"); + package_manifest_directory == current_dir + } else { + panic!("could not read current directory") + } + }) + .expect("could not find matching package"); + let package = metadata.packages.remove(package_index); + for target in package.targets { let args = std::env::args().skip(2); if let Some(first) = target.kind.get(0) { if target.kind.len() > 1 || first.ends_with("lib") { diff --git a/tests/versioncheck.rs b/tests/versioncheck.rs index e216c8015..8abb95c57 100644 --- a/tests/versioncheck.rs +++ b/tests/versioncheck.rs @@ -3,9 +3,9 @@ use clippy_lints::utils::cargo; #[test] fn check_that_clippy_lints_has_the_same_version_as_clippy() { - let clippy_meta = cargo::metadata(None).expect("could not obtain cargo metadata"); + let clippy_meta = cargo::metadata(&None).expect("could not obtain cargo metadata"); std::env::set_current_dir(std::env::current_dir().unwrap().join("clippy_lints")).unwrap(); - let clippy_lints_meta = cargo::metadata(None).expect("could not obtain cargo metadata"); + let clippy_lints_meta = cargo::metadata(&None).expect("could not obtain cargo metadata"); assert_eq!(clippy_lints_meta.packages[0].version, clippy_meta.packages[0].version); for package in &clippy_meta.packages[0].dependencies { if package.name == "clippy_lints" { From 2315a817ce6589f3f27d0f80192c037c3e9bb5ad Mon Sep 17 00:00:00 2001 From: Arnavion Date: Sun, 23 Oct 2016 12:24:16 -0700 Subject: [PATCH 2/3] Changed signature of cargo::metadata according to review comment. --- clippy_lints/src/utils/cargo.rs | 4 ++-- src/main.rs | 2 +- tests/versioncheck.rs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/utils/cargo.rs b/clippy_lints/src/utils/cargo.rs index 0722cef96..b9ce8626e 100644 --- a/clippy_lints/src/utils/cargo.rs +++ b/clippy_lints/src/utils/cargo.rs @@ -65,10 +65,10 @@ impl From for Error { } } -pub fn metadata(manifest_path_arg: &Option) -> Result { +pub fn metadata(manifest_path_arg: Option<&str>) -> Result { let mut cmd = Command::new("cargo"); cmd.arg("metadata").arg("--no-deps"); - if let Some(ref mani) = *manifest_path_arg { + if let Some(mani) = manifest_path_arg { cmd.arg(mani); } let output = cmd.output()?; diff --git a/src/main.rs b/src/main.rs index 29713a183..c008dae2f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -140,7 +140,7 @@ pub fn main() { // this arm is executed on the initial call to `cargo clippy` let manifest_path_arg = std::env::args().skip(2).find(|val| val.starts_with("--manifest-path=")); - let mut metadata = cargo::metadata(&manifest_path_arg).expect("could not obtain cargo metadata"); + let mut metadata = cargo::metadata(manifest_path_arg.as_ref().map(AsRef::as_ref)).expect("could not obtain cargo metadata"); assert_eq!(metadata.version, 1); let manifest_path = manifest_path_arg.map(|arg| PathBuf::from(Path::new(&arg["--manifest-path=".len()..]))); diff --git a/tests/versioncheck.rs b/tests/versioncheck.rs index 8abb95c57..e216c8015 100644 --- a/tests/versioncheck.rs +++ b/tests/versioncheck.rs @@ -3,9 +3,9 @@ use clippy_lints::utils::cargo; #[test] fn check_that_clippy_lints_has_the_same_version_as_clippy() { - let clippy_meta = cargo::metadata(&None).expect("could not obtain cargo metadata"); + let clippy_meta = cargo::metadata(None).expect("could not obtain cargo metadata"); std::env::set_current_dir(std::env::current_dir().unwrap().join("clippy_lints")).unwrap(); - let clippy_lints_meta = cargo::metadata(&None).expect("could not obtain cargo metadata"); + let clippy_lints_meta = cargo::metadata(None).expect("could not obtain cargo metadata"); assert_eq!(clippy_lints_meta.packages[0].version, clippy_meta.packages[0].version); for package in &clippy_meta.packages[0].dependencies { if package.name == "clippy_lints" { From 604694bc0b2f445520469d62ecf54d60f3cc39db Mon Sep 17 00:00:00 2001 From: Arnavion Date: Sun, 23 Oct 2016 12:29:33 -0700 Subject: [PATCH 3/3] Use .expect() for extracting the current_dir. --- src/main.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main.rs b/src/main.rs index c008dae2f..0eef99bfc 100644 --- a/src/main.rs +++ b/src/main.rs @@ -152,11 +152,10 @@ pub fn main() { let package_manifest_path = Path::new(&package.manifest_path); if let Some(ref manifest_path) = manifest_path { package_manifest_path == manifest_path - } else if let Ok(ref current_dir) = current_dir { + } else { + let current_dir = current_dir.as_ref().expect("could not read current directory"); let package_manifest_directory = package_manifest_path.parent().expect("could not find parent directory of package manifest"); package_manifest_directory == current_dir - } else { - panic!("could not read current directory") } }) .expect("could not find matching package");