Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Commit

Permalink
fix(rome_cli): incorrectly compute configuration
Browse files Browse the repository at this point in the history
  • Loading branch information
ematipico committed Sep 7, 2022
1 parent 96ce01c commit 1bd44ca
Show file tree
Hide file tree
Showing 8 changed files with 205 additions and 66 deletions.
15 changes: 5 additions & 10 deletions crates/rome_cli/src/commands/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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");
Expand Down
14 changes: 5 additions & 9 deletions crates/rome_cli/src/commands/ci.rs
Original file line number Diff line number Diff line change
@@ -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)
}
98 changes: 56 additions & 42 deletions crates/rome_cli/src/commands/format.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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");
Expand Down Expand Up @@ -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<Configuration>,
) -> Result<Configuration, Termination> {
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")
Expand All @@ -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
Expand All @@ -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)
}
99 changes: 99 additions & 0 deletions crates/rome_cli/tests/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
source: crates/rome_cli/tests/snap_test.rs
expression: content
---
## `file.js`

```js
function f() {
return {
a,
b,
};
}

```

# Emitted Messages


Original file line number Diff line number Diff line change
@@ -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


2 changes: 1 addition & 1 deletion crates/rome_service/src/configuration/formatter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
7 changes: 3 additions & 4 deletions crates/rome_service/src/configuration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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)]
Expand Down

0 comments on commit 1bd44ca

Please sign in to comment.