fix(rome_cli): correctly compute configuration (#3176)

crates/rome_cli/src/commands/check.rs

@@ -2,14 +2,11 @@ use crate::commands::format::apply_format_settings_from_cli;
 use crate::{execute_mode, CliSession, Execution, Termination, TraversalMode};
 use rome_diagnostics::MAXIMUM_DISPLAYABLE_DIAGNOSTICS;
 use rome_service::load_config;
-use rome_service::settings::WorkspaceSettings;
 use rome_service::workspace::{FixFileMode, UpdateSettingsParams};
 
 /// Handler for the "check" command of the Rome CLI
 pub(crate) fn check(mut session: CliSession) -> Result<(), Termination> {
     let configuration = load_config(&session.app.fs, None)?;
-    let mut workspace_settings = WorkspaceSettings::default();
-
     let max_diagnostics: Option<u16> = session
         .args
         .opt_value_from_str("--max-diagnostics")
@@ -31,13 +28,11 @@ pub(crate) fn check(mut session: CliSession) -> Result<(), Termination> {
         20
     };
 
-    if let Some(configuration) = configuration {
-        session
-            .app
-            .workspace
-            .update_settings(UpdateSettingsParams { configuration })?;
-    }
-    apply_format_settings_from_cli(&mut session, &mut workspace_settings)?;
+    let configuration = apply_format_settings_from_cli(&mut session, configuration)?;
+    session
+        .app
+        .workspace
+        .update_settings(UpdateSettingsParams { configuration })?;
 
     let apply = session.args.contains("--apply");
     let apply_suggested = session.args.contains("--apply-suggested");

crates/rome_cli/src/commands/ci.rs

@@ -1,22 +1,18 @@
 use crate::{execute_mode, CliSession, Execution, Termination, TraversalMode};
 use rome_service::load_config;
-use rome_service::settings::WorkspaceSettings;
 use rome_service::workspace::UpdateSettingsParams;
 
 use super::format::apply_format_settings_from_cli;
 
 /// Handler for the "ci" command of the Rome CLI
 pub(crate) fn ci(mut session: CliSession) -> Result<(), Termination> {
     let configuration = load_config(&session.app.fs, None)?;
-    let mut workspace_settings = WorkspaceSettings::default();
+    let configuration = apply_format_settings_from_cli(&mut session, configuration)?;
 
-    if let Some(configuration) = configuration {
-        session
-            .app
-            .workspace
-            .update_settings(UpdateSettingsParams { configuration })?;
-    }
-    apply_format_settings_from_cli(&mut session, &mut workspace_settings)?;
+    session
+        .app
+        .workspace
+        .update_settings(UpdateSettingsParams { configuration })?;
 
     execute_mode(Execution::new(TraversalMode::CI), session)
 }

crates/rome_cli/src/commands/format.rs

@@ -1,5 +1,8 @@
 use rome_formatter::IndentStyle;
-use rome_service::{load_config, settings::WorkspaceSettings, workspace::UpdateSettingsParams};
+use rome_service::configuration::{
+    FormatterConfiguration, JavascriptConfiguration, JavascriptFormatter, PlainIndentStyle,
+};
+use rome_service::{load_config, workspace::UpdateSettingsParams, Configuration};
 use std::path::PathBuf;
 
 use crate::execute::ReportMode;
@@ -8,16 +11,12 @@ use crate::{execute_mode, CliSession, Execution, Termination, TraversalMode};
 /// Handler for the "format" command of the Rome CLI
 pub(crate) fn format(mut session: CliSession) -> Result<(), Termination> {
     let configuration = load_config(&session.app.fs, None)?;
-    let mut workspace_settings = WorkspaceSettings::default();
+    let configuration = apply_format_settings_from_cli(&mut session, configuration)?;
 
-    if let Some(configuration) = configuration {
-        session
-            .app
-            .workspace
-            .update_settings(UpdateSettingsParams { configuration })?;
-    }
-
-    apply_format_settings_from_cli(&mut session, &mut workspace_settings)?;
+    session
+        .app
+        .workspace
+        .update_settings(UpdateSettingsParams { configuration })?;
 
     let is_write = session.args.contains("--write");
     let ignore_errors = session.args.contains("--skip-errors");
@@ -67,8 +66,21 @@ pub(crate) fn format(mut session: CliSession) -> Result<(), Termination> {
 /// into the workspace settings
 pub(crate) fn apply_format_settings_from_cli(
     session: &mut CliSession,
-    workspace_settings: &mut WorkspaceSettings,
-) -> Result<(), Termination> {
+    configuration: Option<Configuration>,
+) -> Result<Configuration, Termination> {
+    let mut configuration = if let Some(configuration) = configuration {
+        configuration
+    } else {
+        Configuration {
+            formatter: Some(FormatterConfiguration::default()),
+            javascript: Some(JavascriptConfiguration {
+                formatter: Some(JavascriptFormatter::default()),
+                globals: None,
+            }),
+            ..Configuration::default()
+        }
+    };
+
     let size = session
         .args
         .opt_value_from_str("--indent-size")
@@ -85,27 +97,29 @@ pub(crate) fn apply_format_settings_from_cli(
             source,
         })?;
 
-    match indent_style {
-        Some(IndentStyle::Tab) => {
-            workspace_settings.formatter.indent_style = Some(IndentStyle::Tab);
-        }
-        Some(IndentStyle::Space(default_size)) => {
-            workspace_settings.formatter.indent_style =
-                Some(IndentStyle::Space(size.unwrap_or(default_size)));
-        }
-        None => {}
-    }
-
-    let quote_style = session
+    let line_width = session
         .args
-        .opt_value_from_str("--quote-style")
+        .opt_value_from_str("--line-width")
         .map_err(|source| Termination::ParseError {
-            argument: "--quote-style",
+            argument: "--line-width",
             source,
         })?;
 
-    if let Some(quote_style) = quote_style {
-        workspace_settings.languages.javascript.format.quote_style = Some(quote_style);
+    if let Some(formatter) = configuration.formatter.as_mut() {
+        match indent_style {
+            Some(IndentStyle::Tab) => {
+                formatter.indent_style = PlainIndentStyle::Tab;
+            }
+            Some(IndentStyle::Space(default_size)) => {
+                formatter.indent_style = PlainIndentStyle::Space;
+                formatter.indent_size = size.unwrap_or(default_size);
+            }
+            None => {}
+        }
+
+        if let Some(line_width) = line_width {
+            formatter.line_width = line_width;
+        }
     }
 
     let quote_properties = session
@@ -116,25 +130,26 @@ pub(crate) fn apply_format_settings_from_cli(
             source,
         })?;
 
-    if let Some(quote_properties) = quote_properties {
-        workspace_settings
-            .languages
-            .javascript
-            .format
-            .quote_properties = Some(quote_properties);
-    }
-
-    let line_width = session
+    let quote_style = session
         .args
-        .opt_value_from_str("--line-width")
+        .opt_value_from_str("--quote-style")
         .map_err(|source| Termination::ParseError {
-            argument: "--line-width",
+            argument: "--quote-style",
             source,
         })?;
+    if let Some(javascript) = configuration
+        .javascript
+        .as_mut()
+        .and_then(|j| j.formatter.as_mut())
+    {
+        if let Some(quote_properties) = quote_properties {
+            javascript.quote_properties = quote_properties;
+        }
 
-    if let Some(line_width) = line_width {
-        workspace_settings.formatter.line_width = Some(line_width);
+        if let Some(quote_style) = quote_style {
+            javascript.quote_style = quote_style;
+        }
     }
 
-    Ok(())
+    Ok(configuration)
 }

crates/rome_cli/tests/main.rs

@@ -82,6 +82,27 @@ const NO_DEAD_CODE_ERROR: &str = r#"function f() {
 }
 "#;
 
+const APPLY_QUOTE_STYLE_BEFORE: &str = r#"
+let a = "something";
+let b = {
+    "hey": "hello"
+};"#;
+
+const APPLY_QUOTE_STYLE_AFTER: &str = "let a = 'something';
+let b = {\n\t'hey': 'hello',\n};\n";
+
+const CUSTOM_CONFIGURATION_BEFORE: &str = r#"function f() {
+  return { a, b }
+}"#;
+
+const CUSTOM_CONFIGURATION_AFTER: &str = "function f() {
+        return {
+                a,
+                b,
+        };
+}
+";
+
 mod check {
     use super::*;
     use crate::configs::{
@@ -825,6 +846,132 @@ mod format {
         assert_cli_snapshot(module_path!(), "formatter_lint_warning", fs, console);
     }
 
+    #[test]
+    fn applies_custom_configuration() {
+        let mut fs = MemoryFileSystem::default();
+        let mut console = BufferConsole::default();
+
+        let file_path = Path::new("file.js");
+        fs.insert(file_path.into(), CUSTOM_CONFIGURATION_BEFORE.as_bytes());
+
+        let result = run_cli(
+            DynRef::Borrowed(&mut fs),
+            DynRef::Borrowed(&mut console),
+            Arguments::from_vec(vec![
+                OsString::from("format"),
+                OsString::from("--line-width"),
+                OsString::from("10"),
+                OsString::from("--indent-style"),
+                OsString::from("space"),
+                OsString::from("--indent-size"),
+                OsString::from("8"),
+                OsString::from("--write"),
+                file_path.as_os_str().into(),
+            ]),
+        );
+
+        assert!(result.is_ok(), "run_cli returned {result:?}");
+
+        let mut file = fs
+            .open(file_path)
+            .expect("formatting target file was removed by the CLI");
+
+        let mut content = String::new();
+        file.read_to_string(&mut content)
+            .expect("failed to read file from memory FS");
+
+        assert_eq!(content, CUSTOM_CONFIGURATION_AFTER);
+
+        drop(file);
+        assert_cli_snapshot(module_path!(), "applies_custom_configuration", fs, console);
+    }
+
+    #[test]
+    fn applies_custom_configuration_over_config_file() {
+        let mut fs = MemoryFileSystem::default();
+        let mut console = BufferConsole::default();
+
+        let file_path = Path::new("rome.json");
+        fs.insert(file_path.into(), CONFIG_FORMAT.as_bytes());
+
+        let file_path = Path::new("file.js");
+        fs.insert(file_path.into(), CUSTOM_CONFIGURATION_BEFORE.as_bytes());
+
+        let result = run_cli(
+            DynRef::Borrowed(&mut fs),
+            DynRef::Borrowed(&mut console),
+            Arguments::from_vec(vec![
+                OsString::from("format"),
+                OsString::from("--line-width"),
+                OsString::from("10"),
+                OsString::from("--indent-style"),
+                OsString::from("space"),
+                OsString::from("--indent-size"),
+                OsString::from("8"),
+                OsString::from("--write"),
+                file_path.as_os_str().into(),
+            ]),
+        );
+
+        assert!(result.is_ok(), "run_cli returned {result:?}");
+
+        let mut file = fs
+            .open(file_path)
+            .expect("formatting target file was removed by the CLI");
+
+        let mut content = String::new();
+        file.read_to_string(&mut content)
+            .expect("failed to read file from memory FS");
+
+        assert_eq!(content, CUSTOM_CONFIGURATION_AFTER);
+
+        drop(file);
+        assert_cli_snapshot(
+            module_path!(),
+            "applies_custom_configuration_over_config_file",
+            fs,
+            console,
+        );
+    }
+
+    #[test]
+    fn applies_custom_quote_style() {
+        let mut fs = MemoryFileSystem::default();
+        let mut console = BufferConsole::default();
+
+        let file_path = Path::new("file.js");
+        fs.insert(file_path.into(), APPLY_QUOTE_STYLE_BEFORE.as_bytes());
+
+        let result = run_cli(
+            DynRef::Borrowed(&mut fs),
+            DynRef::Borrowed(&mut console),
+            Arguments::from_vec(vec![
+                OsString::from("format"),
+                OsString::from("--quote-style"),
+                OsString::from("single"),
+                OsString::from("--quote-properties"),
+                OsString::from("preserve"),
+                OsString::from("--write"),
+                file_path.as_os_str().into(),
+            ]),
+        );
+
+        assert!(result.is_ok(), "run_cli returned {result:?}");
+
+        let mut file = fs
+            .open(file_path)
+            .expect("formatting target file was removed by the CLI");
+
+        let mut content = String::new();
+        file.read_to_string(&mut content)
+            .expect("failed to read file from memory FS");
+
+        assert_eq!(content, APPLY_QUOTE_STYLE_AFTER);
+
+        drop(file);
+        assert_cli_snapshot(module_path!(), "applies_custom_quote_style", fs, console);
+    }
+
     #[test]
     fn indent_style_parse_errors() {
         let mut console = BufferConsole::default();

crates/rome_cli/tests/snapshots/main_format/applies_custom_configuration.snap

@@ -0,0 +1,19 @@
+---
+source: crates/rome_cli/tests/snap_test.rs
+expression: content
+---
+## `file.js`
+
+```js
+function f() {
+        return {
+                a,
+                b,
+        };
+}
+
+```
+
+# Emitted Messages
+
+