From 85ecdb4955cd882c5416b8902f80a48d9077f7db Mon Sep 17 00:00:00 2001 From: Juraj Oceliak Date: Tue, 29 Sep 2026 16:39:13 +0200 Subject: [PATCH 1/2] fix(workspace): honor project(":name").projectDir relocation in settings.gradle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Module directory discovery derived every subproject's path purely from its include(":name") string, ignoring a subsequent project(":name").projectDir = file("...") relocation. A project that groups its modules under a parent directory (include(":kmp-module-one") building from modules/kmp-module-one/, not ./kmp-module-one/) had that module's source roots silently lost — detect_build_layout_source_paths probed a directory that doesn't exist, read_dir failed, and the module's src/*/kotlin dirs never became source roots. Highlighting still worked (per-file, source-root-independent); navigation and find-references, which depend on correct source-root scoping, mostly didn't. Fixes #326. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF --- src/workspace_json.rs | 80 +++++++++++++++++++++++++++------- src/workspace_json_tests.rs | 86 +++++++++++++++++++++++++++++++++++++ 2 files changed, 151 insertions(+), 15 deletions(-) diff --git a/src/workspace_json.rs b/src/workspace_json.rs index 9ebf9dbf..74071518 100644 --- a/src/workspace_json.rs +++ b/src/workspace_json.rs @@ -603,7 +603,11 @@ fn probe_source_set_roots(module_dir: &Path) -> Vec { roots } -/// Extracts subproject directory names from `settings.gradle` / `settings.gradle.kts`. +/// Extracts subproject directory names from `settings.gradle` / `settings.gradle.kts`, +/// honoring a `project(":name").projectDir = file("...")` relocation when one is +/// present — the directory Gradle actually builds from, which can differ from the +/// `include(...)`-derived name when a project groups its modules under a parent +/// directory (e.g. `modules/kmp-module-one` for `include(":kmp-module-one")`). /// /// Handles both forms: /// - `include(":app", ":core")` — Gradle convention (colon prefix) @@ -615,11 +619,28 @@ fn settings_subprojects(workspace_root: &Path) -> Vec { let Ok(content) = std::fs::read_to_string(&path) else { continue; }; - return parse_include_calls(&content); + let overrides = parse_project_dir_overrides(&content); + return parse_include_calls(&content) + .into_iter() + .map(|dir| overrides.get(&dir).cloned().unwrap_or(dir)) + .collect(); } Vec::new() } +/// Extracts every single- or double-quoted string literal on `line`, in order. +fn quoted_tokens(line: &str) -> Vec { + let mut tokens = Vec::new(); + let mut chars = line.chars().peekable(); + while let Some(c) = chars.next() { + if c == '"' || c == '\'' { + let quote = c; + tokens.push(chars.by_ref().take_while(|&d| d != quote).collect()); + } + } + tokens +} + /// Parses `include("...", "...")` calls and returns directory paths. /// /// Handles both double- and single-quoted project names, and both Kotlin DSL @@ -633,25 +654,54 @@ fn parse_include_calls(content: &str) -> Vec { if !trimmed.starts_with("include(") { continue; } - // Extract all single- or double-quoted strings on this line. - let mut chars = trimmed.chars().peekable(); - while let Some(c) = chars.next() { - if c == '"' || c == '\'' { - let quote = c; - let token: String = chars.by_ref().take_while(|&d| d != quote).collect(); - // ":app" → "app", ":feature:login" → "feature/login" - let dir = token - .trim_start_matches(':') - .replace(':', std::path::MAIN_SEPARATOR_STR); - if !dir.is_empty() && !result.contains(&dir) { - result.push(dir); - } + for token in quoted_tokens(trimmed) { + // ":app" → "app", ":feature:login" → "feature/login" + let dir = token + .trim_start_matches(':') + .replace(':', std::path::MAIN_SEPARATOR_STR); + if !dir.is_empty() && !result.contains(&dir) { + result.push(dir); } } } result } +/// Parses `project(":name").projectDir = file("...")`-shaped relocations, +/// mapping the `include(...)`-derived directory name (same colon-to-separator +/// transform as [`parse_include_calls`]) to the real relative path Gradle +/// actually builds that project from. +/// +/// The project name is always the first quoted string literal on the line, +/// the path is always the last — true regardless of whether the path is +/// wrapped in `file(...)`, `File(...)`, or assigned as a bare string, and +/// regardless of Kotlin DSL vs. Groovy syntax, since all of those shapes are +/// still just one quoted literal for the name and one for the path. +fn parse_project_dir_overrides(content: &str) -> HashMap { + let mut overrides = HashMap::new(); + for line in content.lines() { + let trimmed = line.trim(); + if !trimmed.starts_with("project(") || !trimmed.contains("projectDir") { + continue; + } + let tokens = quoted_tokens(trimmed); + let (Some(name_token), Some(path_token)) = (tokens.first(), tokens.last()) else { + continue; + }; + if name_token == path_token { + continue; + } + let dir = name_token + .trim_start_matches(':') + .replace(':', std::path::MAIN_SEPARATOR_STR); + let real_path = path_token.replace('/', std::path::MAIN_SEPARATOR_STR); + if !dir.is_empty() && !real_path.is_empty() { + overrides.insert(dir, real_path); + } + } + overrides +} + /// Auto-detect Android SDK source directories. /// /// Checks, in order: diff --git a/src/workspace_json_tests.rs b/src/workspace_json_tests.rs index e00d102f..059e2a58 100644 --- a/src/workspace_json_tests.rs +++ b/src/workspace_json_tests.rs @@ -148,6 +148,37 @@ fn settings_gradle_multimodule() { assert!(paths.contains(&core_src)); } +#[test] +fn settings_gradle_relocated_module_dir_is_discovered() { + // Real regression (issue #326): modules grouped under a parent directory + // via `project(":name").projectDir = file(...)`, matching the reporter's + // exact settings.gradle.kts shape. + let dir = TempDir::new().unwrap(); + fs::write( + dir.path().join("settings.gradle.kts"), + "include(\"kmp-module-one\")\n\ + include(\"kmp-module-two\")\n\ + \n\ + project(\":kmp-module-one\").projectDir = file(\"modules/kmp-module-one\")\n\ + project(\":kmp-module-two\").projectDir = file(\"modules/kmp-module-two\")\n", + ) + .unwrap(); + let module_one_src = dir.path().join("modules/kmp-module-one/src/main/kotlin"); + let module_two_src = dir.path().join("modules/kmp-module-two/src/main/kotlin"); + fs::create_dir_all(&module_one_src).unwrap(); + fs::create_dir_all(&module_two_src).unwrap(); + + let paths = detect_build_layout_source_paths(dir.path()); + assert!( + paths.contains(&module_one_src), + "relocated module-one source root must be discovered; got {paths:?}" + ); + assert!( + paths.contains(&module_two_src), + "relocated module-two source root must be discovered; got {paths:?}" + ); +} + #[test] fn kmp_source_sets_discovered_structurally() { // probe_source_set_roots() must discover non-standard KMP source sets by @@ -226,6 +257,61 @@ fn ignores_include_build_lines() { assert_eq!(result, vec!["app"]); } +// ─── parse_project_dir_overrides unit tests ──────────────────────────────────── +// +// Real regression (issue #326): a project that groups its modules under a +// parent directory (`modules/kmp-module-one` for `include(":kmp-module-one")`) +// via `project(":name").projectDir = file(...)` had every module's directory +// silently derived as just the include name, so `settings_subprojects` pointed +// at a directory that doesn't exist and every source root under it was lost. + +#[test] +fn parses_kotlin_dsl_project_dir_override() { + let content = r#"project(":kmp-module-one").projectDir = file("modules/kmp-module-one")"#; + let overrides = parse_project_dir_overrides(content); + let sep = std::path::MAIN_SEPARATOR_STR; + assert_eq!( + overrides.get("kmp-module-one"), + Some(&format!("modules{sep}kmp-module-one")) + ); +} + +#[test] +fn parses_groovy_project_dir_override_with_new_file() { + let content = "project(':app').projectDir = new File('modules/app')"; + let overrides = parse_project_dir_overrides(content); + let sep = std::path::MAIN_SEPARATOR_STR; + assert_eq!(overrides.get("app"), Some(&format!("modules{sep}app"))); +} + +#[test] +fn parses_bare_string_project_dir_override() { + let content = r#"project(":app").projectDir = "modules/app""#; + let overrides = parse_project_dir_overrides(content); + let sep = std::path::MAIN_SEPARATOR_STR; + assert_eq!(overrides.get("app"), Some(&format!("modules{sep}app"))); +} + +#[test] +fn ignores_project_lines_without_project_dir() { + // A bare `project(":app")` reference with no `.projectDir` assignment + // (e.g. dependency configuration) must not be mistaken for a relocation. + let content = r#"implementation(project(":app"))"#; + let overrides = parse_project_dir_overrides(content); + assert!(overrides.is_empty()); +} + +#[test] +fn no_override_for_a_module_that_is_not_relocated() { + let content = "include(\":app\")\ninclude(\":core\")\n\ + project(\":app\").projectDir = file(\"modules/app\")"; + let overrides = parse_project_dir_overrides(content); + assert!( + !overrides.contains_key("core"), + "core was never relocated, must not appear in the override map" + ); +} + // ─── Android SDK detection tests ───────────────────────────────────────────── #[test] From d63e784ca513682933ee6064ded89dff14a9b81b Mon Sep 17 00:00:00 2001 From: Juraj Oceliak Date: Tue, 29 Sep 2026 16:42:57 +0200 Subject: [PATCH 2/2] test(workspace): tighten decoy coverage for project() line parsing Review found two gaps: no test covered a project(...) line that starts correctly but configures something other than projectDir (only the !starts_with("project(") disjunct was exercised); the non-relocated-module test only checked isolated parse_project_dir_overrides output rather than settings_subprojects' real unwrap_or(dir) fallback path end-to-end. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF --- src/workspace_json_tests.rs | 40 +++++++++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/src/workspace_json_tests.rs b/src/workspace_json_tests.rs index 059e2a58..ce8d06b4 100644 --- a/src/workspace_json_tests.rs +++ b/src/workspace_json_tests.rs @@ -293,22 +293,50 @@ fn parses_bare_string_project_dir_override() { } #[test] -fn ignores_project_lines_without_project_dir() { +fn ignores_a_project_reference_with_no_project_dir_prefix() { // A bare `project(":app")` reference with no `.projectDir` assignment // (e.g. dependency configuration) must not be mistaken for a relocation. + // This line doesn't even start with `project(` once trimmed (it starts + // with `implementation(`) — see the sibling test below for the case + // where the line DOES start with `project(` but still isn't a relocation. let content = r#"implementation(project(":app"))"#; let overrides = parse_project_dir_overrides(content); assert!(overrides.is_empty()); } #[test] -fn no_override_for_a_module_that_is_not_relocated() { - let content = "include(\":app\")\ninclude(\":core\")\n\ - project(\":app\").projectDir = file(\"modules/app\")"; +fn ignores_a_project_line_that_configures_something_other_than_project_dir() { + // Starts with `project(` (unlike the sibling test above) but configures + // an unrelated property — must not be mistaken for a relocation either. + let content = r#"project(":app").version = "1.0""#; let overrides = parse_project_dir_overrides(content); + assert!(overrides.is_empty()); +} + +#[test] +fn settings_subprojects_passes_through_a_non_relocated_module_unchanged() { + // End-to-end through settings_subprojects (not just the override parser + // in isolation): a mixed project with one relocated and one ordinary + // module must resolve the ordinary one to its plain include-derived + // directory, exercising the real `unwrap_or(dir)` fallback path. + let dir = TempDir::new().unwrap(); + fs::write( + dir.path().join("settings.gradle.kts"), + "include(\":app\")\ninclude(\":core\")\n\ + project(\":app\").projectDir = file(\"modules/app\")\n", + ) + .unwrap(); + + let subprojects = settings_subprojects(dir.path()); + assert!( + subprojects.contains(&"core".to_string()), + "core was never relocated, must resolve to its plain include-derived \ + directory; got {subprojects:?}" + ); + let sep = std::path::MAIN_SEPARATOR_STR; assert!( - !overrides.contains_key("core"), - "core was never relocated, must not appear in the override map" + subprojects.contains(&format!("modules{sep}app")), + "app must resolve to its relocated directory; got {subprojects:?}" ); }