fix(rome_cli): exit with error code while applying fixes

crates/rome_cli/src/diagnostics.rs

@@ -1,4 +1,4 @@
-use rome_console::fmt::Formatter;
+use rome_console::fmt::{Display, Formatter};
 use rome_console::markup;
 use rome_diagnostics::adapters::{IoError, PicoArgsError};
 use rome_diagnostics::{
@@ -147,9 +147,49 @@ pub struct IncompatibleArguments {
 #[diagnostic(
     category = "internalError/io",
     severity = Error,
-    message = "Some errors were emitted while running checks"
+    message(
+        description = "{action_kind}",
+        message({{self.action_kind}})
+    )
 )]
-pub struct CheckError;
+pub struct CheckError {
+    action_kind: CheckActionKind,
+}
+
+#[derive(Debug, Clone, Copy)]
+pub enum CheckActionKind {
+    Check,
+    Apply,
+}
+
+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! {
+                <Emphasis>"Fixes applied"</Emphasis>", but there are still diagnostics to be addressed"
+            }),
+        }
+    }
+}
+
+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,
+                    "Fixes applied but there are still diagnostics to be addressed"
+                )
+            }
+        }
+    }
+}
 
 #[derive(Debug, Diagnostic)]
 #[diagnostic(
@@ -300,9 +340,18 @@ impl CliDiagnostic {
         })
     }
 
-    /// Emitted when errors were emitted while traversing the file system
+    /// Emitted when errors were emitted while running `check` command
     pub fn check_error() -> Self {
-        Self::CheckError(CheckError)
+        Self::CheckError(CheckError {
+            action_kind: CheckActionKind::Check,
+        })
+    }
+
+    /// Emitted when errors were emitted while apply code fixes
+    pub fn apply_error() -> Self {
+        Self::CheckError(CheckError {
+            action_kind: CheckActionKind::Apply,
+        })
     }
 
     /// Emitted when the server is not running

crates/rome_cli/src/traversal.rs

@@ -811,7 +811,13 @@ fn process_file(ctx: &TraversalOptions, path: &Path) -> FileResult {
                     .with_file_path(path.display().to_string())?;
             }
 
-            return Ok(FileStatus::Success);
+            return if fixed.errors == 0 {
+                Ok(FileStatus::Success)
+            } else {
+                Ok(FileStatus::Message(Message::Error(
+                    CliDiagnostic::apply_error().into(),
+                )))
+            };
         }
 
         let categories = if ctx.execution.is_format() || supported_lint.reason.is_some() {

crates/rome_cli/tests/snapshots/main_commands_check/apply_noop.snap

@@ -12,6 +12,14 @@ if(a != 0) {}
 
 # Emitted Messages
 
+```block
+internalError/io ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  × Fixes applied, but there are still diagnostics to be addressed
+  
+
+```
+
 ```block
 Skipped 1 suggested fixes.
 If you wish to apply the suggested (unsafe) fixes, use the command rome check --apply-unsafe

crates/rome_cli/tests/snapshots/main_commands_check/apply_ok.snap

@@ -12,6 +12,14 @@ if(a != 0) {}
 
 # Emitted Messages
 
+```block
+internalError/io ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  × Fixes applied, but there are still diagnostics to be addressed
+  
+
+```
+
 ```block
 Skipped 1 suggested fixes.
 If you wish to apply the suggested (unsafe) fixes, use the command rome check --apply-unsafe

crates/rome_cli/tests/snapshots/main_commands_check/should_disable_a_rule_group.snap

@@ -30,6 +30,14 @@ try {
 
 # Emitted Messages
 
+```block
+internalError/io ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  × Fixes applied, but there are still diagnostics to be addressed
+  
+
+```
+
 ```block
 Fixed 1 file(s) in <TIME>
 ```

crates/rome_service/src/file_handlers/javascript.rs

@@ -3,7 +3,7 @@ use super::{
     LintResults, Mime, ParserCapabilities,
 };
 use crate::configuration::to_analyzer_configuration;
-use crate::file_handlers::{FixAllParams, Language as LanguageId};
+use crate::file_handlers::{is_diagnostic_error, FixAllParams, Language as LanguageId};
 use crate::{
     settings::{FormatSettings, Language, LanguageSettings, LanguagesSettings, SettingsHandle},
     workspace::{
@@ -391,20 +391,32 @@ fn fix_all(params: FixAllParams) -> Result<FixFileResult, WorkspaceError> {
     };
 
     let mut skipped_suggested_fixes = 0;
+    let mut errors = 0;
     let analyzer_options = compute_analyzer_options(&settings);
     loop {
         let (action, _) = analyze(&tree, filter, &analyzer_options, |signal| {
+            let current_diagnostic = signal.diagnostic();
+
+            // all rules have at least an action, which is he suppression
+            if let Some(diagnostic) = current_diagnostic.as_ref() {
+                if is_diagnostic_error(diagnostic, params.rules) {
+                    errors += 1;
+                }
+            }
+
             for action in signal.actions() {
                 // suppression actions should not be part of the fixes (safe or suggested)
                 if action.is_suppression() {
                     continue;
                 }
+
                 match fix_file_mode {
                     FixFileMode::SafeFixes => {
                         if action.applicability == Applicability::MaybeIncorrect {
                             skipped_suggested_fixes += 1;
                         }
                         if action.applicability == Applicability::Always {
+                            errors -= 1;
                             return ControlFlow::Break(action);
                         }
                     }
@@ -413,6 +425,7 @@ fn fix_all(params: FixAllParams) -> Result<FixFileResult, WorkspaceError> {
                             action.applicability,
                             Applicability::Always | Applicability::MaybeIncorrect
                         ) {
+                            errors -= 1;
                             return ControlFlow::Break(action);
                         }
                     }
@@ -450,6 +463,7 @@ fn fix_all(params: FixAllParams) -> Result<FixFileResult, WorkspaceError> {
                     code: tree.syntax().to_string(),
                     skipped_suggested_fixes,
                     actions,
+                    errors,
                 });
             }
         }

crates/rome_service/src/file_handlers/mod.rs

@@ -6,9 +6,10 @@ use crate::{
     Rules, WorkspaceError,
 };
 pub use javascript::JsFormatterSettings;
-use rome_analyze::AnalysisFilter;
+use rome_analyze::{AnalysisFilter, AnalyzerDiagnostic};
 use rome_console::fmt::Formatter;
 use rome_console::markup;
+use rome_diagnostics::{Diagnostic, Severity};
 use rome_formatter::Printed;
 use rome_fs::RomePath;
 use rome_js_syntax::{TextRange, TextSize};
@@ -292,3 +293,23 @@ impl Features {
         }
     }
 }
+
+/// Checks whether a diagnostic coming from the analyzer is an [error](Severity::Error)
+///
+/// The function checks the diagnostic against the current configured rules.
+pub(crate) fn is_diagnostic_error(
+    diagnostic: &'_ AnalyzerDiagnostic,
+    rules: Option<&'_ Rules>,
+) -> bool {
+    let severity = diagnostic
+        .category()
+        .filter(|category| category.name().starts_with("lint/"))
+        .map(|category| {
+            rules
+                .and_then(|rules| rules.get_severity_from_code(category))
+                .unwrap_or(Severity::Warning)
+        })
+        .unwrap_or_else(|| diagnostic.severity());
+
+    severity >= Severity::Error
+}

crates/rome_service/src/workspace.rs

@@ -254,6 +254,9 @@ pub struct FixFileResult {
     /// List of all the code actions applied to the file
     pub actions: Vec<FixAction>,
 
+    /// List of errors
+    pub errors: usize,
+
     /// number of skipped suggested fixes
     pub skipped_suggested_fixes: u32,
 }