diff --git a/src/core.rs b/src/core.rs index 90c32d7..b395c0c 100644 --- a/src/core.rs +++ b/src/core.rs @@ -22,6 +22,39 @@ struct AffectedState<'a> { visited: &'a mut FxHashSet<(PathBuf, String)>, } +/// Record a `DirectChange` cause for `pkg` for each changed line (or a +/// single line-0 cause when no lines are known). Shared by both the source +/// root-fallback path (Step 6a) and the asset path (Step 6b), which +/// otherwise had identical inlined blocks. +fn record_direct_change_causes( + project_causes: &mut FxHashMap>, + pkg: &str, + file: &Path, + changed_lines: &[usize], +) { + if changed_lines.is_empty() { + project_causes + .entry(pkg.to_string()) + .or_default() + .push(AffectCause::DirectChange { + file: file.to_path_buf(), + symbol: None, + line: 0, + }); + } else { + for &line in changed_lines { + project_causes + .entry(pkg.to_string()) + .or_default() + .push(AffectCause::DirectChange { + file: file.to_path_buf(), + symbol: None, + line, + }); + } + } +} + /// Main true-affected algorithm implementation pub fn find_affected( config: TrueAffectedConfig, @@ -135,9 +168,34 @@ fn find_affected_internal( for changed_file in &source_files { let file_path = &changed_file.file_path; - // Check if file exists in our analyzed files + // Check if file exists in our analyzed files. A source-typed file (.ts/.tsx/.js/.jsx) + // can live inside a project's root but outside its sourceRoot (e.g. jest.config.js, + // webpack.config.js at project root when sourceRoot = "/src"). The semantic + // analyzer only walks sourceRoot, so such files never reach it — but they still + // belong to the project and changing them must mark it affected. Fall back to the + // same root-based ownership lookup used for assets. if !analyzer.files.contains_key(file_path) { - debug!("Skipping unanalyzed source file: {:?}", file_path); + debug!( + "Source file not in analyzer.files, using root fallback: {:?}", + file_path + ); + let owning_packages = project_index.get_package_names_by_path(file_path); + for pkg in &owning_packages { + debug!( + "File {:?} belongs to package '{}' (via root fallback)", + file_path, pkg + ); + affected_packages.insert(pkg.clone()); + + if generate_report { + record_direct_change_causes( + &mut project_causes, + pkg, + file_path, + &changed_file.changed_lines, + ); + } + } continue; } @@ -264,29 +322,13 @@ fn find_affected_internal( debug!("Asset {:?} belongs to package '{}'", asset_path, pkg); affected_packages.insert(pkg.clone()); - // Record direct change cause if generating report if generate_report { - if asset_file.changed_lines.is_empty() { - project_causes - .entry(pkg.clone()) - .or_default() - .push(AffectCause::DirectChange { - file: asset_path.clone(), - symbol: None, - line: 0, - }); - } else { - for &line in &asset_file.changed_lines { - project_causes - .entry(pkg.clone()) - .or_default() - .push(AffectCause::DirectChange { - file: asset_path.clone(), - symbol: None, - line, - }); - } - } + record_direct_change_causes( + &mut project_causes, + pkg, + asset_path, + &asset_file.changed_lines, + ); } } diff --git a/src/utils.rs b/src/utils.rs index b3b9d5d..69429f3 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -53,8 +53,18 @@ impl ProjectIndex { map.push((project.source_root.clone(), vec![project.name.clone()])); } - // Index by root (fallback) — only when root differs from sourceRoot - if project.root != project.source_root { + // Index by root (fallback) — only when root differs from sourceRoot. + // Skip workspace-root projects (root == "" or "."): their root is a + // prefix of every path in the repo, so they'd match every fallback + // lookup and cause massive false positives on asset/config changes. + // The Nx loader produces `root = ""` via `strip_prefix(cwd)` when the + // project lives at the workspace root; guard against "." too for + // loaders that preserve it literally or when paths come in with a + // `./` prefix. + if project.root != project.source_root + && !project.root.as_os_str().is_empty() + && project.root != Path::new(".") + { if let Some(entry) = root_map.iter_mut().find(|(root, _)| *root == project.root) { entry.1.push(project.name.clone()); } else { @@ -536,4 +546,96 @@ mod tests { "file inside parent only must not attribute to child" ); } + + #[test] + fn test_project_index_workspace_root_project_excluded_from_fallback() { + // Nx workspaces commonly have a root-level project with `root: "."`. + // Such a project must NOT be added to `root_entries` — otherwise every + // file that falls through the sourceRoot check would match it (since + // every path `starts_with(".")`), producing massive false positives + // on asset and source-file changes. + let tmp = tempfile::TempDir::new().unwrap(); + let projects = vec![ + Project { + name: "workspace".to_string(), + root: PathBuf::from("."), + source_root: PathBuf::from("src"), + ts_config: None, + implicit_dependencies: vec![], + targets: vec![], + }, + Project { + name: "my-lib".to_string(), + root: "libs/my-lib".into(), + source_root: "libs/my-lib/src".into(), + ts_config: None, + implicit_dependencies: vec![], + targets: vec![], + }, + ]; + + let index = ProjectIndex::new(&projects, tmp.path()); + + // A config file at a nested project root (outside its sourceRoot) must + // resolve ONLY to that project — not to the workspace-root project. + let mut result = index.get_package_names_by_path(Path::new("libs/my-lib/jest.config.js")); + result.sort(); + assert_eq!( + result, + vec!["my-lib"], + "config file inside nested project must NOT also attribute to root workspace project" + ); + + // Likewise for a file inside the nested project's sourceRoot. + let mut result = index.get_package_names_by_path(Path::new("libs/my-lib/src/index.ts")); + result.sort(); + assert_eq!( + result, + vec!["my-lib"], + "source file inside nested project must NOT also attribute to root workspace project" + ); + + // Workspace-root sourceRoot still resolves normally — the filter only + // affects root_entries, not entries. + let result = index.get_package_names_by_path(Path::new("src/main.ts")); + assert_eq!( + result, + vec!["workspace"], + "file inside workspace sourceRoot must still attribute to workspace" + ); + } + + #[test] + fn test_project_index_empty_root_excluded_from_fallback() { + // Defensive: an empty-path root would also match everything via + // `starts_with(Path::new(""))`. Treat empty the same as ".". + let tmp = tempfile::TempDir::new().unwrap(); + let projects = vec![ + Project { + name: "workspace".to_string(), + root: PathBuf::new(), + source_root: PathBuf::from("src"), + ts_config: None, + implicit_dependencies: vec![], + targets: vec![], + }, + Project { + name: "my-lib".to_string(), + root: "libs/my-lib".into(), + source_root: "libs/my-lib/src".into(), + ts_config: None, + implicit_dependencies: vec![], + targets: vec![], + }, + ]; + + let index = ProjectIndex::new(&projects, tmp.path()); + + let result = index.get_package_names_by_path(Path::new("libs/my-lib/jest.config.js")); + assert_eq!( + result, + vec!["my-lib"], + "empty-path root must not over-attribute" + ); + } } diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 6abd72f..d6fd74e 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -3293,6 +3293,110 @@ fn test_named_inputs_negation_with_root_differs_from_source_root() { ); } +#[test] +fn test_source_file_outside_sourceroot_affects_owning_project() { + // lib-a has sourceRoot = "libs/lib-a/src" but project root = "libs/lib-a". + // A source-typed config file (jest.config.js) at project root lives OUTSIDE + // sourceRoot, so the semantic analyzer never parses it. It must still mark + // its owning project as affected via the root fallback — otherwise changes + // to project-level config files would be silently ignored. + let repo = TempNxRepo::new(r#"{}"#); + + repo.change_and_commit( + "libs/lib-a/jest.config.js", + "module.exports = { workerIdleMemoryLimit: '2048MB' };\n", + ); + + let mut affected = repo.get_affected(); + affected.sort(); + + // Exact match — guards against the root-fallback over-attributing. lib-b + // owns nothing at this path and must not appear; a workspace-root project + // (if one existed) must not appear either. + assert_eq!( + affected, + vec!["lib-a".to_string()], + "Only lib-a should be affected by its own jest.config.js" + ); +} + +#[test] +fn test_workspace_root_project_not_over_attributed() { + // Nx workspaces commonly have a root-level project (e.g. the workspace itself + // registered with `root: ""` when loaded via strip_prefix(cwd)). Without the + // root==""/"." guard in ProjectIndex::new(), its root would prefix-match every + // path in the repo and a change to any nested project's config file would + // incorrectly cascade to the workspace project. + let tmp = tempfile::TempDir::new().unwrap(); + let root = tmp.path(); + + git_in(root, &["init", "-q"]); + git_in(root, &["config", "user.email", "test@example.com"]); + git_in(root, &["config", "user.name", "Test"]); + git_in(root, &["branch", "-M", "main"]); + + fs::write(root.join("nx.json"), r#"{}"#).unwrap(); + + // Workspace-root project (root == cwd) + fs::write( + root.join("project.json"), + r#"{ "name": "workspace", "sourceRoot": "src" }"#, + ) + .unwrap(); + fs::create_dir_all(root.join("src")).unwrap(); + fs::write(root.join("src/main.ts"), "export const a = 1;\n").unwrap(); + + // Nested project with sourceRoot != root + fs::create_dir_all(root.join("libs/lib-a/src")).unwrap(); + fs::write( + root.join("libs/lib-a/project.json"), + r#"{ "name": "lib-a", "sourceRoot": "libs/lib-a/src" }"#, + ) + .unwrap(); + fs::write( + root.join("libs/lib-a/src/index.ts"), + "export const b = 2;\n", + ) + .unwrap(); + + git_in(root, &["add", "."]); + git_in(root, &["commit", "-q", "-m", "init"]); + git_in(root, &["checkout", "-q", "-b", "test-branch"]); + + // Change a config file inside lib-a's root but outside lib-a's sourceRoot. + fs::write( + root.join("libs/lib-a/jest.config.js"), + "module.exports = {};\n", + ) + .unwrap(); + git_in(root, &["add", "."]); + git_in(root, &["commit", "-q", "-m", "change jest config"]); + + let projects = domino::workspace::discover_projects(root).unwrap(); + let config = TrueAffectedConfig { + cwd: root.to_path_buf(), + base: "main".to_string(), + head: None, + root_ts_config: None, + projects, + include: vec![], + ignored_paths: vec![], + lockfile_strategy: LockfileStrategy::None, + }; + + let profiler = Arc::new(Profiler::new(false)); + let mut affected = find_affected(config, profiler) + .expect("find_affected failed") + .affected_projects; + affected.sort(); + + assert_eq!( + affected, + vec!["lib-a".to_string()], + "Only lib-a should be affected — workspace-root project must not match via root fallback" + ); +} + #[test] fn test_head_flag_commit_to_commit_diff() { let branch = TestBranch::new("test-head-flag");