Skip to content

Commit

Permalink
chore: only use a compiler filter if not empty
Browse files Browse the repository at this point in the history
  • Loading branch information
@DaniPopes
DaniPopes committed Jan 30, 2024
1 parent bf3b4b0 commit 03fadd5
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 87 deletions.
80 changes: 47 additions & 33 deletions crates/common/src/compile.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,10 @@ pub fn compile_target_with_filter(
let graph = Graph::resolve(&project.paths)?;

// Checking if it's a standalone script, or part of a project.
let mut compiler = ProjectCompiler::new().filter(Box::new(SkipBuildFilters(skip))).quiet(quiet);
let mut compiler = ProjectCompiler::new().quiet(quiet);
if !skip.is_empty() {
compiler = compiler.filter(Box::new(SkipBuildFilters::new(skip)?));
}
if !graph.files().contains_key(target_path) {
if verify {
eyre::bail!("You can only verify deployments from inside a project! Make sure it exists with `forge tree`.");
Expand Down Expand Up @@ -469,13 +472,20 @@ pub fn etherscan_project(metadata: &Metadata, target_path: impl AsRef<Path>) ->
}

/// Bundles multiple `SkipBuildFilter` into a single `FileFilter`
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct SkipBuildFilters(pub Vec<SkipBuildFilter>);
#[derive(Clone, Debug)]
pub struct SkipBuildFilters(Vec<GlobMatcher>);

impl FileFilter for SkipBuildFilters {
/// Only returns a match if _no_ exclusion filter matches
fn is_match(&self, file: &Path) -> bool {
self.0.iter().all(|filter| filter.is_match(file))
self.0.iter().all(|matcher| is_match_exclude(matcher, file))
}
}

impl SkipBuildFilters {
/// Creates a new `SkipBuildFilters` from multiple `SkipBuildFilter`.
pub fn new(matchers: impl IntoIterator<Item = SkipBuildFilter>) -> Result<Self> {
matchers.into_iter().map(|m| m.compile()).collect::<Result<_>>().map(Self)
}
}

Expand All @@ -491,6 +501,14 @@ pub enum SkipBuildFilter {
}

impl SkipBuildFilter {
fn new(s: &str) -> Self {
match s {
"test" | "tests" => SkipBuildFilter::Tests,
"script" | "scripts" => SkipBuildFilter::Scripts,
s => SkipBuildFilter::Custom(s.to_string()),
}
}

/// Returns the pattern to match against a file
fn file_pattern(&self) -> &str {
match self {
Expand All @@ -499,39 +517,30 @@ impl SkipBuildFilter {
SkipBuildFilter::Custom(s) => s.as_str(),
}
}
}

impl<T: AsRef<str>> From<T> for SkipBuildFilter {
fn from(s: T) -> Self {
match s.as_ref() {
"test" | "tests" => SkipBuildFilter::Tests,
"script" | "scripts" => SkipBuildFilter::Scripts,
s => SkipBuildFilter::Custom(s.to_string()),
}
fn compile(&self) -> Result<GlobMatcher> {
self.file_pattern().parse().map_err(Into::into)
}
}

impl FromStr for SkipBuildFilter {
type Err = Infallible;

fn from_str(s: &str) -> result::Result<Self, Self::Err> {
Ok(s.into())
Ok(Self::new(s))
}
}

impl FileFilter for SkipBuildFilter {
/// Matches file only if the filter does not apply
///
/// This is returns the inverse of `file.name.contains(pattern) || matcher.is_match(file)`
fn is_match(&self, file: &Path) -> bool {
fn exclude(file: &Path, pattern: &str) -> Option<bool> {
let matcher: GlobMatcher = pattern.parse().unwrap();
let file_name = file.file_name()?.to_str()?;
Some(file_name.contains(pattern) || matcher.is_match(file.as_os_str().to_str()?))
}

!exclude(file, self.file_pattern()).unwrap_or_default()
/// Matches file only if the filter does not apply.
///
/// This returns the inverse of `file.name.contains(pattern) || matcher.is_match(file)`.
fn is_match_exclude(matcher: &GlobMatcher, path: &Path) -> bool {
fn is_match(matcher: &GlobMatcher, path: &Path) -> Option<bool> {
let file_name = path.file_name()?.to_str()?;
Some(file_name.contains(matcher.as_str()) || matcher.is_match(path))
}

!is_match(matcher, path).unwrap_or_default()
}

#[cfg(test)]
Expand All @@ -540,19 +549,24 @@ mod tests {

#[test]
fn test_build_filter() {
let tests = SkipBuildFilter::Tests.compile().unwrap();
let scripts = SkipBuildFilter::Scripts.compile().unwrap();
let custom = |s: &str| SkipBuildFilter::Custom(s.to_string()).compile().unwrap();

let file = Path::new("A.t.sol");
assert!(!SkipBuildFilter::Tests.is_match(file));
assert!(SkipBuildFilter::Scripts.is_match(file));
assert!(!SkipBuildFilter::Custom("A.t".to_string()).is_match(file));
assert!(!is_match_exclude(&tests, file));
assert!(is_match_exclude(&scripts, file));
assert!(!is_match_exclude(&custom("A.t"), file));

let file = Path::new("A.s.sol");
assert!(SkipBuildFilter::Tests.is_match(file));
assert!(!SkipBuildFilter::Scripts.is_match(file));
assert!(!SkipBuildFilter::Custom("A.s".to_string()).is_match(file));
assert!(is_match_exclude(&tests, file));
assert!(!is_match_exclude(&scripts, file));
assert!(!is_match_exclude(&custom("A.s"), file));

let file = Path::new("/home/test/Foo.sol");
assert!(!SkipBuildFilter::Custom("*/test/**".to_string()).is_match(file));
assert!(!is_match_exclude(&custom("*/test/**"), file));

let file = Path::new("/home/script/Contract.sol");
assert!(!SkipBuildFilter::Custom("*/script/**".to_string()).is_match(file));
assert!(!is_match_exclude(&custom("*/script/**"), file));
}
}
18 changes: 11 additions & 7 deletions crates/common/src/glob.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,16 @@ impl GlobMatcher {
///
/// The glob `./test/*` won't match absolute paths like `test/Contract.sol`, which is common
/// format here, so we also handle this case here
pub fn is_match(&self, path: &str) -> bool {
let mut matches = self.matcher.is_match(path);
if !matches && !path.starts_with("./") && self.as_str().starts_with("./") {
matches = self.matcher.is_match(format!("./{path}"));
pub fn is_match(&self, path: &Path) -> bool {
if self.matcher.is_match(path) {
return true;
}
matches

if !path.starts_with("./") && self.as_str().starts_with("./") {
return self.matcher.is_match(format!("./{}", path.display()));
}

false
}

/// Returns the `Glob` string used to compile this matcher.
Expand Down Expand Up @@ -76,7 +80,7 @@ mod tests {
#[test]
fn can_match_glob_paths() {
let matcher: GlobMatcher = "./test/*".parse().unwrap();
assert!(matcher.is_match("test/Contract.sol"));
assert!(matcher.is_match("./test/Contract.sol"));
assert!(matcher.is_match(Path::new("test/Contract.sol")));
assert!(matcher.is_match(Path::new("./test/Contract.sol")));
}
}
13 changes: 9 additions & 4 deletions crates/forge/bin/cmd/build.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,18 @@ impl BuildArgs {
project = config.project()?;
}

let output = ProjectCompiler::new()
let mut compiler = ProjectCompiler::new()
.print_names(self.names)
.print_sizes(self.sizes)
.quiet(self.format_json)
.bail(!self.format_json)
.filter(Box::new(SkipBuildFilters(self.skip.unwrap_or_default())))
.compile(&project)?;
.bail(!self.format_json);
if let Some(skip) = self.skip {
if !skip.is_empty() {
compiler = compiler.filter(Box::new(SkipBuildFilters::new(skip)?));
}
}
let output = compiler.compile(&project)?;

if self.format_json {
println!("{}", serde_json::to_string_pretty(&output.clone().output())?);
}
Expand Down
75 changes: 33 additions & 42 deletions crates/forge/bin/cmd/test/filter.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,28 +44,26 @@ pub struct FilterArgs {

impl FilterArgs {
/// Merges the set filter globs with the config's values
pub fn merge_with_config(&self, config: &Config) -> ProjectPathsAwareFilter {
let mut filter = self.clone();
if filter.test_pattern.is_none() {
filter.test_pattern = config.test_pattern.clone().map(|p| p.into());
pub fn merge_with_config(mut self, config: &Config) -> ProjectPathsAwareFilter {
if self.test_pattern.is_none() {
self.test_pattern = config.test_pattern.clone().map(Into::into);
}
if filter.test_pattern_inverse.is_none() {
filter.test_pattern_inverse = config.test_pattern_inverse.clone().map(|p| p.into());
if self.test_pattern_inverse.is_none() {
self.test_pattern_inverse = config.test_pattern_inverse.clone().map(Into::into);
}
if filter.contract_pattern.is_none() {
filter.contract_pattern = config.contract_pattern.clone().map(|p| p.into());
if self.contract_pattern.is_none() {
self.contract_pattern = config.contract_pattern.clone().map(Into::into);
}
if filter.contract_pattern_inverse.is_none() {
filter.contract_pattern_inverse =
config.contract_pattern_inverse.clone().map(|p| p.into());
if self.contract_pattern_inverse.is_none() {
self.contract_pattern_inverse = config.contract_pattern_inverse.clone().map(Into::into);
}
if filter.path_pattern.is_none() {
filter.path_pattern = config.path_pattern.clone().map(Into::into);
if self.path_pattern.is_none() {
self.path_pattern = config.path_pattern.clone().map(Into::into);
}
if filter.path_pattern_inverse.is_none() {
filter.path_pattern_inverse = config.path_pattern_inverse.clone().map(Into::into);
if self.path_pattern_inverse.is_none() {
self.path_pattern_inverse = config.path_pattern_inverse.clone().map(Into::into);
}
ProjectPathsAwareFilter { args_filter: filter, paths: config.project_paths() }
ProjectPathsAwareFilter { args_filter: self, paths: config.project_paths() }
}
}

Expand All @@ -86,15 +84,13 @@ impl FileFilter for FilterArgs {
/// Returns true if the file regex pattern match the `file`
///
/// If no file regex is set this returns true if the file ends with `.t.sol`, see
/// [FoundryPathExr::is_sol_test()]
/// [`FoundryPathExt::is_sol_test()`].
fn is_match(&self, file: &Path) -> bool {
if let Some(file) = file.as_os_str().to_str() {
if let Some(ref glob) = self.path_pattern {
return glob.is_match(file)
}
if let Some(ref glob) = self.path_pattern_inverse {
return !glob.is_match(file)
}
if let Some(glob) = &self.path_pattern {
return glob.is_match(file)
}
if let Some(glob) = &self.path_pattern_inverse {
return !glob.is_match(file)
}
file.is_sol_test()
}
Expand Down Expand Up @@ -124,10 +120,6 @@ impl TestFilter for FilterArgs {
}

fn matches_path(&self, path: &Path) -> bool {
let Some(path) = path.to_str() else {
return false;
};

let mut ok = true;
if let Some(re) = &self.path_pattern {
ok = ok && re.is_match(path);
Expand All @@ -141,26 +133,25 @@ impl TestFilter for FilterArgs {

impl fmt::Display for FilterArgs {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let mut patterns = Vec::new();
if let Some(ref p) = self.test_pattern {
patterns.push(format!("\tmatch-test: `{}`", p.as_str()));
if let Some(p) = &self.test_pattern {
writeln!(f, "\tmatch-test: `{}`", p.as_str())?;
}
if let Some(ref p) = self.test_pattern_inverse {
patterns.push(format!("\tno-match-test: `{}`", p.as_str()));
if let Some(p) = &self.test_pattern_inverse {
writeln!(f, "\tno-match-test: `{}`", p.as_str())?;
}
if let Some(ref p) = self.contract_pattern {
patterns.push(format!("\tmatch-contract: `{}`", p.as_str()));
if let Some(p) = &self.contract_pattern {
writeln!(f, "\tmatch-contract: `{}`", p.as_str())?;
}
if let Some(ref p) = self.contract_pattern_inverse {
patterns.push(format!("\tno-match-contract: `{}`", p.as_str()));
if let Some(p) = &self.contract_pattern_inverse {
writeln!(f, "\tno-match-contract: `{}`", p.as_str())?;
}
if let Some(ref p) = self.path_pattern {
patterns.push(format!("\tmatch-path: `{}`", p.as_str()));
if let Some(p) = &self.path_pattern {
writeln!(f, "\tmatch-path: `{}`", p.as_str())?;
}
if let Some(ref p) = self.path_pattern_inverse {
patterns.push(format!("\tno-match-path: `{}`", p.as_str()));
if let Some(p) = &self.path_pattern_inverse {
writeln!(f, "\tno-match-path: `{}`", p.as_str())?;
}
write!(f, "{}", patterns.join("\n"))
Ok(())
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/forge/bin/cmd/test/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ impl TestArgs {

/// Returns the flattened [`FilterArgs`] arguments merged with [`Config`].
pub fn filter(&self, config: &Config) -> ProjectPathsAwareFilter {
self.filter.merge_with_config(config)
self.filter.clone().merge_with_config(config)
}

/// Returns whether `BuildArgs` was configured with `--watch`
Expand Down

0 comments on commit 03fadd5

Please sign in to comment.