feat(rome_cli): exit with error if there are warnings (#4676)

CHANGELOG.md

@@ -84,6 +84,12 @@ multiple files:
 - Fix the commands `rome check` and `rome lint`, they won't exit with an error code
 if no error diagnostics are emitted.
 
+- Add a new option `--error-on-warnings`, which instructs Rome to exit with an error code when warnings are emitted.
+
+  ```shell
+  rome check --error-on-wanrings ./src
+  ```
+
 ### Editors
 
 #### Other changes

crates/rome_cli/src/cli_options.rs

@@ -20,9 +20,14 @@ pub struct CliOptions {
     #[bpaf(long("config-path"), argument("PATH"), optional)]
     pub config_path: Option<String>,
 
-    /// Cap the amount of diagnostics displayed (default: 20)
-    #[bpaf(long("max-diagnostics"), argument("NUMBER"), optional)]
-    pub max_diagnostics: Option<u16>,
+    /// Cap the amount of diagnostics displayed.
+    #[bpaf(
+        long("max-diagnostics"),
+        argument("NUMBER"),
+        fallback(20),
+        display_fallback
+    )]
+    pub max_diagnostics: u16,
 
     /// Skip over files containing syntax errors instead of emitting an error diagnostic.
     #[bpaf(long("skip-errors"), switch)]
@@ -32,6 +37,10 @@ pub struct CliOptions {
     #[bpaf(long("no-errors-on-unmatched"), switch)]
     pub no_errors_on_unmatched: bool,
 
+    /// Tell Rome to exit with an error code if some diagnostics emit warnings.
+    #[bpaf(long("error-on-warnings"), switch)]
+    pub error_on_warnings: bool,
+
     /// Reports information using the JSON format
     #[bpaf(long("json"), switch, hide_usage)]
     pub json: bool,

crates/rome_cli/src/diagnostics.rs

@@ -1,4 +1,4 @@
-use rome_console::fmt::{Display, Formatter};
+use rome_console::fmt::Formatter;
 use rome_console::markup;
 use rome_diagnostics::adapters::{BpafError, IoError};
 use rome_diagnostics::{
@@ -153,54 +153,13 @@ pub struct IncompatibleArguments {
 #[derive(Debug, Diagnostic)]
 #[diagnostic(
     severity = Error,
-    message(
-        description = "{action_kind}",
-        message({{self.action_kind}})
-    )
 )]
 pub struct CheckError {
-    action_kind: CheckActionKind,
-
     #[category]
     category: &'static Category,
-}
 
-#[derive(Clone, Copy)]
-pub enum CheckActionKind {
-    Check,
-    Apply,
-}
-
-impl Debug for CheckActionKind {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        std::fmt::Display::fmt(self, f)
-    }
-}
-
-impl Display for CheckActionKind {
-    fn fmt(&self, fmt: &mut Formatter) -> std::io::Result<()> {
-        match self {
-            CheckActionKind::Check => fmt.write_markup(markup! {
-                "Some errors were emitted while "<Emphasis>"running checks"</Emphasis>
-            }),
-            CheckActionKind::Apply => fmt.write_markup(markup! {
-                "Some errors were emitted while "<Emphasis>"applying fixes"</Emphasis>
-            }),
-        }
-    }
-}
-
-impl std::fmt::Display for CheckActionKind {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        match self {
-            CheckActionKind::Check => {
-                write!(f, "Some errors were emitted while running checks")
-            }
-            CheckActionKind::Apply => {
-                write!(f, "Some errors were emitted while applying fixes")
-            }
-        }
-    }
+    #[message]
+    message: MessageAndDescription,
 }
 
 #[derive(Debug, Diagnostic)]
@@ -406,16 +365,51 @@ impl CliDiagnostic {
     /// Emitted when errors were emitted while running `check` command
     pub fn check_error(category: &'static Category) -> Self {
         Self::CheckError(CheckError {
-            action_kind: CheckActionKind::Check,
             category,
+            message: MessageAndDescription::from(
+                markup! {
+                    "Some "<Emphasis>"errors"</Emphasis>" were emitted while "<Emphasis>"running checks"</Emphasis>"."
+                }
+                .to_owned(),
+            ),
+        })
+    }
+
+    /// Emitted when warnings were emitted while running `check` command
+    pub fn check_warnings(category: &'static Category) -> Self {
+        Self::CheckError(CheckError {
+            category,
+            message: MessageAndDescription::from(
+                markup! {
+                    "Some "<Emphasis>"warnings"</Emphasis>" were emitted while "<Emphasis>"running checks"</Emphasis>"."
+                }
+                .to_owned(),
+            ),
         })
     }
 
     /// Emitted when errors were emitted while apply code fixes
     pub fn apply_error(category: &'static Category) -> Self {
         Self::CheckError(CheckError {
-            action_kind: CheckActionKind::Apply,
             category,
+            message: MessageAndDescription::from(
+                markup! {
+                    "Some "<Emphasis>"errors"</Emphasis>" were emitted while "<Emphasis>"applying fixes"</Emphasis>"."
+                }
+                .to_owned(),
+            ),
+        })
+    }
+    /// Emitted when warnings were emitted while apply code fixes
+    pub fn apply_warnings(category: &'static Category) -> Self {
+        Self::CheckError(CheckError {
+            category,
+            message: MessageAndDescription::from(
+                markup! {
+                    "Some "<Emphasis>"warnings"</Emphasis>" were emitted while "<Emphasis>"running checks"</Emphasis>"."
+                }
+                .to_owned(),
+            ),
         })
     }
 

crates/rome_cli/src/execute/mod.rs

@@ -7,7 +7,7 @@ mod traverse;
 use crate::cli_options::CliOptions;
 use crate::execute::traverse::traverse;
 use crate::{CliDiagnostic, CliSession};
-use rome_diagnostics::MAXIMUM_DISPLAYABLE_DIAGNOSTICS;
+use rome_diagnostics::{category, Category, MAXIMUM_DISPLAYABLE_DIAGNOSTICS};
 use rome_fs::RomePath;
 use rome_service::workspace::{FeatureName, FixFileMode};
 use std::ffi::OsString;
@@ -145,6 +145,16 @@ impl Execution {
         }
     }
 
+    pub(crate) fn as_diagnostic_category(&self) -> &'static Category {
+        match self.traversal_mode {
+            TraversalMode::Check { .. } => category!("check"),
+            TraversalMode::Lint { .. } => category!("lint"),
+            TraversalMode::CI => category!("ci"),
+            TraversalMode::Format { .. } => category!("format"),
+            TraversalMode::Migrate { .. } => category!("migrate"),
+        }
+    }
+
     pub(crate) const fn is_ci(&self) -> bool {
         matches!(self.traversal_mode, TraversalMode::CI { .. })
     }
@@ -210,28 +220,19 @@ pub(crate) fn execute_mode(
     cli_options: &CliOptions,
     paths: Vec<OsString>,
 ) -> Result<(), CliDiagnostic> {
-    mode.max_diagnostics = if let Some(max_diagnostics) = cli_options.max_diagnostics {
-        if max_diagnostics > MAXIMUM_DISPLAYABLE_DIAGNOSTICS {
-            return Err(CliDiagnostic::overflown_argument(
-                "--max-diagnostics",
-                MAXIMUM_DISPLAYABLE_DIAGNOSTICS,
-            ));
-        }
+    if cli_options.max_diagnostics > MAXIMUM_DISPLAYABLE_DIAGNOSTICS {
+        return Err(CliDiagnostic::overflown_argument(
+            "--max-diagnostics",
+            MAXIMUM_DISPLAYABLE_DIAGNOSTICS,
+        ));
+    }
 
-        max_diagnostics
-    } else {
-        // The commands `rome check` and `rome lint` give a default value of 20.
-        // In case of other commands that pass here, we limit to 50 to avoid to delay the terminal.
-        match &mode.traversal_mode {
-            TraversalMode::Check { .. } | TraversalMode::Lint { .. } => 20,
-            TraversalMode::CI | TraversalMode::Format { .. } | TraversalMode::Migrate { .. } => 50,
-        }
-    };
+    mode.max_diagnostics = cli_options.max_diagnostics;
 
     // don't do any traversal if there's some content coming from stdin
     if let Some((path, content)) = mode.as_stdin_file() {
         let rome_path = RomePath::new(path);
-        std_in::run(session, &mode, rome_path, content.as_str(), cli_options)
+        std_in::run(session, &mode, rome_path, content.as_str())
     } else if let TraversalMode::Migrate {
         write,
         configuration_path,

crates/rome_cli/src/execute/std_in.rs

@@ -1,13 +1,11 @@
 //! In here, there are the operations that run via standard input
 //!
+use crate::execute::diagnostics::{ContentDiffAdvice, FormatDiffDiagnostic};
 use crate::execute::Execution;
 use crate::{CliDiagnostic, CliSession};
 use rome_console::{markup, ConsoleExt};
+use rome_diagnostics::PrintDiagnostic;
 use rome_fs::RomePath;
-
-use crate::cli_options::CliOptions;
-use crate::execute::diagnostics::{ContentDiffAdvice, FormatDiffDiagnostic};
-use rome_diagnostics::{PrintDiagnostic, MAXIMUM_DISPLAYABLE_DIAGNOSTICS};
 use rome_service::workspace::{
     ChangeFileParams, FeatureName, FeaturesBuilder, FixFileParams, FormatFileParams, Language,
     OpenFileParams, OrganizeImportsParams, PullDiagnosticsParams, RuleCategories,
@@ -20,7 +18,6 @@ pub(crate) fn run<'a>(
     mode: &'a Execution,
     rome_path: RomePath,
     content: &'a str,
-    cli_options: &CliOptions,
 ) -> Result<(), CliDiagnostic> {
     let workspace = &*session.app.workspace;
     let console = &mut *session.app.console;
@@ -109,10 +106,7 @@ pub(crate) fn run<'a>(
             let result = workspace.pull_diagnostics(PullDiagnosticsParams {
                 categories: RuleCategories::LINT | RuleCategories::SYNTAX,
                 path: rome_path.clone(),
-                max_diagnostics: cli_options
-                    .max_diagnostics
-                    .unwrap_or(MAXIMUM_DISPLAYABLE_DIAGNOSTICS)
-                    as u64,
+                max_diagnostics: mode.max_diagnostics.into(),
             })?;
             diagnostics.extend(result.diagnostics);
         }

crates/rome_cli/src/execute/traverse.rs

@@ -85,6 +85,7 @@ pub(crate) fn traverse(
     let remaining_diagnostics = AtomicU16::new(max_diagnostics);
 
     let mut errors: usize = 0;
+    let mut warnings: usize = 0;
     let mut report = Report::default();
 
     let duration = thread::scope(|s| {
@@ -102,6 +103,7 @@ pub(crate) fn traverse(
                     errors: &mut errors,
                     report: &mut report,
                     verbose: cli_options.verbose,
+                    warnings: &mut warnings,
                 });
             })
             .expect("failed to spawn console thread");
@@ -201,16 +203,19 @@ pub(crate) fn traverse(
         });
     }
 
+    let should_exit_on_warnings = warnings > 0 && cli_options.error_on_warnings;
     // Processing emitted error diagnostics, exit with a non-zero code
     if count.saturating_sub(skipped) == 0 && !cli_options.no_errors_on_unmatched {
         Err(CliDiagnostic::no_files_processed())
-    } else if errors > 0 {
-        let category = if execution.is_ci() {
-            category!("ci")
-        } else {
-            category!("check")
-        };
-        if execution.is_check_apply() {
+    } else if errors > 0 || should_exit_on_warnings {
+        let category = execution.as_diagnostic_category();
+        if should_exit_on_warnings {
+            if execution.is_check_apply() {
+                Err(CliDiagnostic::apply_warnings(category))
+            } else {
+                Err(CliDiagnostic::check_warnings(category))
+            }
+        } else if execution.is_check_apply() {
             Err(CliDiagnostic::apply_error(category))
         } else {
             Err(CliDiagnostic::check_error(category))
@@ -266,6 +271,9 @@ struct ProcessMessagesOptions<'ctx> {
     /// Mutable reference to a boolean flag tracking whether the console thread
     /// printed any error-level message
     errors: &'ctx mut usize,
+    /// Mutable reference to a boolean flag tracking whether the console thread
+    /// printed any warnings-level message
+    warnings: &'ctx mut usize,
     /// Mutable handle to a [Report] instance the console thread should write
     /// stats into
     report: &'ctx mut Report,
@@ -287,6 +295,7 @@ fn process_messages(options: ProcessMessagesOptions) {
         errors,
         report,
         verbose,
+        warnings,
     } = options;
 
     let mut paths: HashSet<String> = HashSet::new();
@@ -345,6 +354,9 @@ fn process_messages(options: ProcessMessagesOptions) {
 
             Message::Error(mut err) => {
                 let location = err.location();
+                if err.severity() == Severity::Warning {
+                    *warnings += 1;
+                }
                 if let Some(Resource::File(file_path)) = location.resource.as_ref() {
                     // Retrieves the file name from the file ID cache, if it's a miss
                     // flush entries from the interner channel until it's found
@@ -432,6 +444,9 @@ fn process_messages(options: ProcessMessagesOptions) {
                         if severity == Severity::Error {
                             *errors += 1;
                         }
+                        if severity == Severity::Warning {
+                            *warnings += 1;
+                        }
 
                         let should_print = printed_diagnostics < max_diagnostics;
                         if should_print {