Address feedback

benches/std_api_crate.rs

@@ -1,3 +1,11 @@
+// Copyright 2017 The RLS Project Developers.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
 #![feature(test)]
 
 extern crate rls_analysis;
@@ -33,7 +41,7 @@ impl AnalysisLoader for TestAnalysisLoader {
         panic!();
     }
 
-    fn paths(&self) -> Vec<PathBuf> { vec![self.path.clone()] }
+    fn search_directories(&self) -> Vec<PathBuf> { vec![self.path.clone()] }
 }
 
 lazy_static! {

src/analysis.rs

@@ -37,7 +37,6 @@ pub struct PerCrateAnalysis {
     pub globs: HashMap<Span, Glob>,
     pub impls: HashMap<Id, Vec<Span>>,
 
-    pub crate_id: CrateId,
     pub root_id: Option<Id>,
     pub timestamp: SystemTime,
 }
@@ -79,7 +78,7 @@ pub struct Glob {
 
 
 impl PerCrateAnalysis {
-    pub fn new(crate_id: CrateId, timestamp: SystemTime) -> PerCrateAnalysis {
+    pub fn new(timestamp: SystemTime) -> PerCrateAnalysis {
         PerCrateAnalysis {
             def_id_for_span: HashMap::new(),
             defs: HashMap::new(),
@@ -89,7 +88,6 @@ impl PerCrateAnalysis {
             ref_spans: HashMap::new(),
             globs: HashMap::new(),
             impls: HashMap::new(),
-            crate_id,
             root_id: None,
             timestamp,
         }
@@ -109,7 +107,7 @@ impl Analysis {
     pub fn timestamps(&self) -> HashMap<CrateId, SystemTime> {
         self.per_crate
             .iter()
-            .map(|(id,c)| (id.clone(), c.timestamp.clone()))
+            .map(|(id, c)| (id.clone(), c.timestamp.clone()))
             .collect()
     }
 

src/lib.rs

@@ -162,8 +162,10 @@ impl<L: AnalysisLoader> AnalysisHost<L> {
         }
 
         let timestamps = self.analysis.lock()?.as_ref().unwrap().timestamps();
-        let loader = self.loader.lock()?;
-        let raw_analysis = read_analysis_from_files(&*loader, timestamps, blacklist);
+        let raw_analysis = {
+            let loader = self.loader.lock()?;
+            read_analysis_from_files(&*loader, timestamps, blacklist)
+        };
 
         lowering::lower(raw_analysis, base_dir, self, |host, per_crate, id| {
             let mut a = host.analysis.lock()?;
@@ -273,7 +275,12 @@ impl<L: AnalysisLoader> AnalysisHost<L> {
     pub fn def_roots(&self) -> AResult<Vec<(Id, String)>> {
         self.with_analysis(|a| {
             Some(
-                a.for_all_crates(|c| c.root_id.map(|id| vec![(id, c.crate_id.name.clone())])),
+                a.per_crate
+                .iter()
+                .filter_map(|(crate_id, data)| {
+                    data.root_id.map(|id| (id, crate_id.name.clone()))
+                })
+                .collect()
             )
         })
     }

src/loader.rs

@@ -40,9 +40,8 @@ pub trait AnalysisLoader: Sized {
     fn fresh_host(&self) -> AnalysisHost<Self>;
     fn set_path_prefix(&mut self, path_prefix: &Path);
     fn abs_path_prefix(&self) -> Option<PathBuf>;
-    /// Returns every directory in which paths are to be considered.
-    /// TODO: Can it also return files?
-    fn paths(&self) -> Vec<PathBuf>;
+    /// Returns every directory in which analysis files are to be considered.
+    fn search_directories(&self) -> Vec<PathBuf>;
 }
 
 impl AnalysisLoader for CargoAnalysisLoader {
@@ -66,7 +65,7 @@ impl AnalysisLoader for CargoAnalysisLoader {
             .map(|s| Path::new(s).canonicalize().unwrap().to_owned())
     }
 
-    fn paths(&self) -> Vec<PathBuf> {
+    fn search_directories(&self) -> Vec<PathBuf> {
         let path_prefix = self.path_prefix.as_ref().unwrap();
         let target = self.target.to_string();
 

src/lowering.rs

@@ -6,7 +6,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-//! For processing the raw save-analysis data from rustc into rustw's
+//! For processing the raw save-analysis data from rustc into the rls
 //! in-memory representation.
 
 use analysis::{Def, Glob, PerCrateAnalysis};
@@ -146,15 +146,15 @@ impl CrateReader {
             base_dir,
         );
 
-        let mut per_crate = PerCrateAnalysis::new(krate.id.clone(), krate.timestamp);
+        let mut per_crate = PerCrateAnalysis::new(krate.timestamp);
 
         let is_distro_crate = krate.analysis.config.distro_crate;
         reader.read_defs(krate.analysis.defs, &mut per_crate, is_distro_crate);
         reader.read_imports(krate.analysis.imports, &mut per_crate, project_analysis);
         reader.read_refs(krate.analysis.refs, &mut per_crate, project_analysis);
         reader.read_impls(krate.analysis.relations, &mut per_crate, project_analysis);
 
-        (per_crate, krate.id.clone())
+        (per_crate, krate.id)
     }
 
     fn read_imports<L: AnalysisLoader>(

src/raw.rs

@@ -44,7 +44,7 @@ pub fn read_analysis_from_files<L: AnalysisLoader>(
 ) -> Vec<Crate> {
     let mut result = vec![];
 
-    loader.paths()
+    loader.search_directories()
     .iter()
     .inspect(|path| trace!("Considering analysis files at {}", path.display()))
     .filter_map(|p| DirectoryListing::from_path(p).ok().map(|list| (p, list)))
@@ -59,10 +59,10 @@ pub fn read_analysis_from_files<L: AnalysisLoader>(
                 }
 
                 let path = p.join(&l.name);
-                // TODO: Bring back path-based timestamps?
-                // This would speed up a bit reloading time, since non-blacklisted
-                // can be older than the ones loaded, so don't waste time
-                // reading the file and deserializing
+                // TODO: Bring back path-based timestamps, so we can discard
+                // stale data before reading the file and attempting the
+                // deserialization, as it can take a considerate amount of time
+                // for big analysis data files.
                 //let is_fresh = timestamps.get(&path).map_or(true, |t| time > t);
                 read_crate_data(&path).map(|analysis| {
                     let is_fresh = {

src/test/mod.rs

@@ -31,7 +31,7 @@ impl AnalysisLoader for TestAnalysisLoader {
         panic!();
     }
 
-    fn paths(&self) -> Vec<PathBuf> { vec![self.path.clone()] }
+    fn search_directories(&self) -> Vec<PathBuf> { vec![self.path.clone()] }
 }
 
 #[test]