ruff server: Fix multiple issues with Neovim and Helix (#11497)

snowsignal · web-flow · commit 94abea4b0822 · 2024-05-22T20:50:58.000Z
## Summary Fixes #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
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -237,12 +237,27 @@ impl Index {
         Ok(())
     }
 
-    pub(super) fn make_document_ref(&self, key: DocumentKey) -> Option<DocumentQuery> {
+    pub(super) fn make_document_ref(
+        &self,
+        key: DocumentKey,
+        global_settings: &ClientSettings,
+    ) -> Option<DocumentQuery> {
         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
@@ -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<RuffSettings> {
@@ -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
@@ -130,7 +130,8 @@ enum InitializationOptions {
         workspace_settings: Vec<WorkspaceSettings>,
     },
     GlobalOnly {
-        settings: Option<ClientSettings>,
+        #[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,
+            },
         }
         "###);
     }

Original file line number	Diff line number	Diff line change
`@@ -119,12 +119,14 @@ pub(crate) fn check(`
`119`	`119`
`120`	`120`	`let mut diagnostics = Diagnostics::default();`
`121`	`121`
`122`		`- // Populate all cell URLs with an empty diagnostic list.`
`123`		`- // This ensures that cells without diagnostics still get updated.`
	`122`	`+ // Populates all relevant URLs with an empty diagnostic list.`
	`123`	`+ // This ensures that documents without diagnostics still get updated.`
`124`	`124`	`if let Some(notebook) = query.as_notebook() {`
`125`	`125`	`for url in notebook.urls() {`
`126`	`126`	`diagnostics.entry(url.clone()).or_default();`
`127`	`127`	`}`
	`128`	`+ } else {`
	`129`	`+ diagnostics.entry(query.make_key().into_url()).or_default();`
`128`	`130`	`}`
`129`	`131`
`130`	`132`	`let lsp_diagnostics = data`
Original file line number	Diff line number	Diff line change
`@@ -67,7 +67,7 @@ impl Session {`
`67`	`67`	`Some(DocumentSnapshot {`
`68`	`68`	`resolved_client_capabilities: self.resolved_client_capabilities.clone(),`
`69`	`69`	`client_settings: self.index.client_settings(&key, &self.global_settings),`
`70`		`- document_ref: self.index.make_document_ref(key)?,`
	`70`	`+ document_ref: self.index.make_document_ref(key, &self.global_settings)?,`
`71`	`71`	`position_encoding: self.position_encoding,`
`72`	`72`	`})`
`73`	`73`	`}`