Skip to content

fix(rust): keep reporter source paths resolvable from CWD#869

Open
saltyfireball wants to merge 2 commits into
kucherenko:masterfrom
saltyfireball:fix/reporter-cwd-paths
Open

fix(rust): keep reporter source paths resolvable from CWD#869
saltyfireball wants to merge 2 commits into
kucherenko:masterfrom
saltyfireball:fix/reporter-cwd-paths

Conversation

@saltyfireball

@saltyfireball saltyfireball commented Jul 10, 2026

Copy link
Copy Markdown

fix(rust): keep reporter source paths resolvable from CWD (empty HTML/JSON code)

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    Bug fix (Rust cpd / v5).

  • What is the current behavior? (You can also link to an open issue here)

When a scan target is a subdirectory of the current working directory - e.g. path: ["frontend", "backend"] in .jscpd.json, or cpd some-subdir - the HTML report's "show code" blocks are empty, and the JSON report's fragment field is an empty string. Running cpd . (scanning the CWD directly) works, which is the confusing part.

Root cause: the file reporters (html, json, console-full) re-read each source file from disk to render its snippet. Since #827, source paths are rewritten to be relative to the scan root for display (/proj/frontend/src/a.js -> src/a.js). The reporters then call std::fs::read_to_string("src/a.js"), which resolves against the CWD (/proj), where that path does not exist. The read fails and is swallowed by read_file_cached's unwrap_or_default(), yielding empty code. Passing . masks the bug because the scan root then equals the CWD, so the rewritten path still resolves.

  • What is the new behavior (if this is a feature change)?

Display paths are now made relative to the CWD instead of the scan root, so they remain resolvable from the CWD and the reporters can re-read the source. Scanning frontend from the project root now yields frontend/src/a.js -- which both renders code correctly and disambiguates multiple scan roots (frontend/... vs backend/...). Files outside the CWD are left absolute (still readable). --absolute is unaffected.

  • Other information:
  • Implementation: replaced relativize_to_scan_root with relativize_to_cwd in rust/crates/cpd/src/main.rs (canonicalizes the CWD for reliable prefix stripping); updated unit tests.
  • Added an integration regression test (report_snippets_populated_when_scan_root_differs_from_cwd) that scans a subdirectory (scan root != CWD) and asserts both HTML and JSON contain the snippet. Verified it fails on the old behavior and passes with the fix.
  • Test suite: cargo test passes (620+ across the workspace; 375 in the jscpd crate).
  • Behavior note for reviewers familiar with absolute:false should always emit scan-root-relative SARIF artifactLocation.uri paths #827: scanning an absolute path outside the CWD now displays absolute paths rather than scan-root-relative ones. This is the deliberate tradeoff -- a scan-root-relative path is not resolvable from the CWD and is exactly what caused the empty-code bug.
  • Known follow-up (not in this PR): read_file_cached swallows read failures via unwrap_or_default(); it should emit a warning so this class of bug is not silent.
  • If this behavior is preferred, an alternative is to keep scan-root-relative display and resolve reads against the scan roots.

This change is Reviewable

Summary by CodeRabbit

  • Bug Fixes

    • Fixed source path handling in JSON and HTML reports when the scan root is a subdirectory of the current working directory, preventing rendered snippet output from appearing empty.
    • Improved path normalization so paths inside the current directory stay relative, while paths outside it keep absolute forms to remain resolvable.
  • Tests

    • Added a regression test covering snippet rendering and source path preservation across JSON and HTML outputs when scan roots differ from the process working directory.

@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c61536f4-7578-4e64-bf67-151c9244e9a1

📥 Commits

Reviewing files that changed from the base of the PR and between 354e05c and 6e1ff5a.

📒 Files selected for processing (1)
  • rust/crates/cpd/tests/integration.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • rust/crates/cpd/tests/integration.rs

Walkthrough

CPD now normalizes non-absolute clone source IDs relative to the canonical current working directory. Paths outside it remain absolute, with unit and integration tests covering subdirectory scans and report output.

Changes

CWD path normalization

Layer / File(s) Summary
CWD relativization contract
rust/crates/cpd/src/main.rs
Clone source IDs use CWD-based normalization, preserving outside-CWD paths and covering subpaths, dot prefixes, and helper behavior with unit tests.
Subdirectory scan regression coverage
rust/crates/cpd/tests/integration.rs
Integration coverage verifies duplicated snippets and CWD-relative paths in JSON and HTML reports when scanning a subdirectory.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Poem

A bunny hops through paths so neat,
CWD keeps each source complete.
JSON and HTML happily show,
The snippets where the files still grow.
No empty reports—what a treat! 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: making reporter source paths resolvable from the CWD.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@rust/crates/cpd/tests/integration.rs`:
- Around line 385-388: Update the path assertion in the integration test to use
a raw string for the escaped Windows path: replace the current
`json.contains("pkg\\a.js")` check with `json.contains(r"pkg\\a.js")`, while
retaining the forward-slash alternative.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a5f235bb-0c61-44d1-9907-f14bd2af2022

📥 Commits

Reviewing files that changed from the base of the PR and between 64258a9 and 354e05c.

📒 Files selected for processing (2)
  • rust/crates/cpd/src/main.rs
  • rust/crates/cpd/tests/integration.rs

Comment thread rust/crates/cpd/tests/integration.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant