chore: only use a compiler filter if not empty (#6954)

crates/common/src/compile.rs

@@ -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`.");
@@ -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)
     }
 }
 
@@ -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 {
@@ -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)]
@@ -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));
     }
 }

crates/common/src/glob.rs

@@ -38,12 +38,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 `globset::Glob`.
@@ -84,7 +88,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")));
     }
 }

crates/forge/bin/cmd/build.rs

@@ -87,16 +87,22 @@ 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())?);
         }
+
         Ok(output)
     }
 

crates/forge/bin/cmd/test/filter.rs

@@ -43,29 +43,37 @@ pub struct FilterArgs {
 }
 
 impl FilterArgs {
+    /// Returns true if the filter is empty.
+    pub fn is_empty(&self) -> bool {
+        self.test_pattern.is_none() &&
+            self.test_pattern_inverse.is_none() &&
+            self.contract_pattern.is_none() &&
+            self.contract_pattern_inverse.is_none() &&
+            self.path_pattern.is_none() &&
+            self.path_pattern_inverse.is_none()
+    }
+
     /// 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() }
     }
 }
 
@@ -86,15 +94,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()
     }
@@ -124,10 +130,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);
@@ -141,26 +143,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(())
     }
 }
 
@@ -174,12 +175,17 @@ pub struct ProjectPathsAwareFilter {
 // === impl ProjectPathsAwareFilter ===
 
 impl ProjectPathsAwareFilter {
-    /// Returns the CLI arguments
+    /// Returns true if the filter is empty.
+    pub fn is_empty(&self) -> bool {
+        self.args_filter.is_empty()
+    }
+
+    /// Returns the CLI arguments.
     pub fn args(&self) -> &FilterArgs {
         &self.args_filter
     }
 
-    /// Returns the CLI arguments mutably
+    /// Returns the CLI arguments mutably.
     pub fn args_mut(&mut self) -> &mut FilterArgs {
         &mut self.args_filter
     }

crates/forge/bin/cmd/test/mod.rs

@@ -319,14 +319,16 @@ impl TestArgs {
         trace!(target: "forge::test", "running all tests");
 
         if runner.matching_test_function_count(filter) == 0 {
-            let filter_str = filter.to_string();
-            if filter_str.is_empty() {
+            println!();
+            if filter.is_empty() {
                 println!(
-                    "\nNo tests found in project! \
+                    "No tests found in project! \
                      Forge looks for functions that starts with `test`."
                 );
             } else {
-                println!("\nNo tests match the provided pattern:\n{filter_str}");
+                println!("No tests match the provided pattern:");
+                print!("{filter}");
+
                 // Try to suggest a test when there's no match
                 if let Some(test_pattern) = &filter.args().test_pattern {
                     let test_name = test_pattern.as_str();
@@ -507,7 +509,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`