mirror of
				https://github.com/astral-sh/ruff.git
				synced 2025-10-31 20:08:19 +00:00 
			
		
		
		
	[ty] Warn users if server received unknown options (#19779)
## Summary This PR updates the client settings handling to recognize unknown options provided by the user and show a warning popup along with a warning log message. ## Test Plan Add E2E tests.
This commit is contained in:
		
							parent
							
								
									1f29a04e9a
								
							
						
					
					
						commit
						fa711fa40f
					
				
					 4 changed files with 90 additions and 2 deletions
				
			
		|  | @ -98,6 +98,15 @@ impl Server { | ||||||
|         let (main_loop_sender, main_loop_receiver) = crossbeam::channel::bounded(32); |         let (main_loop_sender, main_loop_receiver) = crossbeam::channel::bounded(32); | ||||||
|         let client = Client::new(main_loop_sender.clone(), connection.sender.clone()); |         let client = Client::new(main_loop_sender.clone(), connection.sender.clone()); | ||||||
| 
 | 
 | ||||||
|  |         if !initialization_options.options.unknown.is_empty() { | ||||||
|  |             let options = serde_json::to_string_pretty(&initialization_options.options.unknown) | ||||||
|  |                 .unwrap_or_else(|_| "<invalid JSON>".to_string()); | ||||||
|  |             tracing::warn!("Received unknown options during initialization: {options}"); | ||||||
|  |             client.show_warning_message(format_args!( | ||||||
|  |                 "Received unknown options during initialization: {options}" | ||||||
|  |             )); | ||||||
|  |         } | ||||||
|  | 
 | ||||||
|         // Get workspace URLs without settings - settings will come from workspace/configuration
 |         // Get workspace URLs without settings - settings will come from workspace/configuration
 | ||||||
|         let workspace_urls = workspace_folders |         let workspace_urls = workspace_folders | ||||||
|             .filter(|folders| !folders.is_empty()) |             .filter(|folders| !folders.is_empty()) | ||||||
|  |  | ||||||
|  | @ -450,12 +450,23 @@ impl Session { | ||||||
| 
 | 
 | ||||||
|             // Combine the global options specified during initialization with the
 |             // Combine the global options specified during initialization with the
 | ||||||
|             // workspace-specific options to create the final workspace options.
 |             // workspace-specific options to create the final workspace options.
 | ||||||
|             let ClientOptions { global, workspace } = self |             let ClientOptions { | ||||||
|  |                 global, workspace, .. | ||||||
|  |             } = self | ||||||
|                 .initialization_options |                 .initialization_options | ||||||
|                 .options |                 .options | ||||||
|                 .clone() |                 .clone() | ||||||
|                 .combine(options.clone()); |                 .combine(options.clone()); | ||||||
| 
 | 
 | ||||||
|  |             if !options.unknown.is_empty() { | ||||||
|  |                 let options = serde_json::to_string_pretty(&options.unknown) | ||||||
|  |                     .unwrap_or_else(|_| "<invalid JSON>".to_string()); | ||||||
|  |                 tracing::warn!("Received unknown options for workspace `{url}`: {options}"); | ||||||
|  |                 client.show_warning_message(format!( | ||||||
|  |                     "Received unknown options for workspace `{url}`: {options}", | ||||||
|  |                 )); | ||||||
|  |             } | ||||||
|  | 
 | ||||||
|             combined_global_options.combine_with(Some(global)); |             combined_global_options.combine_with(Some(global)); | ||||||
| 
 | 
 | ||||||
|             let workspace_settings = workspace.into_settings(); |             let workspace_settings = workspace.into_settings(); | ||||||
|  |  | ||||||
|  | @ -1,3 +1,5 @@ | ||||||
|  | use std::collections::HashMap; | ||||||
|  | 
 | ||||||
| use lsp_types::Url; | use lsp_types::Url; | ||||||
| use ruff_db::system::SystemPathBuf; | use ruff_db::system::SystemPathBuf; | ||||||
| use ruff_macros::Combine; | use ruff_macros::Combine; | ||||||
|  | @ -84,6 +86,11 @@ pub struct ClientOptions { | ||||||
| 
 | 
 | ||||||
|     #[serde(flatten)] |     #[serde(flatten)] | ||||||
|     pub(crate) workspace: WorkspaceOptions, |     pub(crate) workspace: WorkspaceOptions, | ||||||
|  | 
 | ||||||
|  |     /// Additional options that aren't valid as per the schema but we accept it to provide better
 | ||||||
|  |     /// error message to the user.
 | ||||||
|  |     #[serde(flatten)] | ||||||
|  |     pub(crate) unknown: HashMap<String, Value>, | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| impl ClientOptions { | impl ClientOptions { | ||||||
|  | @ -98,6 +105,12 @@ impl ClientOptions { | ||||||
|         self.workspace.disable_language_services = Some(disable_language_services); |         self.workspace.disable_language_services = Some(disable_language_services); | ||||||
|         self |         self | ||||||
|     } |     } | ||||||
|  | 
 | ||||||
|  |     #[must_use] | ||||||
|  |     pub fn with_unknown(mut self, unknown: HashMap<String, Value>) -> Self { | ||||||
|  |         self.unknown = unknown; | ||||||
|  |         self | ||||||
|  |     } | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| /// Options that are global to the language server.
 | /// Options that are global to the language server.
 | ||||||
|  |  | ||||||
|  | @ -1,6 +1,7 @@ | ||||||
| use anyhow::Result; | use anyhow::Result; | ||||||
| use lsp_types::{Position, request::RegisterCapability}; | use lsp_types::{Position, notification::ShowMessage, request::RegisterCapability}; | ||||||
| use ruff_db::system::SystemPath; | use ruff_db::system::SystemPath; | ||||||
|  | use serde_json::Value; | ||||||
| use ty_server::{ClientOptions, DiagnosticMode}; | use ty_server::{ClientOptions, DiagnosticMode}; | ||||||
| 
 | 
 | ||||||
| use crate::TestServerBuilder; | use crate::TestServerBuilder; | ||||||
|  | @ -382,3 +383,57 @@ def bar() -> str: | ||||||
| 
 | 
 | ||||||
|     Ok(()) |     Ok(()) | ||||||
| } | } | ||||||
|  | 
 | ||||||
|  | /// Tests that the server sends a warning notification if user provided unknown options during
 | ||||||
|  | /// initialization.
 | ||||||
|  | #[test] | ||||||
|  | fn unknown_initialization_options() -> Result<()> { | ||||||
|  |     let workspace_root = SystemPath::new("foo"); | ||||||
|  |     let mut server = TestServerBuilder::new()? | ||||||
|  |         .with_workspace(workspace_root, None)? | ||||||
|  |         .with_initialization_options( | ||||||
|  |             ClientOptions::default() | ||||||
|  |                 .with_unknown([("foo".to_string(), Value::String("bar".to_string()))].into()), | ||||||
|  |         ) | ||||||
|  |         .build()? | ||||||
|  |         .wait_until_workspaces_are_initialized()?; | ||||||
|  | 
 | ||||||
|  |     let show_message_params = server.await_notification::<ShowMessage>()?; | ||||||
|  | 
 | ||||||
|  |     insta::assert_json_snapshot!(show_message_params, @r#" | ||||||
|  |     { | ||||||
|  |       "type": 2, | ||||||
|  |       "message": "Received unknown options during initialization: {\n  /"foo/": /"bar/"\n}" | ||||||
|  |     } | ||||||
|  |     "#);
 | ||||||
|  | 
 | ||||||
|  |     Ok(()) | ||||||
|  | } | ||||||
|  | 
 | ||||||
|  | /// Tests that the server sends a warning notification if user provided unknown options in the
 | ||||||
|  | /// workspace configuration.
 | ||||||
|  | #[test] | ||||||
|  | fn unknown_options_in_workspace_configuration() -> Result<()> { | ||||||
|  |     let workspace_root = SystemPath::new("foo"); | ||||||
|  |     let mut server = TestServerBuilder::new()? | ||||||
|  |         .with_workspace( | ||||||
|  |             workspace_root, | ||||||
|  |             Some( | ||||||
|  |                 ClientOptions::default() | ||||||
|  |                     .with_unknown([("foo".to_string(), Value::String("bar".to_string()))].into()), | ||||||
|  |             ), | ||||||
|  |         )? | ||||||
|  |         .build()? | ||||||
|  |         .wait_until_workspaces_are_initialized()?; | ||||||
|  | 
 | ||||||
|  |     let show_message_params = server.await_notification::<ShowMessage>()?; | ||||||
|  | 
 | ||||||
|  |     insta::assert_json_snapshot!(show_message_params, @r#" | ||||||
|  |     { | ||||||
|  |       "type": 2, | ||||||
|  |       "message": "Received unknown options for workspace `file://<temp_dir>/foo`: {\n  /"foo/": /"bar/"\n}" | ||||||
|  |     } | ||||||
|  |     "#);
 | ||||||
|  | 
 | ||||||
|  |     Ok(()) | ||||||
|  | } | ||||||
|  |  | ||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Dhruv Manilawala
						Dhruv Manilawala