From d21fa343915eb5b22502bd7a3023429b681ae66e Mon Sep 17 00:00:00 2001 From: Lewis Russell Date: Wed, 15 Apr 2026 12:21:13 +0100 Subject: [PATCH] fix(config): resolve file-relative paths before merge Preprocess each config file while its declaring directory is still known, then write back only the resulting path changes before merging. This keeps library/package/ignoreDir/resource path resolution correct without materializing defaults into unrelated fields. Keep the raw single-file config loader for edit commands and cover the sparse merge behavior with config loader regressions. --- .../src/config/config_loader.rs | 439 +++++++++++++++--- .../emmylua_code_analysis/src/config/mod.rs | 2 +- .../src/config/pre_process.rs | 82 +++- .../command/commands/emmy_add_doc_tag.rs | 4 +- .../command/commands/emmy_disable_code.rs | 4 +- 5 files changed, 461 insertions(+), 70 deletions(-) diff --git a/crates/emmylua_code_analysis/src/config/config_loader.rs b/crates/emmylua_code_analysis/src/config/config_loader.rs index e54e101ba..cb3b2b21f 100644 --- a/crates/emmylua_code_analysis/src/config/config_loader.rs +++ b/crates/emmylua_code_analysis/src/config/config_loader.rs @@ -6,89 +6,161 @@ use crate::{config::lua_loader::load_lua_config, read_file_with_encoding}; use super::{Emmyrc, flatten_config::FlattenConfigObject}; -pub fn load_configs_raw(config_files: Vec, partial_emmyrcs: Option>) -> Value { - let mut config_jsons = Vec::new(); +/// Load a config file into an Emmyrc-shaped JSON value without path preprocessing. +/// +/// This keeps file-relative paths exactly as written in the source config. It is +/// intended for callers that need to inspect or edit the raw config shape before +/// writing it back to disk. +pub fn load_config_json_unprocessed(config_file: PathBuf) -> Value { + let Some(config_value) = load_config_file_value(&config_file) else { + log::info!("No valid config file found."); + return Value::Object(Default::default()); + }; + + FlattenConfigObject::parse(config_value).to_emmyrc() +} + +/// Load config files into a final [`Emmyrc`]. +/// +/// Unlike [`load_config_json_unprocessed`], this is the semantic runtime loader. +/// Each file config is flattened and preprocessed relative to the directory that +/// declared it before configs are merged. That has to happen pre-merge because +/// relative settings such as `library`, `packages`, and per-entry ignores lose +/// their originating config path once they have been folded into one JSON value. +pub fn load_configs(config_files: Vec, partial_emmyrcs: Option>) -> Emmyrc { + let mut merged_emmyrc = None; for config_file in config_files { - log::info!("Loading config file: {:?}", config_file); - let config_content = match read_file_with_encoding(&config_file, "utf-8") { - Some(content) => content, - None => { - log::error!( - "Failed to read config file: {:?}, error: File not found or unreadable", - config_file - ); - continue; - } + let Some(config_value) = load_config_file_value(&config_file) else { + continue; }; - let config_value = if config_file.extension().and_then(|s| s.to_str()) == Some("lua") { - match load_lua_config(&config_content) { - Ok(value) => value, - Err(e) => { - log::error!( - "Failed to parse lua config file: {:?}, error: {:?}", - &config_file, - e - ); - continue; - } - } - } else { - match serde_json::from_str(&config_content) { - Ok(json) => json, - Err(e) => { - log::error!( - "Failed to parse config file: {:?}, error: {:?}", - &config_file, - e - ); - continue; - } + let mut config_emmyrc = FlattenConfigObject::parse(config_value).to_emmyrc(); + if let Some(config_root) = config_file.parent() { + // Relative config paths only make sense while we still know which + // config file they came from. + if let Ok(mut emmyrc) = serde_json::from_value::(config_emmyrc.clone()) { + let original_emmyrc = serde_json::to_value(&emmyrc).unwrap(); + emmyrc.pre_process_emmyrc(config_root); + // Write back only the leaves changed by preprocessing. Replacing + // the whole config would materialize defaults and change merge + // semantics for sparse per-file configs. + apply_changed_values( + &mut config_emmyrc, + &original_emmyrc, + serde_json::to_value(emmyrc).unwrap(), + ); } - }; + } - config_jsons.push(config_value); + if let Err(err) = serde_json::from_value::(config_emmyrc.clone()) { + log::error!( + "Failed to parse config file as emmyrc {:?}: {:?}", + config_file, + err + ); + continue; + } + + if let Some(merged_emmyrc) = merged_emmyrc.as_mut() { + merge_values(merged_emmyrc, config_emmyrc); + } else { + merged_emmyrc = Some(config_emmyrc); + } } if let Some(partial_emmyrcs) = partial_emmyrcs { + // Partial configs are late overlays from the client, so they win over + // file-backed config. They are not file-relative; callers preprocess + // the merged runtime config against the active workspace root later. for partial_emmyrc in partial_emmyrcs { - config_jsons.push(partial_emmyrc); + let partial_emmyrc = FlattenConfigObject::parse(partial_emmyrc).to_emmyrc(); + if let Some(merged_emmyrc) = merged_emmyrc.as_mut() { + merge_values(merged_emmyrc, partial_emmyrc); + } else { + merged_emmyrc = Some(partial_emmyrc); + } } } - if config_jsons.is_empty() { + let Some(merged_emmyrc) = merged_emmyrc else { log::info!("No valid config file found."); - Value::Object(Default::default()) - } else if config_jsons.len() == 1 { - let first_config = config_jsons.into_iter().next().unwrap_or_else(|| { - log::error!("No valid config file found."); - Value::Object(Default::default()) - }); - - let flatten_config = FlattenConfigObject::parse(first_config); - flatten_config.to_emmyrc() - } else { - let merge_config = - config_jsons - .into_iter() - .fold(Value::Object(Default::default()), |mut acc, item| { - merge_values(&mut acc, item); - acc - }); - let flatten_config = FlattenConfigObject::parse(merge_config.clone()); - flatten_config.to_emmyrc() - } -} + return Emmyrc::default(); + }; -pub fn load_configs(config_files: Vec, partial_emmyrcs: Option>) -> Emmyrc { - let emmyrc_json_value = load_configs_raw(config_files, partial_emmyrcs); - serde_json::from_value(emmyrc_json_value).unwrap_or_else(|err| { + serde_json::from_value(merged_emmyrc).unwrap_or_else(|err| { log::error!("Failed to parse config: error: {:?}", err); Emmyrc::default() }) } +fn load_config_file_value(config_file: &PathBuf) -> Option { + log::info!("Loading config file: {:?}", config_file); + let config_content = match read_file_with_encoding(config_file, "utf-8") { + Some(content) => content, + None => { + log::error!( + "Failed to read config file: {:?}, error: File not found or unreadable", + config_file + ); + return None; + } + }; + + if config_file.extension().and_then(|s| s.to_str()) == Some("lua") { + match load_lua_config(&config_content) { + Ok(value) => Some(value), + Err(err) => { + log::error!( + "Failed to parse lua config file: {:?}, error: {:?}", + config_file, + err + ); + None + } + } + } else { + match serde_json::from_str(&config_content) { + Ok(json) => Some(json), + Err(err) => { + log::error!( + "Failed to parse config file: {:?}, error: {:?}", + config_file, + err + ); + None + } + } + } +} + +fn apply_changed_values(target: &mut Value, before: &Value, after: Value) { + match (target, before, after) { + (Value::Object(target_map), Value::Object(before_map), Value::Object(after_map)) => { + for (key, after_value) in after_map { + let Some(before_value) = before_map.get(&key) else { + continue; + }; + + if before_value == &after_value { + continue; + } + + if let Some(target_value) = target_map.get_mut(&key) { + apply_changed_values(target_value, before_value, after_value); + } else { + target_map.insert(key, after_value); + } + } + } + (target_slot, before_value, after_value) => { + if before_value != &after_value { + *target_slot = after_value; + } + } + } +} + fn merge_values(base: &mut Value, overlay: Value) { match (base, overlay) { (Value::Object(base_map), Value::Object(overlay_map)) => { @@ -116,3 +188,244 @@ fn merge_values(base: &mut Value, overlay: Value) { } } } + +#[cfg(test)] +mod tests { + use std::{ + fs, + path::{Path, PathBuf}, + sync::atomic::{AtomicU64, Ordering}, + time::{SystemTime, UNIX_EPOCH}, + }; + + use super::{Emmyrc, load_config_json_unprocessed, load_configs}; + + static TEST_CONFIG_COUNTER: AtomicU64 = AtomicU64::new(0); + + struct TestConfigRoot { + root: PathBuf, + } + + impl TestConfigRoot { + fn new() -> Self { + let unique = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos(); + let counter = TEST_CONFIG_COUNTER.fetch_add(1, Ordering::Relaxed); + let root = std::env::temp_dir().join(format!( + "emmylua-config-loader-{}-{}-{}", + std::process::id(), + unique, + counter, + )); + fs::create_dir_all(&root).unwrap(); + Self { root } + } + + fn write_file(&self, relative_path: &str, contents: &str) -> PathBuf { + let path = self.root.join(relative_path); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(&path, contents).unwrap(); + path + } + + fn path(&self, relative_path: &str) -> PathBuf { + self.root.join(relative_path) + } + } + + impl Drop for TestConfigRoot { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.root); + } + } + + fn to_string(path: &Path) -> String { + path.to_string_lossy().to_string() + } + + #[test] + fn merged_configs_should_resolve_relative_workspace_paths_from_declaring_config_file() { + let workspace = TestConfigRoot::new(); + let shared_config = workspace.write_file( + "shared/config/.luarc.json", + r#"{ + "workspace": { + "library": ["../shared-lib"] + } + }"#, + ); + let workspace_config = workspace.write_file( + "project/.luarc.json", + r#"{ + "workspace": { + "library": ["./vendor"] + } + }"#, + ); + let expected = vec![ + to_string(&workspace.path("shared/shared-lib")), + to_string(&workspace.path("project/vendor")), + ]; + + let emmyrc = load_configs(vec![shared_config, workspace_config], None); + + let actual = emmyrc + .workspace + .library + .iter() + .map(|item| item.get_path().clone()) + .collect::>(); + + assert_eq!(actual, expected); + } + + #[test] + fn path_preprocessing_preserves_unrelated_workspace_fields() { + let workspace = TestConfigRoot::new(); + let config = workspace.write_file( + "project/.luarc.json", + r#"{ + "workspace": { + "library": ["./vendor"], + "moduleMap": [ + { + "pattern": "^(.*)$", + "replace": "$1" + } + ] + } + }"#, + ); + + let emmyrc = load_configs(vec![config], None); + let actual = emmyrc + .workspace + .library + .iter() + .map(|item| item.get_path().clone()) + .collect::>(); + + assert_eq!(actual, vec![to_string(&workspace.path("project/vendor"))]); + assert_eq!(emmyrc.workspace.module_map.len(), 1); + assert_eq!(emmyrc.workspace.module_map[0].pattern, "^(.*)$"); + } + + #[test] + fn raw_config_loader_keeps_relative_workspace_paths_as_authored() { + let workspace = TestConfigRoot::new(); + let config = workspace.write_file( + "project/.luarc.json", + r#"{ + "workspace": { + "library": ["../shared-lib"] + } + }"#, + ); + + let emmyrc_value = load_config_json_unprocessed(config); + let emmyrc: Emmyrc = serde_json::from_value(emmyrc_value).unwrap(); + + let actual = emmyrc + .workspace + .library + .iter() + .map(|item| item.get_path().clone()) + .collect::>(); + + assert_eq!(actual, vec!["../shared-lib".to_string()]); + } + + #[test] + fn invalid_config_does_not_override_earlier_valid_settings() { + let workspace = TestConfigRoot::new(); + let valid_config = workspace.write_file( + "project/.luarc.json", + r#"{ + "diagnostics": { + "enable": false + } + }"#, + ); + let invalid_config = workspace.write_file( + "project/invalid.luarc.json", + r#"{ + "diagnostics": { + "enable": "oops" + } + }"#, + ); + + let emmyrc = load_configs(vec![valid_config, invalid_config], None); + + assert!(!emmyrc.diagnostics.enable); + } + + #[test] + fn dotted_partial_configs_are_flattened_before_merge() { + let emmyrc = load_configs( + vec![], + Some(vec![serde_json::json!({ + "diagnostics.enable": false + })]), + ); + + assert!(!emmyrc.diagnostics.enable); + } + + #[test] + fn later_valid_configs_do_not_restore_defaults() { + let workspace = TestConfigRoot::new(); + let first_config = workspace.write_file( + "project/first.luarc.json", + r#"{ + "diagnostics": { + "enable": false + } + }"#, + ); + let second_config = workspace.write_file( + "project/second.luarc.json", + r#"{ + "strict": { + "requirePath": true + } + }"#, + ); + + let emmyrc = load_configs(vec![first_config, second_config], None); + + assert!(!emmyrc.diagnostics.enable); + assert!(emmyrc.strict.require_path); + } + + #[test] + fn later_workspace_path_config_does_not_restore_workspace_defaults() { + let workspace = TestConfigRoot::new(); + let first_config = workspace.write_file( + "project/first.luarc.json", + r#"{ + "workspace": { + "enableReindex": true + } + }"#, + ); + let second_config = workspace.write_file( + "project/second.luarc.json", + r#"{ + "workspace": { + "library": ["./vendor"] + } + }"#, + ); + + let emmyrc = load_configs(vec![first_config, second_config], None); + + assert!(emmyrc.workspace.enable_reindex); + assert_eq!( + emmyrc.workspace.library[0].get_path(), + &to_string(&workspace.path("project/vendor")) + ); + } +} diff --git a/crates/emmylua_code_analysis/src/config/mod.rs b/crates/emmylua_code_analysis/src/config/mod.rs index e23432604..ae770ea5a 100644 --- a/crates/emmylua_code_analysis/src/config/mod.rs +++ b/crates/emmylua_code_analysis/src/config/mod.rs @@ -6,7 +6,7 @@ mod pre_process; use std::{collections::HashMap, path::Path}; -pub use config_loader::{load_configs, load_configs_raw}; +pub use config_loader::{load_config_json_unprocessed, load_configs}; pub use configs::{ DiagnosticSeveritySetting, DocSyntax, EmmyLibraryConfig, EmmyLibraryItem, EmmyrcCodeAction, EmmyrcCodeLens, EmmyrcCompletion, EmmyrcDiagnostic, EmmyrcDoc, EmmyrcDocumentColor, diff --git a/crates/emmylua_code_analysis/src/config/pre_process.rs b/crates/emmylua_code_analysis/src/config/pre_process.rs index 681d4c4e0..f6107ca4c 100644 --- a/crates/emmylua_code_analysis/src/config/pre_process.rs +++ b/crates/emmylua_code_analysis/src/config/pre_process.rs @@ -1,5 +1,9 @@ use regex::Regex; -use std::{collections::HashSet, path::PathBuf, process::Command}; +use std::{ + collections::HashSet, + path::{Component, PathBuf}, + process::Command, +}; use crate::config::configs::{EmmyrcWorkspacePathConfig, EmmyrcWorkspacePathItem}; @@ -95,7 +99,9 @@ impl PreProcessContext { path = self.workspace.join(&path).to_string_lossy().to_string(); } - path + normalize_path(PathBuf::from(path)) + .to_string_lossy() + .to_string() } fn pre_process_workspace_path_item( @@ -165,6 +171,31 @@ impl PreProcessContext { } } +fn normalize_path(path: PathBuf) -> PathBuf { + let mut normalized = PathBuf::new(); + for component in path.components() { + match component { + Component::CurDir => {} + Component::ParentDir => { + let can_pop = matches!( + normalized.components().next_back(), + Some(Component::Normal(_)) + ); + if can_pop { + normalized.pop(); + } else if !normalized.has_root() { + normalized.push(component.as_os_str()); + } + } + Component::Normal(_) | Component::RootDir | Component::Prefix(_) => { + normalized.push(component.as_os_str()); + } + } + } + + normalized +} + fn get_luarocks_deploy_dir() -> String { Command::new("luarocks") .args(["config", "deploy_lua_dir"]) @@ -179,3 +210,50 @@ fn get_luarocks_deploy_dir() -> String { }) .unwrap_or_default() } + +#[cfg(test)] +mod tests { + use crate::{EmmyLibraryConfig, EmmyLibraryItem, Emmyrc, EmmyrcWorkspacePathItem}; + + #[test] + fn pre_process_emmyrc_normalizes_parent_package_paths() { + let workspace = std::env::temp_dir() + .join("emmylua-pre-process") + .join("packages"); + let expected = workspace.parent().unwrap().to_string_lossy().to_string(); + let mut emmyrc = Emmyrc::default(); + emmyrc.workspace.packages = vec![EmmyrcWorkspacePathItem::Path("..".to_string())]; + + emmyrc.pre_process_emmyrc(&workspace); + + assert_eq!( + emmyrc.workspace.packages, + vec![EmmyrcWorkspacePathItem::Path(expected)] + ); + } + + #[test] + fn pre_process_workspace_path_config_resolves_ignore_dirs_from_entry_root() { + let workspace = std::env::temp_dir() + .join("emmylua-pre-process") + .join("workspace"); + let entry_root = workspace.join("vendor").join("socket"); + let expected_root = entry_root.to_string_lossy().to_string(); + let expected_ignore_dir = entry_root.join("tests").to_string_lossy().to_string(); + let mut emmyrc = Emmyrc::default(); + emmyrc.workspace.library = vec![EmmyLibraryItem::Config(EmmyLibraryConfig { + path: "vendor/socket".to_string(), + ignore_dir: vec!["tests".to_string()], + ignore_globs: vec!["**/*.spec.lua".to_string()], + })]; + + emmyrc.pre_process_emmyrc(&workspace); + + let EmmyLibraryItem::Config(config) = &emmyrc.workspace.library[0] else { + panic!("expected configured workspace path item"); + }; + assert_eq!(config.path, expected_root); + assert_eq!(config.ignore_dir, vec![expected_ignore_dir]); + assert_eq!(config.ignore_globs, vec!["**/*.spec.lua".to_string()]); + } +} diff --git a/crates/emmylua_ls/src/handlers/command/commands/emmy_add_doc_tag.rs b/crates/emmylua_ls/src/handlers/command/commands/emmy_add_doc_tag.rs index 154ae8228..487a65953 100644 --- a/crates/emmylua_ls/src/handlers/command/commands/emmy_add_doc_tag.rs +++ b/crates/emmylua_ls/src/handlers/command/commands/emmy_add_doc_tag.rs @@ -1,6 +1,6 @@ use std::{fs::OpenOptions, io::Write}; -use emmylua_code_analysis::load_configs_raw; +use emmylua_code_analysis::load_config_json_unprocessed; use lsp_types::Command; use serde_json::Value; use tokio::sync::RwLock; @@ -35,7 +35,7 @@ async fn add_doc_tag(workspace_manager: &RwLock, tag_name: Str let workspace_manager = workspace_manager.read().await; let main_workspace = workspace_manager.workspace_folders.first()?; let emmyrc_path = main_workspace.root.join(".emmyrc.json"); - let mut emmyrc = load_configs_raw(vec![emmyrc_path.clone()], None); + let mut emmyrc = load_config_json_unprocessed(emmyrc_path.clone()); drop(workspace_manager); emmyrc diff --git a/crates/emmylua_ls/src/handlers/command/commands/emmy_disable_code.rs b/crates/emmylua_ls/src/handlers/command/commands/emmy_disable_code.rs index d1967e7fe..217895964 100644 --- a/crates/emmylua_ls/src/handlers/command/commands/emmy_disable_code.rs +++ b/crates/emmylua_ls/src/handlers/command/commands/emmy_disable_code.rs @@ -1,6 +1,6 @@ use std::{fs::OpenOptions, io::Write}; -use emmylua_code_analysis::{DiagnosticCode, FileId, load_configs_raw}; +use emmylua_code_analysis::{DiagnosticCode, FileId, load_config_json_unprocessed}; use lsp_types::{Command, Range}; use serde::{Deserialize, Serialize}; use serde_json::Value; @@ -63,7 +63,7 @@ async fn add_disable_project( let workspace_manager = workspace_manager.read().await; let main_workspace = workspace_manager.workspace_folders.first()?; let emmyrc_path = main_workspace.root.join(".emmyrc.json"); - let mut emmyrc = load_configs_raw(vec![emmyrc_path.clone()], None); + let mut emmyrc = load_config_json_unprocessed(emmyrc_path.clone()); drop(workspace_manager); emmyrc