Merge pull request rust-lang#1896 from Arnavion/fix-workspace

Fix logic that determines closest parent crate when invoked from a subdirectory.

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

clippy_workspace_tests/Cargo.toml

@@ -0,0 +1,6 @@
+[package]
+name = "clippy_workspace_tests"
+version = "0.1.0"
+
+[workspace]
+members = ["subcrate"]

clippy_workspace_tests/src/main.rs

@@ -0,0 +1,2 @@
+fn main() {
+}

clippy_workspace_tests/subcrate/Cargo.toml

@@ -0,0 +1,3 @@
+[package]
+name = "subcrate"
+version = "0.1.0"

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<Item = &'a T>>(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::<Vec<_>>();
+
+                    let mut current_path: &Path = &current_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::<Vec<_>>();
-                            if has_prefix(&current_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");