From 94abea4b0822f99f97e01d9adadddc82acd8d8b8 Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Wed, 22 May 2024 13:50:58 -0700 Subject: [PATCH] `ruff server`: Fix multiple issues with Neovim and Helix (#11497) ## Summary Fixes https://github.com/astral-sh/ruff/issues/11236. This PR fixes several issues, most of which relate to non-VS Code editors (Helix and Neovim). 1. Global-only initialization options are now correctly deserialized from Neovim and Helix 2. Empty diagnostics are now published correctly for Neovim and Helix. 3. A workspace folder is created at the current working directory if the initialization parameters send an empty list of workspace folders. 4. The server now gracefully handles opening files outside of any known workspace, and will use global fallback settings taken from client editor settings and a user settings TOML, if it exists. ## Test Plan I've tested to confirm that each issue has been fixed. * Global-only initialization options are now correctly deserialized from Neovim and Helix + the server gracefully handles opening files outside of any known workspace https://github.com/astral-sh/ruff/assets/19577865/4f33477f-20c8-4e50-8214-6608b1a1ea6b * Empty diagnostics are now published correctly for Neovim and Helix https://github.com/astral-sh/ruff/assets/19577865/c93f56a0-f75d-466f-9f40-d77f99cf0637 * A workspace folder is created at the current working directory if the initialization parameters send an empty list of workspace folders. https://github.com/astral-sh/ruff/assets/19577865/b4b2e818-4b0d-40ce-961d-5831478cc726 --- .../test/fixtures/settings/global_only.json | 28 +++--- crates/ruff_server/src/lint.rs | 6 +- crates/ruff_server/src/server.rs | 9 +- crates/ruff_server/src/session.rs | 2 +- crates/ruff_server/src/session/index.rs | 23 ++++- .../src/session/index/ruff_settings.rs | 58 ++++++----- crates/ruff_server/src/session/settings.rs | 95 ++++++++++--------- 7 files changed, 120 insertions(+), 101 deletions(-) diff --git a/crates/ruff_server/resources/test/fixtures/settings/global_only.json b/crates/ruff_server/resources/test/fixtures/settings/global_only.json index 0ed3bf16d55262..29c9956c771566 100644 --- a/crates/ruff_server/resources/test/fixtures/settings/global_only.json +++ b/crates/ruff_server/resources/test/fixtures/settings/global_only.json @@ -1,17 +1,15 @@ { - "settings": { - "codeAction": { - "disableRuleComment": { - "enable": false - } - }, - "lint": { - "ignore": ["RUF001"], - "run": "onSave" - }, - "fixAll": false, - "logLevel": "warn", - "lineLength": 80, - "exclude": ["third_party"] - } + "codeAction": { + "disableRuleComment": { + "enable": false + } + }, + "lint": { + "ignore": ["RUF001"], + "run": "onSave" + }, + "fixAll": false, + "logLevel": "warn", + "lineLength": 80, + "exclude": ["third_party"] } diff --git a/crates/ruff_server/src/lint.rs b/crates/ruff_server/src/lint.rs index 91aa2424e1d2df..4815a9d5ef1a9e 100644 --- a/crates/ruff_server/src/lint.rs +++ b/crates/ruff_server/src/lint.rs @@ -119,12 +119,14 @@ pub(crate) fn check( let mut diagnostics = Diagnostics::default(); - // Populate all cell URLs with an empty diagnostic list. - // This ensures that cells without diagnostics still get updated. + // Populates all relevant URLs with an empty diagnostic list. + // This ensures that documents without diagnostics still get updated. if let Some(notebook) = query.as_notebook() { for url in notebook.urls() { diagnostics.entry(url.clone()).or_default(); } + } else { + diagnostics.entry(query.make_key().into_url()).or_default(); } let lsp_diagnostics = data diff --git a/crates/ruff_server/src/server.rs b/crates/ruff_server/src/server.rs index affeeb67f0f1d7..9c995b402a0e43 100644 --- a/crates/ruff_server/src/server.rs +++ b/crates/ruff_server/src/server.rs @@ -69,7 +69,11 @@ impl Server { let AllSettings { global_settings, mut workspace_settings, - } = AllSettings::from_value(init_params.initialization_options.unwrap_or_default()); + } = AllSettings::from_value( + init_params + .initialization_options + .unwrap_or_else(|| serde_json::Value::Object(serde_json::Map::default())), + ); let mut workspace_for_path = |path: PathBuf| { let Some(workspace_settings) = workspace_settings.as_mut() else { @@ -84,11 +88,12 @@ impl Server { let workspaces = init_params .workspace_folders + .filter(|folders| !folders.is_empty()) .map(|folders| folders.into_iter().map(|folder| { workspace_for_path(folder.uri.to_file_path().unwrap()) }).collect()) .or_else(|| { - tracing::debug!("No workspace(s) were provided during initialization. Using the current working directory as a default workspace..."); + tracing::warn!("No workspace(s) were provided during initialization. Using the current working directory as a default workspace..."); let uri = types::Url::from_file_path(std::env::current_dir().ok()?).ok()?; Some(vec![workspace_for_path(uri.to_file_path().unwrap())]) }) diff --git a/crates/ruff_server/src/session.rs b/crates/ruff_server/src/session.rs index 9058eed1fb73ed..683b3ea82c288d 100644 --- a/crates/ruff_server/src/session.rs +++ b/crates/ruff_server/src/session.rs @@ -67,7 +67,7 @@ impl Session { Some(DocumentSnapshot { resolved_client_capabilities: self.resolved_client_capabilities.clone(), client_settings: self.index.client_settings(&key, &self.global_settings), - document_ref: self.index.make_document_ref(key)?, + document_ref: self.index.make_document_ref(key, &self.global_settings)?, position_encoding: self.position_encoding, }) } diff --git a/crates/ruff_server/src/session/index.rs b/crates/ruff_server/src/session/index.rs index 453e8eb8f2e2ad..aa5a1efcad7121 100644 --- a/crates/ruff_server/src/session/index.rs +++ b/crates/ruff_server/src/session/index.rs @@ -237,12 +237,27 @@ impl Index { Ok(()) } - pub(super) fn make_document_ref(&self, key: DocumentKey) -> Option { + pub(super) fn make_document_ref( + &self, + key: DocumentKey, + global_settings: &ClientSettings, + ) -> Option { let path = self.path_for_key(&key)?.clone(); let document_settings = self - .settings_for_path(&path)? - .workspace_settings_index - .get(&path); + .settings_for_path(&path) + .map(|settings| settings.workspace_settings_index.get(&path)) + .unwrap_or_else(|| { + tracing::warn!( + "No settings available for {} - falling back to default settings", + path.display() + ); + let resolved_global = ResolvedClientSettings::global(global_settings); + let root = path.parent().unwrap_or(&path); + Arc::new(RuffSettings::fallback( + resolved_global.editor_settings(), + root, + )) + }); let controller = self.documents.get(&path)?; let cell_uri = match key { diff --git a/crates/ruff_server/src/session/index/ruff_settings.rs b/crates/ruff_server/src/session/index/ruff_settings.rs index 3570dbaaa75002..e54b16020bc3f2 100644 --- a/crates/ruff_server/src/session/index/ruff_settings.rs +++ b/crates/ruff_server/src/session/index/ruff_settings.rs @@ -43,6 +43,32 @@ impl std::fmt::Display for RuffSettings { } impl RuffSettings { + pub(crate) fn fallback(editor_settings: &ResolvedEditorSettings, root: &Path) -> RuffSettings { + let fallback = find_user_settings_toml() + .and_then(|user_settings| { + ruff_workspace::resolver::resolve_root_settings( + &user_settings, + Relativity::Cwd, + &EditorConfigurationTransformer(editor_settings, root), + ) + .ok() + }) + .unwrap_or_else(|| { + let default_configuration = ruff_workspace::configuration::Configuration::default(); + EditorConfigurationTransformer(editor_settings, root) + .transform(default_configuration) + .into_settings(root) + .expect( + "editor configuration should merge successfully with default configuration", + ) + }); + + RuffSettings { + formatter: fallback.formatter, + linter: fallback.linter, + } + } + pub(crate) fn linter(&self) -> &ruff_linter::settings::LinterSettings { &self.linter } @@ -80,32 +106,9 @@ impl RuffSettingsIndex { } } - let fallback = find_user_settings_toml() - .and_then(|user_settings| { - ruff_workspace::resolver::resolve_root_settings( - &user_settings, - Relativity::Cwd, - &EditorConfigurationTransformer(editor_settings, root), - ) - .ok() - }) - .unwrap_or_else(|| { - let default_configuration = ruff_workspace::configuration::Configuration::default(); - EditorConfigurationTransformer(editor_settings, root) - .transform(default_configuration) - .into_settings(root) - .expect( - "editor configuration should merge successfully with default configuration", - ) - }); + let fallback = Arc::new(RuffSettings::fallback(editor_settings, root)); - Self { - index, - fallback: Arc::new(RuffSettings { - formatter: fallback.formatter, - linter: fallback.linter, - }), - } + Self { index, fallback } } pub(super) fn get(&self, document_path: &Path) -> Arc { @@ -118,11 +121,6 @@ impl RuffSettingsIndex { return settings.clone(); } - tracing::info!( - "No Ruff settings file found for {}; falling back to default configuration", - document_path.display() - ); - self.fallback.clone() } } diff --git a/crates/ruff_server/src/session/settings.rs b/crates/ruff_server/src/session/settings.rs index 02d4acbd079255..bfbfeef5c42fea 100644 --- a/crates/ruff_server/src/session/settings.rs +++ b/crates/ruff_server/src/session/settings.rs @@ -130,7 +130,8 @@ enum InitializationOptions { workspace_settings: Vec, }, GlobalOnly { - settings: Option, + #[serde(flatten)] + settings: ClientSettings, }, } @@ -157,7 +158,7 @@ impl AllSettings { fn from_init_options(options: InitializationOptions) -> Self { let (global_settings, workspace_settings) = match options { - InitializationOptions::GlobalOnly { settings } => (settings.unwrap_or_default(), None), + InitializationOptions::GlobalOnly { settings } => (settings, None), InitializationOptions::HasWorkspaces { global_settings, workspace_settings, @@ -341,7 +342,9 @@ impl ResolvedClientSettings { impl Default for InitializationOptions { fn default() -> Self { - Self::GlobalOnly { settings: None } + Self::GlobalOnly { + settings: ClientSettings::default(), + } } } @@ -626,52 +629,50 @@ mod tests { assert_debug_snapshot!(options, @r###" GlobalOnly { - settings: Some( - ClientSettings { - configuration: None, - fix_all: Some( - false, - ), - organize_imports: None, - lint: Some( - LintOptions { - enable: None, - preview: None, - select: None, - extend_select: None, - ignore: Some( - [ - "RUF001", - ], - ), - }, - ), - format: None, - code_action: Some( - CodeActionOptions { - disable_rule_comment: Some( - CodeActionParameters { - enable: Some( - false, - ), - }, - ), - fix_violation: None, - }, - ), - exclude: Some( - [ - "third_party", - ], - ), - line_length: Some( - LineLength( - 80, + settings: ClientSettings { + configuration: None, + fix_all: Some( + false, + ), + organize_imports: None, + lint: Some( + LintOptions { + enable: None, + preview: None, + select: None, + extend_select: None, + ignore: Some( + [ + "RUF001", + ], ), + }, + ), + format: None, + code_action: Some( + CodeActionOptions { + disable_rule_comment: Some( + CodeActionParameters { + enable: Some( + false, + ), + }, + ), + fix_violation: None, + }, + ), + exclude: Some( + [ + "third_party", + ], + ), + line_length: Some( + LineLength( + 80, ), - configuration_preference: None, - }, - ), + ), + configuration_preference: None, + }, } "###); }