diff --git a/src/artifacts/mod.rs b/src/artifacts/mod.rs index a130979d..70052fdf 100644 --- a/src/artifacts/mod.rs +++ b/src/artifacts/mod.rs @@ -3,7 +3,8 @@ #![allow(ambiguous_glob_reexports)] use crate::{ - compile::*, error::SolcIoError, remappings::Remapping, utils, ProjectPathsConfig, SolcError, + compile::*, error::SolcIoError, output::ErrorFilter, remappings::Remapping, utils, + ProjectPathsConfig, SolcError, }; use alloy_primitives::hex; use md5::Digest; @@ -1570,14 +1571,26 @@ impl CompilerOutput { self.errors.iter().any(|err| err.severity.is_error()) } - /// Whether the output contains a compiler warning - pub fn has_warning(&self, ignored_error_codes: &[u64]) -> bool { - self.errors.iter().any(|err| { - if err.severity.is_warning() { - err.error_code.as_ref().map_or(false, |code| !ignored_error_codes.contains(code)) - } else { - false + /// Checks if there are any compiler warnings that are not ignored by the specified error codes + /// and file paths. + pub fn has_warning<'a>(&self, filter: impl Into>) -> bool { + let filter: ErrorFilter<'_> = filter.into(); + self.errors.iter().any(|error| { + if !error.severity.is_warning() { + return false; } + + let is_code_ignored = filter.is_code_ignored(error.error_code); + + let is_file_ignored = error + .source_location + .as_ref() + .map_or(false, |location| filter.is_file_ignored(Path::new(&location.file))); + + // Only consider warnings that are not ignored by either code or file path. + // Hence, return `true` for warnings that are not ignored, making the function + // return `true` if any such warnings exist. + !(is_code_ignored || is_file_ignored) }) } diff --git a/src/compile/output/mod.rs b/src/compile/output/mod.rs index f3ada08b..c9334711 100644 --- a/src/compile/output/mod.rs +++ b/src/compile/output/mod.rs @@ -13,7 +13,12 @@ use crate::{ use contracts::{VersionedContract, VersionedContracts}; use semver::Version; use serde::{Deserialize, Serialize}; -use std::{collections::BTreeMap, fmt, path::Path}; +use std::{ + borrow::Cow, + collections::BTreeMap, + fmt, + path::{Path, PathBuf}, +}; use yansi::Paint; pub mod contracts; @@ -32,6 +37,8 @@ pub struct ProjectCompileOutput { pub(crate) cached_artifacts: Artifacts, /// errors that should be omitted pub(crate) ignored_error_codes: Vec, + /// paths that should be omitted + pub(crate) ignored_file_paths: Vec, /// set minimum level of severity that is treated as an error pub(crate) compiler_severity_filter: Severity, } @@ -245,12 +252,17 @@ impl ProjectCompileOutput { /// Returns whether any errors were emitted by the compiler. pub fn has_compiler_errors(&self) -> bool { - self.compiler_output.has_error(&self.ignored_error_codes, &self.compiler_severity_filter) + self.compiler_output.has_error( + &self.ignored_error_codes, + &self.ignored_file_paths, + &self.compiler_severity_filter, + ) } /// Returns whether any warnings were emitted by the compiler. pub fn has_compiler_warnings(&self) -> bool { - self.compiler_output.has_warning(&self.ignored_error_codes) + let filter = ErrorFilter::new(&self.ignored_error_codes, &self.ignored_file_paths); + self.compiler_output.has_warning(filter) } /// Panics if any errors were emitted by the compiler. @@ -463,7 +475,11 @@ impl fmt::Display for ProjectCompileOutput { f.write_str("Nothing to compile") } else { self.compiler_output - .diagnostics(&self.ignored_error_codes, self.compiler_severity_filter) + .diagnostics( + &self.ignored_error_codes, + &self.ignored_file_paths, + self.compiler_severity_filter, + ) .fmt(f) } } @@ -484,6 +500,46 @@ pub struct AggregatedCompilerOutput { pub build_infos: BTreeMap, } +/// How to filter errors/warnings +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ErrorFilter<'a> { + /// Ignore errors/warnings with these codes + pub error_codes: Cow<'a, [u64]>, + /// Ignore errors/warnings from these file paths + pub ignored_file_paths: Cow<'a, [PathBuf]>, +} + +impl<'a> ErrorFilter<'a> { + /// Creates a new `ErrorFilter` with the given error codes and ignored file paths + pub fn new(error_codes: &'a [u64], ignored_file_paths: &'a [PathBuf]) -> Self { + ErrorFilter { + error_codes: Cow::Borrowed(error_codes), + ignored_file_paths: Cow::Borrowed(ignored_file_paths), + } + } + /// Helper function to check if an error code is ignored + pub fn is_code_ignored(&self, code: Option) -> bool { + match code { + Some(code) => self.error_codes.contains(&code), + None => false, + } + } + + /// Helper function to check if an error's file path is ignored + pub fn is_file_ignored(&self, file_path: &Path) -> bool { + self.ignored_file_paths.iter().any(|ignored_path| file_path.starts_with(ignored_path)) + } +} + +impl<'a> From<&'a [u64]> for ErrorFilter<'a> { + fn from(error_codes: &'a [u64]) -> Self { + ErrorFilter { + error_codes: Cow::Borrowed(error_codes), + ignored_file_paths: Cow::Borrowed(&[]), + } + } +} + impl AggregatedCompilerOutput { /// Converts all `\\` separators in _all_ paths to `/` pub fn slash_paths(&mut self) { @@ -495,12 +551,15 @@ impl AggregatedCompilerOutput { pub fn has_error( &self, ignored_error_codes: &[u64], + ignored_file_paths: &[PathBuf], compiler_severity_filter: &Severity, ) -> bool { self.errors.iter().any(|err| { if compiler_severity_filter.ge(&err.severity) { if compiler_severity_filter.is_warning() { - return self.has_warning(ignored_error_codes); + // skip ignored error codes and file path from warnings + let filter = ErrorFilter::new(ignored_error_codes, ignored_file_paths); + return self.has_warning(filter); } return true; } @@ -508,23 +567,41 @@ impl AggregatedCompilerOutput { }) } - /// Whether the output contains a compiler warning - pub fn has_warning(&self, ignored_error_codes: &[u64]) -> bool { - self.errors.iter().any(|err| { - if err.severity.is_warning() { - err.error_code.as_ref().map_or(false, |code| !ignored_error_codes.contains(code)) - } else { - false + /// Checks if there are any compiler warnings that are not ignored by the specified error codes + /// and file paths. + pub fn has_warning<'a>(&self, filter: impl Into>) -> bool { + let filter: ErrorFilter<'_> = filter.into(); + self.errors.iter().any(|error| { + if !error.severity.is_warning() { + return false; } + + let is_code_ignored = filter.is_code_ignored(error.error_code); + + let is_file_ignored = error + .source_location + .as_ref() + .map_or(false, |location| filter.is_file_ignored(Path::new(&location.file))); + + // Only consider warnings that are not ignored by either code or file path. + // Hence, return `true` for warnings that are not ignored, making the function + // return `true` if any such warnings exist. + !(is_code_ignored || is_file_ignored) }) } pub fn diagnostics<'a>( &'a self, ignored_error_codes: &'a [u64], + ignored_file_paths: &'a [PathBuf], compiler_severity_filter: Severity, ) -> OutputDiagnostics<'a> { - OutputDiagnostics { compiler_output: self, ignored_error_codes, compiler_severity_filter } + OutputDiagnostics { + compiler_output: self, + ignored_error_codes, + ignored_file_paths, + compiler_severity_filter, + } } pub fn is_empty(&self) -> bool { @@ -784,6 +861,8 @@ pub struct OutputDiagnostics<'a> { compiler_output: &'a AggregatedCompilerOutput, /// the error codes to ignore ignored_error_codes: &'a [u64], + /// the file paths to ignore + ignored_file_paths: &'a [PathBuf], /// set minimum level of severity that is treated as an error compiler_severity_filter: Severity, } @@ -791,12 +870,17 @@ pub struct OutputDiagnostics<'a> { impl<'a> OutputDiagnostics<'a> { /// Returns true if there is at least one error of high severity pub fn has_error(&self) -> bool { - self.compiler_output.has_error(self.ignored_error_codes, &self.compiler_severity_filter) + self.compiler_output.has_error( + self.ignored_error_codes, + self.ignored_file_paths, + &self.compiler_severity_filter, + ) } /// Returns true if there is at least one warning pub fn has_warning(&self) -> bool { - self.compiler_output.has_warning(self.ignored_error_codes) + let filter = ErrorFilter::new(self.ignored_error_codes, self.ignored_file_paths); + self.compiler_output.has_warning(filter) } /// Returns true if the contract is a expected to be a test @@ -833,6 +917,13 @@ impl<'a> fmt::Display for OutputDiagnostics<'a> { // from a test file we skip ignored = self.is_test(&source_location.file) && (code == 1878 || code == 5574); + + // we ignore warnings coming from ignored files + let source_path = Path::new(&source_location.file); + ignored |= self + .ignored_file_paths + .iter() + .any(|ignored_path| source_path.starts_with(ignored_path)); } ignored |= self.ignored_error_codes.contains(&code); diff --git a/src/compile/project.rs b/src/compile/project.rs index 95944496..e9c7c993 100644 --- a/src/compile/project.rs +++ b/src/compile/project.rs @@ -303,8 +303,11 @@ impl<'a, T: ArtifactOutput> CompiledState<'a, T> { ctx, &project.paths, ) - } else if output.has_error(&project.ignored_error_codes, &project.compiler_severity_filter) - { + } else if output.has_error( + &project.ignored_error_codes, + &project.ignored_file_paths, + &project.compiler_severity_filter, + ) { trace!("skip writing cache file due to solc errors: {:?}", output.errors); project.artifacts_handler().output_to_artifacts( &output.contracts, @@ -352,8 +355,10 @@ impl<'a, T: ArtifactOutput> ArtifactsState<'a, T> { let ArtifactsState { output, cache, compiled_artifacts } = self; let project = cache.project(); let ignored_error_codes = project.ignored_error_codes.clone(); + let ignored_file_paths = project.ignored_file_paths.clone(); let compiler_severity_filter = project.compiler_severity_filter; - let has_error = output.has_error(&ignored_error_codes, &compiler_severity_filter); + let has_error = + output.has_error(&ignored_error_codes, &ignored_file_paths, &compiler_severity_filter); let skip_write_to_disk = project.no_artifacts || has_error; trace!(has_error, project.no_artifacts, skip_write_to_disk, cache_path=?project.cache_path(),"prepare writing cache file"); @@ -363,6 +368,7 @@ impl<'a, T: ArtifactOutput> ArtifactsState<'a, T> { compiled_artifacts, cached_artifacts, ignored_error_codes, + ignored_file_paths, compiler_severity_filter, }) } diff --git a/src/lib.rs b/src/lib.rs index d8503253..92813620 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -82,6 +82,8 @@ pub struct Project { pub artifacts: T, /// Errors/Warnings which match these error codes are not going to be logged pub ignored_error_codes: Vec, + /// Errors/Warnings which match these file paths are not going to be logged + pub ignored_file_paths: Vec, /// The minimum severity level that is treated as a compiler error pub compiler_severity_filter: Severity, /// The paths which will be allowed for library inclusion @@ -565,6 +567,8 @@ pub struct ProjectBuilder { artifacts: T, /// Which error codes to ignore pub ignored_error_codes: Vec, + /// Which file paths to ignore + pub ignored_file_paths: Vec, /// The minimum severity level that is treated as a compiler error compiler_severity_filter: Severity, /// All allowed paths for solc's `--allowed-paths` @@ -589,6 +593,7 @@ impl ProjectBuilder { slash_paths: true, artifacts, ignored_error_codes: Vec::new(), + ignored_file_paths: Vec::new(), compiler_severity_filter: Severity::Error, allowed_paths: Default::default(), include_paths: Default::default(), @@ -628,6 +633,11 @@ impl ProjectBuilder { self } + pub fn ignore_paths(mut self, paths: Vec) -> Self { + self.ignored_file_paths = paths; + self + } + #[must_use] pub fn set_compiler_severity_filter(mut self, compiler_severity_filter: Severity) -> Self { self.compiler_severity_filter = compiler_severity_filter; @@ -739,6 +749,7 @@ impl ProjectBuilder { offline, build_info, slash_paths, + ignored_file_paths, .. } = self; ProjectBuilder { @@ -752,6 +763,7 @@ impl ProjectBuilder { slash_paths, artifacts, ignored_error_codes, + ignored_file_paths, compiler_severity_filter, allowed_paths, include_paths, @@ -810,6 +822,7 @@ impl ProjectBuilder { auto_detect, artifacts, ignored_error_codes, + ignored_file_paths, compiler_severity_filter, mut allowed_paths, include_paths, @@ -842,6 +855,7 @@ impl ProjectBuilder { auto_detect, artifacts, ignored_error_codes, + ignored_file_paths, compiler_severity_filter, allowed_paths, include_paths, diff --git a/src/project_util/mock.rs b/src/project_util/mock.rs index af8e4034..46324390 100644 --- a/src/project_util/mock.rs +++ b/src/project_util/mock.rs @@ -380,6 +380,7 @@ trait NamingStrategy { fn new_source_file_name(&mut self, id: usize) -> String; /// Return a new name for the given source file id + #[allow(unused)] fn new_lib_file_name(&mut self, id: usize) -> String; /// Return a new name for the given lib id @@ -388,9 +389,8 @@ trait NamingStrategy { /// A primitive naming that simply uses ids to create unique names #[derive(Debug, Clone, Copy, Default)] -pub struct SimpleNamingStrategy { - _priv: (), -} +#[non_exhaustive] +pub struct SimpleNamingStrategy; impl NamingStrategy for SimpleNamingStrategy { fn new_source_file_name(&mut self, id: usize) -> String { diff --git a/src/project_util/mod.rs b/src/project_util/mod.rs index cbc0cb6d..0810c61e 100644 --- a/src/project_util/mod.rs +++ b/src/project_util/mod.rs @@ -402,6 +402,14 @@ impl TempProject { Ok(Self::create_new(tmp_dir, inner)?) } + pub fn dapptools_with_ignore_paths(paths_to_ignore: Vec) -> Result { + let tmp_dir = tempdir("tmp_dapp")?; + let paths = ProjectPathsConfig::dapptools(tmp_dir.path())?; + + let inner = Project::builder().paths(paths).ignore_paths(paths_to_ignore).build()?; + Ok(Self::create_new(tmp_dir, inner)?) + } + /// Creates an initialized dapptools style workspace in a new temporary dir pub fn dapptools_init() -> Result { let mut project = Self::dapptools()?; diff --git a/tests/project.rs b/tests/project.rs index 53b99af9..ef922143 100644 --- a/tests/project.rs +++ b/tests/project.rs @@ -1797,6 +1797,54 @@ library MyLib { assert!(!bytecode.is_unlinked()); } +#[test] +fn can_ignore_warning_from_paths() { + let setup_and_compile = |ignore_paths: Option>| { + let tmp = match ignore_paths { + Some(paths) => TempProject::dapptools_with_ignore_paths(paths).unwrap(), + None => TempProject::dapptools().unwrap(), + }; + + tmp.add_source( + "LinkTest", + r#" + // SPDX-License-Identifier: MIT + import "./MyLib.sol"; + contract LinkTest { + function foo() public returns (uint256) { + } + } + "#, + ) + .unwrap(); + + tmp.add_source( + "MyLib", + r" + // SPDX-License-Identifier: MIT + library MyLib { + function foobar(uint256 a) public view returns (uint256) { + return a * 100; + } + } + ", + ) + .unwrap(); + + tmp.compile().unwrap() + }; + + // Test without ignoring paths + let compiled_without_ignore = setup_and_compile(None); + compiled_without_ignore.assert_success(); + assert!(compiled_without_ignore.has_compiler_warnings()); + + // Test with ignoring paths + let paths_to_ignore = vec![Path::new("src").to_path_buf()]; + let compiled_with_ignore = setup_and_compile(Some(paths_to_ignore)); + compiled_with_ignore.assert_success(); + assert!(!compiled_with_ignore.has_compiler_warnings()); +} #[test] fn can_apply_libraries_with_remappings() { let mut tmp = TempProject::dapptools().unwrap(); @@ -2456,46 +2504,68 @@ fn test_compiler_severity_filter() { assert!(compiled.has_compiler_errors()); } -#[test] -fn test_compiler_severity_filter_and_ignored_error_codes() { - fn gen_test_data_licensing_warning() -> ProjectPathsConfig { - let root = PathBuf::from(env!("CARGO_MANIFEST_DIR")) - .join("test-data/test-contract-warnings/LicenseWarning.sol"); +fn gen_test_data_licensing_warning() -> ProjectPathsConfig { + let root = PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("test-data/test-contract-warnings/LicenseWarning.sol"); - ProjectPathsConfig::builder().sources(root).build().unwrap() + ProjectPathsConfig::builder().sources(root).build().unwrap() +} + +fn compile_project_with_options( + severity_filter: Option, + ignore_paths: Option>, + ignore_error_code: Option, +) -> ProjectCompileOutput { + let mut builder = + Project::builder().no_artifacts().paths(gen_test_data_licensing_warning()).ephemeral(); + + if let Some(paths) = ignore_paths { + builder = builder.ignore_paths(paths); } + if let Some(code) = ignore_error_code { + builder = builder.ignore_error_code(code); + } + if let Some(severity) = severity_filter { + builder = builder.set_compiler_severity_filter(severity); + } + + let project = builder.build().unwrap(); + project.compile().unwrap() +} +#[test] +fn test_compiler_ignored_file_paths() { + let compiled = compile_project_with_options(None, None, None); + // no ignored paths set, so the warning should be present + assert!(compiled.has_compiler_warnings()); + compiled.assert_success(); + + let compiled = compile_project_with_options( + Some(foundry_compilers::artifacts::Severity::Warning), + Some(vec![PathBuf::from("test-data")]), + None, + ); + // ignored paths set, so the warning shouldnt be present + assert!(!compiled.has_compiler_warnings()); + compiled.assert_success(); +} + +#[test] +fn test_compiler_severity_filter_and_ignored_error_codes() { let missing_license_error_code = 1878; - let project = Project::builder() - .no_artifacts() - .paths(gen_test_data_licensing_warning()) - .ephemeral() - .build() - .unwrap(); - let compiled = project.compile().unwrap(); + let compiled = compile_project_with_options(None, None, None); assert!(compiled.has_compiler_warnings()); - let project = Project::builder() - .no_artifacts() - .paths(gen_test_data_licensing_warning()) - .ephemeral() - .ignore_error_code(missing_license_error_code) - .build() - .unwrap(); - let compiled = project.compile().unwrap(); + let compiled = compile_project_with_options(None, None, Some(missing_license_error_code)); assert!(!compiled.has_compiler_warnings()); compiled.assert_success(); - let project = Project::builder() - .no_artifacts() - .paths(gen_test_data_licensing_warning()) - .ephemeral() - .ignore_error_code(missing_license_error_code) - .set_compiler_severity_filter(foundry_compilers::artifacts::Severity::Warning) - .build() - .unwrap(); - let compiled = project.compile().unwrap(); + let compiled = compile_project_with_options( + Some(foundry_compilers::artifacts::Severity::Warning), + None, + Some(missing_license_error_code), + ); assert!(!compiled.has_compiler_warnings()); compiled.assert_success(); }