feat: warnings as errors (#1838)

* feat: warnings as errors

* changed the bool arg to Severity and updated its traits

* reformat the test based on the linter

* renamed variable based on property type change and changed a few refs

* updated changelog

* revert changelog iden change

* added test for combining compiler severity filter and ignored error codes and adjusted has error for the added test case

* adjusted has_error to utilize ge functionality in case of info errors

Co-authored-by: Georgios Konstantopoulos <me@gakonst.com>

ethers-solc/src/artifacts/mod.rs

@@ -1733,8 +1733,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,

ethers-solc/src/compile/output/mod.rs

@@ -3,7 +3,7 @@
 use crate::{
     artifacts::{
         contract::{CompactContractBytecode, CompactContractRef, Contract},
-        Error,
+        Error, Severity,
     },
     buildinfo::RawBuildInfo,
     info::ContractInfoRef,
@@ -31,6 +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>,
+    /// set minimum level of severity that is treated as an error
+    pub(crate) compiler_severity_filter: Severity,
 }
 
 impl<T: ArtifactOutput> ProjectCompileOutput<T> {
@@ -197,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.compiler_output.has_error(&self.ignored_error_codes, &self.compiler_severity_filter)
     }
 
     /// Whether there were warnings
@@ -398,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).fmt(f)
+            self.compiler_output.diagnostics(&self.ignored_error_codes, self.compiler_severity_filter.clone()).fmt(f)
         }
     }
 }
@@ -426,8 +428,16 @@ impl AggregatedCompilerOutput {
     }
 
     /// Whether the output contains a compiler error
-    pub fn has_error(&self) -> bool {
+    pub fn has_error(&self, ignored_error_codes: &[u64], compiler_severity_filter: &Severity) -> bool {
-        self.errors.iter().any(|err| err.severity.is_error())
+        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)
+                }
+                return true
+            }
+            return false
+        })
     }
 
     /// Whether the output contains a compiler warning
@@ -441,8 +451,8 @@ impl AggregatedCompilerOutput {
         })
     }
 
-    pub fn diagnostics<'a>(&'a self, ignored_error_codes: &'a [u64]) -> OutputDiagnostics {
+    pub fn diagnostics<'a>(&'a self, ignored_error_codes: &'a [u64], compiler_severity_filter: Severity) -> OutputDiagnostics {
-        OutputDiagnostics { compiler_output: self, ignored_error_codes }
+        OutputDiagnostics { compiler_output: self, ignored_error_codes, compiler_severity_filter }
     }
 
     pub fn is_empty(&self) -> bool {
@@ -702,12 +712,14 @@ pub struct OutputDiagnostics<'a> {
     compiler_output: &'a AggregatedCompilerOutput,
     /// the error codes to ignore
     ignored_error_codes: &'a [u64],
+    /// set minimum level of severity that is treated as an error
+    compiler_severity_filter: Severity,
 }
 
 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.compiler_output.has_error(&self.ignored_error_codes, &self.compiler_severity_filter)
     }
 
     /// Returns true if there is at least one warning

ethers-solc/src/compile/project.rs

@@ -311,7 +311,7 @@ impl<'a, T: ArtifactOutput> CompiledState<'a, T> {
                 ctx,
                 &project.paths,
             )
-        } else if output.has_error() {
+        } else if output.has_error(&project.ignored_error_codes, &project.compiler_severity_filter) {
             trace!("skip writing cache file due to solc errors: {:?}", output.errors);
             project.artifacts_handler().output_to_artifacts(
                 &output.contracts,
@@ -358,14 +358,17 @@ impl<'a, T: ArtifactOutput> ArtifactsState<'a, T> {
     fn write_cache(self) -> Result<ProjectCompileOutput<T>> {
         trace!("write cache");
         let ArtifactsState { output, cache, compiled_artifacts } = self;
-        let ignored_error_codes = cache.project().ignored_error_codes.clone();
+        let project = cache.project();
-        let skip_write_to_disk = cache.project().no_artifacts || output.has_error();
+        let ignored_error_codes = project.ignored_error_codes.clone();
+        let compiler_severity_filter = project.compiler_severity_filter.clone();
+        let skip_write_to_disk = project.no_artifacts || output.has_error(&ignored_error_codes, &compiler_severity_filter);
         let cached_artifacts = cache.consume(&compiled_artifacts, !skip_write_to_disk)?;
         Ok(ProjectCompileOutput {
             compiler_output: output,
             compiled_artifacts,
             cached_artifacts,
             ignored_error_codes,
+            compiler_severity_filter
         })
     }
 }

ethers-solc/src/lib.rs

@@ -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;
@@ -73,6 +73,8 @@ pub struct Project<T: ArtifactOutput = ConfigurableArtifacts> {
     pub artifacts: T,
     /// Errors/Warnings which match these error codes are not going to be logged
     pub ignored_error_codes: Vec<u64>,
+    /// 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
     pub allowed_paths: AllowedLibPaths,
     /// The paths which will be used with solc's `--include-path` attribute
@@ -569,6 +571,8 @@ pub struct ProjectBuilder<T: ArtifactOutput = ConfigurableArtifacts> {
     artifacts: T,
     /// Which error codes to ignore
     pub ignored_error_codes: Vec<u64>,
+    /// The minimum severity level that is treated as a compiler error
+    compiler_severity_filter: Severity,
     /// All allowed paths for solc's `--allowed-paths`
     allowed_paths: AllowedLibPaths,
     /// Paths to use for solc's `--include-path`
@@ -591,6 +595,7 @@ impl<T: ArtifactOutput> ProjectBuilder<T> {
             slash_paths: true,
             artifacts,
             ignored_error_codes: Vec::new(),
+            compiler_severity_filter: Severity::Error,
             allowed_paths: Default::default(),
             include_paths: Default::default(),
             solc_jobs: None,
@@ -629,6 +634,12 @@ impl<T: ArtifactOutput> ProjectBuilder<T> {
         self
     }
 
+    #[must_use]
+    pub fn set_compiler_severity_filter(mut self, compiler_severity_filter: Severity) -> Self {
+        self.compiler_severity_filter = compiler_severity_filter;
+        self
+    }
+
     /// Disables cached builds
     #[must_use]
     pub fn ephemeral(self) -> Self {
@@ -727,6 +738,7 @@ impl<T: ArtifactOutput> ProjectBuilder<T> {
             no_artifacts,
             auto_detect,
             ignored_error_codes,
+            compiler_severity_filter,
             allowed_paths,
             include_paths,
             solc_jobs,
@@ -746,6 +758,7 @@ impl<T: ArtifactOutput> ProjectBuilder<T> {
             slash_paths,
             artifacts,
             ignored_error_codes,
+            compiler_severity_filter,
             allowed_paths,
             include_paths,
             solc_jobs,
@@ -803,6 +816,7 @@ impl<T: ArtifactOutput> ProjectBuilder<T> {
             auto_detect,
             artifacts,
             ignored_error_codes,
+            compiler_severity_filter,
             mut allowed_paths,
             include_paths,
             solc_jobs,
@@ -834,6 +848,7 @@ impl<T: ArtifactOutput> ProjectBuilder<T> {
             auto_detect,
             artifacts,
             ignored_error_codes,
+            compiler_severity_filter,
             allowed_paths,
             include_paths,
             solc_jobs: solc_jobs.unwrap_or_else(num_cpus::get),

ethers-solc/test-data/test-contract-warnings/LicenseWarning.sol

@@ -0,0 +1,3 @@
+pragma solidity 0.8.6;
+
+contract LicenseWarning {}

ethers-solc/tests/project.rs

@@ -1618,6 +1618,81 @@ fn can_compile_model_checker_sample() {
     assert!(compiled.has_compiler_warnings());
 }
 
+#[test]
+fn test_compiler_severity_filter() {
+    fn gen_test_data_warning_path() -> ProjectPathsConfig {
+        let root =
+            PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("test-data/test-contract-warnings");
+        let paths = ProjectPathsConfig::builder().sources(root).build().unwrap();
+        paths
+    }
+
+    let project = Project::builder()
+        .no_artifacts()
+        .paths(gen_test_data_warning_path())
+        .ephemeral()
+        .build()
+        .unwrap();
+    let compiled = project.compile().unwrap();
+    assert!(compiled.has_compiler_warnings());
+    assert!(!compiled.has_compiler_errors());
+
+    let project = Project::builder()
+        .no_artifacts()
+        .paths(gen_test_data_warning_path())
+        .ephemeral()
+        .set_compiler_severity_filter(ethers_solc::artifacts::Severity::Warning)
+        .build()
+        .unwrap();
+    let compiled = project.compile().unwrap();
+    assert!(compiled.has_compiler_warnings());
+    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");
+        let paths = ProjectPathsConfig::builder().sources(root).build().unwrap();
+        paths
+    }
+
+    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();
+    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();
+    assert!(!compiled.has_compiler_warnings());
+    assert!(!compiled.has_compiler_errors());
+
+    let project = Project::builder()
+    .no_artifacts()
+    .paths(gen_test_data_licensing_warning())
+    .ephemeral()
+    .ignore_error_code(missing_license_error_code)
+    .set_compiler_severity_filter(ethers_solc::artifacts::Severity::Warning)
+    .build()
+    .unwrap();
+    let compiled = project.compile().unwrap();
+    assert!(!compiled.has_compiler_warnings());
+    assert!(!compiled.has_compiler_errors());
+}
+
 fn remove_solc_if_exists(version: &Version) {
     if Solc::find_svm_installed_version(version.to_string()).unwrap().is_some() {
         svm::remove_version(version).expect("failed to remove version")