Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 66 additions & 24 deletions src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, Vec<AffectCause>>,
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,
Expand Down Expand Up @@ -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 = "<proj>/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 {
Comment thread
nirsky marked this conversation as resolved.
record_direct_change_causes(
&mut project_causes,
pkg,
file_path,
&changed_file.changed_lines,
);
}
}
continue;
}

Expand Down Expand Up @@ -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,
);
}
}

Expand Down
106 changes: 104 additions & 2 deletions src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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"
);
}
}
104 changes: 104 additions & 0 deletions tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Loading