Skip to content

Commit

Permalink
Merge pull request #141 from tavianator/into-box
Browse files Browse the repository at this point in the history
matchers: Replace new_box() with an into_box() trait method
  • Loading branch information
sylvestre authored Feb 8, 2022
2 parents 48d6f63 + 979a442 commit 7ffd5f8
Show file tree
Hide file tree
Showing 14 changed files with 110 additions and 172 deletions.
4 changes: 0 additions & 4 deletions src/find/matchers/delete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ impl DeleteMatcher {
DeleteMatcher
}

pub fn new_box() -> io::Result<Box<dyn Matcher>> {
Ok(Box::new(Self::new()))
}

fn delete(&self, file_path: &Path, file_type: FileType) -> io::Result<()> {
if file_type.is_dir() {
fs::remove_dir(file_path)
Expand Down
4 changes: 0 additions & 4 deletions src/find/matchers/empty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@ impl EmptyMatcher {
pub fn new() -> EmptyMatcher {
EmptyMatcher
}

pub fn new_box() -> Box<dyn Matcher> {
Box::new(EmptyMatcher::new())
}
}

impl Matcher for EmptyMatcher {
Expand Down
8 changes: 0 additions & 8 deletions src/find/matchers/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,6 @@ impl SingleExecMatcher {
exec_in_parent_dir,
})
}

pub fn new_box(
executable: &str,
args: &[&str],
exec_in_parent_dir: bool,
) -> Result<Box<dyn Matcher>, Box<dyn Error>> {
Ok(Box::new(Self::new(executable, args, exec_in_parent_dir)?))
}
}

impl Matcher for SingleExecMatcher {
Expand Down
88 changes: 34 additions & 54 deletions src/find/matchers/logical_matchers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ impl AndMatcherBuilder {
}
}

pub fn new_and_condition(&mut self, matcher: Box<dyn Matcher>) {
self.submatchers.push(matcher);
pub fn new_and_condition(&mut self, matcher: impl Matcher) {
self.submatchers.push(matcher.into_box());
}

/// Builds a Matcher: consuming the builder in the process.
Expand Down Expand Up @@ -131,7 +131,7 @@ pub struct OrMatcherBuilder {
}

impl OrMatcherBuilder {
pub fn new_and_condition(&mut self, matcher: Box<dyn Matcher>) {
pub fn new_and_condition(&mut self, matcher: impl Matcher) {
// safe to unwrap. submatchers always has at least one member
self.submatchers
.last_mut()
Expand Down Expand Up @@ -222,7 +222,7 @@ pub struct ListMatcherBuilder {
}

impl ListMatcherBuilder {
pub fn new_and_condition(&mut self, matcher: Box<dyn Matcher>) {
pub fn new_and_condition(&mut self, matcher: impl Matcher) {
// safe to unwrap. submatchers always has at least one member
self.submatchers
.last_mut()
Expand Down Expand Up @@ -291,12 +291,6 @@ impl ListMatcherBuilder {
/// A simple matcher that always matches.
pub struct TrueMatcher;

impl TrueMatcher {
pub fn new_box() -> Box<dyn Matcher> {
Box::new(Self {})
}
}

impl Matcher for TrueMatcher {
fn matches(&self, _dir_entry: &DirEntry, _: &mut MatcherIO) -> bool {
true
Expand All @@ -312,24 +306,16 @@ impl Matcher for FalseMatcher {
}
}

impl FalseMatcher {
pub fn new_box() -> Box<dyn Matcher> {
Box::new(Self {})
}
}

/// Matcher that wraps another matcher and inverts matching criteria.
pub struct NotMatcher {
submatcher: Box<dyn Matcher>,
}

impl NotMatcher {
pub fn new(submatcher: Box<dyn Matcher>) -> Self {
Self { submatcher }
}

pub fn new_box(submatcher: Box<dyn Matcher>) -> Box<Self> {
Box::new(Self::new(submatcher))
pub fn new(submatcher: impl Matcher) -> Self {
Self {
submatcher: submatcher.into_box(),
}
}
}

Expand Down Expand Up @@ -361,7 +347,7 @@ mod tests {
use walkdir::DirEntry;

/// Simple Matcher impl that has side effects
pub struct HasSideEffects {}
pub struct HasSideEffects;

impl Matcher for HasSideEffects {
fn matches(&self, _: &DirEntry, _: &mut MatcherIO) -> bool {
Expand All @@ -373,25 +359,19 @@ mod tests {
}
}

impl HasSideEffects {
pub fn new_box() -> Box<dyn Matcher> {
Box::new(Self {})
}
}

#[test]
fn and_matches_works() {
let abbbc = get_dir_entry_for("test_data/simple", "abbbc");
let mut builder = AndMatcherBuilder::new();
let deps = FakeDependencies::new();

// start with one matcher returning true
builder.new_and_condition(TrueMatcher::new_box());
builder.new_and_condition(TrueMatcher);
assert!(builder.build().matches(&abbbc, &mut deps.new_matcher_io()));

builder = AndMatcherBuilder::new();
builder.new_and_condition(TrueMatcher::new_box());
builder.new_and_condition(FalseMatcher::new_box());
builder.new_and_condition(TrueMatcher);
builder.new_and_condition(FalseMatcher);
assert!(!builder.build().matches(&abbbc, &mut deps.new_matcher_io()));
}

Expand All @@ -402,13 +382,13 @@ mod tests {
let deps = FakeDependencies::new();

// start with one matcher returning false
builder.new_and_condition(FalseMatcher::new_box());
builder.new_and_condition(FalseMatcher);
assert!(!builder.build().matches(&abbbc, &mut deps.new_matcher_io()));

let mut builder = OrMatcherBuilder::new();
builder.new_and_condition(FalseMatcher::new_box());
builder.new_and_condition(FalseMatcher);
builder.new_or_condition("-o").unwrap();
builder.new_and_condition(TrueMatcher::new_box());
builder.new_and_condition(TrueMatcher);
assert!(builder.build().matches(&abbbc, &mut deps.new_matcher_io()));
}

Expand All @@ -419,21 +399,21 @@ mod tests {
let deps = FakeDependencies::new();

// result should always match that of the last pushed submatcher
builder.new_and_condition(FalseMatcher::new_box());
builder.new_and_condition(FalseMatcher);
assert!(!builder.build().matches(&abbbc, &mut deps.new_matcher_io()));

builder = ListMatcherBuilder::new();
builder.new_and_condition(FalseMatcher::new_box());
builder.new_and_condition(FalseMatcher);
builder.new_list_condition().unwrap();
builder.new_and_condition(TrueMatcher::new_box());
builder.new_and_condition(TrueMatcher);
assert!(builder.build().matches(&abbbc, &mut deps.new_matcher_io()));

builder = ListMatcherBuilder::new();
builder.new_and_condition(FalseMatcher::new_box());
builder.new_and_condition(FalseMatcher);
builder.new_list_condition().unwrap();
builder.new_and_condition(TrueMatcher::new_box());
builder.new_and_condition(TrueMatcher);
builder.new_list_condition().unwrap();
builder.new_and_condition(FalseMatcher::new_box());
builder.new_and_condition(FalseMatcher);
assert!(!builder.build().matches(&abbbc, &mut deps.new_matcher_io()));
}

Expand All @@ -460,12 +440,12 @@ mod tests {
let mut builder = AndMatcherBuilder::new();

// start with one matcher with no side effects false
builder.new_and_condition(TrueMatcher::new_box());
builder.new_and_condition(TrueMatcher);
assert!(!builder.build().has_side_effects());

builder = AndMatcherBuilder::new();
builder.new_and_condition(TrueMatcher::new_box());
builder.new_and_condition(HasSideEffects::new_box());
builder.new_and_condition(TrueMatcher);
builder.new_and_condition(HasSideEffects);
assert!(builder.build().has_side_effects());
}

Expand All @@ -474,12 +454,12 @@ mod tests {
let mut builder = OrMatcherBuilder::new();

// start with one matcher with no side effects false
builder.new_and_condition(TrueMatcher::new_box());
builder.new_and_condition(TrueMatcher);
assert!(!builder.build().has_side_effects());

builder = OrMatcherBuilder::new();
builder.new_and_condition(TrueMatcher::new_box());
builder.new_and_condition(HasSideEffects::new_box());
builder.new_and_condition(TrueMatcher);
builder.new_and_condition(HasSideEffects);
assert!(builder.build().has_side_effects());
}

Expand All @@ -488,12 +468,12 @@ mod tests {
let mut builder = ListMatcherBuilder::new();

// start with one matcher with no side effects false
builder.new_and_condition(TrueMatcher::new_box());
builder.new_and_condition(TrueMatcher);
assert!(!builder.build().has_side_effects());

builder = ListMatcherBuilder::new();
builder.new_and_condition(TrueMatcher::new_box());
builder.new_and_condition(HasSideEffects::new_box());
builder.new_and_condition(TrueMatcher);
builder.new_and_condition(HasSideEffects);
assert!(builder.build().has_side_effects());
}

Expand All @@ -512,17 +492,17 @@ mod tests {
#[test]
fn not_matches_works() {
let abbbc = get_dir_entry_for("test_data/simple", "abbbc");
let not_true = NotMatcher::new(TrueMatcher::new_box());
let not_false = NotMatcher::new(FalseMatcher::new_box());
let not_true = NotMatcher::new(TrueMatcher);
let not_false = NotMatcher::new(FalseMatcher);
let deps = FakeDependencies::new();
assert!(!not_true.matches(&abbbc, &mut deps.new_matcher_io()));
assert!(not_false.matches(&abbbc, &mut deps.new_matcher_io()));
}

#[test]
fn not_has_side_effects_works() {
let has_fx = NotMatcher::new(HasSideEffects::new_box());
let hasnt_fx = NotMatcher::new(FalseMatcher::new_box());
let has_fx = NotMatcher::new(HasSideEffects);
let hasnt_fx = NotMatcher::new(FalseMatcher);
assert!(has_fx.has_side_effects());
assert!(!hasnt_fx.has_side_effects());
}
Expand Down
Loading

0 comments on commit 7ffd5f8

Please sign in to comment.