From 846ac9570584f94f0346f5e51f3f8bea593219b3 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Fri, 23 Dec 2022 12:07:55 +0100 Subject: [PATCH] feat(editors/vscode): Add `requiresConfiguration` option (#4023) Co-authored-by: Emanuele Stoppa Co-authored-by: l3ops --- crates/rome_lsp/src/capabilities.rs | 10 +- .../src/{config.rs => extension_settings.rs} | 12 +- crates/rome_lsp/src/lib.rs | 4 +- crates/rome_lsp/src/server.rs | 148 ++++++++-------- crates/rome_lsp/src/session.rs | 165 ++++++++++++++++-- editors/vscode/README.md | 12 +- editors/vscode/package.json | 15 +- editors/vscode/src/main.ts | 18 +- 8 files changed, 275 insertions(+), 109 deletions(-) rename crates/rome_lsp/src/{config.rs => extension_settings.rs} (74%) diff --git a/crates/rome_lsp/src/capabilities.rs b/crates/rome_lsp/src/capabilities.rs index 18727ff39ac..853d7da20b2 100644 --- a/crates/rome_lsp/src/capabilities.rs +++ b/crates/rome_lsp/src/capabilities.rs @@ -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`] @@ -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() } diff --git a/crates/rome_lsp/src/config.rs b/crates/rome_lsp/src/extension_settings.rs similarity index 74% rename from crates/rome_lsp/src/config.rs rename to crates/rome_lsp/src/extension_settings.rs index 9c5096e3f44..97ba9bed9ee 100644 --- a/crates/rome_lsp/src/config.rs +++ b/crates/rome_lsp/src/extension_settings.rs @@ -14,14 +14,18 @@ pub struct WorkspaceSettings { /// Enable rename capability pub rename: Option, + + /// Only run Rome if a `rome.json` configuration file exists. + pub require_configuration: Option, } +/// 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(), @@ -37,4 +41,8 @@ impl Config { ); Ok(()) } + + pub(crate) fn requires_configuration(&self) -> bool { + self.settings.require_configuration.unwrap_or_default() + } } diff --git a/crates/rome_lsp/src/lib.rs b/crates/rome_lsp/src/lib.rs index 6f823c949eb..d626f27619c 100644 --- a/crates/rome_lsp/src/lib.rs +++ b/crates/rome_lsp/src/lib.rs @@ -1,6 +1,6 @@ mod capabilities; -mod config; mod documents; +mod extension_settings; mod handlers; mod line_index; mod requests; @@ -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}; diff --git a/crates/rome_lsp/src/server.rs b/crates/rome_lsp/src/server.rs index d3c90106074..b4f3a907a50 100644 --- a/crates/rome_lsp/src/server.rs +++ b/crates/rome_lsp/src/server.rs @@ -1,6 +1,8 @@ use crate::capabilities::server_capabilities; use crate::requests::syntax_tree::{SyntaxTreePayload, SYNTAX_TREE_REQUEST}; -use crate::session::{ClientInformation, Session, SessionHandle, SessionKey}; +use crate::session::{ + CapabilitySet, CapabilityStatus, ClientInformation, Session, SessionHandle, SessionKey, +}; use crate::utils::{into_lsp_error, panic_to_lsp_error}; use crate::{handlers, requests}; use futures::future::ready; @@ -9,6 +11,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}; @@ -109,78 +112,82 @@ impl LSPServer { Ok(RageResult { entries }) } - async fn register_capability(&self, registration: Registration) { - let method = registration.method.clone(); + async fn setup_capabilities(&self) { + let mut capabilities = CapabilitySet::default(); - if let Err(e) = self - .session - .client - .register_capability(vec![registration]) - .await - { - error!("Error registering {:?} capability: {}", method, e); - } - } + capabilities.add_capability( + "rome_did_change_extension_settings", + "workspace/didChangeConfiguration", + if self.session.can_register_did_change_configuration() { + CapabilityStatus::Enable(None) + } else { + CapabilityStatus::Disable + }, + ); - async fn unregister_capability(&self, unregistration: Unregistration) { - let method = unregistration.method.clone(); + capabilities.add_capability( + "rome_did_change_workspace_settings", + "workspace/didChangeWatchedFiles", + if let Some(base_path) = self.session.base_path() { + CapabilityStatus::Enable(Some(json!(DidChangeWatchedFilesRegistrationOptions { + watchers: vec![FileSystemWatcher { + glob_pattern: format!("{}/rome.json", base_path.display()), + kind: Some(WatchKind::all()), + }], + }))) + } else { + CapabilityStatus::Disable + }, + ); - if let Err(e) = self - .session - .client - .unregister_capability(vec![unregistration]) - .await - { - error!("Error unregistering {:?} capability: {}", method, e); - } - } + capabilities.add_capability( + "rome_formatting", + "textDocument/formatting", + if self.session.is_linting_and_formatting_disabled() { + CapabilityStatus::Disable + } else { + CapabilityStatus::Enable(None) + }, + ); + capabilities.add_capability( + "rome_range_formatting", + "textDocument/rangeFormatting", + if self.session.is_linting_and_formatting_disabled() { + CapabilityStatus::Disable + } else { + CapabilityStatus::Enable(None) + }, + ); + capabilities.add_capability( + "rome_on_type_formatting", + "textDocument/onTypeFormatting", + if self.session.is_linting_and_formatting_disabled() { + CapabilityStatus::Disable + } else { + CapabilityStatus::Enable(Some(json!(DocumentOnTypeFormattingRegistrationOptions { + document_selector: None, + first_trigger_character: String::from("}"), + more_trigger_character: Some(vec![String::from("]"), String::from(")")]), + }))) + }, + ); - 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) }; - if self.session.can_register_did_change_configuration() { - self.register_capability(Registration { - id: "workspace/didChangeConfiguration".to_string(), - method: "workspace/didChangeConfiguration".to_string(), - register_options: None, - }) - .await; - } - - let base_path = self.session.base_path(); - - if let Some(base_path) = base_path { - let registration_options = DidChangeWatchedFilesRegistrationOptions { - watchers: vec![FileSystemWatcher { - glob_pattern: format!("{}/rome.json", base_path.display()), - kind: Some(WatchKind::all()), - }], - }; - self.register_capability(Registration { - id: "workspace/didChangeWatchedFiles".to_string(), - method: "workspace/didChangeWatchedFiles".to_string(), - register_options: Some(serde_json::to_value(registration_options).unwrap()), - }) - .await; - } + capabilities.add_capability( + "rome_rename", + "textDocument/rename", + if rename { + CapabilityStatus::Enable(None) + } else { + CapabilityStatus::Disable + }, + ); - if rename { - self.register_capability(Registration { - id: "textDocument/rename".to_string(), - method: "textDocument/rename".to_string(), - register_options: None, - }) - .await; - } else { - self.unregister_capability(Unregistration { - id: "textDocument/rename".to_string(), - method: "textDocument/rename".to_string(), - }) - .await; - } + self.session.register_capabilities(capabilities).await; } } @@ -237,8 +244,10 @@ impl LanguageServer for LSPServer { info!("Attempting to load the configuration from 'rome.json' file"); - self.session.load_client_configuration().await; - self.session.load_workspace_settings().await; + futures::join!( + self.session.load_extension_settings(), + self.session.load_workspace_settings() + ); let msg = format!("Server initialized with PID: {}", std::process::id()); self.session @@ -256,11 +265,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.load_client_configuration().await; + self.session.load_extension_settings().await; self.setup_capabilities().await; + self.session.update_all_diagnostics().await; } #[tracing::instrument(level = "debug", skip(self))] @@ -278,6 +289,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.load_workspace_settings().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 @@ -353,7 +365,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) diff --git a/crates/rome_lsp/src/session.rs b/crates/rome_lsp/src/session.rs index 46844668c7d..27dbc15837c 100644 --- a/crates/rome_lsp/src/session.rs +++ b/crates/rome_lsp/src/session.rs @@ -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 anyhow::{anyhow, Result}; @@ -14,13 +14,18 @@ use rome_service::workspace::{FeatureName, PullDiagnosticsParams, SupportsFeatur use rome_service::workspace::{RageEntry, RageParams, RageResult, UpdateSettingsParams}; use rome_service::{load_config, Workspace}; use rome_service::{DynRef, WorkspaceError}; +use serde_json::Value; use std::collections::HashMap; use std::path::PathBuf; +use std::sync::atomic::AtomicU8; +use std::sync::atomic::Ordering; use std::sync::Arc; use std::sync::RwLock; use tokio::sync::Notify; use tokio::sync::OnceCell; use tower_lsp::lsp_types; +use tower_lsp::lsp_types::Registration; +use tower_lsp::lsp_types::Unregistration; use tower_lsp::lsp_types::Url; use tracing::{error, info}; @@ -47,10 +52,11 @@ pub(crate) struct Session { /// The parameters provided by the client in the "initialize" request initialize_params: OnceCell, - /// the configuration of the LSP - pub(crate) config: RwLock, + /// The settings of the Rome extension (under the `rome` namespace) + pub(crate) extension_settings: RwLock, pub(crate) workspace: Arc, + configuration_status: AtomicU8, /// File system to read files inside the workspace pub(crate) fs: DynRef<'static, dyn FileSystem>, @@ -69,8 +75,57 @@ struct InitializeParams { root_uri: Option, } +#[repr(u8)] +enum ConfigurationStatus { + /// The configuration file was properly loaded + Loaded = 0, + /// The configuration file does not exist + Missing = 1, + /// The configuration file exists but could not be loaded + Error = 2, +} + +impl TryFrom for ConfigurationStatus { + type Error = (); + + fn try_from(value: u8) -> Result { + match value { + 0 => Ok(Self::Loaded), + 1 => Ok(Self::Missing), + 2 => Ok(Self::Error), + _ => Err(()), + } + } +} + pub(crate) type SessionHandle = Arc; +/// Holds the set of capabilities supported by the Language Server +/// instance and whether they are enabled or not +#[derive(Default)] +pub(crate) struct CapabilitySet { + registry: HashMap<&'static str, (&'static str, CapabilityStatus)>, +} + +/// Represents whether a capability is enabled or not, optionally holding the +/// configuration associated with the capability +pub(crate) enum CapabilityStatus { + Enable(Option), + Disable, +} + +impl CapabilitySet { + /// Insert a capability in the set + pub(crate) fn add_capability( + &mut self, + id: &'static str, + method: &'static str, + status: CapabilityStatus, + ) { + self.registry.insert(id, (method, status)); + } +} + impl Session { pub(crate) fn new( key: SessionKey, @@ -80,15 +135,16 @@ impl Session { ) -> Self { let documents = Default::default(); let url_interner = Default::default(); - let config = RwLock::new(Config::new()); + let config = RwLock::new(ExtensionSettings::new()); Self { key, client, initialize_params: OnceCell::default(), workspace, + configuration_status: AtomicU8::new(ConfigurationStatus::Missing as u8), documents, url_interner, - config, + extension_settings: config, fs: DynRef::Owned(Box::new(OsFileSystem)), cancellation, } @@ -112,6 +168,57 @@ impl Session { } } + /// Register a set of capabilities with the client + pub(crate) async fn register_capabilities(&self, capabilities: CapabilitySet) { + let mut registrations = Vec::new(); + let mut unregistrations = Vec::new(); + + let mut register_methods = String::new(); + let mut unregister_methods = String::new(); + + for (id, (method, status)) in capabilities.registry { + unregistrations.push(Unregistration { + id: id.to_string(), + method: method.to_string(), + }); + + if !unregister_methods.is_empty() { + unregister_methods.push_str(", "); + } + + unregister_methods.push_str(method); + + if let CapabilityStatus::Enable(register_options) = status { + registrations.push(Registration { + id: id.to_string(), + method: method.to_string(), + register_options, + }); + + if !register_methods.is_empty() { + register_methods.push_str(", "); + } + + register_methods.push_str(method); + } + } + + if let Err(e) = self.client.unregister_capability(unregistrations).await { + error!( + "Error unregistering {unregister_methods:?} capabilities: {}", + e + ); + } else { + info!("Unregister capabilities {unregister_methods:?}"); + } + + if let Err(e) = self.client.register_capability(registrations).await { + error!("Error registering {register_methods:?} capabilities: {}", e); + } else { + info!("Register capabilities {register_methods:?}"); + } + } + /// Get a [`Document`] matching the provided [`lsp_types::Url`] /// /// If document does not exist, result is [SessionError::DocumentNotFound] @@ -173,7 +280,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![] @@ -268,7 +378,7 @@ impl Session { pub(crate) async fn load_workspace_settings(&self) { let base_path = self.base_path(); - match load_config(&self.fs, base_path) { + let status = match load_config(&self.fs, base_path) { Ok(Some(configuration)) => { info!("Loaded workspace settings: {configuration:#?}"); @@ -277,21 +387,28 @@ impl Session { .update_settings(UpdateSettingsParams { configuration }); if let Err(error) = result { - error!("Failed to set workspace settings: {}", error) + error!("Failed to set workspace settings: {}", error); + ConfigurationStatus::Error + } else { + ConfigurationStatus::Loaded } } Ok(None) => { // Ignore, load_config already logs an error in this case + ConfigurationStatus::Missing } Err(err) => { error!("Couldn't load the workspace settings, reason:\n {}", err); + ConfigurationStatus::Error } - } + }; + + self.set_configuration_status(status); } /// Requests "workspace/configuration" from client and updates Session config #[tracing::instrument(level = "debug", skip(self))] - pub(crate) async fn load_client_configuration(&self) { + pub(crate) async fn load_extension_settings(&self) { let item = lsp_types::ConfigurationItem { scope_uri: None, section: Some(String::from(CONFIGURATION_SECTION)), @@ -310,7 +427,7 @@ impl Session { if let Some(client_configuration) = client_configuration { info!("Loaded client configuration: {client_configuration:#?}"); - let mut config = self.config.write().unwrap(); + let mut config = self.extension_settings.write().unwrap(); if let Err(err) = config.set_workspace_settings(client_configuration) { error!("Couldn't set client configuration: {}", err); } @@ -339,4 +456,28 @@ impl Session { } } } + + fn configuration_status(&self) -> ConfigurationStatus { + self.configuration_status + .load(Ordering::Relaxed) + .try_into() + .unwrap() + } + + fn set_configuration_status(&self, status: ConfigurationStatus) { + self.configuration_status + .store(status as u8, Ordering::Relaxed); + } + + pub(crate) fn is_linting_and_formatting_disabled(&self) -> bool { + match self.configuration_status() { + ConfigurationStatus::Loaded => false, + ConfigurationStatus::Missing => self + .extension_settings + .read() + .unwrap() + .requires_configuration(), + ConfigurationStatus::Error => true, + } + } } diff --git a/editors/vscode/README.md b/editors/vscode/README.md index 2f8bba87e83..41ed785c849 100644 --- a/editors/vscode/README.md +++ b/editors/vscode/README.md @@ -88,6 +88,10 @@ The `rome.lspBin` option overrides the Rome binary used by the extension. The wo Enables Rome to handle renames in the workspace (experimental). +### `rome.requireConfiguration` + +Disables formatting, linting, and syntax errors for projects without a `rome.json` file. Requires Rome 12 or newer. + ## Known limitations ### Configuration per sub-directory @@ -100,10 +104,4 @@ Rome isn't yet able to pick up the `rome.json` configuration in [multi-root work ### Disable Rome for workspaces without a `rome.json` configuration -Rome's VS Code extension is active for every workspace regardless if the workspace contains a `rome.json` configuration ([issue 3506](https://github.com/rome/tools/issues/3506)). That may be surprising to you if you use other extensions like ESLint where the extension is disabled if the configuration is missing. This behavior is intentional because it's Rome's philosophy that the configuration should be optional. - -You can work around this limitation by [disabling Rome per workspace](https://code.visualstudio.com/docs/editor/extension-marketplace#_disable-an-extension): - -- Open the _Extensions_ panel -- Search for Rome -- Right-click on _Rome_ and select _Disable (Workspace)_ +You can set Rome's [`rome.requireConfiguration`](#romerequireconfiguration) setting to `true` to disable Rome's formatter, linter, and syntax errors for projects without a `rome.json` file. diff --git a/editors/vscode/package.json b/editors/vscode/package.json index d52397d17d2..713df69cd28 100644 --- a/editors/vscode/package.json +++ b/editors/vscode/package.json @@ -24,6 +24,14 @@ "vscode": "^1.70.0", "npm": "^8" }, + "capabilities": { + "untrustedWorkspaces": { + "supported": "limited", + "restrictedConfigurations": [ + "rome.lspBin" + ] + } + }, "contributes": { "languages": [ { @@ -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 syntax errors, formatting and linting. Requires Rome 12 or newer." } } }, @@ -137,4 +150,4 @@ "resolve": "^1.22.1", "vscode-languageclient": "^8.0.2" } -} +} \ No newline at end of file diff --git a/editors/vscode/src/main.ts b/editors/vscode/src/main.ts index f33771a53a8..a66fe5d63c3 100644 --- a/editors/vscode/src/main.ts +++ b/editors/vscode/src/main.ts @@ -1,12 +1,18 @@ +import { Commands } from "./commands"; +import { syntaxTree } from "./commands/syntaxTree"; +import { Session } from "./session"; +import { StatusBar } from "./statusBar"; +import { setContextValue } from "./utils"; import { type ChildProcess, spawn } from "child_process"; -import { connect, type Socket } from "net"; +import { type Socket, connect } from "net"; +import { isAbsolute } from "path"; import { promisify, TextDecoder } from "util"; import { ExtensionContext, - languages, OutputChannel, TextEditor, Uri, + languages, window, workspace, } from "vscode"; @@ -17,12 +23,6 @@ import { ServerOptions, StreamInfo, } from "vscode-languageclient/node"; -import { isAbsolute } from "path"; -import { setContextValue } from "./utils"; -import { Session } from "./session"; -import { syntaxTree } from "./commands/syntaxTree"; -import { Commands } from "./commands"; -import { StatusBar } from "./statusBar"; import resolveImpl = require("resolve/async"); import type * as Resolve from "resolve"; @@ -118,7 +118,7 @@ export async function activate(context: ExtensionContext) { ); handleActiveTextEditorChanged(window.activeTextEditor); - client.start(); + await client.start(); } type Architecture = "x64" | "arm64";