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.

Sign up for GitHub

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

Jump to bottom

fix: unify logic for ignored warnings #179

Merged
merged 2 commits into from
Jul 30, 2024
Merged

fix: unify logic for ignored warnings #179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7168aaa
fix: unify logic for ignored warnings
klkvr Jul 30, 2024
fa84234
clippy
klkvr Jul 30, 2024
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
64 changes: 0 additions & 64 deletions crates/artifacts/solc/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ use semver::Version;
use serde::{de::Visitor, Deserialize, Deserializer, Serialize, Serializer};
use serde_repr::{Deserialize_repr, Serialize_repr};
use std::{
borrow::Cow,
collections::{BTreeMap, HashSet},
fmt,
path::{Path, PathBuf},
Expand Down Expand Up @@ -1437,46 +1436,6 @@ pub struct DocLibraries {
pub libs: BTreeMap<String, serde_json::Value>,
}

/// How to filter errors/warnings
#[derive(Clone, Debug, 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<u64>) -> 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(&[]),
}
}
}

/// Output type `solc` produces
#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)]
pub struct CompilerOutput {
Expand All @@ -1494,29 +1453,6 @@ impl CompilerOutput {
self.errors.iter().any(|err| err.severity.is_error())
}

/// 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<ErrorFilter<'a>>) -> 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)
})
}

/// Finds the _first_ contract with the given name
pub fn find(&self, contract_name: &str) -> Option<CompactContractRef<'_>> {
self.contracts_iter().find_map(|(name, contract)| {
Expand Down
107 changes: 51 additions & 56 deletions crates/compilers/src/compile/output/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! The output of a compiled project
use contracts::{VersionedContract, VersionedContracts};
use foundry_compilers_artifacts::{
CompactContractBytecode, CompactContractRef, Contract, ErrorFilter, Severity,
CompactContractBytecode, CompactContractRef, Contract, Severity,
};
use foundry_compilers_core::error::{SolcError, SolcIoError};
use info::ContractInfoRef;
Expand Down Expand Up @@ -483,8 +483,7 @@ impl<C: Compiler, T: ArtifactOutput> ProjectCompileOutput<C, T> {

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

/// Panics if any errors were emitted by the compiler.
Expand Down Expand Up @@ -836,8 +835,7 @@ impl<C: Compiler> AggregatedCompilerOutput<C> {
if compiler_severity_filter.ge(&err.severity()) {
if compiler_severity_filter.is_warning() {
// 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 self.has_warning(ignored_error_codes, ignored_file_paths);
}
return true;
}
Expand All @@ -847,25 +845,52 @@ impl<C: Compiler> AggregatedCompilerOutput<C> {

/// 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<ErrorFilter<'a>>) -> bool {
let filter: ErrorFilter<'_> = filter.into();
self.errors.iter().any(|error| {
if !error.is_warning() {
return false;
pub fn has_warning(&self, ignored_error_codes: &[u64], ignored_file_paths: &[PathBuf]) -> bool {
self.errors
.iter()
.any(|error| !self.should_ignore(ignored_error_codes, ignored_file_paths, error))
}

pub fn should_ignore(
&self,
ignored_error_codes: &[u64],
ignored_file_paths: &[PathBuf],
error: &C::CompilationError,
) -> bool {
if !error.is_warning() {
return false;
}

let mut ignore = false;

if let Some(code) = error.error_code() {
ignore |= ignored_error_codes.contains(&code);
if let Some(loc) = error.source_location() {
let path = Path::new(&loc.file);
ignore |=
ignored_file_paths.iter().any(|ignored_path| path.starts_with(ignored_path));

// we ignore spdx and contract size warnings in test
// files. if we are looking at one of these warnings
// from a test file we skip
ignore |= self.is_test(path) && (code == 1878 || code == 5574);
}
}

let is_code_ignored = filter.is_code_ignored(error.error_code());
ignore
}

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

// 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)
})
self.contracts.contracts_with_files().filter(|(path, _, _)| *path == contract_path).any(
|(_, _, contract)| {
contract.abi.as_ref().map_or(false, |abi| abi.functions.contains_key("IS_TEST"))
},
)
}
}

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

/// Returns true if there is at least one warning
pub fn has_warning(&self) -> bool {
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
fn is_test(&self, contract_path: &str) -> bool {
if contract_path.ends_with(".t.sol") {
return true;
}

self.compiler_output.find_first(contract_path).map_or(false, |contract| {
contract.abi.map_or(false, |abi| abi.functions.contains_key("IS_TEST"))
})
self.compiler_output.has_warning(self.ignored_error_codes, self.ignored_file_paths)
}
}

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

for err in &self.compiler_output.errors {
let mut ignored = false;
if err.is_warning() {
if let Some(code) = err.error_code() {
if let Some(source_location) = &err.source_location() {
// we ignore spdx and contract size warnings in test
// files. if we are looking at one of these warnings
// 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);
}
}

if !ignored {
if !self.compiler_output.should_ignore(
self.ignored_error_codes,
self.ignored_file_paths,
err,
) {
f.write_str("\n")?;
fmt::Display::fmt(&err, f)?;
}
Expand Down