Ignore SPDX and contract size for tests #775
Conversation
ethers-solc/src/artifacts.rs
Outdated
| // files. if we are looking at one of these warnings | ||
| // from a test file we skip | ||
| if self.is_test(&source_location.file) | ||
| && (*code == 1878 || *code == 5574) { |
There was a problem hiding this comment.
Not super keen on this, but I am not entirely sure what direction to go in with fully configurable error codes based on types either in terms of DX or configuration. Is this fine for now or does anyone have any pointers on how to make this more elegant?
ethers-solc/src/artifacts.rs
Outdated
| pub fn has_warning<'a>(&self, ignored_error_codes: &'a [u64]) -> bool { | ||
| self.errors.iter().any(|err| { | ||
| let is_ignored = err.error_code.as_ref().map_or(false, |code| { | ||
| !ignored_error_codes.contains(code) | ||
| }); | ||
|
|
||
| err.severity.is_warning() && !is_ignored | ||
| }) |
There was a problem hiding this comment.
Realized we had to filter for errors here too
| self.compiler_output.has_warning(&self.ignored_error_codes) | ||
| } | ||
|
|
||
| fn is_test<T: AsRef<str>>(&self, contract_path: T) -> bool { |
There was a problem hiding this comment.
Not sure how robust this is and I couldn't really figure out a good way to do it for deps, should the test be "if it sits in node_modules or lib then it is a dependency"?
There was a problem hiding this comment.
hmm, one idea I have is to leave this completely up to the user,
rn we're using it like this:
ethers-rs/ethers-solc/src/lib.rs
Line 864 in 371afb9
perhaps we can add something like
filter: dyn ContractFilter
trait ContractFilter {
fn matches(&self, contract: &Contract) -> bool;
}
impl ContractFilter for () {
fn matches(&self) -> boo {true}
}then implement this for Fn(&str) -> bool
so we can add a CompilerOutput::diagnostics_with_filter(error_codes, dyn ContractFilter)
There was a problem hiding this comment.
we could also do that for filtering of warnings in general, so instead of a contract filter etc. we just give the user
has_warningshas_errors- a way to iterate the errors and filter them themselves
unless there is a specific reason we have this in ethers-solc as opposed to foundry?
There was a problem hiding this comment.
sg, so we should map each error to its contract and ignore an error if:
- ignored error code
- contract should be ignored
onbjerg
force-pushed
the
granular-error-codes
branch
from
08a001f to
371afb9
Compare
ethers-solc/src/artifacts.rs
Outdated
| let is_ignored = | ||
| err.error_code.as_ref().map_or(false, |code| !ignored_error_codes.contains(code)); |
There was a problem hiding this comment.
I'd be more efficient to compute this only if err.severity.is_warning() == true
* Pull latest ethers * Do not suppress warnings on build * Add `--ignored-error-codes` option * chore: remove double success with warnings log * chore: fix dai resolve addr test https://github.com/gakonst/ethers-rs/pull/771/files * fix: do not log output warnings if ignored using gakonst/ethers-rs#775, warnings for test contracts are not going to be logged. Co-authored-by: Georgios Konstantopoulos <[email protected]>
* Pull latest ethers * Do not suppress warnings on build * Add `--ignored-error-codes` option * chore: remove double success with warnings log * chore: fix dai resolve addr test https://github.com/gakonst/ethers-rs/pull/771/files * fix: do not log output warnings if ignored using gakonst/ethers-rs#775, warnings for test contracts are not going to be logged. Co-authored-by: Georgios Konstantopoulos <[email protected]>
* Pull latest ethers * Do not suppress warnings on build * Add `--ignored-error-codes` option * chore: remove double success with warnings log * chore: fix dai resolve addr test https://github.com/gakonst/ethers-rs/pull/771/files * fix: do not log output warnings if ignored using gakonst/ethers-rs#775, warnings for test contracts are not going to be logged. Co-authored-by: Georgios Konstantopoulos <[email protected]>
Motivation
Following discussion in foundry-rs/foundry#399 we want to be able to ignore some errors for tests and dependencies, but not for source.
Solution
This PR just ignores 2 errors for both tests and dependencies by default: the SPDX and contract size warnings.