From 1bd44caa3f430c3514352e58667cf0f38986e51a Mon Sep 17 00:00:00 2001 From: ematipico Date: Wed, 7 Sep 2022 13:45:30 +0100 Subject: [PATCH] fix(rome_cli): incorrectly compute configuration --- crates/rome_cli/src/commands/check.rs | 15 +-- crates/rome_cli/src/commands/ci.rs | 14 +-- crates/rome_cli/src/commands/format.rs | 98 ++++++++++-------- crates/rome_cli/tests/main.rs | 99 +++++++++++++++++++ .../applies_custom_configuration.snap | 19 ++++ .../applies_custom_quote_style.snap | 17 ++++ .../src/configuration/formatter.rs | 2 +- crates/rome_service/src/configuration/mod.rs | 7 +- 8 files changed, 205 insertions(+), 66 deletions(-) create mode 100644 crates/rome_cli/tests/snapshots/main_format/applies_custom_configuration.snap create mode 100644 crates/rome_cli/tests/snapshots/main_format/applies_custom_quote_style.snap diff --git a/crates/rome_cli/src/commands/check.rs b/crates/rome_cli/src/commands/check.rs index 3043292c76b1..307cd524ccc4 100644 --- a/crates/rome_cli/src/commands/check.rs +++ b/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 = 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"); diff --git a/crates/rome_cli/src/commands/ci.rs b/crates/rome_cli/src/commands/ci.rs index 8f3ca15cc61f..29a1589f0221 100644 --- a/crates/rome_cli/src/commands/ci.rs +++ b/crates/rome_cli/src/commands/ci.rs @@ -1,6 +1,5 @@ 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; @@ -8,15 +7,12 @@ 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) } diff --git a/crates/rome_cli/src/commands/format.rs b/crates/rome_cli/src/commands/format.rs index c548babda566..83f2fbc77ca4 100644 --- a/crates/rome_cli/src/commands/format.rs +++ b/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,20 @@ 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, +) -> Result { + let mut configuration = if let Some(configuration) = configuration { + configuration + } else { + let mut configuration = Configuration::default(); + configuration.formatter = Some(FormatterConfiguration::default()); + configuration.javascript = Some(JavascriptConfiguration { + formatter: Some(JavascriptFormatter::default()), + globals: None, + }); + configuration + }; + let size = session .args .opt_value_from_str("--indent-size") @@ -85,27 +96,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 +129,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) } diff --git a/crates/rome_cli/tests/main.rs b/crates/rome_cli/tests/main.rs index b4da331714b9..2e4c466787c0 100644 --- a/crates/rome_cli/tests/main.rs +++ b/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::{ @@ -758,6 +779,84 @@ 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_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(); diff --git a/crates/rome_cli/tests/snapshots/main_format/applies_custom_configuration.snap b/crates/rome_cli/tests/snapshots/main_format/applies_custom_configuration.snap new file mode 100644 index 000000000000..f88f56d0fd46 --- /dev/null +++ b/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 + + diff --git a/crates/rome_cli/tests/snapshots/main_format/applies_custom_quote_style.snap b/crates/rome_cli/tests/snapshots/main_format/applies_custom_quote_style.snap new file mode 100644 index 000000000000..d95c9e6477d0 --- /dev/null +++ b/crates/rome_cli/tests/snapshots/main_format/applies_custom_quote_style.snap @@ -0,0 +1,17 @@ +--- +source: crates/rome_cli/tests/snap_test.rs +expression: content +--- +## `file.js` + +```js +let a = 'something'; +let b = { + 'hey': 'hello', +}; + +``` + +# Emitted Messages + + diff --git a/crates/rome_service/src/configuration/formatter.rs b/crates/rome_service/src/configuration/formatter.rs index 4f04d27dc591..181fd6ca69d3 100644 --- a/crates/rome_service/src/configuration/formatter.rs +++ b/crates/rome_service/src/configuration/formatter.rs @@ -17,7 +17,7 @@ pub struct FormatterConfiguration { pub indent_style: PlainIndentStyle, /// The size of the indentation, 2 by default - indent_size: u8, + pub indent_size: u8, /// What's the max width of a line. Defaults to 80. #[serde( diff --git a/crates/rome_service/src/configuration/mod.rs b/crates/rome_service/src/configuration/mod.rs index 57bc7eb416bd..9035b7345a4e 100644 --- a/crates/rome_service/src/configuration/mod.rs +++ b/crates/rome_service/src/configuration/mod.rs @@ -3,9 +3,6 @@ //! The configuration is divided by "tool", and then it's possible to further customise it //! by language. The language might further options divided by tool. -use crate::configuration::formatter::FormatterConfiguration; -use crate::configuration::javascript::JavascriptConfiguration; -use crate::configuration::linter::LinterConfiguration; use crate::{DynRef, RomeError}; use rome_fs::{FileSystem, OpenOptions}; use serde::{Deserialize, Serialize}; @@ -18,7 +15,9 @@ mod formatter; mod javascript; pub mod linter; -pub use linter::{RuleConfiguration, Rules}; +pub use formatter::{FormatterConfiguration, PlainIndentStyle}; +pub use javascript::{JavascriptConfiguration, JavascriptFormatter}; +pub use linter::{LinterConfiguration, RuleConfiguration, Rules}; /// The configuration that is contained inside the file `rome.json` #[derive(Debug, Deserialize, Serialize)]