fix(go): handle replace directives in go list -m all parsing#468
Conversation
The stream filter in getFinalPackagesVersionsForModule() only accepted 2-part lines from `go list -m all`, silently dropping replace directive lines (5-part format: name v1 => replacement v2). This caused replaced modules to be absent from the MVS version map, resulting in incorrect SBOM versions. Extract parseModuleVersions() to handle both standard and replace directive lines, mirroring the JS client fix (PR guacsec#505). Implements TC-4359 Assisted-by: Claude Code
Reviewer's GuideRefactors Go module version parsing into a dedicated helper that correctly handles File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
parseModuleVersionslogic currently usesline.split(" "), which will produce empty tokens with multiple spaces and won't handle tab-separatedgo list -m alloutput well; consider splitting on\s+instead to robustly handle varying whitespace. - To make
parseModuleVersionsmore resilient, consider skipping blank/comment lines explicitly before splitting and perhaps guarding against malformed lines rather than relying solely on theparts.lengthfilter.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `parseModuleVersions` logic currently uses `line.split(" ")`, which will produce empty tokens with multiple spaces and won't handle tab-separated `go list -m all` output well; consider splitting on `\s+` instead to robustly handle varying whitespace.
- To make `parseModuleVersions` more resilient, consider skipping blank/comment lines explicitly before splitting and perhaps guarding against malformed lines rather than relying solely on the `parts.length` filter.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
ruromero
left a comment
There was a problem hiding this comment.
I agree that splitting on \s+ it's worth implementing.
The blank lines suggestion should already be handled implicitly.
You can also add a unit test for the parseModuleVersions
Use regex whitespace splitting instead of single space to handle tabs and multiple spaces in go list -m all output. Add unit tests covering standard lines, replace directives, varied whitespace, blank lines, malformed input, and duplicate handling. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Verification Report for TC-4359 (commit 8628bff)
Acceptance Criteria Detail
Overall: WARNOut-of-scope test file added per reviewer request. One commit missing explicit issue reference. Test functions could be parameterized per project conventions. No blocking issues. This comment was AI-generated by sdlc-workflow/verify-pr v0.5.11. |
Summary
go list -m allparsing inGoModulesProvider.getFinalPackagesVersionsForModule()to handle Go replace directives (5-part format:name v1 => replacement v2)parseModuleVersions()method mirroring JS client PR build(deps): bump org.pitest:pitest-maven from 1.25.1 to 1.25.4 #505go_mod_light_no_ignoreand update expected SBOMImplements TC-4359
Test plan
go_mod_no_pathfixture with no-op replace still passesTest_Golang_MvS_Logic_Disabled,Test_Golang_MvS_Enabled_Preserves_All_Transitive_Dependencies)mvn spotless:applypasses (code formatted)mvn verifypasses (only pre-existing Python env failures)🤖 Generated with Claude Code
Summary by Sourcery
Handle Go module replace directives when resolving final package versions from
go list -m alloutput.Bug Fixes:
go list -m alloutput.Enhancements:
parseModuleVersionshelper to parsego list -m alloutput into a module-to-version map.Tests: