Add LocationRole=lockfile to all BlockLocation extractors#140
Closed
anderruiz wants to merge 36 commits into
Closed
Add LocationRole=lockfile to all BlockLocation extractors#140anderruiz wants to merge 36 commits into
anderruiz wants to merge 36 commits into
Conversation
The npm-lock extractor previously set FilePosition on intermediate types (NpmLockPackage/NpmLockDependency) via InJSON but never transferred those positions to PackageDetails.BlockLocation. This meant transitive deps had no lockfile position data. Changes: - Thread sourceFile parameter through all npm-lock processing functions - Copy FilePosition to BlockLocation in both processPackage() and parseNpmLockDependencies() - Fix matcher de-duplication check in match-package-json.go and match-composer.go: compare BlockLocation.Filename against the source file path instead of checking Line.Start != 0, since extractors now set BlockLocation from the lockfile for all packages - Add BlockLocation assertion tests for npm v1 and v2 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Switch strict ExpectPackages to ExpectPackagesWithoutLocations in tests that are not specifically testing location data. Dedicated BlockLocation assertion tests (v1 and v2) verify positions are set. Update WorkspacesComplex test to expect lockfile-sourced BlockLocation for the transitive dep colors@1.4.0. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add line counter to bufio.Scanner loop and set BlockLocation with line number and column range for each parsed package. Column.Start is 1 (start of line), Column.End is the raw line length + 1 (before TrimSpace, preserving original positions). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add line counter to bufio.Scanner loop and set BlockLocation with line number and full line column range for each parsed package. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add dedicated BlockLocation assertion test. Switch strict ExpectPackages to ExpectPackagesWithoutLocations for non-location tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add line number and source file tracking to the gemfileLockfileParser state machine. Each package gets BlockLocation set with the line where it appears in the Gemfile.lock file. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add dedicated BlockLocation assertion test for gemfile-lock extractor verifying line numbers and filenames. Switch existing tests to ExpectPackagesWithoutLocations since dedicated test covers location assertions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Use the InTOML utility to compute block positions for each [[package]] section in Cargo.lock. Read file content into buffer for both TOML decode and line-based position computation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add dedicated BlockLocation assertion test for cargo-lock extractor verifying multi-line block positions from InTOML utility. Switch existing tests to ExpectPackagesWithoutLocations. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Use the InTOML utility to compute block positions for each [[package]] section in poetry.lock. Uses [metadata] as the otherKey to properly close the last package block. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add dedicated BlockLocation assertion test for poetry-lock extractor verifying multi-line block positions from InTOML utility. Switch existing tests to ExpectPackagesWithoutLocations. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Use the InTOML utility to compute block positions for each [[package]] section in pdm.lock. Buffer file content with io.ReadAll for both TOML decode and line-based position computation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add dedicated BlockLocation assertion test for pdm-lock extractor verifying multi-line block positions from InTOML utility. Switch existing tests to ExpectPackagesWithoutLocations. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Use the InTOML utility to compute block positions for all [[package]] sections including root, then assign positions only to non-root packages. Handles the root package skip correctly by computing positions for all TOML entries and indexing by position. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add dedicated BlockLocation assertion test for uv-lock extractor verifying that root package is skipped while non-root packages get correct positions. Switch existing tests to ExpectPackagesWithoutLocations. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add custom JSON array scanner to compute block positions for each package object in the "packages" and "packages-dev" arrays. Tracks brace depth to find start/end lines of each package block. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add dedicated BlockLocation assertion test for composer-lock extractor verifying JSON block positions for both packages and packages-dev arrays. Switch existing tests to ExpectPackagesWithoutLocations. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Buffer content with io.ReadAll, use InJSON utility for "default" and "develop" map sections to compute block positions for each package. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Switch 5 existing tests to ExpectPackagesWithoutLocations and add dedicated TestParsePipenvLock_TwoPackages_BlockLocation test verifying positions for packages in both "default" and "develop" sections. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Buffer content with io.ReadAll, use InJSON utility for "Packages" map section to compute block positions for each package. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Switch 4 existing tests to ExpectPackagesWithoutLocations and add dedicated TestParseRenvLock_TwoPackages_BlockLocation test verifying positions for packages in the "Packages" section. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Buffer content with io.ReadAll, use InJSON utility per framework target (e.g. "net6.0") to compute block positions for each package within the nested "dependencies" structure. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Switch 6 existing tests to ExpectPackagesWithoutLocations, update MultipleVersionsNonDeterministicOrder to expect lockfile positions for packages not matched by .csproj, and add dedicated BlockLocation test for one-framework-two-packages fixture. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
V1 (nodes map): Uses InJSON with "nodes" group key to get positions for each node block, keyed by node ID. V2 (string arrays): Uses ExtractDelimitedStringPositionInBlock to find the line containing each reference string within quotes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Switch existing v1-revisions and v2 tests from ExpectPackages to ExpectPackagesWithoutLocations, and add dedicated BlockLocation tests for both v1-revisions and v2 formats. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ractor Buffer XML content and scan for <component> elements to extract line positions. Handles duplicate group:name:version entries (multiple versions) by consuming positions sequentially. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ocation assertions Switch existing tests to ExpectPackagesWithoutLocations and add dedicated BlockLocation test for two-packages fixture verifying line ranges for each <component> block. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Buffer YAML content and scan for package name entries at 2-space indent under the "packages:" key. Track block boundaries from package name line to the line before the next package or end of section. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Switch existing tests to ExpectPackagesWithoutLocations and add dedicated BlockLocation test for two-packages fixture verifying line ranges for shelf and shelf_web_socket packages. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add lockfile position tracking for both pnpm v9+ and legacy (<v7) formats. Buffer file content before YAML decode, scan for package entries under "packages:" section using indent-based detection, and set BlockLocation on all returned PackageDetails. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add dedicated BlockLocation tests for v9 one-package, v9 mixed-groups, legacy one-package, and legacy multiple-packages fixtures. Update workspace-complex test to expect lockfile position for transitive dependency (colors@1.4.0). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add lockfile position tracking for both yarn v1-v3 (text format) and v4+ (JSON format). For text format, track line numbers during block grouping. For JSON format, scan lines for entry keys within "entries" object. Buffer full content for both formats to enable line scanning. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add dedicated BlockLocation tests for v1 one-package, v1 two-packages, v2 one-package, and v2 two-packages fixtures. Update workspace-complex tests for both v1 and v2 to expect lockfile positions for transitive dependency (colors@1.4.0). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two fixes for non-deterministic output after adding lockfile positions: - Sort evidence occurrences by location string in cyclonedx.go so that packages with multiple locations (lockfile + source file) always emit occurrences in a consistent order. - In npmPackageDetailsMap.add(), prefer the earlier lockfile position when the same name@version appears at multiple node_modules paths, ensuring deterministic location selection regardless of map iteration order. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Update integration test snapshots to reflect new lockfile positions for transitive dependencies. Fix testifylint warnings in new test code: - Use assert.Len instead of assert.Equal with len() - Use assert.Positive instead of assert.Greater with 0 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
🎯 Code Coverage (details) 🔗 Commit SHA: 8c59c1e | Docs | Datadog PR Page | Give us feedback! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c59c1e4b3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Commit: commit, | ||
| DepGroups: depGroups, | ||
| IsDirect: isDirect, | ||
| BlockLocation: blockLocation, |
There was a problem hiding this comment.
For pnpm lockfiles with lockfileVersion < 7.0, Extract routes through extractLegacyPnpm, and this newly populated BlockLocation is emitted with the zero-value LocationRole. The scanner passes p.LocationRole into location.NewPackageLocations, so legacy pnpm SBOM evidence now has a lockfile position but no role while the other lockfile parsers in this change tag the same kind of location as models.LocationRoleLockfile.
Useful? React with 👍 / 👎.
Contributor
Author
|
Closing — content pushed directly to ander/transitive-dep-locations (PR #127). |
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
1 participant
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
ander/transitive-dep-locationsbranch (36 commits addingBlockLocationto 17 lockfile extractors) onto latest main which includesLocationRolesupportLocationRole = models.LocationRoleLockfilein every extractor that setsBlockLocation, so lockfile positions are tagged with the correct roleAffected Extractors (17)
npm-lock, gradle-lock, mix-lock, gemfile-lock, cargo-lock, poetry-lock, pdm-lock, uv-lock, composer-lock, pipenv-lock, renv-lock, nuget-lock, conan-lock (v1+v2), gradle-verification-metadata, pubspec-lock, pnpm-lock, yarn-lock
Changes Per Commit
Each of the 17 extractor implementation commits now includes both
BlockLocationandLocationRoleassignments (achieved via fixup+autosquash to maintain clean history).Test plan
./scripts/run_tests.shpasses./scripts/run_lints.shpasses./scripts/build.shsucceedsLocationRole: models.LocationRoleLockfileHasPackageupdated to ignoreLocationRolewhenignoreLocations=trueLocationRoleLockfilefor transitive deps