From 540023262e35ae6658fa2bfb71c271dac093c7e3 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Mon, 12 Aug 2024 21:12:45 +0530 Subject: [PATCH] Collect errors while building up the settings index (#12781) ## Summary Related to https://github.com/astral-sh/ruff-vscode/issues/571, this PR updates the settings index builder to trace all the errors it encountered. Without this, there's no way for user to know that something failed and some of the capability might not work as expected. For example, in the linked PR, the settings were invalid which means notebooks weren't included and there were no log messages for it. ## Test Plan Create an invalid `ruff.toml` file: ```toml [tool.ruff] extend-exclude = ["*.ipynb"] ``` Logs: ``` 2024-08-12 18:33:09.873 [info] [Trace - 6:33:09 PM] 12.217043000s ERROR ruff:main ruff_server::session::index::ruff_settings: Failed to parse /Users/dhruv/playground/ruff/pyproject.toml ``` Notification Preview: Screenshot 2024-08-12 at 18 33 20 Another way to trigger is to provide an invalid `cache-dir` value: ```toml [tool.ruff] cache-dir = "$UNKNOWN" ``` Same notification preview but different log message: ``` 2024-08-12 18:41:37.571 [info] [Trace - 6:41:37 PM] 21.700112208s ERROR ThreadId(30) ruff_server::session::index::ruff_settings: Error while resolving settings from /Users/dhruv/playground/ruff/pyproject.toml: Invalid `cache-dir` value: error looking key 'UNKNOWN' up: environment variable not found ``` With multiple `pyproject.toml` file: ``` 2024-08-12 18:41:15.887 [info] [Trace - 6:41:15 PM] 0.016636833s ERROR ThreadId(04) ruff_server::session::index::ruff_settings: Error while resolving settings from /Users/dhruv/playground/ruff/pyproject.toml: Invalid `cache-dir` value: error looking key 'UNKNOWN' up: environment variable not found 2024-08-12 18:41:15.888 [info] [Trace - 6:41:15 PM] 0.017378833s ERROR ThreadId(13) ruff_server::session::index::ruff_settings: Failed to parse /Users/dhruv/playground/ruff/tools/pyproject.toml ``` --- .../src/session/index/ruff_settings.rs | 120 ++++++++++++------ 1 file changed, 81 insertions(+), 39 deletions(-) diff --git a/crates/ruff_server/src/session/index/ruff_settings.rs b/crates/ruff_server/src/session/index/ruff_settings.rs index 5fa65b2453..a172b58e10 100644 --- a/crates/ruff_server/src/session/index/ruff_settings.rs +++ b/crates/ruff_server/src/session/index/ruff_settings.rs @@ -1,5 +1,6 @@ use std::collections::BTreeMap; use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use ignore::{WalkBuilder, WalkState}; @@ -99,32 +100,49 @@ impl RuffSettings { impl RuffSettingsIndex { pub(super) fn new(root: &Path, editor_settings: &ResolvedEditorSettings) -> Self { + let mut error = false; let mut index = BTreeMap::default(); let mut respect_gitignore = None; - // Add any settings from above the workspace root. - for directory in root.ancestors() { - if let Some(pyproject) = settings_toml(directory).ok().flatten() { - let Ok(settings) = ruff_workspace::resolver::resolve_root_settings( - &pyproject, - Relativity::Parent, - &EditorConfigurationTransformer(editor_settings, root), - ) else { + // Add any settings from above the workspace root, excluding the workspace root itself. + for directory in root.ancestors().skip(1) { + match settings_toml(directory) { + Ok(Some(pyproject)) => { + match ruff_workspace::resolver::resolve_root_settings( + &pyproject, + Relativity::Parent, + &EditorConfigurationTransformer(editor_settings, root), + ) { + Ok(settings) => { + respect_gitignore = Some(settings.file_resolver.respect_gitignore); + + index.insert( + directory.to_path_buf(), + Arc::new(RuffSettings { + path: Some(pyproject), + file_resolver: settings.file_resolver, + linter: settings.linter, + formatter: settings.formatter, + }), + ); + break; + } + Err(err) => { + tracing::error!( + "Error while resolving settings from {}: {err}", + pyproject.display() + ); + error = true; + continue; + } + } + } + Ok(None) => continue, + Err(err) => { + tracing::error!("{err}"); + error = true; continue; - }; - - respect_gitignore = Some(settings.file_resolver.respect_gitignore); - - index.insert( - directory.to_path_buf(), - Arc::new(RuffSettings { - path: Some(pyproject), - file_resolver: settings.file_resolver, - linter: settings.linter, - formatter: settings.formatter, - }), - ); - break; + } } } @@ -144,6 +162,8 @@ impl RuffSettingsIndex { let walker = builder.build_parallel(); let index = std::sync::RwLock::new(index); + let error = AtomicBool::new(error); + walker.run(|| { Box::new(|result| { let Ok(entry) = result else { @@ -186,29 +206,51 @@ impl RuffSettingsIndex { } } - if let Some(pyproject) = settings_toml(&directory).ok().flatten() { - let Ok(settings) = ruff_workspace::resolver::resolve_root_settings( - &pyproject, - Relativity::Parent, - &EditorConfigurationTransformer(editor_settings, root), - ) else { - return WalkState::Continue; - }; - index.write().unwrap().insert( - directory, - Arc::new(RuffSettings { - path: Some(pyproject), - file_resolver: settings.file_resolver, - linter: settings.linter, - formatter: settings.formatter, - }), - ); + match settings_toml(&directory) { + Ok(Some(pyproject)) => { + match ruff_workspace::resolver::resolve_root_settings( + &pyproject, + Relativity::Parent, + &EditorConfigurationTransformer(editor_settings, root), + ) { + Ok(settings) => { + index.write().unwrap().insert( + directory, + Arc::new(RuffSettings { + path: Some(pyproject), + file_resolver: settings.file_resolver, + linter: settings.linter, + formatter: settings.formatter, + }), + ); + } + Err(err) => { + tracing::error!( + "Error while resolving settings from {}: {err}", + pyproject.display() + ); + error.store(true, Ordering::Relaxed); + } + } + } + Ok(None) => {} + Err(err) => { + tracing::error!("{err}"); + error.store(true, Ordering::Relaxed); + } } WalkState::Continue }) }); + if error.load(Ordering::Relaxed) { + let root = root.display(); + show_err_msg!( + "Error while resolving settings from workspace {root}. Please refer to the logs for more details.", + ); + } + Self { index: index.into_inner().unwrap(), fallback,