Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(forge): implement glob pattern for forge build --skip #5267

Merged
merged 3 commits into from
Jul 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ async-trait = "0.1"
# disk / paths
walkdir = "2"
dunce = "1"
globset = "0.4"
path-slash = "0.2"
tempfile = "3"

Expand Down Expand Up @@ -89,6 +88,7 @@ toml = "0.7"
serial_test = "2"
criterion = "0.4"
svm = { package = "svm-rs", version = "0.2", default-features = false, features = ["rustls"] }
globset = "0.4"

[features]
default = ["rustls"]
Expand Down
67 changes: 2 additions & 65 deletions cli/src/cmd/forge/test/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ use crate::utils::FoundryPathExt;
use clap::Parser;
use ethers::solc::{FileFilter, ProjectPathsConfig};
use forge::TestFilter;
use foundry_common::glob::GlobMatcher;
use foundry_config::Config;
use std::{fmt, path::Path, str::FromStr};
use std::{fmt, path::Path};

/// The filter to use during testing.
///
Expand Down Expand Up @@ -214,67 +215,3 @@ impl fmt::Display for ProjectPathsAwareFilter {
self.args_filter.fmt(f)
}
}

/// A `globset::Glob` that creates its `globset::GlobMatcher` when its created, so it doesn't need
/// to be compiled when the filter functions `TestFilter` functions are called.
#[derive(Debug, Clone)]
pub struct GlobMatcher {
/// The parsed glob
pub glob: globset::Glob,
/// The compiled `glob`
pub matcher: globset::GlobMatcher,
}

// === impl GlobMatcher ===

impl GlobMatcher {
/// Tests whether the given path matches this pattern or not.
///
/// 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}"));
}
matches
}

/// Returns the `Glob` string used to compile this matcher.
pub fn as_str(&self) -> &str {
self.glob.glob()
}
}

impl fmt::Display for GlobMatcher {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
self.glob.fmt(f)
}
}

impl FromStr for GlobMatcher {
type Err = globset::Error;

fn from_str(s: &str) -> Result<Self, Self::Err> {
s.parse::<globset::Glob>().map(Into::into)
}
}

impl From<globset::Glob> for GlobMatcher {
fn from(glob: globset::Glob) -> Self {
let matcher = glob.compile_matcher();
Self { glob, matcher }
}
}

#[cfg(test)]
mod tests {
use super::*;

#[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"));
}
}
3 changes: 3 additions & 0 deletions cli/tests/fixtures/can_build_skip_glob.stdout
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Compiling 1 files with 0.8.17
Solc 0.8.17 finished in 33.25ms
Compiler run successful!
23 changes: 23 additions & 0 deletions cli/tests/it/cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1575,6 +1575,29 @@ forgetest_init!(can_build_skip_contracts, |prj: TestProject, mut cmd: TestComman
assert!(out.trim().contains("No files changed, compilation skipped"), "{}", out);
});

forgetest_init!(can_build_skip_glob, |prj: TestProject, mut cmd: TestCommand| {
// explicitly set to run with 0.8.17 for consistent output
let config = Config { solc: Some("0.8.17".into()), ..Default::default() };
prj.write_config(config);
prj.inner()
.add_test(
"Foo",
r#"
// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.17;
contract TestDemo {
function test_run() external {}
}"#,
)
.unwrap();
// only builds the single template contract `src/*` even if `*.t.sol` or `.s.sol` is absent
cmd.args(["build", "--skip", "*/test/**", "--skip", "*/script/**"]);

cmd.unchecked_output().stdout_matches_path(
PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/can_build_skip_glob.stdout"),
);
});

// checks that build --sizes includes all contracts even if unchanged
forgetest_init!(can_build_sizes_repeatedly, |_prj: TestProject, mut cmd: TestCommand| {
cmd.args(["build", "--sizes"]);
Expand Down
1 change: 1 addition & 0 deletions common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ semver = "1"
once_cell = "1"
dunce = "1"
regex = "1"
globset = "0.4"

[dev-dependencies]
tokio = { version = "1", features = ["rt-multi-thread", "macros"] }
12 changes: 9 additions & 3 deletions common/src/compile.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//! Support for compiling [ethers::solc::Project]
use crate::{term, TestFunctionExt};
use crate::{glob::GlobMatcher, term, TestFunctionExt};
use comfy_table::{presets::ASCII_MARKDOWN, *};
use ethers_etherscan::contract::Metadata;
use ethers_solc::{
Expand Down Expand Up @@ -525,11 +525,12 @@ impl FromStr for SkipBuildFilter {
impl FileFilter for SkipBuildFilter {
/// Matches file only if the filter does not apply
///
/// This is returns the inverse of `file.name.contains(pattern)`
/// 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))
Some(file_name.contains(pattern) || matcher.is_match(file.as_os_str().to_str()?))
}

!exclude(file, self.file_pattern()).unwrap_or_default()
Expand All @@ -551,5 +552,10 @@ mod tests {
assert!(SkipBuildFilter::Tests.is_match(file));
assert!(!SkipBuildFilter::Scripts.is_match(file));
assert!(!SkipBuildFilter::Custom("A.s".to_string()).is_match(file));

let file = Path::new("/home/test/Foo.sol");
assert!(!SkipBuildFilter::Custom("*/test/**".to_string()).is_match(file));
let file = Path::new("/home/script/Contract.sol");
assert!(!SkipBuildFilter::Custom("*/script/**".to_string()).is_match(file));
}
}
64 changes: 64 additions & 0 deletions common/src/glob.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
//! Contains `globset::Glob` wrapper functions used for filtering
use std::{fmt, str::FromStr};
/// A `globset::Glob` that creates its `globset::GlobMatcher` when its created, so it doesn't need
/// to be compiled when the filter functions `TestFilter` functions are called.
#[derive(Debug, Clone)]
pub struct GlobMatcher {
/// The parsed glob
pub glob: globset::Glob,
/// The compiled glob
pub matcher: globset::GlobMatcher,
}

// === impl GlobMatcher ===

impl GlobMatcher {
/// Tests whether the given path matches this pattern or not.
///
/// 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 {
Copy link
Contributor Author

@0xalpharush 0xalpharush Jul 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally to match /private/var/folders/test/Foo.sol (this is how it appears in the tests) one wouldn't need */test/** and could instead use ./test/*. I don't have insight into how paths are passed around in Foundry (are they always absolute?), but I could appreciate clarification on this point.

For example, this is the cause of the failing test:

        let file = Path::new("/private/var/folders/test/Foo.sol");
        assert!(!SkipBuildFilter::Custom("*/test/**".to_string()).is_match(file)); // passes 
        assert!(!SkipBuildFilter::Custom("./test/**".to_string()).is_match(file)); // fails

        let file = Path::new("script/Contract.sol");
        assert!(!SkipBuildFilter::Custom("*/script/**".to_string()).is_match(file)); // fails
        assert!(!SkipBuildFilter::Custom("./script/**".to_string()).is_match(file)); // passes

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have insight into how paths are passed around in Foundry (are they always absolute?)

they're relative to the configured root when dealing solc, outside of solc they always should be absolute, so foundry should always have absolute paths

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}"));
}
matches
}

/// Returns the `Glob` string used to compile this matcher.
pub fn as_str(&self) -> &str {
self.glob.glob()
}
}

impl fmt::Display for GlobMatcher {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
self.glob.fmt(f)
}
}

impl FromStr for GlobMatcher {
type Err = globset::Error;

fn from_str(s: &str) -> Result<Self, Self::Err> {
s.parse::<globset::Glob>().map(Into::into)
}
}

impl From<globset::Glob> for GlobMatcher {
fn from(glob: globset::Glob) -> Self {
let matcher = glob.compile_matcher();
Self { glob, matcher }
}
}
#[cfg(test)]
mod tests {
use super::*;

#[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"));
}
}
1 change: 1 addition & 0 deletions common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ pub mod errors;
pub mod evm;
pub mod fmt;
pub mod fs;
pub mod glob;
pub mod provider;
pub mod selectors;
pub mod shell;
Expand Down