-
Notifications
You must be signed in to change notification settings - Fork 570
Duplicate Code: Repeated package.json JSON parsing blocks in repoPolicyCheck handlers #26891
Copy link
Copy link
Open
Labels
Description
Analysis of commit e85150e681f69d58a07dadcfbe5804dccde749c5
Assignee: @copilot
Summary
Multiple repo policy-check handlers repeat the same package.json JSON parsing + try/catch error-return logic. This is duplicated across 3 files and heavily repeated within npmPackages.ts, increasing maintenance cost and making error handling inconsistent over time.
Duplication Details
Pattern: JSON.parse(readFile(...)) with identical catch { return \Error parsing JSON file: ...`; }`
-
Severity: Medium
-
Occurrences:
build-tools/packages/build-cli/src/library/repoPolicyCheck/npmPackages.ts: 22JSON.parse(readFile(blocks (21 with the exactError parsing JSON file:message)build-tools/packages/build-cli/src/library/repoPolicyCheck/fluidBuildTasks.ts: 2build-tools/packages/build-cli/src/library/repoPolicyCheck/pnpm.ts: 1- Total: 25 parse blocks across these policy-check modules
-
Locations (examples):
build-tools/packages/build-cli/src/library/repoPolicyCheck/npmPackages.ts(lines 776–781)build-tools/packages/build-cli/src/library/repoPolicyCheck/npmPackages.ts(lines 1857–1862)build-tools/packages/build-cli/src/library/repoPolicyCheck/fluidBuildTasks.ts(lines 770–775)build-tools/packages/build-cli/src/library/repoPolicyCheck/pnpm.ts(lines 23–28)
-
Code Sample (representative):
let json: PackageJson; try { json = JSON.parse(readFile(file)) as PackageJson; } catch { return `Error parsing JSON file: \$\{file}`; }
Impact Analysis
- Maintainability: When package.json parsing requirements change (e.g., improved diagnostics, schema validation, stricter typing), the change must be repeated in many places.
- Bug Risk: Future edits can easily introduce subtle divergence (different error messages/return types, partial handling, inconsistent logging).
- Code Bloat: The same 6–8 line block appears ~25 times in this area alone.
Refactoring Recommendations
-
Extract a shared helper into
build-tools/packages/build-cli/src/library/repoPolicyCheck/common.ts- Example:
tryReadJsonFile(T)(path: string): { ok: true; value: T } | { ok: false; message: string } - Update handlers to use the helper and return the helper’s message on failure.
- Benefits: Single point of change, consistent error reporting.
- Example:
-
Add a package.json-focused wrapper
- Example:
tryReadPackageJson(path: string)built on the generic helper. - Benefits: Keeps handler bodies small and uniform, reduces repeated
as PackageJsoncasts.
- Example:
Implementation Checklist
- Add helper(s) in
repoPolicyCheck/common.ts - Refactor
npmPackages.tsto use helper in each handler/resolver - Refactor
fluidBuildTasks.tsandpnpm.tsto use helper - Ensure return types remain unchanged (
string | undefinedvs resolver objects) - Run existing policy-check tooling to confirm behavior unchanged
Analysis Metadata
- Detection Method: Serena semantic + targeted pattern search
- Commit:
e85150e681f69d58a07dadcfbe5804dccde749c5 - Analysis Date: 2026-03-30T21:50:25.550Z
Generated by Duplicate Code Detector · ◷
To install this agentic workflow, run
gh aw add github/gh-aw/.github/workflows/duplicate-code-detector.md@94662b1dee8ce96c876ba9f33b3ab8be32de82a4
Reactions are currently unavailable