feat: honor --usage and --only-used in multi-folder scans - #904
feat: honor --usage and --only-used in multi-folder scans#904Ayush7614 wants to merge 2 commits into
Conversation
sonukapoor
left a comment
There was a problem hiding this comment.
Nice fix @Ayush7614, thanks for tracking this down. I traced the loop and the per-folder scoping is right: subfolderAbs is already computed per iteration for readDirectDependencyNames, and you reuse it for scanProjectForPackageUsage, so there is no risk of one folder's findings getting filtered against another folder's imports.
One thing before merging: none of the three new tests exercise more than one subfolder, so nothing proves the per-folder isolation this PR is about. The --check-overrides tests just above yours already do this (two subfolders, a and b, asserting the mock is called twice with the right args). Could you add a similar one, ideally with a package imported in one folder and not the other, so we get a regression test for the actual bug rather than just the annotate/filter mechanics?
Two small things while you are in there: the if (!params.options.json) guard around the logInfo call is redundant, since logInfo already no-ops when options.json is set (which is why the single-folder call site does not wrap it). And longer-term (not blocking) it would be worth pulling the annotate-then-filter block into a shared helper used by both index.ts and multi-folder-scan.ts. This repo keeps hitting the "single-folder has X, multi-folder quietly does not" pattern (#866, #878, #879), and this logic living in two files is exactly how that happens again.
Also worth linking an issue in the PR body if one exists. Happy to open one on our side if you did not file it.
|
Addressed review feedback:
No issue was filed for this gap when the PR was opened — happy for you to open one on your side if you want it linked; otherwise this can land as a direct parity fix. |
Flags were accepted but never wired: multi-folder skipped the source usage scan that single-folder runs. Annotate and filter findings per subfolder so Action/CLI usage inputs work on monorepos.
Add a two-folder regression where lodash is imported only in packages/a so --only-used keeps it there and drops it from packages/b. Also drop the redundant json guard around logInfo (it already no-ops).
ac63534 to
4f1b251
Compare
|
@sonukapoor thanks for the review — addressed:
No issue was filed for this gap when the PR was opened — happy for you to open one on your side if you want it linked. Also rebased this branch onto latest |
Summary
scanProjectwhen--usage/--only-usedare set.--only-usedfilters each subfolder’s findings to packages actually imported in that folder’s source.Why
Package-based monorepos (no root lockfile, ≥2 nested lockfiles) route through
handleMultiFolderScan, which never calledscanProjectForPackageUsage. Teams enabling usage via CLI or Action inputs got unannotated / unfiltered results with no warning.Test plan
npm test -- tests/multi-folder-scan.test.ts(usage annotate, only-used filter, no-op without flag)npm run buildcve-lite examples/nx-package-based --usageshows usage on findings;--only-usedreduces the set