feat(lint): fail on configured diagnostic level (#11445)

Author: 0xrusowsky

Cargo.lock

@@ -418,5 +418,8 @@ rexpect = { git = "https://github.com/rust-cli/rexpect", rev = "2ed0b1898d7edaf6
 # foundry-compilers = { git = "https://github.com/foundry-rs/compilers.git", branch = "dani/bump-solar" }
 # foundry-fork-db = { git = "https://github.com/foundry-rs/foundry-fork-db", rev = "eee6563" }
 
-## solar
+# solar
 # solar = { package = "solar-compiler", git = "https://github.com/paradigmxyz/solar.git", branch = "main" }
+# solar-interface = { package = "solar-interface", git = "https://github.com/paradigmxyz/solar.git", branch = "main" }
+# solar-ast = { package = "solar-ast", git = "https://github.com/paradigmxyz/solar.git", branch = "main" }
+# solar-sema = { package = "solar-sema", git = "https://github.com/paradigmxyz/solar.git", branch = "main" }

Cargo.toml

@@ -9,9 +9,9 @@ use foundry_compilers::{
     utils::canonicalized,
 };
 use foundry_config::{
-    Config, Remappings, figment,
+    Config, DenyLevel, Remappings,
     figment::{
-        Figment, Metadata, Profile, Provider,
+        self, Figment, Metadata, Profile, Provider,
         error::Kind::InvalidType,
         value::{Dict, Map, Value},
     },
@@ -48,9 +48,26 @@ pub struct BuildOpts {
     #[serde(skip_serializing_if = "Vec::is_empty")]
     pub ignored_error_codes: Vec<u64>,
 
-    /// Warnings will trigger a compiler error
-    #[arg(long, help_heading = "Compiler options")]
+    /// A compiler error will be triggered at the specified diagnostic level.
+    ///
+    /// Replaces the deprecated `--deny-warnings` flag.
+    ///
+    /// Possible values:
+    ///  - `never`: Do not treat any diagnostics as errors.
+    ///  - `warnings`: Treat warnings as errors.
+    ///  - `notes`: Treat both, warnings and notes, as errors.
+    #[arg(
+        long,
+        short = 'D',
+        help_heading = "Compiler options",
+        value_name = "LEVEL",
+        conflicts_with = "deny_warnings"
+    )]
     #[serde(skip)]
+    pub deny: Option<DenyLevel>,
+
+    /// Deprecated: use `--deny=warnings` instead.
+    #[arg(long = "deny-warnings", hide = true)]
     pub deny_warnings: bool,
 
     /// Do not auto-detect the `solc` version.
@@ -220,7 +237,10 @@ impl Provider for BuildOpts {
         }
 
         if self.deny_warnings {
-            dict.insert("deny_warnings".to_string(), true.into());
+            dict.insert("deny".to_string(), figment::value::Value::serialize(DenyLevel::Warnings)?);
+            _ = sh_warn!("`--deny-warnings` is being deprecated in favor of `--deny warnings`.");
+        } else if let Some(deny) = self.deny {
+            dict.insert("deny".to_string(), figment::value::Value::serialize(deny)?);
         }
 
         if self.via_ir {

crates/cli/src/opts/build/core.rs

@@ -107,7 +107,11 @@ etherscan_api_key = "YOURETHERSCANAPIKEY"
 # additional warnings can be added using their numeric error code: ["license", 1337]
 ignored_error_codes = ["license", "code-size"]
 ignored_warnings_from = ["path_to_ignore"]
-deny_warnings = false
+# Deny compiler warnings and/or notes with:
+# - "never": default behavior, no denial
+# - "warnings": warnings treated as errors
+# - "notes": notes and warnings treated as notes
+deny = "never"
 match_test = "Foo"
 no_match_test = "Bar"
 match_contract = "Foo"

crates/config/README.md

@@ -494,8 +494,8 @@ contract FuzzInlineConf is DSTest {
 
     fn natspec() -> NatSpec {
         let conf = r"
-        forge-config: default.fuzz.runs = 600 
-        forge-config: ci.fuzz.runs = 500 
+        forge-config: default.fuzz.runs = 600
+        forge-config: ci.fuzz.runs = 500
         ========= SOME NOISY TEXT =============
          䩹𧀫Jx닧Ʀ̳盅K擷􅟽Ɂw첊}ꏻk86ᖪk-檻ܴ렝[Dz𐤬oᘓƤ
         ꣖ۻ%Ƅ㪕ς:(饁΍av/烲ڻ̛߉橞㗡𥺃̹M봓䀖ؿ̄󵼁)𯖛d􂽰񮍃

crates/config/src/inline/natspec.rs

@@ -42,7 +42,7 @@ use foundry_compilers::{
 use regex::Regex;
 use revm::primitives::hardfork::SpecId;
 use semver::Version;
-use serde::{Deserialize, Serialize, Serializer};
+use serde::{Deserialize, Deserializer, Serialize, Serializer, de};
 use std::{
     borrow::Cow,
     collections::BTreeMap,
@@ -306,7 +306,10 @@ pub struct Config {
     /// list of file paths to ignore
     #[serde(rename = "ignored_warnings_from")]
     pub ignored_file_paths: Vec<PathBuf>,
-    /// When true, compiler warnings are treated as errors
+    /// Diagnostic level (minimum) at which the process should finish with a non-zero exit.
+    pub deny: DenyLevel,
+    /// DEPRECATED: use `deny` instead.
+    #[serde(default, skip_serializing)]
     pub deny_warnings: bool,
     /// Only run test functions matching the specified regex pattern.
     #[serde(rename = "match_test")]
@@ -562,13 +565,109 @@ pub struct Config {
     pub _non_exhaustive: (),
 }
 
+/// Diagnostic level (minimum) at which the process should finish with a non-zero exit.
+#[derive(Debug, Clone, Copy, PartialEq, Eq, clap::ValueEnum, Default, Serialize)]
+#[serde(rename_all = "lowercase")]
+pub enum DenyLevel {
+    /// Always exit with zero code.
+    #[default]
+    Never,
+    /// Exit with a non-zero code if any warnings are found.
+    Warnings,
+    /// Exit with a non-zero code if any notes or warnings are found.
+    Notes,
+}
+
+// Custom deserialization to make `DenyLevel` parsing case-insensitive and backwards compatible with
+// booleans.
+impl<'de> Deserialize<'de> for DenyLevel {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        struct DenyLevelVisitor;
+
+        impl<'de> de::Visitor<'de> for DenyLevelVisitor {
+            type Value = DenyLevel;
+
+            fn expecting(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+                formatter.write_str("one of the following strings: `never`, `warnings`, `notes`")
+            }
+
+            fn visit_bool<E>(self, value: bool) -> Result<Self::Value, E>
+            where
+                E: de::Error,
+            {
+                Ok(DenyLevel::from(value))
+            }
+
+            fn visit_str<E>(self, value: &str) -> Result<Self::Value, E>
+            where
+                E: de::Error,
+            {
+                DenyLevel::from_str(value).map_err(de::Error::custom)
+            }
+        }
+
+        deserializer.deserialize_any(DenyLevelVisitor)
+    }
+}
+
+impl FromStr for DenyLevel {
+    type Err = String;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        match s.to_lowercase().as_str() {
+            "warnings" | "warning" | "w" => Ok(Self::Warnings),
+            "notes" | "note" | "n" => Ok(Self::Notes),
+            "never" | "false" | "f" => Ok(Self::Never),
+            _ => Err(format!(
+                "unknown variant: found `{s}`, expected one of `never`, `warnings`, `notes`"
+            )),
+        }
+    }
+}
+
+impl From<bool> for DenyLevel {
+    fn from(deny: bool) -> Self {
+        if deny { Self::Warnings } else { Self::Never }
+    }
+}
+
+impl DenyLevel {
+    /// Returns `true` if the deny level includes warnings.
+    pub fn warnings(&self) -> bool {
+        match self {
+            Self::Never => false,
+            Self::Warnings | Self::Notes => true,
+        }
+    }
+
+    /// Returns `true` if the deny level includes notes.
+    pub fn notes(&self) -> bool {
+        match self {
+            Self::Never | Self::Warnings => false,
+            Self::Notes => true,
+        }
+    }
+
+    /// Returns `true` if the deny level is set to never (only errors).
+    pub fn never(&self) -> bool {
+        match self {
+            Self::Never => true,
+            Self::Warnings | Self::Notes => false,
+        }
+    }
+}
+
 /// Mapping of fallback standalone sections. See [`FallbackProfileProvider`].
 pub const STANDALONE_FALLBACK_SECTIONS: &[(&str, &str)] = &[("invariant", "fuzz")];
 
 /// Deprecated keys and their replacements.
 ///
 /// See [Warning::DeprecatedKey]
-pub const DEPRECATIONS: &[(&str, &str)] = &[("cancun", "evm_version = Cancun")];
+pub const DEPRECATIONS: &[(&str, &str)] =
+    &[("cancun", "evm_version = Cancun"), ("deny_warnings", "deny = warnings")];
 
 impl Config {
     /// The default profile: "default"
@@ -1051,7 +1150,7 @@ impl Config {
             .paths(paths)
             .ignore_error_codes(self.ignored_error_codes.iter().copied().map(Into::into))
             .ignore_paths(self.ignored_file_paths.clone())
-            .set_compiler_severity_filter(if self.deny_warnings {
+            .set_compiler_severity_filter(if self.deny.warnings() {
                 Severity::Warning
             } else {
                 Severity::Error
@@ -2160,6 +2259,13 @@ impl Config {
             figment = figment.merge(("evm_version", version));
         }
 
+        // Normalize `deny` based on the provided `deny_warnings` version.
+        if self.deny_warnings
+            && let Ok(DenyLevel::Never) = figment.extract_inner("deny")
+        {
+            figment = figment.merge(("deny", DenyLevel::Warnings));
+        }
+
         figment
     }
 }
@@ -2432,6 +2538,7 @@ impl Default for Config {
                 SolidityErrorCode::TransientStorageUsed,
             ],
             ignored_file_paths: vec![],
+            deny: DenyLevel::Never,
             deny_warnings: false,
             via_ir: false,
             ast: false,
@@ -3780,7 +3887,7 @@ mod tests {
                 gas_reports = ['*']
                 ignored_error_codes = [1878]
                 ignored_warnings_from = ["something"]
-                deny_warnings = false
+                deny = "never"
                 initial_balance = '0xffffffffffffffffffffffff'
                 libraries = []
                 libs = ['lib']
@@ -6221,6 +6328,24 @@ mod tests {
         });
     }
 
+    #[test]
+    fn test_deprecated_deny_warnings_is_handled() {
+        figment::Jail::expect_with(|jail| {
+            jail.create_file(
+                "foundry.toml",
+                r#"
+                [profile.default]
+                deny_warnings = true
+                "#,
+            )?;
+            let config = Config::load().unwrap();
+
+            // Assert that the deprecated flag is correctly interpreted
+            assert_eq!(config.deny, DenyLevel::Warnings);
+            Ok(())
+        });
+    }
+
     #[test]
     fn test_evm_version_solc_compatibility_warning() {
         figment::Jail::expect_with(|jail| {

crates/config/src/lib.rs

@@ -7,7 +7,7 @@ use solar::interface::diagnostics::Level;
 use std::str::FromStr;
 use yansi::Paint;
 
-/// Contains the config and rule set
+/// Contains the config and rule set.
 #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
 pub struct LinterConfig {
     /// Specifies which lints to run based on severity.
@@ -18,7 +18,7 @@ pub struct LinterConfig {
     /// Deny specific lints based on their ID (e.g. "mixed-case-function").
     pub exclude_lints: Vec<String>,
 
-    /// Globs to ignore
+    /// Globs to ignore.
     pub ignore: Vec<String>,
 
     /// Whether to run linting during `forge build`.
@@ -45,7 +45,7 @@ impl Default for LinterConfig {
     }
 }
 
-/// Severity of a lint
+/// Severity of a lint.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, ValueEnum, Serialize)]
 pub enum Severity {
     High,

crates/config/src/lint.rs

@@ -302,6 +302,12 @@ impl<P: Provider> Provider for BackwardsCompatTomlProvider<P> {
                     dict.insert("solc".to_string(), v);
                 }
             }
+            if let Some(v) = dict.remove("deny_warnings")
+                && !dict.contains_key("deny")
+            {
+                dict.insert("deny".to_string(), v);
+            }
+
             map.insert(profile, dict);
         }
         Ok(map)

crates/config/src/providers/ext.rs

@@ -177,7 +177,7 @@ impl BindJsonArgs {
                     .source_map()
                     .new_source_file(path.clone(), source.content.as_str())
                 {
-                    target_files.insert(src_file.clone());
+                    target_files.insert(Arc::clone(&src_file));
                     pcx.add_file(src_file);
                 }
             }

crates/forge/src/cmd/bind_json.rs

@@ -175,7 +175,7 @@ impl BuildArgs {
 
             if !input_files.is_empty() {
                 let compiler = output.parser_mut().solc_mut().compiler_mut();
-                linter.lint(&input_files, compiler)?;
+                linter.lint(&input_files, config.deny, compiler)?;
             }
         }