diff --git a/lib/src/fileset.rs b/lib/src/fileset.rs index 19ee786986..39712f5890 100644 --- a/lib/src/fileset.rs +++ b/lib/src/fileset.rs @@ -612,7 +612,7 @@ mod tests { @r###" PrefixMatcher { tree: Dir { - "foo": Dir|File {}, + "foo": Prefix {}, }, } "###); @@ -627,7 +627,7 @@ mod tests { insta::assert_debug_snapshot!(expr.to_matcher(), @r###" FilesMatcher { tree: Dir { - "foo": Dir|File { + "foo": File { "bar": File {}, }, }, @@ -641,8 +641,8 @@ mod tests { insta::assert_debug_snapshot!(expr.to_matcher(), @r###" PrefixMatcher { tree: Dir { - "bar": Dir|File { - "baz": Dir|File {}, + "bar": Prefix { + "baz": Prefix {}, }, }, } @@ -664,7 +664,7 @@ mod tests { }, input2: PrefixMatcher { tree: Dir { - "bar": Dir|File {}, + "bar": Prefix {}, }, }, } @@ -714,7 +714,7 @@ mod tests { }, input2: PrefixMatcher { tree: Dir { - "bar": Dir|File {}, + "bar": Prefix {}, }, }, }, diff --git a/lib/src/matchers.rs b/lib/src/matchers.rs index 41fe9c1e00..d319be7cf7 100644 --- a/lib/src/matchers.rs +++ b/lib/src/matchers.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -#![allow(dead_code, missing_docs)] +#![allow(missing_docs)] use std::collections::{HashMap, HashSet}; use std::fmt::Debug; @@ -122,14 +122,14 @@ impl Matcher for EverythingMatcher { #[derive(PartialEq, Eq, Debug)] pub struct FilesMatcher { - tree: RepoPathTree, + tree: RepoPathTree, } impl FilesMatcher { pub fn new(files: impl IntoIterator>) -> Self { - let mut tree = RepoPathTree::new(); + let mut tree = RepoPathTree::default(); for f in files { - tree.add_file(f.as_ref()); + tree.add(f.as_ref()).value = FilesNodeKind::File; } FilesMatcher { tree } } @@ -137,27 +137,53 @@ impl FilesMatcher { impl Matcher for FilesMatcher { fn matches(&self, file: &RepoPath) -> bool { - self.tree.get(file).map(|sub| sub.is_file).unwrap_or(false) + self.tree + .get(file) + .map_or(false, |sub| sub.value == FilesNodeKind::File) } fn visit(&self, dir: &RepoPath) -> Visit { - self.tree.get_visit_sets(dir) + self.tree + .get(dir) + .map_or(Visit::Nothing, files_tree_to_visit_sets) } } +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] +enum FilesNodeKind { + /// Represents an intermediate directory. + #[default] + Dir, + /// Represents a file (which might also be an intermediate directory.) + File, +} + +fn files_tree_to_visit_sets(tree: &RepoPathTree) -> Visit { + let mut dirs = HashSet::new(); + let mut files = HashSet::new(); + for (name, sub) in &tree.entries { + // should visit only intermediate directories + if !sub.entries.is_empty() { + dirs.insert(name.clone()); + } + if sub.value == FilesNodeKind::File { + files.insert(name.clone()); + } + } + Visit::sets(dirs, files) +} + #[derive(Debug)] pub struct PrefixMatcher { - tree: RepoPathTree, + tree: RepoPathTree, } impl PrefixMatcher { #[instrument(skip(prefixes))] pub fn new(prefixes: impl IntoIterator>) -> Self { - let mut tree = RepoPathTree::new(); + let mut tree = RepoPathTree::default(); for prefix in prefixes { - let sub = tree.add(prefix.as_ref()); - sub.is_dir = true; - sub.is_file = true; + tree.add(prefix.as_ref()).value = PrefixNodeKind::Prefix; } PrefixMatcher { tree } } @@ -165,24 +191,48 @@ impl PrefixMatcher { impl Matcher for PrefixMatcher { fn matches(&self, file: &RepoPath) -> bool { - self.tree.walk_to(file).any(|(sub, _)| sub.is_file) + self.tree + .walk_to(file) + .any(|(sub, _)| sub.value == PrefixNodeKind::Prefix) } fn visit(&self, dir: &RepoPath) -> Visit { for (sub, tail_path) in self.tree.walk_to(dir) { - // 'is_file' means the current path matches prefix paths - if sub.is_file { + // ancestor of 'dir' matches prefix paths + if sub.value == PrefixNodeKind::Prefix { return Visit::AllRecursively; } // 'dir' found, and is an ancestor of prefix paths if tail_path.is_root() { - return sub.to_visit_sets(); + return prefix_tree_to_visit_sets(sub); } } Visit::Nothing } } +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] +enum PrefixNodeKind { + /// Represents an intermediate directory. + #[default] + Dir, + /// Represents a file and prefix directory. + Prefix, +} + +fn prefix_tree_to_visit_sets(tree: &RepoPathTree) -> Visit { + let mut dirs = HashSet::new(); + let mut files = HashSet::new(); + for (name, sub) in &tree.entries { + // should visit both intermediate and prefix directories + dirs.insert(name.clone()); + if sub.value == PrefixNodeKind::Prefix { + files.insert(name.clone()); + } + } + Visit::sets(dirs, files) +} + /// Matches paths that are matched by any of the input matchers. #[derive(Clone, Debug)] pub struct UnionMatcher { @@ -338,93 +388,50 @@ impl Matcher for IntersectionMatcher { } } -/// Keeps track of which subdirectories and files of each directory need to be -/// visited. -#[derive(PartialEq, Eq)] -struct RepoPathTree { - entries: HashMap, - // is_dir/is_file aren't exclusive, both can be set to true. If entries is not empty, - // is_dir should be set. - is_dir: bool, - is_file: bool, +/// Tree that maps `RepoPath` to value of type `V`. +#[derive(Clone, Default, Eq, PartialEq)] +struct RepoPathTree { + entries: HashMap>, + value: V, } -impl RepoPathTree { - fn new() -> Self { - RepoPathTree { - entries: HashMap::new(), - is_dir: false, - is_file: false, - } - } - - fn add(&mut self, dir: &RepoPath) -> &mut RepoPathTree { +impl RepoPathTree { + fn add(&mut self, dir: &RepoPath) -> &mut Self + where + V: Default, + { dir.components().fold(self, |sub, name| { // Avoid name.clone() if entry already exists. if !sub.entries.contains_key(name) { - sub.is_dir = true; - sub.entries.insert(name.to_owned(), RepoPathTree::new()); + sub.entries.insert(name.to_owned(), Self::default()); } sub.entries.get_mut(name).unwrap() }) } - fn add_dir(&mut self, dir: &RepoPath) { - self.add(dir).is_dir = true; - } - - fn add_file(&mut self, file: &RepoPath) { - self.add(file).is_file = true; - } - - fn get(&self, dir: &RepoPath) -> Option<&RepoPathTree> { + fn get(&self, dir: &RepoPath) -> Option<&Self> { dir.components() .try_fold(self, |sub, name| sub.entries.get(name)) } - fn get_visit_sets(&self, dir: &RepoPath) -> Visit { - self.get(dir) - .map(RepoPathTree::to_visit_sets) - .unwrap_or(Visit::Nothing) - } - /// Walks the tree from the root to the given `dir`, yielding each sub tree /// and remaining path. fn walk_to<'a, 'b: 'a>( &'a self, dir: &'b RepoPath, - ) -> impl Iterator + 'a { + ) -> impl Iterator + 'a { iter::successors(Some((self, dir)), |(sub, dir)| { let mut components = dir.components(); let name = components.next()?; Some((sub.entries.get(name)?, components.as_path())) }) } - - fn to_visit_sets(&self) -> Visit { - let mut dirs = HashSet::new(); - let mut files = HashSet::new(); - for (name, sub) in &self.entries { - if sub.is_dir { - dirs.insert(name.clone()); - } - if sub.is_file { - files.insert(name.clone()); - } - } - Visit::sets(dirs, files) - } } -impl Debug for RepoPathTree { +impl Debug for RepoPathTree { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let type_name = match (self.is_dir, self.is_file) { - (true, true) => "Dir|File", - (true, false) => "Dir", - (false, true) => "File", - (false, false) => "_", - }; - write!(f, "{type_name} ")?; + self.value.fmt(f)?; + f.write_str(" ")?; f.debug_map() .entries( self.entries @@ -445,48 +452,6 @@ mod tests { RepoPath::from_internal_string(value) } - #[test] - fn test_repo_path_tree_empty() { - let tree = RepoPathTree::new(); - assert_eq!(tree.get_visit_sets(RepoPath::root()), Visit::Nothing); - } - - #[test] - fn test_repo_path_tree_root() { - let mut tree = RepoPathTree::new(); - tree.add_dir(RepoPath::root()); - assert_eq!(tree.get_visit_sets(RepoPath::root()), Visit::Nothing); - } - - #[test] - fn test_repo_path_tree_dir() { - let mut tree = RepoPathTree::new(); - tree.add_dir(repo_path("dir")); - assert_eq!( - tree.get_visit_sets(RepoPath::root()), - Visit::sets(hashset! {RepoPathComponentBuf::from("dir")}, hashset! {}), - ); - tree.add_dir(repo_path("dir/sub")); - assert_eq!( - tree.get_visit_sets(repo_path("dir")), - Visit::sets(hashset! {RepoPathComponentBuf::from("sub")}, hashset! {}), - ); - } - - #[test] - fn test_repo_path_tree_file() { - let mut tree = RepoPathTree::new(); - tree.add_file(repo_path("dir/file")); - assert_eq!( - tree.get_visit_sets(RepoPath::root()), - Visit::sets(hashset! {RepoPathComponentBuf::from("dir")}, hashset! {}), - ); - assert_eq!( - tree.get_visit_sets(repo_path("dir")), - Visit::sets(hashset! {}, hashset! {RepoPathComponentBuf::from("file")}), - ); - } - #[test] fn test_nothingmatcher() { let m = NothingMatcher;