From ba31075161fcc595aa1aee5ca6055f25735bc980 Mon Sep 17 00:00:00 2001 From: Austin Henriksen Date: Tue, 13 Feb 2024 11:24:01 -0500 Subject: [PATCH 1/2] Centralize server-level config in new 'ServerConfig' struct. --- server/src/configuration_set.rs | 64 ++++++-------------------- server/src/diagnostic_ext.rs | 11 +++-- server/src/main.rs | 35 ++++++++------- server/src/session.rs | 62 +++++++++----------------- server/src/slice_config.rs | 79 ++++++++++++++++++++------------- 5 files changed, 108 insertions(+), 143 deletions(-) diff --git a/server/src/configuration_set.rs b/server/src/configuration_set.rs index fa68828..aa0df48 100644 --- a/server/src/configuration_set.rs +++ b/server/src/configuration_set.rs @@ -1,29 +1,18 @@ // Copyright (c) ZeroC, Inc. -use crate::slice_config; +use crate::slice_config::{compute_slice_options, ServerConfig, SliceConfig}; use crate::utils::sanitize_path; -use std::path::{Path, PathBuf}; use std::collections::HashMap; -use slice_config::SliceConfig; use slicec::slice_options::SliceOptions; use slicec::{ast::Ast, diagnostics::Diagnostic, slice_file::SliceFile}; use slicec::compilation_state::CompilationState; -#[derive(Debug)] +#[derive(Debug, Default)] pub struct CompilationData { pub ast: Ast, pub files: HashMap, } -impl Default for CompilationData { - fn default() -> Self { - Self { - ast: Ast::create(), - files: HashMap::default(), - } - } -} - // Necessary for using `CompilationData` within async functions. // // # Safety @@ -33,7 +22,7 @@ impl Default for CompilationData { unsafe impl Send for CompilationData {} unsafe impl Sync for CompilationData {} -#[derive(Debug)] +#[derive(Debug, Default)] pub struct ConfigurationSet { pub slice_config: SliceConfig, pub compilation_data: CompilationData, @@ -42,53 +31,28 @@ pub struct ConfigurationSet { } impl ConfigurationSet { - /// Creates a new `ConfigurationSet` using the given root and built-in-slice paths. - pub fn new(root_path: PathBuf, built_in_path: String) -> Self { - let slice_config = SliceConfig { - paths: vec![], - workspace_root_path: Some(root_path), - built_in_slice_path: Some(built_in_path), - }; - let compilation_data = CompilationData::default(); - Self { slice_config, compilation_data, cached_slice_options: None } - } - - /// Parses a vector of `ConfigurationSet` from a JSON array, root path, and built-in path. - pub fn parse_configuration_sets( - config_array: &[serde_json::Value], - root_path: &Path, - built_in_path: &str, - ) -> Vec { + /// Parses a vector of `ConfigurationSet` from a JSON array. + pub fn parse_configuration_sets(config_array: &[serde_json::Value]) -> Vec { config_array .iter() - .map(|value| ConfigurationSet::from_json(value, root_path, built_in_path)) + .map(ConfigurationSet::from_json) .collect::>() } /// Constructs a `ConfigurationSet` from a JSON value. - fn from_json(value: &serde_json::Value, root_path: &Path, built_in_path: &str) -> Self { - // Parse the paths and `include_built_in_types` from the configuration set - let paths = parse_paths(value); - let include_built_in = parse_include_built_in(value); - - // Create the SliceConfig and CompilationState + fn from_json(value: &serde_json::Value) -> Self { let slice_config = SliceConfig { - paths, - workspace_root_path: Some(root_path.to_owned()), - built_in_slice_path: include_built_in.then(|| built_in_path.to_owned()), + slice_search_paths: parse_paths(value), + include_built_in_slice_files: parse_include_built_in(value), }; - let compilation_data = CompilationData::default(); - Self { slice_config, compilation_data, cached_slice_options: None } + Self { slice_config, ..Self::default() } } - pub fn trigger_compilation(&mut self) -> Vec { + pub fn trigger_compilation(&mut self, server_config: &ServerConfig) -> Vec { // Re-compute the `slice_options` we're going to pass into the compiler, if necessary. - if self.cached_slice_options.is_none() { - let mut slice_options = SliceOptions::default(); - slice_options.references = self.slice_config.resolve_paths(); - self.cached_slice_options = Some(slice_options); - } - let slice_options = self.cached_slice_options.as_ref().unwrap(); + let slice_options = self.cached_slice_options.get_or_insert_with(|| { + compute_slice_options(server_config, &self.slice_config) + }); // Perform the compilation. let compilation_state = slicec::compile_from_options(slice_options, |_| {}, |_| {}); diff --git a/server/src/diagnostic_ext.rs b/server/src/diagnostic_ext.rs index 908f70a..1b42b23 100644 --- a/server/src/diagnostic_ext.rs +++ b/server/src/diagnostic_ext.rs @@ -1,6 +1,7 @@ // Copyright (c) ZeroC, Inc. use crate::configuration_set::ConfigurationSet; +use crate::session::Session; use crate::utils::convert_slice_url_to_uri; use crate::{notifications, show_popup}; @@ -48,11 +49,9 @@ pub async fn publish_diagnostics_for_set( /// Triggers and compilation and publishes any diagnostics that are reported. /// It does this for all configuration sets. -pub async fn compile_and_publish_diagnostics( - client: &Client, - configuration_sets: &Mutex>, -) { - let mut configuration_sets = configuration_sets.lock().await; +pub async fn compile_and_publish_diagnostics(client: &Client, session: &Session) { + let mut configuration_sets = session.configuration_sets.lock().await; + let server_config = session.server_config.read().await; client .log_message( @@ -62,7 +61,7 @@ pub async fn compile_and_publish_diagnostics( .await; for configuration_set in configuration_sets.iter_mut() { // Trigger a compilation and get any diagnostics that were reported during it. - let diagnostics = configuration_set.trigger_compilation(); + let diagnostics = configuration_set.trigger_compilation(&server_config); // Publish those diagnostics. publish_diagnostics_for_set(client, diagnostics, configuration_set).await; } diff --git a/server/src/main.rs b/server/src/main.rs index 603a8e4..47dc032 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -1,15 +1,15 @@ // Copyright (c) ZeroC, Inc. -use diagnostic_ext::{clear_diagnostics, compile_and_publish_diagnostics, process_diagnostics}; -use hover::try_into_hover_result; -use jump_definition::get_definition_span; -use notifications::{ShowNotification, ShowNotificationParams}; +use crate::diagnostic_ext::{clear_diagnostics, compile_and_publish_diagnostics, process_diagnostics}; +use crate::hover::try_into_hover_result; +use crate::jump_definition::get_definition_span; +use crate::notifications::{ShowNotification, ShowNotificationParams}; +use crate::session::Session; +use crate::slice_config::compute_slice_options; use std::{collections::HashMap, path::Path}; use tower_lsp::{jsonrpc::Error, lsp_types::*, Client, LanguageServer, LspService, Server}; use utils::{convert_slice_url_to_uri, url_to_sanitized_file_path, FindFile}; -use crate::session::Session; - mod configuration_set; mod diagnostic_ext; mod hover; @@ -35,7 +35,7 @@ struct Backend { impl Backend { pub fn new(client: tower_lsp::Client) -> Self { - let session = Session::new(); + let session = Session::default(); Self { client, session } } @@ -77,19 +77,24 @@ impl Backend { .await; let mut configuration_sets = self.session.configuration_sets.lock().await; + let server_config = self.session.server_config.read().await; + let mut publish_map = HashMap::new(); let mut diagnostics = Vec::new(); // Process each configuration set that contains the changed file for set in configuration_sets.iter_mut().filter(|set| { - set.slice_config.resolve_paths().into_iter().any(|f| { - let key_path = Path::new(&f); - let file_path = Path::new(file_name); - key_path == file_path || file_path.starts_with(key_path) - }) + compute_slice_options(&server_config, &set.slice_config) + .references + .into_iter() + .any(|f| { + let key_path = Path::new(&f); + let file_path = Path::new(file_name); + key_path == file_path || file_path.starts_with(key_path) + }) }) { // `trigger_compilation` compiles the configuration set's files and returns any diagnostics. - diagnostics.extend(set.trigger_compilation()); + diagnostics.extend(set.trigger_compilation(&server_config)); // Update publish_map with files to be updated publish_map.extend( @@ -149,7 +154,7 @@ impl LanguageServer for Backend { async fn initialized(&self, _: InitializedParams) { // Now that the server and client are fully initialized, it's safe to compile and publish any diagnostics. - compile_and_publish_diagnostics(&self.client, &self.session.configuration_sets).await; + compile_and_publish_diagnostics(&self.client, &self.session).await; } async fn shutdown(&self) -> tower_lsp::jsonrpc::Result<()> { @@ -169,7 +174,7 @@ impl LanguageServer for Backend { self.session.update_configurations_from_params(params).await; // Trigger a compilation and publish the diagnostics for all files - compile_and_publish_diagnostics(&self.client, &self.session.configuration_sets).await; + compile_and_publish_diagnostics(&self.client, &self.session).await; } async fn goto_definition( diff --git a/server/src/session.rs b/server/src/session.rs index 10c075b..ae65691 100644 --- a/server/src/session.rs +++ b/server/src/session.rs @@ -1,33 +1,23 @@ // Copyright (c) ZeroC, Inc. -use crate::{ - configuration_set::ConfigurationSet, - utils::{sanitize_path, url_to_sanitized_file_path}, -}; +use crate::configuration_set::ConfigurationSet; +use crate::slice_config::ServerConfig; +use crate::utils::{sanitize_path, url_to_sanitized_file_path}; use std::path::PathBuf; use tokio::sync::{Mutex, RwLock}; use tower_lsp::lsp_types::DidChangeConfigurationParams; +#[derive(Debug, Default)] pub struct Session { /// This vector contains all of the configuration sets for the language server. Each element is a tuple containing /// `SliceConfig` and `CompilationState`. The `SliceConfig` is used to determine which configuration set to use when /// publishing diagnostics. The `CompilationState` is used to retrieve the diagnostics for a given file. pub configuration_sets: Mutex>, - /// This is the root path of the workspace. It is used to resolve relative paths in the configuration. - pub root_path: RwLock>, - /// This is the path to the built-in Slice files that are included with the extension. - pub built_in_slice_path: RwLock, + /// Configuration that affects the entire server. + pub server_config: RwLock, } impl Session { - pub fn new() -> Self { - Self { - configuration_sets: Mutex::new(Vec::new()), - root_path: RwLock::new(None), - built_in_slice_path: RwLock::new(String::new()), - } - } - // Update the properties of the session from `InitializeParams` pub async fn update_from_initialize_params( &self, @@ -35,6 +25,16 @@ impl Session { ) { let initialization_options = params.initialization_options; + // Use the root_uri if it exists temporarily as we cannot access configuration until + // after initialization. Additionally, LSP may provide the windows path with escaping or a lowercase + // drive letter. To fix this, we convert the path to a URL and then back to a path. + let workspace_root_path = params + .root_uri + .and_then(|uri| url_to_sanitized_file_path(&uri)) + .map(PathBuf::from) + .map(|path| path.display().to_string()) + .expect("`root_uri` was not sent by the client, or was malformed"); + // This is the path to the built-in Slice files that are included with the extension. It should always // be present. let built_in_slice_path = initialization_options @@ -44,43 +44,28 @@ impl Session { .map(sanitize_path) .expect("builtInSlicePath not found in initialization options"); - // Use the root_uri if it exists temporarily as we cannot access configuration until - // after initialization. Additionally, LSP may provide the windows path with escaping or a lowercase - // drive letter. To fix this, we convert the path to a URL and then back to a path. - let root_path = params - .root_uri - .and_then(|uri| url_to_sanitized_file_path(&uri)) - .map(PathBuf::from) - .expect("`root_uri` was not sent by the client, or was malformed"); + *self.server_config.write().await = ServerConfig { workspace_root_path, built_in_slice_path }; // Load any user configuration from the 'slice.configurations' option. let configuration_sets = initialization_options .as_ref() .and_then(|opts| opts.get("configurations")) .and_then(|v| v.as_array()) - .map(|arr| { - ConfigurationSet::parse_configuration_sets(arr, &root_path, &built_in_slice_path) - }) + .map(|arr| ConfigurationSet::parse_configuration_sets(arr)) .unwrap_or_default(); - *self.built_in_slice_path.write().await = built_in_slice_path; - *self.root_path.write().await = Some(root_path); self.update_configurations(configuration_sets).await; } // Update the configuration sets from the `DidChangeConfigurationParams` notification. pub async fn update_configurations_from_params(&self, params: DidChangeConfigurationParams) { - let built_in_path = &self.built_in_slice_path.read().await; - let root_path_guard = self.root_path.read().await; - let root_path = (*root_path_guard).clone().expect("root_path not set"); - // Parse the configurations from the notification let configurations = params .settings .get("slice") .and_then(|v| v.get("configurations")) .and_then(|v| v.as_array()) - .map(|arr| ConfigurationSet::parse_configuration_sets(arr, &root_path, built_in_path)) + .map(|arr| ConfigurationSet::parse_configuration_sets(arr)) .unwrap_or_default(); // Update the configuration sets @@ -92,14 +77,9 @@ impl Session { async fn update_configurations(&self, mut configurations: Vec) { // Insert the default configuration set if needed if configurations.is_empty() { - let root_path = self.root_path.read().await; - let built_in_slice_path = self.built_in_slice_path.read().await; - let default = - ConfigurationSet::new(root_path.clone().unwrap(), built_in_slice_path.clone()); - configurations.push(default); + configurations.push(ConfigurationSet::default()); } - let mut configuration_sets = self.configuration_sets.lock().await; - *configuration_sets = configurations; + *self.configuration_sets.lock().await = configurations; } } diff --git a/server/src/slice_config.rs b/server/src/slice_config.rs index 959910c..6b25562 100644 --- a/server/src/slice_config.rs +++ b/server/src/slice_config.rs @@ -1,45 +1,62 @@ // Copyright (c) ZeroC, Inc. -use std::path::{Path, PathBuf}; +use std::path::Path; -// This struct holds the configuration for a single compilation set. +use slicec::slice_options::SliceOptions; + +/// This struct holds configuration that affects the entire server. +#[derive(Debug, Default)] +pub struct ServerConfig { + /// This is the root path of the workspace, used to resolve relative paths. It must be an absolute path. + pub workspace_root_path: String, + /// This is the path to the built-in Slice files that are included with the extension. It must be an absolute path. + pub built_in_slice_path: String, +} + +/// This struct holds the configuration for a single compilation set. #[derive(Debug)] pub struct SliceConfig { - pub paths: Vec, - pub workspace_root_path: Option, - pub built_in_slice_path: Option, + /// List of paths that will be passed to the compiler as reference files/directories. + pub slice_search_paths: Vec, + /// Specifies whether to include the built-in Slice files that are bundled with the extension. + pub include_built_in_slice_files: bool, } -impl SliceConfig { - // Resolve path URIs to file paths to be used by the Slice compiler. - pub fn resolve_paths(&self) -> Vec { - // If `root_path` isn't set, relative path resolution is impossible, so we return. - let Some(root_path) = &self.workspace_root_path else { - return vec![]; - }; +impl Default for SliceConfig { + fn default() -> Self { + SliceConfig { + slice_search_paths: vec![], + include_built_in_slice_files: true, + } + } +} - let mut resolved_paths = Vec::new(); - for string_path in &self.paths { - let path = Path::new(string_path); +pub fn compute_slice_options(server_config: &ServerConfig, unit_config: &SliceConfig) -> SliceOptions { + let root_path = Path::new(&server_config.workspace_root_path); + let mut slice_options = SliceOptions::default(); + let references = &mut slice_options.references; - // If the path is absolute, add it as-is. Otherwise, preface it with the workspace root. - let absolute_path = match path.is_absolute() { - true => path.to_owned(), - false => root_path.join(path), - }; - resolved_paths.push(absolute_path.display().to_string()); - } + // Add any user specified search paths at the front of the list. + for string_path in &unit_config.slice_search_paths { + let path = Path::new(string_path); - // If the user didn't specify any paths, default to using the workspace root. - if resolved_paths.is_empty() { - resolved_paths.push(root_path.display().to_string()); - } + // If the path is absolute, add it as-is. Otherwise, preface it with the workspace root. + let absolute_path = match path.is_absolute() { + true => path.to_owned(), + false => root_path.join(path), + }; + references.push(absolute_path.display().to_string()); + } - // Add the built-in Slice files (WellKnownTypes, etc.) to the end of the list if it's present. - if let Some(path) = &self.built_in_slice_path { - resolved_paths.push(path.clone()); - } + // If the user didn't specify any paths, default to using the workspace root. + if references.is_empty() { + references.push(root_path.display().to_string()); + } - resolved_paths + // Add the built-in Slice files (WellKnownTypes, etc.) to the end of the list, if they should be included. + if unit_config.include_built_in_slice_files { + references.push(server_config.built_in_slice_path.clone()); } + + slice_options } From 4f0633a6a5b8973cf62e227cd4b6723dafc52fb1 Mon Sep 17 00:00:00 2001 From: Austin Henriksen Date: Tue, 13 Feb 2024 12:08:08 -0500 Subject: [PATCH 2/2] Rename variable to 'set_config'. --- server/src/slice_config.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/slice_config.rs b/server/src/slice_config.rs index 6b25562..c4788a7 100644 --- a/server/src/slice_config.rs +++ b/server/src/slice_config.rs @@ -31,13 +31,13 @@ impl Default for SliceConfig { } } -pub fn compute_slice_options(server_config: &ServerConfig, unit_config: &SliceConfig) -> SliceOptions { +pub fn compute_slice_options(server_config: &ServerConfig, set_config: &SliceConfig) -> SliceOptions { let root_path = Path::new(&server_config.workspace_root_path); let mut slice_options = SliceOptions::default(); let references = &mut slice_options.references; // Add any user specified search paths at the front of the list. - for string_path in &unit_config.slice_search_paths { + for string_path in &set_config.slice_search_paths { let path = Path::new(string_path); // If the path is absolute, add it as-is. Otherwise, preface it with the workspace root. @@ -54,7 +54,7 @@ pub fn compute_slice_options(server_config: &ServerConfig, unit_config: &SliceCo } // Add the built-in Slice files (WellKnownTypes, etc.) to the end of the list, if they should be included. - if unit_config.include_built_in_slice_files { + if set_config.include_built_in_slice_files { references.push(server_config.built_in_slice_path.clone()); }