Expose source-level manifest declarations - #67
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request extends the public parsing API to expose source-level “declaration” entries (dependency-like references with stable, parser-defined logical locations) while keeping the existing effective Dependencies output unchanged. For Maven POMs, it adds collection of version-bearing references beyond direct dependencies (parent, dependencyManagement, plugins, extensions, and profile-scoped variants) so consumers can diff edits without implementing a Maven XML parser.
Changes:
- Adds a new
Declarationtype andParseResult.Declarationsto surface source-level references with stableLocation. - Implements Maven POM declaration extraction (
internal/maven/declarations.go) and wires it into the Maven parser. - Adds tests validating Maven declaration locations and PURL generation.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| manifests.go | Re-exports Declaration and adds ParseResult.Declarations, including PURL generation for declarations. |
| manifests_test.go | Adds an integration test asserting Maven declaration PURLs are generated as expected. |
| internal/maven/maven.go | Populates core.Result.Declarations for pom.xml parsing. |
| internal/maven/declarations.go | New Maven POM source-level declaration collector (parent, deps, depMgmt, plugins, extensions, profiles). |
| internal/maven/declarations_test.go | New unit tests for declaration collection semantics/locations/scopes. |
| internal/core/types.go | Introduces core.Declaration and adds Declarations to core.Result. |
| go.mod | Updates github.com/git-pkgs/pom dependency version. |
| go.sum | Updates checksums for the new github.com/git-pkgs/pom version. |
Suppressed comments (1)
internal/maven/declarations.go:43
- Profile-scoped sections (DependencyManagement, Build, PluginManagement) are dereferenced without nil checks; profiles can omit these blocks, which will panic if the corresponding fields are nil pointers.
collectPOMDependencies(&declarations, prefix+"/dependencies", profile.Dependencies)
collectPOMDependencies(&declarations, prefix+"/dependencyManagement/dependencies", profile.DependencyManagement.Dependencies)
collectMavenPlugins(&declarations, prefix+"/build/plugins", profile.Build.Plugins)
collectMavenPlugins(&declarations, prefix+"/build/pluginManagement/plugins", profile.Build.PluginManagement.Plugins)
collectMavenExtensions(&declarations, prefix+"/build/extensions", profile.Build.Extensions)
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| collectPOMDependencies(&declarations, "dependencies", project.Dependencies) | ||
| collectPOMDependencies(&declarations, "dependencyManagement/dependencies", project.DependencyManagement.Dependencies) | ||
| collectMavenPlugins(&declarations, "build/plugins", project.Build.Plugins) | ||
| collectMavenPlugins(&declarations, "build/pluginManagement/plugins", project.Build.PluginManagement.Plugins) | ||
| collectMavenExtensions(&declarations, "build/extensions", project.Build.Extensions) |
There was a problem hiding this comment.
The relevant fields in pom v0.1.7 are value fields: DepMgmt, Build, and PluginManagement. Omitted sections therefore produce zero values rather than nil pointers, so these dereferences cannot panic. No change is needed here.
| dependencyLocation := location + "/" + url.PathEscape(pluginName) + "/dependencies" | ||
| for _, dependency := range plugin.Dependencies { | ||
| appendMavenDeclaration( | ||
| declarations, | ||
| dependencyLocation, | ||
| dependency.GroupID, | ||
| dependency.ArtifactID, | ||
| dependency.Version, | ||
| core.Build, | ||
| "", | ||
| ) | ||
| } |
There was a problem hiding this comment.
Build scope is intentional here. Declaration.Scope describes when the referenced package is required by the project. Plugin dependencies are loaded on the plugin classpath during the build; mapping an omitted nested Maven scope to Runtime would misclassify them for manifests consumers.
| for _, declaration := range result.Declarations { | ||
| if declaration.PURL != want[declaration.Location] { | ||
| t.Errorf("declaration at %q has PURL %q, want %q", declaration.Location, declaration.PURL, want[declaration.Location]) | ||
| } | ||
| } |
There was a problem hiding this comment.
Fixed in 56ea5fb. The test now checks map membership and reports an unexpected declaration location directly.
Add
ParseResult.Declarationsfor dependency-like source references at stable parser-defined locations. Populate it for Maven parents, dependencies, dependency management, plugins, plugin dependencies, plugin management, extensions, and profile-scoped forms.This keeps effective
Dependenciesbehavior unchanged and lets consumers compare source edits without carrying a Maven XML parser. It usespomv0.1.7 to retain raw build coordinates during the existing parse.Closes #48.