From db7a5c69f10430b3a3853ca559adccc468b72d52 Mon Sep 17 00:00:00 2001 From: Arnavion Date: Thu, 27 Jul 2017 00:04:17 -0700 Subject: [PATCH] Fix logic that determines closest parent crate when invoked from a subdirectory. The previous logic incorrectly matches the deepest child of the current directory that is a crate. --- .travis.yml | 4 ++ clippy_workspace_tests/Cargo.toml | 6 +++ clippy_workspace_tests/src/main.rs | 2 + clippy_workspace_tests/subcrate/Cargo.toml | 3 ++ clippy_workspace_tests/subcrate/src/lib.rs | 0 src/main.rs | 59 +++++++++++----------- 6 files changed, 44 insertions(+), 30 deletions(-) create mode 100644 clippy_workspace_tests/Cargo.toml create mode 100644 clippy_workspace_tests/src/main.rs create mode 100644 clippy_workspace_tests/subcrate/Cargo.toml create mode 100644 clippy_workspace_tests/subcrate/src/lib.rs diff --git a/.travis.yml b/.travis.yml index 7abe1025ad9d..e4fa4608f0e8 100644 --- a/.travis.yml +++ b/.travis.yml @@ -34,6 +34,10 @@ script: - cp clippy_tests/target/debug/cargo-clippy ~/rust/cargo/bin/cargo-clippy - PATH=$PATH:~/rust/cargo/bin cargo clippy -- -D clippy - cd clippy_lints && PATH=$PATH:~/rust/cargo/bin cargo clippy -- -D clippy && cd .. + - cd clippy_workspace_tests && PATH=$PATH:~/rust/cargo/bin cargo clippy -- -D clippy && cd .. + - cd clippy_workspace_tests/src && PATH=$PATH:~/rust/cargo/bin cargo clippy -- -D clippy && cd ../.. + - cd clippy_workspace_tests/subcrate && PATH=$PATH:~/rust/cargo/bin cargo clippy -- -D clippy && cd ../.. + - cd clippy_workspace_tests/subcrate/src && PATH=$PATH:~/rust/cargo/bin cargo clippy -- -D clippy && cd ../../.. - set +e after_success: | diff --git a/clippy_workspace_tests/Cargo.toml b/clippy_workspace_tests/Cargo.toml new file mode 100644 index 000000000000..a282378c951c --- /dev/null +++ b/clippy_workspace_tests/Cargo.toml @@ -0,0 +1,6 @@ +[package] +name = "clippy_workspace_tests" +version = "0.1.0" + +[workspace] +members = ["subcrate"] diff --git a/clippy_workspace_tests/src/main.rs b/clippy_workspace_tests/src/main.rs new file mode 100644 index 000000000000..f79c691f0853 --- /dev/null +++ b/clippy_workspace_tests/src/main.rs @@ -0,0 +1,2 @@ +fn main() { +} diff --git a/clippy_workspace_tests/subcrate/Cargo.toml b/clippy_workspace_tests/subcrate/Cargo.toml new file mode 100644 index 000000000000..83ea5868160b --- /dev/null +++ b/clippy_workspace_tests/subcrate/Cargo.toml @@ -0,0 +1,3 @@ +[package] +name = "subcrate" +version = "0.1.0" diff --git a/clippy_workspace_tests/subcrate/src/lib.rs b/clippy_workspace_tests/subcrate/src/lib.rs new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/src/main.rs b/src/main.rs index 82c87b737c03..a48e91aa342a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -15,6 +15,7 @@ extern crate syntax; use rustc_driver::{driver, CompilerCalls, RustcDefaultCalls, Compilation}; use rustc::session::{config, Session, CompileIncomplete}; use rustc::session::config::{Input, ErrorOutputType}; +use std::collections::HashMap; use std::path::PathBuf; use std::process::{self, Command}; use syntax::ast; @@ -158,12 +159,6 @@ fn show_version() { println!("{}", env!("CARGO_PKG_VERSION")); } -// FIXME: false positive for needless_lifetimes -#[allow(needless_lifetimes)] -fn has_prefix<'a, T: PartialEq, I: Iterator>(v: &'a [T], itr: I) -> bool { - v.iter().zip(itr).all(|(a, b)| a == b) -} - pub fn main() { use std::env; @@ -199,43 +194,47 @@ pub fn main() { let manifest_path = manifest_path_arg.map(|arg| PathBuf::from(Path::new(&arg["--manifest-path=".len()..]))); let package_index = { - let mut iterator = metadata.packages.iter(); - if let Some(ref manifest_path) = manifest_path { - iterator.position(|package| { + metadata.packages.iter().position(|package| { let package_manifest_path = Path::new(&package.manifest_path); package_manifest_path == manifest_path }) } else { + let package_manifest_paths: HashMap<_, _> = + metadata.packages.iter() + .enumerate() + .map(|(i, package)| { + let package_manifest_path = Path::new(&package.manifest_path) + .parent() + .expect("could not find parent directory of package manifest") + .canonicalize() + .expect("package directory cannot be canonicalized"); + (package_manifest_path, i) + }) + .collect(); + let current_dir = std::env::current_dir() .expect("could not read current directory") .canonicalize() .expect("current directory cannot be canonicalized"); - let current_dir_components = current_dir.components().collect::>(); + + let mut current_path: &Path = ¤t_dir; // This gets the most-recent parent (the one that takes the fewest `cd ..`s to // reach). - iterator - .enumerate() - .filter_map(|(i, package)| { - let package_manifest_path = Path::new(&package.manifest_path); - let canonical_path = package_manifest_path + loop { + if let Some(&package_index) = package_manifest_paths.get(current_path) { + break Some(package_index); + } + else { + // We'll never reach the filesystem root, because to get to this point in the code + // the call to `cargo_metadata::metadata` must have succeeded. So it's okay to + // unwrap the current path's parent. + current_path = current_path .parent() - .expect("could not find parent directory of package manifest") - .canonicalize() - .expect("package directory cannot be canonicalized"); - - // TODO: We can do this in `O(1)` by combining the `len` and the - // iteration. - let components = canonical_path.components().collect::>(); - if has_prefix(¤t_dir_components, components.iter()) { - Some((i, components.len())) - } else { - None - } - }) - .max_by_key(|&(_, length)| length) - .map(|(i, _)| i) + .unwrap_or_else(|| panic!("could not find parent of path {}", current_path.display())); + } + } } } .expect("could not find matching package");