From ffac554f12385ef49c310bb055504158e3594aa6 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Tue, 21 Feb 2023 11:41:04 +0000 Subject: [PATCH 1/4] feat(rome_service): traverse upwards the fs to discover the config file --- crates/rome_cli/src/commands/rage.rs | 4 +- crates/rome_cli/src/configuration.rs | 6 +- crates/rome_lsp/src/session.rs | 6 +- crates/rome_service/src/configuration/mod.rs | 112 ++++++++++++------- crates/rome_service/src/lib.rs | 2 +- website/package.json | 2 +- 6 files changed, 84 insertions(+), 48 deletions(-) diff --git a/crates/rome_cli/src/commands/rage.rs b/crates/rome_cli/src/commands/rage.rs index f26c299c5c9..d1a18b16c18 100644 --- a/crates/rome_cli/src/commands/rage.rs +++ b/crates/rome_cli/src/commands/rage.rs @@ -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, BasePath, DynRef, Workspace}; +use rome_service::{load_config, ConfigurationBasePath, DynRef, Workspace}; use std::{env, io, ops::Deref}; use tokio::runtime::Runtime; @@ -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, BasePath::default()) { + match load_config(self.0, ConfigurationBasePath::default()) { Ok(None) => KeyValuePair("Status", markup!("unset")).fmt(fmt)?, Ok(Some(deserialized)) => { let (configuration, diagnostics) = deserialized.consume(); diff --git a/crates/rome_cli/src/configuration.rs b/crates/rome_cli/src/configuration.rs index a1162040138..f2f63f84f96 100644 --- a/crates/rome_cli/src/configuration.rs +++ b/crates/rome_cli/src/configuration.rs @@ -2,7 +2,7 @@ use std::path::PathBuf; use crate::{CliDiagnostic, CliSession}; use rome_deserialize::Deserialized; -use rome_service::{load_config, BasePath, Configuration}; +use rome_service::{load_config, Configuration, ConfigurationBasePath}; /// 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 @@ -15,8 +15,8 @@ pub(crate) fn load_configuration( .map_err(|source| CliDiagnostic::parse_error("--config-path", source))?; let base_path = match config_path { - None => BasePath::default(), - Some(path) => BasePath::FromUser(path), + None => ConfigurationBasePath::default(), + Some(path) => ConfigurationBasePath::FromUser(path), }; let config = load_config(&session.app.fs, base_path)?; diff --git a/crates/rome_lsp/src/session.rs b/crates/rome_lsp/src/session.rs index 7adbb6e00ad..8dd6ce002f0 100644 --- a/crates/rome_lsp/src/session.rs +++ b/crates/rome_lsp/src/session.rs @@ -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, BasePath, Workspace}; +use rome_service::{load_config, ConfigurationBasePath, Workspace}; use rome_service::{DynRef, WorkspaceError}; use serde_json::Value; use std::collections::HashMap; @@ -365,8 +365,8 @@ impl Session { #[tracing::instrument(level = "debug", skip(self))] pub(crate) async fn load_workspace_settings(&self) { let base_path = match self.base_path() { - None => BasePath::default(), - Some(path) => BasePath::Lsp(path), + None => ConfigurationBasePath::default(), + Some(path) => ConfigurationBasePath::Lsp(path), }; let status = match load_config(&self.fs, base_path) { diff --git a/crates/rome_service/src/configuration/mod.rs b/crates/rome_service/src/configuration/mod.rs index 11a64bc636d..980f4019959 100644 --- a/crates/rome_service/src/configuration/mod.rs +++ b/crates/rome_service/src/configuration/mod.rs @@ -115,10 +115,14 @@ impl FilesConfiguration { const KNOWN_KEYS: &'static [&'static str] = &["maxSize", "ignore"]; } +/// - [Result]: if an error occurred while loading the configuration file. +/// - [Option]: sometimes not having a configuration file should not be an error, so we need this type. +/// - [Deserialized]: result of the deserialization of the configuration. +/// - [Configuration]: the type needed to [Deserialized] to infer the return type. type LoadConfig = Result>, WorkspaceError>; #[derive(Debug, Default, PartialEq)] -pub enum BasePath { +pub enum ConfigurationBasePath { /// The default mode, not having a configuration file is not an error. #[default] None, @@ -129,53 +133,85 @@ pub enum BasePath { FromUser(PathBuf), } +impl ConfigurationBasePath { + const fn is_from_user(&self) -> bool { + matches!(self, ConfigurationBasePath::FromUser(_)) + } + + fn parent(&self) -> Option<&Path> { + match self { + ConfigurationBasePath::None => None, + ConfigurationBasePath::Lsp(path) | ConfigurationBasePath::FromUser(path) => { + path.parent() + } + } + } +} + /// Load the configuration from the file system. /// -/// 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, base_path: BasePath) -> LoadConfig { +/// The configuration file will be read from the `file_system`. A [base path](ConfigurationBasePath) should be provided. +/// +/// The function will try to traverse upwards the file system until if finds a `rome.json` file, or there +/// aren't directories anymore. +/// +/// If a the configuration base path was provided by the user, the function will error. If not, Rome will use +/// its defaults. +pub fn load_config( + file_system: &DynRef, + base_path: ConfigurationBasePath, +) -> LoadConfig { let config_name = file_system.config_name(); - let configuration_path = match base_path { - BasePath::Lsp(ref path) | BasePath::FromUser(ref path) => path.join(config_name), + let mut configuration_path = match base_path { + ConfigurationBasePath::Lsp(ref path) | ConfigurationBasePath::FromUser(ref path) => { + path.join(config_name) + } _ => PathBuf::from(config_name), }; + let should_error = base_path.is_from_user(); info!( "Attempting to read the configuration file from {:?}", configuration_path ); - let options = OpenOptions::default().read(true); - let file = file_system.open_with_options(&configuration_path, options); - match file { - Ok(mut file) => { - let mut buffer = String::new(); - file.read_to_string(&mut buffer).map_err(|_| { - WorkspaceError::cant_read_file(format!("{}", configuration_path.display())) - })?; - - let deserialized = deserialize_from_json::(&buffer) - .with_file_path(&configuration_path.display().to_string()); - Ok(Some(deserialized)) - } - Err(err) => { - // 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() - ))); + + loop { + let options = OpenOptions::default().read(true); + let file = file_system.open_with_options(&configuration_path, options); + return match file { + Ok(mut file) => { + let mut buffer = String::new(); + file.read_to_string(&mut buffer).map_err(|_| { + WorkspaceError::cant_read_file(format!("{}", configuration_path.display())) + })?; + + let deserialized = deserialize_from_json::(&buffer) + .with_file_path(&configuration_path.display().to_string()); + Ok(Some(deserialized)) } - error!( - "Could not read the configuration file from {:?}, reason:\n {}", - configuration_path.display(), - err - ); - Ok(None) - } + Err(err) => { + if let Some(path) = base_path.parent() { + if path.is_dir() { + configuration_path = path.join(config_name); + continue; + } + } + // 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 should_error || err.kind() != ErrorKind::NotFound { + return Err(WorkspaceError::cant_read_file(format!( + "{}", + configuration_path.display() + ))); + } + error!( + "Could not read the configuration file from {:?}, reason:\n {}", + configuration_path.display(), + err + ); + Ok(None) + } + }; } } diff --git a/crates/rome_service/src/lib.rs b/crates/rome_service/src/lib.rs index f448b02d094..e4b2256a80f 100644 --- a/crates/rome_service/src/lib.rs +++ b/crates/rome_service/src/lib.rs @@ -14,7 +14,7 @@ mod diagnostics; pub mod workspace_types; pub use crate::configuration::{ - create_config, load_config, BasePath, Configuration, ConfigurationDiagnostic, + create_config, load_config, Configuration, ConfigurationBasePath, ConfigurationDiagnostic, RuleConfiguration, Rules, }; pub use crate::matcher::{MatchOptions, Matcher, Pattern}; diff --git a/website/package.json b/website/package.json index 9a2475cde1f..f324b87e722 100644 --- a/website/package.json +++ b/website/package.json @@ -5,7 +5,7 @@ "start": "astro dev", "format": "cargo rome-cli-dev format --write .", "tsc": "tsc --skipLibCheck", - "check": "cd ../ && cargo rome-cli-dev check ./website/src", + "check": "cargo rome-cli-dev check ./src", "start:playground": "pnpm build:wasm-dev && pnpm start", "build": "pnpm build:wasm && pnpm build:js", "build:js": "astro build", From 26954062ded3dc13807033cd9850f8ca7e462834 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Tue, 21 Feb 2023 13:24:34 +0000 Subject: [PATCH 2/4] fix: use correct path --- crates/rome_service/src/configuration/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/rome_service/src/configuration/mod.rs b/crates/rome_service/src/configuration/mod.rs index 980f4019959..0e81355b449 100644 --- a/crates/rome_service/src/configuration/mod.rs +++ b/crates/rome_service/src/configuration/mod.rs @@ -189,7 +189,7 @@ pub fn load_config( Ok(Some(deserialized)) } Err(err) => { - if let Some(path) = base_path.parent() { + if let Some(path) = configuration_path.parent() { if path.is_dir() { configuration_path = path.join(config_name); continue; From 82267f4dbab6c9451ada756c6c4d4c23dd35739c Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Wed, 22 Feb 2023 14:10:35 +0000 Subject: [PATCH 3/4] chore: code suggestions --- crates/rome_service/src/configuration/mod.rs | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/crates/rome_service/src/configuration/mod.rs b/crates/rome_service/src/configuration/mod.rs index 0e81355b449..0137e08bca0 100644 --- a/crates/rome_service/src/configuration/mod.rs +++ b/crates/rome_service/src/configuration/mod.rs @@ -137,15 +137,6 @@ impl ConfigurationBasePath { const fn is_from_user(&self) -> bool { matches!(self, ConfigurationBasePath::FromUser(_)) } - - fn parent(&self) -> Option<&Path> { - match self { - ConfigurationBasePath::None => None, - ConfigurationBasePath::Lsp(path) | ConfigurationBasePath::FromUser(path) => { - path.parent() - } - } - } } /// Load the configuration from the file system. @@ -189,10 +180,13 @@ pub fn load_config( Ok(Some(deserialized)) } Err(err) => { - if let Some(path) = configuration_path.parent() { - if path.is_dir() { - configuration_path = path.join(config_name); - continue; + // base paths from users are not eligible for auto discovery + if !base_path.is_from_user() { + if let Some(path) = configuration_path.parent() { + if path.is_dir() { + configuration_path = path.join(config_name); + continue; + } } } // We skip the error when the configuration file is not found. From e91314c277589bfa2ea2131b6b909c2524dfb9bc Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Wed, 22 Feb 2023 14:17:14 +0000 Subject: [PATCH 4/4] chore: code suggestions --- crates/rome_service/src/configuration/mod.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/crates/rome_service/src/configuration/mod.rs b/crates/rome_service/src/configuration/mod.rs index 0137e08bca0..6045e330442 100644 --- a/crates/rome_service/src/configuration/mod.rs +++ b/crates/rome_service/src/configuration/mod.rs @@ -161,10 +161,11 @@ pub fn load_config( }; let should_error = base_path.is_from_user(); info!( - "Attempting to read the configuration file from {:?}", - configuration_path + "Attempting to read the configuration file from {}", + configuration_path.display() ); + let mut from_parent = false; loop { let options = OpenOptions::default().read(true); let file = file_system.open_with_options(&configuration_path, options); @@ -175,6 +176,13 @@ pub fn load_config( WorkspaceError::cant_read_file(format!("{}", configuration_path.display())) })?; + if from_parent { + info!( + "Rome auto discovered a configuration file at following path that wasn't in the working directory: {}", + configuration_path.display() + ); + } + let deserialized = deserialize_from_json::(&buffer) .with_file_path(&configuration_path.display().to_string()); Ok(Some(deserialized)) @@ -185,6 +193,7 @@ pub fn load_config( if let Some(path) = configuration_path.parent() { if path.is_dir() { configuration_path = path.join(config_name); + from_parent = true; continue; } }