Sitelet https://github.com/voronor/compilers/commit/7b8c5e92b6bab6a27fa24dd580c6f4b2440a5ff7
Skip to content

Commit 7b8c5e9

Browse files
klkvrklkvr
authored
fix: unify logic for ignored warnings (foundry-rs#179)
Closes foundry-rs/foundry#8548 we've had logic for determining ignored warnings duplicated, this PR refactors it into helpers on `AggregatedCompilerOutput`
1 parent 909e64d commit 7b8c5e9

2 files changed

Lines changed: 51 additions & 120 deletions

File tree

  • crates

‎crates/artifacts/solc/src/lib.rs‎

Lines changed: 0 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ use semver::Version;
1111
use serde::{de::Visitor, Deserialize, Deserializer, Serialize, Serializer};
1212
use serde_repr::{Deserialize_repr, Serialize_repr};
1313
use std::{
14-
borrow::Cow,
1514
collections::{BTreeMap, HashSet},
1615
fmt,
1716
path::{Path, PathBuf},
@@ -1437,46 +1436,6 @@ pub struct DocLibraries {
14371436
pub libs: BTreeMap<String, serde_json::Value>,
14381437
}
14391438

1440-
/// How to filter errors/warnings
1441-
#[derive(Clone, Debug, PartialEq, Eq)]
1442-
pub struct ErrorFilter<'a> {
1443-
/// Ignore errors/warnings with these codes
1444-
pub error_codes: Cow<'a, [u64]>,
1445-
/// Ignore errors/warnings from these file paths
1446-
pub ignored_file_paths: Cow<'a, [PathBuf]>,
1447-
}
1448-
1449-
impl<'a> ErrorFilter<'a> {
1450-
/// Creates a new `ErrorFilter` with the given error codes and ignored file paths
1451-
pub fn new(error_codes: &'a [u64], ignored_file_paths: &'a [PathBuf]) -> Self {
1452-
ErrorFilter {
1453-
error_codes: Cow::Borrowed(error_codes),
1454-
ignored_file_paths: Cow::Borrowed(ignored_file_paths),
1455-
}
1456-
}
1457-
/// Helper function to check if an error code is ignored
1458-
pub fn is_code_ignored(&self, code: Option<u64>) -> bool {
1459-
match code {
1460-
Some(code) => self.error_codes.contains(&code),
1461-
None => false,
1462-
}
1463-
}
1464-
1465-
/// Helper function to check if an error's file path is ignored
1466-
pub fn is_file_ignored(&self, file_path: &Path) -> bool {
1467-
self.ignored_file_paths.iter().any(|ignored_path| file_path.starts_with(ignored_path))
1468-
}
1469-
}
1470-
1471-
impl<'a> From<&'a [u64]> for ErrorFilter<'a> {
1472-
fn from(error_codes: &'a [u64]) -> Self {
1473-
ErrorFilter {
1474-
error_codes: Cow::Borrowed(error_codes),
1475-
ignored_file_paths: Cow::Borrowed(&[]),
1476-
}
1477-
}
1478-
}
1479-
14801439
/// Output type `solc` produces
14811440
#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)]
14821441
pub struct CompilerOutput {
@@ -1494,29 +1453,6 @@ impl CompilerOutput {
14941453
self.errors.iter().any(|err| err.severity.is_error())
14951454
}
14961455

1497-
/// Checks if there are any compiler warnings that are not ignored by the specified error codes
1498-
/// and file paths.
1499-
pub fn has_warning<'a>(&self, filter: impl Into<ErrorFilter<'a>>) -> bool {
1500-
let filter: ErrorFilter<'_> = filter.into();
1501-
self.errors.iter().any(|error| {
1502-
if !error.severity.is_warning() {
1503-
return false;
1504-
}
1505-
1506-
let is_code_ignored = filter.is_code_ignored(error.error_code);
1507-
1508-
let is_file_ignored = error
1509-
.source_location
1510-
.as_ref()
1511-
.map_or(false, |location| filter.is_file_ignored(Path::new(&location.file)));
1512-
1513-
// Only consider warnings that are not ignored by either code or file path.
1514-
// Hence, return `true` for warnings that are not ignored, making the function
1515-
// return `true` if any such warnings exist.
1516-
!(is_code_ignored || is_file_ignored)
1517-
})
1518-
}
1519-
15201456
/// Finds the _first_ contract with the given name
15211457
pub fn find(&self, contract_name: &str) -> Option<CompactContractRef<'_>> {
15221458
self.contracts_iter().find_map(|(name, contract)| {

‎crates/compilers/src/compile/output/mod.rs‎

Lines changed: 51 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! The output of a compiled project
22
use contracts::{VersionedContract, VersionedContracts};
33
use foundry_compilers_artifacts::{
4-
CompactContractBytecode, CompactContractRef, Contract, ErrorFilter, Severity,
4+
CompactContractBytecode, CompactContractRef, Contract, Severity,
55
};
66
use foundry_compilers_core::error::{SolcError, SolcIoError};
77
use info::ContractInfoRef;
@@ -483,8 +483,7 @@ impl<C: Compiler, T: ArtifactOutput> ProjectCompileOutput<C, T> {
483483

484484
/// Returns whether any warnings were emitted by the compiler.
485485
pub fn has_compiler_warnings(&self) -> bool {
486-
let filter = ErrorFilter::new(&self.ignored_error_codes, &self.ignored_file_paths);
487-
self.compiler_output.has_warning(filter)
486+
self.compiler_output.has_warning(&self.ignored_error_codes, &self.ignored_file_paths)
488487
}
489488

490489
/// Panics if any errors were emitted by the compiler.
@@ -836,8 +835,7 @@ impl<C: Compiler> AggregatedCompilerOutput<C> {
836835
if compiler_severity_filter.ge(&err.severity()) {
837836
if compiler_severity_filter.is_warning() {
838837
// skip ignored error codes and file path from warnings
839-
let filter = ErrorFilter::new(ignored_error_codes, ignored_file_paths);
840-
return self.has_warning(filter);
838+
return self.has_warning(ignored_error_codes, ignored_file_paths);
841839
}
842840
return true;
843841
}
@@ -847,25 +845,52 @@ impl<C: Compiler> AggregatedCompilerOutput<C> {
847845

848846
/// Checks if there are any compiler warnings that are not ignored by the specified error codes
849847
/// and file paths.
850-
pub fn has_warning<'a>(&self, filter: impl Into<ErrorFilter<'a>>) -> bool {
851-
let filter: ErrorFilter<'_> = filter.into();
852-
self.errors.iter().any(|error| {
853-
if !error.is_warning() {
854-
return false;
848+
pub fn has_warning(&self, ignored_error_codes: &[u64], ignored_file_paths: &[PathBuf]) -> bool {
849+
self.errors
850+
.iter()
851+
.any(|error| !self.should_ignore(ignored_error_codes, ignored_file_paths, error))
852+
}
853+
854+
pub fn should_ignore(
855+
&self,
856+
ignored_error_codes: &[u64],
857+
ignored_file_paths: &[PathBuf],
858+
error: &C::CompilationError,
859+
) -> bool {
860+
if !error.is_warning() {
861+
return false;
862+
}
863+
864+
let mut ignore = false;
865+
866+
if let Some(code) = error.error_code() {
867+
ignore |= ignored_error_codes.contains(&code);
868+
if let Some(loc) = error.source_location() {
869+
let path = Path::new(&loc.file);
870+
ignore |=
871+
ignored_file_paths.iter().any(|ignored_path| path.starts_with(ignored_path));
872+
873+
// we ignore spdx and contract size warnings in test
874+
// files. if we are looking at one of these warnings
875+
// from a test file we skip
876+
ignore |= self.is_test(path) && (code == 1878 || code == 5574);
855877
}
878+
}
856879

857-
let is_code_ignored = filter.is_code_ignored(error.error_code());
880+
ignore
881+
}
858882

859-
let is_file_ignored = error
860-
.source_location()
861-
.as_ref()
862-
.map_or(false, |location| filter.is_file_ignored(Path::new(&location.file)));
883+
/// Returns true if the contract is a expected to be a test
884+
fn is_test(&self, contract_path: &Path) -> bool {
885+
if contract_path.to_string_lossy().ends_with(".t.sol") {
886+
return true;
887+
}
863888

864-
// Only consider warnings that are not ignored by either code or file path.
865-
// Hence, return `true` for warnings that are not ignored, making the function
866-
// return `true` if any such warnings exist.
867-
!(is_code_ignored || is_file_ignored)
868-
})
889+
self.contracts.contracts_with_files().filter(|(path, _, _)| *path == contract_path).any(
890+
|(_, _, contract)| {
891+
contract.abi.as_ref().map_or(false, |abi| abi.functions.contains_key("IS_TEST"))
892+
},
893+
)
869894
}
870895
}
871896

@@ -894,19 +919,7 @@ impl<'a, C: Compiler> OutputDiagnostics<'a, C> {
894919

895920
/// Returns true if there is at least one warning
896921
pub fn has_warning(&self) -> bool {
897-
let filter = ErrorFilter::new(self.ignored_error_codes, self.ignored_file_paths);
898-
self.compiler_output.has_warning(filter)
899-
}
900-
901-
/// Returns true if the contract is a expected to be a test
902-
fn is_test(&self, contract_path: &str) -> bool {
903-
if contract_path.ends_with(".t.sol") {
904-
return true;
905-
}
906-
907-
self.compiler_output.find_first(contract_path).map_or(false, |contract| {
908-
contract.abi.map_or(false, |abi| abi.functions.contains_key("IS_TEST"))
909-
})
922+
self.compiler_output.has_warning(self.ignored_error_codes, self.ignored_file_paths)
910923
}
911924
}
912925

@@ -923,29 +936,11 @@ impl<'a, C: Compiler> fmt::Display for OutputDiagnostics<'a, C> {
923936
.fmt(f)?;
924937

925938
for err in &self.compiler_output.errors {
926-
let mut ignored = false;
927-
if err.is_warning() {
928-
if let Some(code) = err.error_code() {
929-
if let Some(source_location) = &err.source_location() {
930-
// we ignore spdx and contract size warnings in test
931-
// files. if we are looking at one of these warnings
932-
// from a test file we skip
933-
ignored =
934-
self.is_test(&source_location.file) && (code == 1878 || code == 5574);
935-
936-
// we ignore warnings coming from ignored files
937-
let source_path = Path::new(&source_location.file);
938-
ignored |= self
939-
.ignored_file_paths
940-
.iter()
941-
.any(|ignored_path| source_path.starts_with(ignored_path));
942-
}
943-
944-
ignored |= self.ignored_error_codes.contains(&code);
945-
}
946-
}
947-
948-
if !ignored {
939+
if !self.compiler_output.should_ignore(
940+
self.ignored_error_codes,
941+
self.ignored_file_paths,
942+
err,
943+
) {
949944
f.write_str("\n")?;
950945
fmt::Display::fmt(&err, f)?;
951946
}

0 commit comments

Comments
 (0)