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

Commit

Permalink
feat(editors/vscode): Add requiresConfiguration option
Browse files Browse the repository at this point in the history
## Summary

This PR introduces a new `rome.requiresConfiguration` option that disables the linter and formatter if a workspace doesn't contain a `rome.json` configuration.

The addition of the option is motivated by #3506 where users want to have an easy way to disable Rome for all projects not using Rome.

The new option doesn't fully disable Rome but only disables the linter and formatter (operations that are performed automatically on save). This has the benefit that other actions like sorting imports, or refactors remain available.

The implementation for the linter and formatter differ:

* Formatter: I changed the extension to not register the formatting capabilities if the `rome.json` file is missing and the `requiresConfiguration` option is set. This so that VS Code can pick up another default formatter (if installed).
* Linter: Returns an empty list of diagnostics from `pull_diagnostic`

## Test Plan

I tested that:
* Formatting and linting is working if the option is false
* Formatting and linting is working if the option is `true` and a `rome.json` file is present
* Formatting and linting is disabled if the option is `true` and the project has no `rome.json` file
  • Loading branch information
MichaReiser committed Dec 9, 2022
1 parent d84f585 commit e5f527c
Show file tree
Hide file tree
Showing 7 changed files with 146 additions and 62 deletions.
10 changes: 2 additions & 8 deletions crates/rome_lsp/src/capabilities.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use tower_lsp::lsp_types::{
CodeActionProviderCapability, DocumentOnTypeFormattingOptions, OneOf, ServerCapabilities,
TextDocumentSyncCapability, TextDocumentSyncKind,
CodeActionProviderCapability, ServerCapabilities, TextDocumentSyncCapability,
TextDocumentSyncKind,
};

/// The capabilities to send from server as part of [`InitializeResult`]
Expand All @@ -12,12 +12,6 @@ pub(crate) fn server_capabilities() -> ServerCapabilities {
TextDocumentSyncKind::INCREMENTAL,
)),
code_action_provider: Some(CodeActionProviderCapability::Simple(true)),
document_formatting_provider: Some(OneOf::Left(true)),
document_range_formatting_provider: Some(OneOf::Left(true)),
document_on_type_formatting_provider: Some(DocumentOnTypeFormattingOptions {
first_trigger_character: String::from("}"),
more_trigger_character: Some(vec![String::from("]"), String::from(")")]),
}),
rename_provider: None,
..Default::default()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,18 @@ pub struct WorkspaceSettings {

/// Enable rename capability
pub rename: Option<bool>,

/// Only run Rome if a `rome.json` configuration file exists.
pub require_configuration: Option<bool>,
}

/// The `rome.*` extension settings
#[derive(Debug)]
pub(crate) struct Config {
pub(crate) struct ExtensionSettings {
pub(crate) settings: WorkspaceSettings,
}

impl Config {
impl ExtensionSettings {
pub(crate) fn new() -> Self {
Self {
settings: WorkspaceSettings::default(),
Expand All @@ -37,4 +41,8 @@ impl Config {
);
Ok(())
}

pub(crate) fn requires_configuration(&self) -> bool {
self.settings.require_configuration.unwrap_or_default()
}
}
4 changes: 2 additions & 2 deletions crates/rome_lsp/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
mod capabilities;
mod config;
mod documents;
mod extension_settings;
mod handlers;
mod line_index;
mod requests;
Expand All @@ -9,5 +9,5 @@ mod session;
mod url_interner;
mod utils;

pub use crate::config::WorkspaceSettings;
pub use crate::extension_settings::WorkspaceSettings;
pub use crate::server::{LSPServer, ServerConnection, ServerFactory};
120 changes: 89 additions & 31 deletions crates/rome_lsp/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use rome_console::markup;
use rome_fs::CONFIG_NAME;
use rome_service::workspace::{RageEntry, RageParams, RageResult};
use rome_service::{workspace, Workspace};
use serde_json::json;
use std::collections::HashMap;
use std::panic::RefUnwindSafe;
use std::sync::atomic::{AtomicBool, AtomicU64, Ordering};
Expand Down Expand Up @@ -111,45 +112,50 @@ impl LSPServer {
Ok(RageResult { entries })
}

async fn register_capability(&self, registration: Registration) {
let method = registration.method.clone();
async fn register_capabilities(&self, registrations: Vec<Registration>) {
let methods = registrations
.iter()
.map(|reg| reg.method.clone())
.collect::<Vec<_>>()
.join(", ");

if let Err(e) = self
.session
.client
.register_capability(vec![registration])
.await
{
error!("Error registering {:?} capability: {}", method, e);
if let Err(e) = self.session.client.register_capability(registrations).await {
error!("Error registering {:?} capability: {}", methods, e);
}
}

async fn unregister_capability(&self, unregistration: Unregistration) {
let method = unregistration.method.clone();
async fn unregister_capabilities(&self, registrations: Vec<Unregistration>) {
let methods = registrations
.iter()
.map(|reg| reg.method.clone())
.collect::<Vec<_>>()
.join(", ");

if let Err(e) = self
.session
.client
.unregister_capability(vec![unregistration])
.unregister_capability(registrations)
.await
{
error!("Error unregistering {:?} capability: {}", method, e);
error!("Error unregistering {:?} capability: {}", methods, e);
}
}

async fn setup_capabilities(&self) {
let rename = {
let config = self.session.config.read().ok();
let config = self.session.extension_settings.read().ok();
config.and_then(|x| x.settings.rename).unwrap_or(false)
};

let mut register = Vec::new();
let mut unregister = Vec::new();

if self.session.can_register_did_change_configuration() {
self.register_capability(Registration {
register.push(Registration {
id: "workspace/didChangeConfiguration".to_string(),
method: "workspace/didChangeConfiguration".to_string(),
register_options: None,
})
.await;
});
}

let base_path = self.session.base_path();
Expand All @@ -161,27 +167,75 @@ impl LSPServer {
kind: Some(WatchKind::all()),
}],
};
self.register_capability(Registration {
register.push(Registration {
id: "workspace/didChangeWatchedFiles".to_string(),
method: "workspace/didChangeWatchedFiles".to_string(),
register_options: Some(serde_json::to_value(registration_options).unwrap()),
})
.await;
});
}

if self.session.is_linting_and_formatting_disabled() {
unregister.extend([
Unregistration {
id: "textDocument/formatting".to_string(),
method: "textDocument/formatting".to_string(),
},
Unregistration {
id: "textDocument/rangeFormatting".to_string(),
method: "textDocument/rangeFormatting".to_string(),
},
Unregistration {
id: "textDocument/onTypeFormatting".to_string(),
method: "textDocument/onTypeFormatting".to_string(),
},
]);
} else {
register.extend([
Registration {
id: "textDocument/formatting".to_string(),
method: "textDocument/formatting".to_string(),
register_options: Some(json!(TextDocumentRegistrationOptions {
document_selector: None
})),
},
Registration {
id: "textDocument/rangeFormatting".to_string(),
method: "textDocument/rangeFormatting".to_string(),
register_options: Some(json!(TextDocumentRegistrationOptions {
document_selector: None
})),
},
Registration {
id: "textDocument/onTypeFormatting".to_string(),
method: "textDocument/onTypeFormatting".to_string(),
register_options: Some(json!(DocumentOnTypeFormattingRegistrationOptions {
document_selector: None,
first_trigger_character: String::from("}"),
more_trigger_character: Some(vec![String::from("]"), String::from(")")]),
})),
},
]);
}

if rename {
self.register_capability(Registration {
register.push(Registration {
id: "textDocument/rename".to_string(),
method: "textDocument/rename".to_string(),
register_options: None,
})
.await;
});
} else {
self.unregister_capability(Unregistration {
unregister.push(Unregistration {
id: "textDocument/rename".to_string(),
method: "textDocument/rename".to_string(),
})
.await;
});
}

if !register.is_empty() {
self.register_capabilities(register).await;
}

if !unregister.is_empty() {
self.unregister_capabilities(unregister).await;
}
}
}
Expand Down Expand Up @@ -228,8 +282,10 @@ impl LanguageServer for LSPServer {

info!("Attempting to load the configuration from 'rome.json' file");

self.session.update_configuration().await;
self.session.fetch_client_configuration().await;
futures::join!(
self.session.update_configuration(),
self.session.fetch_extension_settings()
);

let msg = format!("Server initialized with PID: {}", std::process::id());
self.session
Expand All @@ -247,11 +303,13 @@ impl LanguageServer for LSPServer {
Ok(())
}

/// Called when the user changed the editor settings.
#[tracing::instrument(level = "debug", skip(self))]
async fn did_change_configuration(&self, params: DidChangeConfigurationParams) {
let _ = params;
self.session.fetch_client_configuration().await;
self.session.fetch_extension_settings().await;
self.setup_capabilities().await;
self.session.update_all_diagnostics().await;
}

#[tracing::instrument(level = "debug", skip(self))]
Expand All @@ -269,7 +327,7 @@ impl LanguageServer for LSPServer {
if let Ok(possible_rome_json) = possible_rome_json {
if possible_rome_json.display().to_string() == CONFIG_NAME {
self.session.update_configuration().await;
self.session.fetch_client_configuration().await;
self.setup_capabilities().await;
self.session.update_all_diagnostics().await;
// for now we are only interested to the configuration file,
// so it's OK to exist the loop
Expand Down Expand Up @@ -345,7 +403,7 @@ impl LanguageServer for LSPServer {
rome_diagnostics::panic::catch_unwind(move || {
let rename_enabled = self
.session
.config
.extension_settings
.read()
.ok()
.and_then(|config| config.settings.rename)
Expand Down
31 changes: 21 additions & 10 deletions crates/rome_lsp/src/session.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::config::Config;
use crate::config::CONFIGURATION_SECTION;
use crate::documents::Document;
use crate::extension_settings::ExtensionSettings;
use crate::extension_settings::CONFIGURATION_SECTION;
use crate::url_interner::UrlInterner;
use crate::utils;
use futures::stream::futures_unordered::FuturesUnordered;
Expand Down Expand Up @@ -47,8 +47,8 @@ pub(crate) struct Session {

pub(crate) client_information: Mutex<Option<ClientInformation>>,

/// the configuration of the LSP
pub(crate) config: RwLock<Config>,
/// The settings of the Rome extension (under the `rome` namespace)
pub(crate) extension_settings: RwLock<ExtensionSettings>,

pub(crate) workspace: Arc<dyn Workspace>,

Expand Down Expand Up @@ -78,7 +78,7 @@ impl Session {
let client_capabilities = RwLock::new(Default::default());
let documents = Default::default();
let url_interner = Default::default();
let config = RwLock::new(Config::new());
let config = RwLock::new(ExtensionSettings::new());
let configuration = RwLock::new(None);
let root_uri = RwLock::new(None);
Self {
Expand All @@ -89,7 +89,7 @@ impl Session {
workspace,
documents,
url_interner,
config,
extension_settings: config,
fs: DynRef::Owned(Box::new(OsFileSystem)),
configuration,
root_uri,
Expand Down Expand Up @@ -144,7 +144,10 @@ impl Session {
path: rome_path.clone(),
})?;

let diagnostics = if let Some(reason) = unsupported_lint.reason {
let diagnostics = if self.is_linting_and_formatting_disabled() {
tracing::trace!("Linting disabled because Rome configuration is missing and `requireConfiguration` is true.");
vec![]
} else if let Some(reason) = unsupported_lint.reason {
tracing::trace!("linting not supported: {reason:?}");
// Sending empty vector clears published diagnostics
vec![]
Expand Down Expand Up @@ -242,7 +245,7 @@ impl Session {
}

/// Requests "workspace/configuration" from client and updates Session config
pub(crate) async fn fetch_client_configuration(&self) {
pub(crate) async fn fetch_extension_settings(&self) {
let item = lsp_types::ConfigurationItem {
scope_uri: None,
section: Some(String::from(CONFIGURATION_SECTION)),
Expand All @@ -255,15 +258,14 @@ impl Session {
.into_iter()
.next()
.and_then(|client_configuration| {
let mut config = self.config.write().unwrap();
let mut config = self.extension_settings.write().unwrap();

config
.set_workspace_settings(client_configuration)
.map_err(|err| {
error!("Cannot set workspace settings: {}", err);
})
.ok()?;
self.update_workspace_settings();

Some(())
});
Expand Down Expand Up @@ -316,4 +318,13 @@ impl Session {
}
}
}

pub(crate) fn is_linting_and_formatting_disabled(&self) -> bool {
self.configuration.read().unwrap().is_none()
&& self
.extension_settings
.read()
.unwrap()
.requires_configuration()
}
}
13 changes: 13 additions & 0 deletions editors/vscode/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,14 @@
"vscode": "^1.70.0",
"npm": "^8"
},
"capabilities": {
"untrustedWorkspaces": {
"supported": "limited",
"restrictedConfigurations": [
"rome.lspBin"
]
}
},
"contributes": {
"languages": [
{
Expand Down Expand Up @@ -95,6 +103,11 @@
],
"default": null,
"markdownDescription": "Enable/Disable Rome handling renames in the workspace. (Experimental)"
},
"rome.requireConfiguration": {
"type": "boolean",
"default": false,
"markdownDescription": "Require a Rome configuration file to enable formatting and linting."
}
}
},
Expand Down
Loading

0 comments on commit e5f527c

Please sign in to comment.