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

Commit

Permalink
feat(rome_cli): Add a new option --config-path to set the path to rom…
Browse files Browse the repository at this point in the history
…e.json (#4158)

* feat(rome_cli): Add a new option --config to set the path to rome.json

* rename option config -> config-path

* indentation

* remove the website documentation for the config-path option

* rename config_path -> configuration_path

* refactor the base path option

* debug artifacts

* update comment

* formatting
  • Loading branch information
realtimetodie authored Feb 13, 2023
1 parent eae1259 commit bb3cbc1
Show file tree
Hide file tree
Showing 9 changed files with 207 additions and 22 deletions.
3 changes: 3 additions & 0 deletions crates/rome_cli/src/commands/help.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const CHECK: Markup = markup! {
"<Dim>"--apply"</Dim>" Apply safe fixes
"<Dim>"--apply-unsafe"</Dim>" Apply safe and unsafe fixes
"<Dim>"--max-diagnostics"</Dim>" Cap the amount of diagnostics displayed (default: 20)
"<Dim>"--config-path"</Dim>" Set the filesystem path to the directory of the rome.json configuration file
"<Dim>"--verbose"</Dim>" Print additional verbose advices on diagnostics
"
};
Expand Down Expand Up @@ -65,6 +66,7 @@ const CI: Markup = markup! {
"<Dim>"--formatter-enabled"</Dim>" Allow to enable or disable the formatter check. (default: true)
"<Dim>"--linter-enabled"</Dim>" Allow to enable or disable the linter check. (default: true)
"<Dim>"--max-diagnostics"</Dim>" Cap the amount of diagnostics displayed (default: 50)
"<Dim>"--config-path"</Dim>" Set the filesystem path to the directory of the rome.json configuration file
"<Dim>"--verbose"</Dim>" Print additional verbose advices on diagnostics"
{FORMAT_OPTIONS}
};
Expand All @@ -81,6 +83,7 @@ const FORMAT: Markup = markup! {
"<Dim>"--write"</Dim>" Edit the files in place (beware!) instead of printing the diff to the console
"<Dim>"--skip-errors"</Dim>" Skip over files containing syntax errors instead of emitting an error diagnostic.
"<Dim>"--max-diagnostics"</Dim>" Cap the amount of diagnostics displayed (default: 50)
"<Dim>"--config-path"</Dim>" Set the filesystem path to the directory of the rome.json configuration file
"<Dim>"--verbose"</Dim>" Print additional verbose advices on diagnostics"
{FORMAT_OPTIONS}
""<Dim>"--stdin-file-path <string>"</Dim>" A file name with its extension to pass when reading from standard in, e.g. echo 'let a;' | rome format --stdin-file-path file.js
Expand Down
4 changes: 2 additions & 2 deletions crates/rome_cli/src/commands/rage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use rome_diagnostics::termcolor::{ColorChoice, WriteColor};
use rome_diagnostics::{termcolor, PrintDescription};
use rome_fs::FileSystem;
use rome_service::workspace::{client, RageEntry, RageParams};
use rome_service::{load_config, DynRef, Workspace};
use rome_service::{load_config, BasePath, DynRef, Workspace};
use std::{env, io, ops::Deref};
use tokio::runtime::Runtime;

Expand Down Expand Up @@ -163,7 +163,7 @@ impl Display for RageConfiguration<'_, '_> {
fn fmt(&self, fmt: &mut Formatter) -> io::Result<()> {
Section("Rome Configuration").fmt(fmt)?;

match load_config(self.0, None) {
match load_config(self.0, BasePath::default()) {
Ok(None) => KeyValuePair("Status", markup!(<Dim>"unset"</Dim>)).fmt(fmt)?,
Ok(Some(deserialized)) => {
let (configuration, diagnostics) = deserialized.consume();
Expand Down
18 changes: 16 additions & 2 deletions crates/rome_cli/src/configuration.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,25 @@
use std::path::PathBuf;

use crate::{CliDiagnostic, CliSession};
use rome_deserialize::Deserialized;
use rome_service::{load_config, Configuration};
use rome_service::{load_config, BasePath, Configuration};

/// Load the configuration for this session of the CLI, merging the content of
/// the `rome.json` file if it exists on disk with common command line options
pub(crate) fn load_configuration(
session: &mut CliSession,
) -> Result<Deserialized<Configuration>, CliDiagnostic> {
Ok(load_config(&session.app.fs, None)?.unwrap_or_default())
let config_path: Option<PathBuf> = session
.args
.opt_value_from_str("--config-path")
.map_err(|source| CliDiagnostic::parse_error("--config-path", source))?;

let base_path = match config_path {
None => BasePath::default(),
Some(path) => BasePath::FromUser(path),
};

let config = load_config(&session.app.fs, base_path)?;

Ok(config.unwrap_or_default())
}
90 changes: 90 additions & 0 deletions crates/rome_cli/tests/commands/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,15 @@ const APPLY_TRAILING_COMMA_AFTER: &str = r#"const a = [
];
"#;

const DEFAULT_CONFIGURATION_BEFORE: &str = r#"function f() {
return { a, b }
}"#;

const DEFAULT_CONFIGURATION_AFTER: &str = "function f() {
return { a, b };
}
";

const CUSTOM_CONFIGURATION_BEFORE: &str = r#"function f() {
return { a, b }
}"#;
Expand Down Expand Up @@ -236,6 +245,87 @@ fn lint_warning() {
));
}

#[test]
fn custom_config_file_path() {
let mut fs = MemoryFileSystem::default();
let mut console = BufferConsole::default();

let config_path = Path::new("/test/rome.json");
fs.insert(config_path.into(), CONFIG_FORMAT.as_bytes());

let file_path = Path::new("file.js");
fs.insert(file_path.into(), DEFAULT_CONFIGURATION_BEFORE.as_bytes());

let mut config_path = PathBuf::from(config_path);
config_path.pop();

let result = run_cli(
DynRef::Borrowed(&mut fs),
&mut console,
Arguments::from_vec(vec![
OsString::from("format"),
OsString::from("--config-path"),
OsString::from(config_path),
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, DEFAULT_CONFIGURATION_AFTER);

drop(file);
assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
"custom_config_file_path",
fs,
console,
result,
));
}

// Should throw an error when an invalid configuration path is specified
#[test]
fn invalid_config_file_path() {
let mut fs = MemoryFileSystem::default();
let mut console = BufferConsole::default();

let config_path = Path::new("/test");
let file_path = Path::new("file.js");
fs.insert(file_path.into(), *b"content");

let result = run_cli(
DynRef::Borrowed(&mut fs),
&mut console,
Arguments::from_vec(vec![
OsString::from("format"),
OsString::from("--config-path"),
OsString::from(config_path),
OsString::from("--write"),
file_path.as_os_str().into(),
]),
);

assert!(result.is_err(), "run_cli returned {result:?}");

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
"invalid_config_file_path",
fs,
console,
result,
));
}

#[test]
fn applies_custom_configuration() {
let mut fs = MemoryFileSystem::default();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
---
source: crates/rome_cli/tests/snap_test.rs
assertion_line: 335
expression: content
---
## `/test/rome.json`

```json
{
"formatter": {
"enabled": true,
"lineWidth": 160,
"indentStyle": "space",
"indentSize": 6
}
}

```

## `file.js`

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

```

# Emitted Messages

```block
Formatted 1 file(s) in <TIME>
```


Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
---
source: crates/rome_cli/tests/snap_test.rs
assertion_line: 335
expression: content
---
## `file.js`

```js
content
```

# Termination Message

```block
/test/rome.json internalError/fs ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× Rome couldn't read the following file, maybe for permissions reasons or it doesn't exists: /test/rome.json
```


8 changes: 6 additions & 2 deletions crates/rome_lsp/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use rome_console::markup;
use rome_fs::{FileSystem, OsFileSystem, RomePath};
use rome_service::workspace::{FeatureName, PullDiagnosticsParams, SupportsFeatureParams};
use rome_service::workspace::{RageEntry, RageParams, RageResult, UpdateSettingsParams};
use rome_service::{load_config, Workspace};
use rome_service::{load_config, BasePath, Workspace};
use rome_service::{DynRef, WorkspaceError};
use serde_json::Value;
use std::collections::HashMap;
Expand Down Expand Up @@ -364,14 +364,18 @@ impl Session {
/// the root URI and update the workspace settings accordingly
#[tracing::instrument(level = "debug", skip(self))]
pub(crate) async fn load_workspace_settings(&self) {
let base_path = self.base_path();
let base_path = match self.base_path() {
None => BasePath::default(),
Some(path) => BasePath::Lsp(path),
};

let status = match load_config(&self.fs, base_path) {
Ok(Some(deserialized)) => {
let (configuration, diagnostics) = deserialized.consume();
if diagnostics.is_empty() {
warn!("The deserialization of the configuration resulted in errors. Rome will its defaults where possible.");
}

info!("Loaded workspace settings: {configuration:#?}");

let result = self
Expand Down
45 changes: 30 additions & 15 deletions crates/rome_service/src/configuration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,18 +117,30 @@ impl FilesConfiguration {

type LoadConfig = Result<Option<Deserialized<Configuration>>, WorkspaceError>;

/// This function is responsible to load the rome configuration.
#[derive(Default, PartialEq)]
pub enum BasePath {
/// The default mode, not having a configuration file is not an error.
#[default]
None,
/// The base path provided by the LSP, not having a configuration file is not an error.
Lsp(PathBuf),
/// The base path provided by the user, not having a configuration file is an error.
/// Throws any kind of I/O errors.
FromUser(PathBuf),
}

/// Load the configuration from the file system.
///
/// The `file_system` will read the configuration file. A base path can be passed
pub fn load_config(file_system: &DynRef<dyn FileSystem>, base_path: Option<PathBuf>) -> LoadConfig {
/// The configuration file will be read from the `file_system`.
/// An optional base path can be appended to the configuration file path.
pub fn load_config(file_system: &DynRef<dyn FileSystem>, base_path: BasePath) -> LoadConfig {
let config_name = file_system.config_name();
let configuration_path = if let Some(base_path) = base_path {
base_path.join(config_name)
} else {
PathBuf::from(config_name)
let configuration_path = match base_path {
BasePath::Lsp(ref path) | BasePath::FromUser(ref path) => path.join(config_name),
_ => PathBuf::from(config_name),
};
info!(
"Attempting to load the configuration file at path {:?}",
"Attempting to read the configuration file from {:?}",
configuration_path
);
let options = OpenOptions::default().read(true);
Expand All @@ -145,20 +157,23 @@ pub fn load_config(file_system: &DynRef<dyn FileSystem>, base_path: Option<PathB
Ok(Some(deserialized))
}
Err(err) => {
// We throw an error only when the error is found.
// In case we don't fine the file, we swallow the error and we continue; not having
// a file should not be a cause of error (for now)
if err.kind() != ErrorKind::NotFound {
// We skip the error when the configuration file is not found.
// Not having a configuration file is only an error when the `base_path` is
// set to `BasePath::FromUser`.
if match base_path {
BasePath::FromUser(_) => true,
_ => err.kind() != ErrorKind::NotFound,
} {
return Err(WorkspaceError::cant_read_file(format!(
"{}",
configuration_path.display()
)));
}
error!(
"Could not find the file configuration at {:?}",
configuration_path.display()
"Could not read the configuration file from {:?}, reason:\n {}",
configuration_path.display(),
err
);
error!("Reason: {:?}", err);
Ok(None)
}
}
Expand Down
3 changes: 2 additions & 1 deletion crates/rome_service/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ mod diagnostics;
pub mod workspace_types;

pub use crate::configuration::{
create_config, load_config, Configuration, ConfigurationDiagnostic, RuleConfiguration, Rules,
create_config, load_config, BasePath, Configuration, ConfigurationDiagnostic,
RuleConfiguration, Rules,
};
pub use crate::matcher::{MatchOptions, Matcher, Pattern};

Expand Down

0 comments on commit bb3cbc1

Please sign in to comment.