Add a typed package.json reader - #16179
Open
JamieMagee wants to merge 1 commit into
Open
Conversation
5 tasks
Contributor
There was a problem hiding this comment.
Copilot review overview
馃煝 Approval recommended
The focused implementation follows existing parsing conventions and has thorough behavioral coverage.
Review tier: Balanced
Findings: None
What changed in this PR
Adds a shared, strongly typed package.json reader to Dependabot Common.
Changes:
- Parses dependency sections and package metadata safely.
- Validates malformed JSON structures with contextual errors.
- Adds comprehensive tests for valid, invalid, and optional fields.
| File | Description |
|---|---|
common/鈥媗ib/鈥媎ependabot/鈥媝ackage/鈥媙pm_package_json.rb |
Implements the typed reader. |
common/鈥媠pec/鈥媎ependabot/鈥媝ackage/鈥媙pm_package_json_spec.rb |
Covers parsing, validation, and defaults. |
馃挕 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
What are you trying to accomplish?
Add a shared
NpmPackageJsonreader for the fields used by npm/Yarn and Bun file parsing and workspace filtering. It istyped: strong, yields string dependency requirements, and reusesNpmPackageManagerConfigfor package-manager metadata.This is the bottom layer of the stack. #16180 and #16181 migrate the consumers.
Anything you want to highlight for special attention from reviewers?
The contract follows the existing parser code and fixtures, not a complete npm schema. Fields are read lazily, requirement strings stay unchanged, and non-string requirements are skipped. Missing/null/false dependency sections remain empty; other non-object sections raise
TypeErrorwith file and field context.flatandworkspacesretain their existing Ruby truthiness, including empty arrays and objects. JSON syntax errors propagate for callers to handle. The reader never rewrites the source file.How will you know you've accomplished your goal?
The reader and existing package-manager config specs pass all 48 examples. Repository-wide Sorbet and targeted RuboCop also pass. These checks ran in containers; I did not run the complete repository suite.
Checklist