fix: treat ambiguous paths like /tools/ and /migrations/ as production inside source roots - #1727
Draft
skoshx wants to merge 2 commits into
Draft
fix: treat ambiguous paths like /tools/ and /migrations/ as production inside source roots#1727skoshx wants to merge 2 commits into
skoshx wants to merge 2 commits into
Conversation
…de source roots Fixes #1724 Rules tagged test-noise were silently skipping files in paths containing /tools/, /migrations/, /scripts/, /cli/, /bin/, etc., even when nested inside application source roots like src/components/tools/. The heuristic now distinguishes between: - Unambiguous non-production (test/fixture/story/benchmark/demo/examples): always skipped regardless of depth - Ambiguous build tooling (/tools/, /migrations/, /scripts/, etc.): only skipped at repo root, not inside source roots This lets test-noise rules correctly fire on production feature areas like src/components/tools/ while still skipping top-level build tooling like <repo>/tools/. Component library demos (/demo/, /examples/) remain unambiguous and are skipped everywhere, as they're genuinely non-production even when nested in component source (e.g., components/Button/demos/). Co-authored-by: Skosh <skoshx@users.noreply.github.com>
Co-authored-by: Skosh <skoshx@users.noreply.github.com>
commit: |
Contributor
Interactive terminal E2ETerminal Control verified the built CLI at
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1724
Rules tagged
test-noise(332 of 884 rules) were silently skipping files in paths containing/tools/,/migrations/,/scripts/,/cli/,/bin/,/generators/,/devtools/, etc., even when nested inside application source roots likesrc/components/tools/. This caused false negatives where legitimate production code was not being scanned.Root Cause
The
isTestlikeFilenameheuristic treated these directory names as universally non-production. While this is correct at the repo root (wheretools/is typically build tooling), it's incorrect inside application source roots wheresrc/components/tools/is often a feature area (e.g., an admin tools section).The Fix
The heuristic now distinguishes between two categories of path segments:
Unambiguous non-production (always skipped)
Test/fixture/story/benchmark/demo/examples directories that are never shipped code, regardless of depth:
/test/,/tests/,/__tests__/,/fixtures/,/stories/, etc./demo/,/demos/,/examples/,/example//playground/,/benchmarks/,/e2e/, etc.Component library demos (
/demo/,/examples/) remain unambiguous because even when nested in component source (e.g.,components/Button/demos/), they're demonstration code, not production.Ambiguous build tooling (only skipped at repo root)
Directories that mark build tooling at the repo root but can be feature areas inside source roots:
/tools/,/scripts/,/cli/,/bin/,/tooling//migrations/,/migration/,/generators/,/generator//codemods/,/codemod/,/runbooks/,/devtools//seeds/,/seed/,/dev-seeder/,/internal-tools/These are now only skipped when the file is NOT within a source root (
/src/,/app/,/components/,/pages/,/lib/, etc.).Examples
Before this fix:
src/components/tools/widget.tsx→ skipped (false negative)src/admin/migrations/history.tsx→ skipped (false negative)tools/build-script.ts→ skipped ✅After this fix:
src/components/tools/widget.tsx→ scanned ✅src/admin/migrations/history.tsx→ scanned ✅tools/build-script.ts→ skipped ✅components/Button/demos/demo1.tsx→ skipped ✅ (component library demo)Testing
src/components/chat/(scanned) vssrc/components/tools/(skipped before, now scanned)Validation
Running parity check with
react-doctor-evalsto ensure no unexpected regressions across the OSS corpus. Will update with results.