Skip to content
This repository has been archived by the owner on Oct 19, 2024. It is now read-only.

feat: warnings as errors #1838

Merged
merged 10 commits into from
Nov 10, 2022
3 changes: 2 additions & 1 deletion ethers-solc/src/artifacts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1747,8 +1747,9 @@ impl fmt::Display for Error {
}
}

#[derive(Clone, Debug, Eq, PartialEq, Hash)]
#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd, Default)]
pub enum Severity {
#[default]
Error,
Warning,
Info,
Expand Down
24 changes: 9 additions & 15 deletions ethers-solc/src/compile/output/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use crate::{
artifacts::{
contract::{CompactContractBytecode, CompactContractRef, Contract},
Error,
Error, Severity,
},
buildinfo::RawBuildInfo,
info::ContractInfoRef,
Expand Down Expand Up @@ -31,8 +31,8 @@ pub struct ProjectCompileOutput<T: ArtifactOutput = ConfigurableArtifacts> {
pub(crate) cached_artifacts: Artifacts<T::Artifact>,
/// errors that should be omitted
pub(crate) ignored_error_codes: Vec<u64>,
/// treat warnings as errors
pub(crate) warnings_as_errors: bool,
/// set level of severity that is treated as an error
pub(crate) warnings_as_errors: Severity,
}

impl<T: ArtifactOutput> ProjectCompileOutput<T> {
Expand Down Expand Up @@ -199,7 +199,7 @@ impl<T: ArtifactOutput> ProjectCompileOutput<T> {

/// Whether there were errors
pub fn has_compiler_errors(&self) -> bool {
self.compiler_output.has_error(self.warnings_as_errors)
self.compiler_output.has_error(&self.warnings_as_errors)
}

/// Whether there were warnings
Expand Down Expand Up @@ -400,7 +400,7 @@ impl<T: ArtifactOutput> fmt::Display for ProjectCompileOutput<T> {
if self.compiler_output.is_unchanged() {
f.write_str("Nothing to compile")
} else {
self.compiler_output.diagnostics(&self.ignored_error_codes, self.warnings_as_errors).fmt(f)
self.compiler_output.diagnostics(&self.ignored_error_codes, &self.warnings_as_errors).fmt(f)
}
}
}
Expand Down Expand Up @@ -428,14 +428,8 @@ impl AggregatedCompilerOutput {
}

/// Whether the output contains a compiler error
pub fn has_error(&self, warnings_as_errors: bool) -> bool {
self.errors.iter().any(|err| {
if warnings_as_errors {
true
} else {
err.severity.is_error()
}
})
pub fn has_error(&self, warnings_as_errors: &Severity) -> bool {
self.errors.iter().any(|err| warnings_as_errors.ge(&err.severity))
}

/// Whether the output contains a compiler warning
Expand All @@ -449,7 +443,7 @@ impl AggregatedCompilerOutput {
})
}

pub fn diagnostics<'a>(&'a self, ignored_error_codes: &'a [u64], warnings_as_errors: bool) -> OutputDiagnostics {
pub fn diagnostics<'a>(&'a self, ignored_error_codes: &'a [u64], warnings_as_errors: &'a Severity) -> OutputDiagnostics {
Copy link
Collaborator

Choose a reason for hiding this comment

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

no need to take this as ref

Suggested change
pub fn diagnostics<'a>(&'a self, ignored_error_codes: &'a [u64], warnings_as_errors: &'a Severity) -> OutputDiagnostics {
pub fn diagnostics<'a>(&'a self, ignored_error_codes: &'a [u64], warnings_as_errors: Severity) -> OutputDiagnostics {

OutputDiagnostics { compiler_output: self, ignored_error_codes, warnings_as_errors }
}

Expand Down Expand Up @@ -711,7 +705,7 @@ pub struct OutputDiagnostics<'a> {
/// the error codes to ignore
ignored_error_codes: &'a [u64],
/// treat warnings as errors
warnings_as_errors: bool,
warnings_as_errors: &'a Severity,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
warnings_as_errors: &'a Severity,
warnings_as_errors: Severity,

Copy link
Collaborator

Choose a reason for hiding this comment

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

please rename to something akin to severity

}

impl<'a> OutputDiagnostics<'a> {
Expand Down
7 changes: 4 additions & 3 deletions ethers-solc/src/compile/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ impl<'a, T: ArtifactOutput> CompiledState<'a, T> {
ctx,
&project.paths,
)
} else if output.has_error(project.warnings_as_errors) {
} else if output.has_error(&project.warnings_as_errors) {
trace!("skip writing cache file due to solc errors: {:?}", output.errors);
project.artifacts_handler().output_to_artifacts(
&output.contracts,
Expand Down Expand Up @@ -360,14 +360,15 @@ 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();

Choose a reason for hiding this comment

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

perhaps follow convention here and change to:

let warnings_as_errors = cache.project().warnings_as_errors.clone();

let skip_write_to_disk = project.no_artifacts || output.has_error(project.warnings_as_errors);
let warnings_as_errors = project.warnings_as_errors.clone();
let skip_write_to_disk = project.no_artifacts || output.has_error(&warnings_as_errors);
let cached_artifacts = cache.consume(&compiled_artifacts, !skip_write_to_disk)?;
Ok(ProjectCompileOutput {
compiler_output: output,
compiled_artifacts,
cached_artifacts,
ignored_error_codes,
warnings_as_errors: project.warnings_as_errors
warnings_as_errors
})
}
}
Expand Down
10 changes: 5 additions & 5 deletions ethers-solc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ use crate::{
error::{SolcError, SolcIoError},
sources::{VersionedSourceFile, VersionedSourceFiles},
};
use artifacts::contract::Contract;
use artifacts::{contract::Contract, Severity};
use compile::output::contracts::VersionedContracts;
use error::Result;
use semver::Version;
Expand Down Expand Up @@ -74,7 +74,7 @@ pub struct Project<T: ArtifactOutput = ConfigurableArtifacts> {
/// Errors/Warnings which match these error codes are not going to be logged
pub ignored_error_codes: Vec<u64>,
/// Whether warnings will be filtered out when checking for errors.
pub warnings_as_errors: bool,
pub warnings_as_errors: Severity,
/// The paths which will be allowed for library inclusion
pub allowed_paths: AllowedLibPaths,
/// The paths which will be used with solc's `--include-path` attribute
Expand Down Expand Up @@ -572,7 +572,7 @@ pub struct ProjectBuilder<T: ArtifactOutput = ConfigurableArtifacts> {
/// Which error codes to ignore
pub ignored_error_codes: Vec<u64>,
/// Whether to include warnings when checking for errors
warnings_as_errors: bool,
warnings_as_errors: Severity,
/// All allowed paths for solc's `--allowed-paths`
allowed_paths: AllowedLibPaths,
/// Paths to use for solc's `--include-path`
Expand All @@ -595,7 +595,7 @@ impl<T: ArtifactOutput> ProjectBuilder<T> {
slash_paths: true,
artifacts,
ignored_error_codes: Vec::new(),
warnings_as_errors: false,
warnings_as_errors: Severity::Error,
allowed_paths: Default::default(),
include_paths: Default::default(),
solc_jobs: None,
Expand Down Expand Up @@ -635,7 +635,7 @@ impl<T: ArtifactOutput> ProjectBuilder<T> {
}

#[must_use]
pub fn set_warnings_as_errors(mut self, warnings_as_errors: bool) -> Self {
pub fn set_warnings_as_errors(mut self, warnings_as_errors: Severity) -> Self {
self.warnings_as_errors = warnings_as_errors;
self
}
Expand Down
2 changes: 1 addition & 1 deletion ethers-solc/tests/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1631,7 +1631,7 @@ fn test_warnings_as_errors() {
assert!(compiled.has_compiler_warnings());
assert!(!compiled.has_compiler_errors());

let project = Project::builder().no_artifacts().paths(gen_test_data_warning_path()).ephemeral().set_warnings_as_errors(true).build().unwrap();
let project = Project::builder().no_artifacts().paths(gen_test_data_warning_path()).ephemeral().set_warnings_as_errors(ethers_solc::artifacts::Severity::Warning).build().unwrap();
let compiled = project.compile().unwrap();
assert!(compiled.has_compiler_warnings());
assert!(compiled.has_compiler_errors());
Expand Down