From dc8d79fe40437ab6f2acd841694d03417d377c71 Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Wed, 22 May 2024 12:39:47 -0700 Subject: [PATCH 1/2] Fix miscellaneous issues with configuration and diagnostic publishing --- .../test/fixtures/settings/global_only.json | 28 +++--- crates/ruff_server/src/lint.rs | 6 +- crates/ruff_server/src/server.rs | 17 +++- 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, 126 insertions(+), 103 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 0ed3bf16d5526..29c9956c77156 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 91aa2424e1d2d..4815a9d5ef1a9 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 affeeb67f0f1d..b18cf7e1081d6 100644 --- a/crates/ruff_server/src/server.rs +++ b/crates/ruff_server/src/server.rs @@ -66,10 +66,19 @@ impl Server { crate::message::init_messenger(connection.make_sender()); + tracing::info!( + "Initialization options: {:?}", + init_params.initialization_options + ); + 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 +93,11 @@ impl Server { let workspaces = init_params .workspace_folders - .map(|folders| folders.into_iter().map(|folder| { + .and_then(|folders| (!folders.is_empty()).then(|| folders.into_iter().map(|folder| { workspace_for_path(folder.uri.to_file_path().unwrap()) - }).collect()) + }).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 9058eed1fb73e..683b3ea82c288 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 453e8eb8f2e2a..aa5a1efcad712 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 3570dbaaa7500..e54b16020bc3f 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 02d4acbd07925..bfbfeef5c42fe 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, + }, } "###); } From 66f3f59b208d37f43850b904257e3cda7a46deaa Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Wed, 22 May 2024 13:47:06 -0700 Subject: [PATCH 2/2] Filter workspace folders --- crates/ruff_server/src/server.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/crates/ruff_server/src/server.rs b/crates/ruff_server/src/server.rs index b18cf7e1081d6..9c995b402a0e4 100644 --- a/crates/ruff_server/src/server.rs +++ b/crates/ruff_server/src/server.rs @@ -66,11 +66,6 @@ impl Server { crate::message::init_messenger(connection.make_sender()); - tracing::info!( - "Initialization options: {:?}", - init_params.initialization_options - ); - let AllSettings { global_settings, mut workspace_settings, @@ -93,9 +88,10 @@ impl Server { let workspaces = init_params .workspace_folders - .and_then(|folders| (!folders.is_empty()).then(|| folders.into_iter().map(|folder| { + .filter(|folders| !folders.is_empty()) + .map(|folders| folders.into_iter().map(|folder| { workspace_for_path(folder.uri.to_file_path().unwrap()) - }).collect())) + }).collect()) .or_else(|| { 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()?;