Merge #5433

5433: Simplify exclusion logic r=matklad a=matklad



bors r+
🤖

Co-authored-by: Aleksey Kladov <[email protected]>

5 files changed, 120 insertions, 110 deletions

diff --git a/crates/rust-analyzer/Cargo.toml b/crates/rust-analyzer/Cargo.toml
index 3ed2a689e..5eb2d0bb7 100644
--- a/crates/rust-analyzer/Cargo.toml
+++ b/crates/rust-analyzer/Cargo.toml
@@ -17,7 +17,6 @@ path = "src/bin/main.rs"
 anyhow = "1.0.26"
 crossbeam-channel = "0.4.0"
 env_logger = { version = "0.7.1", default-features = false }
-globset = "0.4.4"
 itertools = "0.9.0"
 jod-thread = "0.1.0"
 log = "0.4.8"
diff --git a/crates/vfs-notify/Cargo.toml b/crates/vfs-notify/Cargo.toml
index 95c56ffa6..fce7bae3a 100644
--- a/crates/vfs-notify/Cargo.toml
+++ b/crates/vfs-notify/Cargo.toml
@@ -13,7 +13,6 @@ log = "0.4.8"
 rustc-hash = "1.0"
 jod-thread = "0.1.0"
 walkdir = "2.3.1"
-globset = "0.4.5"
 crossbeam-channel = "0.4.0"
 notify = "5.0.0-pre.3"
 
diff --git a/crates/vfs-notify/src/include.rs b/crates/vfs-notify/src/include.rs
deleted file mode 100644
index 59da3d77a..000000000
--- a/crates/vfs-notify/src/include.rs
+++ /dev/null
@@ -1,42 +0,0 @@
-//! See `Include`.
-use std::convert::TryFrom;
-
-use globset::{Glob, GlobSet, GlobSetBuilder};
-use paths::{RelPath, RelPathBuf};
-
-/// `Include` is the opposite of .gitignore.
-///
-/// It describes the set of files inside some directory.
-///
-/// The current implementation is very limited, it allows including file globs
-/// and recursively excluding directories.
-#[derive(Debug, Clone)]
-pub(crate) struct Include {
-    include_files: GlobSet,
-    exclude_dirs: Vec<RelPathBuf>,
-}
-
-impl Include {
-    pub(crate) fn new(include: Vec<String>) -> Include {
-        let mut include_files = GlobSetBuilder::new();
-        let mut exclude_dirs = Vec::new();
-
-        for glob in include {
-            if glob.starts_with("!/") {
-                if let Ok(path) = RelPathBuf::try_from(&glob["!/".len()..]) {
-                    exclude_dirs.push(path)
-                }
-            } else {
-                include_files.add(Glob::new(&glob).unwrap());
-            }
-        }
-        let include_files = include_files.build().unwrap();
-        Include { include_files, exclude_dirs }
-    }
-    pub(crate) fn include_file(&self, path: &RelPath) -> bool {
-        self.include_files.is_match(path)
-    }
-    pub(crate) fn exclude_dir(&self, path: &RelPath) -> bool {
-        self.exclude_dirs.iter().any(|excluded| path.starts_with(excluded))
-    }
-}
diff --git a/crates/vfs-notify/src/lib.rs b/crates/vfs-notify/src/lib.rs
index 6aaa53f63..e1e36612a 100644
--- a/crates/vfs-notify/src/lib.rs
+++ b/crates/vfs-notify/src/lib.rs
@@ -6,9 +6,7 @@
 //!
 //! Hopefully, one day a reliable file watching/walking crate appears on
 //! crates.io, and we can reduce this to trivial glue code.
-mod include;
-
-use std::convert::{TryFrom, TryInto};
+use std::convert::TryFrom;
 
 use crossbeam_channel::{never, select, unbounded, Receiver, Sender};
 use notify::{RecommendedWatcher, RecursiveMode, Watcher};
@@ -16,8 +14,6 @@ use paths::{AbsPath, AbsPathBuf};
 use vfs::loader;
 use walkdir::WalkDir;
 
-use crate::include::Include;
-
 #[derive(Debug)]
 pub struct NotifyHandle {
     // Relative order of fields below is significant.
@@ -53,7 +49,7 @@ type NotifyEvent = notify::Result<notify::Event>;
 
 struct NotifyActor {
     sender: loader::Sender,
-    config: Vec<(AbsPathBuf, Include, bool)>,
+    watched_entries: Vec<loader::Entry>,
     // Drop order is significant.
     watcher: Option<(RecommendedWatcher, Receiver<NotifyEvent>)>,
 }
@@ -66,7 +62,7 @@ enum Event {
 
 impl NotifyActor {
     fn new(sender: loader::Sender) -> NotifyActor {
-        NotifyActor { sender, config: Vec::new(), watcher: None }
+        NotifyActor { sender, watched_entries: Vec::new(), watcher: None }
     }
     fn next_event(&self, receiver: &Receiver<Message>) -> Option<Event> {
         let watcher_receiver = self.watcher.as_ref().map(|(_, receiver)| receiver);
@@ -93,15 +89,17 @@ impl NotifyActor {
                         let n_total = config.load.len();
                         self.send(loader::Message::Progress { n_total, n_done: 0 });
 
-                        self.config.clear();
+                        self.watched_entries.clear();
 
                         for (i, entry) in config.load.into_iter().enumerate() {
                             let watch = config.watch.contains(&i);
+                            if watch {
+                                self.watched_entries.push(entry.clone())
+                            }
                             let files = self.load_entry(entry, watch);
                             self.send(loader::Message::Loaded { files });
                             self.send(loader::Message::Progress { n_total, n_done: i + 1 });
                         }
-                        self.config.sort_by(|x, y| x.0.cmp(&y.0));
                     }
                     Message::Invalidate(path) => {
                         let contents = read(path.as_path());
@@ -116,34 +114,27 @@ impl NotifyActor {
                             .into_iter()
                             .map(|path| AbsPathBuf::try_from(path).unwrap())
                             .filter_map(|path| {
-                                let is_dir = path.is_dir();
-                                let is_file = path.is_file();
-
-                                let config_idx =
-                                    match self.config.binary_search_by(|it| it.0.cmp(&path)) {
-                                        Ok(it) => it,
-                                        Err(it) => it.saturating_sub(1),
-                                    };
-                                let include = self.config.get(config_idx).and_then(|it| {
-                                    let rel_path = path.strip_prefix(&it.0)?;
-                                    Some((rel_path, &it.1))
-                                });
-
-                                if let Some((rel_path, include)) = include {
-                                    if is_dir && include.exclude_dir(&rel_path)
-                                        || is_file && !include.include_file(&rel_path)
-                                    {
-                                        return None;
-                                    }
+                                if path.is_dir()
+                                    && self
+                                        .watched_entries
+                                        .iter()
+                                        .any(|entry| entry.contains_dir(&path))
+                                {
+                                    self.watch(path);
+                                    return None;
                                 }
 
-                                if is_dir {
-                                    self.watch(path);
+                                if !path.is_file() {
                                     return None;
                                 }
-                                if !is_file {
+                                if !self
+                                    .watched_entries
+                                    .iter()
+                                    .any(|entry| entry.contains_file(&path))
+                                {
                                     return None;
                                 }
+
                                 let contents = read(&path);
                                 Some((path, contents))
                             })
@@ -170,43 +161,42 @@ impl NotifyActor {
                     (file, contents)
                 })
                 .collect::<Vec<_>>(),
-            loader::Entry::Directory { path, include } => {
-                let include = Include::new(include);
-                self.config.push((path.clone(), include.clone(), watch));
-
-                let files = WalkDir::new(&path)
-                    .into_iter()
-                    .filter_entry(|entry| {
-                        let abs_path: &AbsPath = entry.path().try_into().unwrap();
-                        match abs_path.strip_prefix(&path) {
-                            Some(rel_path) => {
-                                !(entry.file_type().is_dir() && include.exclude_dir(rel_path))
-                            }
-                            None => false,
+            loader::Entry::Directories(dirs) => {
+                let mut res = Vec::new();
+
+                for root in dirs.include.iter() {
+                    let walkdir = WalkDir::new(root).into_iter().filter_entry(|entry| {
+                        if !entry.file_type().is_dir() {
+                            return true;
                         }
-                    })
-                    .filter_map(|entry| entry.ok())
-                    .filter_map(|entry| {
+                        let path = AbsPath::assert(entry.path());
+                        root == path
+                            || dirs.exclude.iter().chain(&dirs.include).all(|it| it != path)
+                    });
+
+                    let files = walkdir.filter_map(|it| it.ok()).filter_map(|entry| {
                         let is_dir = entry.file_type().is_dir();
                         let is_file = entry.file_type().is_file();
-                        let abs_path = AbsPathBuf::try_from(entry.into_path()).unwrap();
+                        let abs_path = AbsPathBuf::assert(entry.into_path());
                         if is_dir && watch {
                             self.watch(abs_path.clone());
                         }
-                        let rel_path = abs_path.strip_prefix(&path)?;
-                        if is_file && include.include_file(&rel_path) {
-                            Some(abs_path)
-                        } else {
-                            None
+                        if !is_file {
+                            return None;
+                        }
+                        let ext = abs_path.extension().unwrap_or_default();
+                        if dirs.extensions.iter().all(|it| it.as_str() != ext) {
+                            return None;
                         }
+                        Some(abs_path)
                     });
 
-                files
-                    .map(|file| {
+                    res.extend(files.map(|file| {
                         let contents = read(file.as_path());
                         (file, contents)
-                    })
-                    .collect()
+                    }));
+                }
+                res
             }
         }
     }
diff --git a/crates/vfs/src/loader.rs b/crates/vfs/src/loader.rs
index 6de2e5b3f..9c6e4b6a7 100644
--- a/crates/vfs/src/loader.rs
+++ b/crates/vfs/src/loader.rs
@@ -3,10 +3,25 @@ use std::fmt;
 
 use paths::{AbsPath, AbsPathBuf};
 
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub enum Entry {
     Files(Vec<AbsPathBuf>),
-    Directory { path: AbsPathBuf, include: Vec<String> },
+    Directories(Directories),
+}
+
+/// Specifies a set of files on the file system.
+///
+/// A file is included if:
+///   * it has included extension
+///   * it is under an `include` path
+///   * it is not under `exclude` path
+///
+/// If many include/exclude paths match, the longest one wins.
+#[derive(Debug, Clone)]
+pub struct Directories {
+    pub extensions: Vec<String>,
+    pub include: Vec<AbsPathBuf>,
+    pub exclude: Vec<AbsPathBuf>,
 }
 
 #[derive(Debug)]
@@ -33,21 +48,70 @@ pub trait Handle: fmt::Debug {
 
 impl Entry {
     pub fn rs_files_recursively(base: AbsPathBuf) -> Entry {
-        Entry::Directory { path: base, include: globs(&["*.rs", "!/.git/"]) }
+        Entry::Directories(dirs(base, &[".git"]))
     }
     pub fn local_cargo_package(base: AbsPathBuf) -> Entry {
-        Entry::Directory { path: base, include: globs(&["*.rs", "!/target/", "!/.git/"]) }
+        Entry::Directories(dirs(base, &[".git", "target"]))
     }
     pub fn cargo_package_dependency(base: AbsPathBuf) -> Entry {
-        Entry::Directory {
-            path: base,
-            include: globs(&["*.rs", "!/tests/", "!/examples/", "!/benches/", "!/.git/"]),
+        Entry::Directories(dirs(base, &[".git", "/tests", "/examples", "/benches"]))
+    }
+
+    pub fn contains_file(&self, path: &AbsPath) -> bool {
+        match self {
+            Entry::Files(files) => files.iter().any(|it| it == path),
+            Entry::Directories(dirs) => dirs.contains_file(path),
+        }
+    }
+    pub fn contains_dir(&self, path: &AbsPath) -> bool {
+        match self {
+            Entry::Files(_) => false,
+            Entry::Directories(dirs) => dirs.contains_dir(path),
+        }
+    }
+}
+
+impl Directories {
+    pub fn contains_file(&self, path: &AbsPath) -> bool {
+        let ext = path.extension().unwrap_or_default();
+        if self.extensions.iter().all(|it| it.as_str() != ext) {
+            return false;
+        }
+        self.includes_path(path)
+    }
+    pub fn contains_dir(&self, path: &AbsPath) -> bool {
+        self.includes_path(path)
+    }
+    fn includes_path(&self, path: &AbsPath) -> bool {
+        let mut include = None;
+        for incl in &self.include {
+            if is_prefix(incl, path) {
+                include = Some(match include {
+                    Some(prev) if is_prefix(incl, prev) => prev,
+                    _ => incl,
+                })
+            }
+        }
+        let include = match include {
+            Some(it) => it,
+            None => return false,
+        };
+        for excl in &self.exclude {
+            if is_prefix(excl, path) && is_prefix(include, excl) {
+                return false;
+            }
+        }
+        return true;
+
+        fn is_prefix(short: &AbsPath, long: &AbsPath) -> bool {
+            long.strip_prefix(short).is_some()
         }
     }
 }
 
-fn globs(globs: &[&str]) -> Vec<String> {
-    globs.iter().map(|it| it.to_string()).collect()
+fn dirs(base: AbsPathBuf, exclude: &[&str]) -> Directories {
+    let exclude = exclude.iter().map(|it| base.join(it)).collect::<Vec<_>>();
+    Directories { extensions: vec!["rs".to_string()], include: vec![base], exclude }
 }
 
 impl fmt::Debug for Message {