fix(java): correctly inherit version and scope from upper/root depManagement and dependencies into parents (#7541)

Signed-off-by: knqyf263 <[email protected]>
Co-authored-by: knqyf263 <[email protected]>

Author: DmitriyLewen

pkg/dependency/parser/java/pom/parse.go

@@ -96,7 +96,7 @@ func NewParser(filePath string, opts ...option) *Parser {
 }
 
 func (p *Parser) Parse(r xio.ReadSeekerAt) ([]ftypes.Package, []ftypes.Dependency, error) {
-	content, err := parsePom(r)
+	content, err := parsePom(r, true)
 	if err != nil {
 		return nil, nil, xerrors.Errorf("failed to parse POM: %w", err)
 	}
@@ -107,7 +107,7 @@ func (p *Parser) Parse(r xio.ReadSeekerAt) ([]ftypes.Package, []ftypes.Dependenc
 	}
 
 	// Analyze root POM
-	result, err := p.analyze(root, analysisOptions{lineNumber: true})
+	result, err := p.analyze(root, analysisOptions{})
 	if err != nil {
 		return nil, nil, xerrors.Errorf("analyze error (%s): %w", p.rootPath, err)
 	}
@@ -330,53 +330,27 @@ type analysisResult struct {
 type analysisOptions struct {
 	exclusions    map[string]struct{}
 	depManagement []pomDependency // from the root POM
-	lineNumber    bool            // Save line numbers
 }
 
 func (p *Parser) analyze(pom *pom, opts analysisOptions) (analysisResult, error) {
-	if pom == nil || pom.content == nil {
+	if pom.nil() {
 		return analysisResult{}, nil
 	}
-
 	// Update remoteRepositories
 	pomReleaseRemoteRepos, pomSnapshotRemoteRepos := pom.repositories(p.servers)
 	p.releaseRemoteRepos = lo.Uniq(append(pomReleaseRemoteRepos, p.releaseRemoteRepos...))
 	p.snapshotRemoteRepos = lo.Uniq(append(pomSnapshotRemoteRepos, p.snapshotRemoteRepos...))
 
-	// We need to forward dependencyManagements from current and root pom to Parent,
-	// to use them for dependencies in parent.
-	// For better understanding see the following tests:
-	// - `dependency from parent uses version from child pom depManagement`
-	// - `dependency from parent uses version from root pom depManagement`
-	//
-	// depManagements from root pom has higher priority than depManagements from current pom.
-	depManagementForParent := lo.UniqBy(append(opts.depManagement, pom.content.DependencyManagement.Dependencies.Dependency...),
-		func(dep pomDependency) string {
-			return dep.Name()
-		})
-
-	// Parent
-	parent, err := p.parseParent(pom.filePath, pom.content.Parent, depManagementForParent)
-	if err != nil {
-		return analysisResult{}, xerrors.Errorf("parent error: %w", err)
+	// Resolve parent POM
+	if err := p.resolveParent(pom); err != nil {
+		return analysisResult{}, xerrors.Errorf("pom resolve error: %w", err)
 	}
 
-	// Inherit values/properties from parent
-	pom.inherit(parent)
-
-	// Generate properties
+	// Resolve dependencies
 	props := pom.properties()
-
-	// dependencyManagements have the next priority:
-	// 1. Managed dependencies from this POM
-	// 2. Managed dependencies from parent of this POM
-	depManagement := p.mergeDependencyManagements(pom.content.DependencyManagement.Dependencies.Dependency,
-		parent.dependencyManagement)
-
-	// Merge dependencies. Child dependencies must be preferred than parent dependencies.
-	// Parents don't have to resolve dependencies.
+	depManagement := pom.content.DependencyManagement.Dependencies.Dependency
 	deps := p.parseDependencies(pom.content.Dependencies.Dependency, props, depManagement, opts)
-	deps = p.mergeDependencies(parent.dependencies, deps, opts.exclusions)
+	deps = p.filterDependencies(deps, opts.exclusions)
 
 	return analysisResult{
 		filePath:             pom.filePath,
@@ -388,6 +362,39 @@ func (p *Parser) analyze(pom *pom, opts analysisOptions) (analysisResult, error)
 	}, nil
 }
 
+// resolveParent resolves its parent POMs and inherits properties, dependencies, and dependencyManagement.
+func (p *Parser) resolveParent(pom *pom) error {
+	if pom.nil() {
+		return nil
+	}
+
+	// Parse parent POM
+	parent, err := p.parseParent(pom.filePath, pom.content.Parent)
+	if err != nil {
+		return xerrors.Errorf("parent error: %w", err)
+	}
+
+	// Inherit values/properties from parent
+	pom.inherit(parent)
+
+	// Merge properties
+	pom.content.Properties = p.mergeProperties(pom.content.Properties, parent.content.Properties)
+
+	// Merge dependencyManagement with the following priority:
+	// 1. Managed dependencies from this POM
+	// 2. Managed dependencies from parent of this POM
+	pom.content.DependencyManagement.Dependencies.Dependency = p.mergeDependencyManagements(
+		pom.content.DependencyManagement.Dependencies.Dependency,
+		parent.content.DependencyManagement.Dependencies.Dependency)
+
+	// Merge dependencies
+	pom.content.Dependencies.Dependency = p.mergeDependencies(
+		pom.content.Dependencies.Dependency,
+		parent.content.Dependencies.Dependency)
+
+	return nil
+}
+
 func (p *Parser) mergeDependencyManagements(depManagements ...[]pomDependency) []pomDependency {
 	uniq := make(map[string]struct{})
 	var depManagement []pomDependency
@@ -463,22 +470,20 @@ func (p *Parser) resolveDepManagement(props map[string]string, depManagement []p
 	return newDepManagement
 }
 
-func (p *Parser) mergeDependencies(parent, child []artifact, exclusions map[string]struct{}) []artifact {
-	var deps []artifact
-	unique := make(map[string]struct{})
+func (p *Parser) mergeProperties(child, parent properties) properties {
+	return lo.Assign(parent, child)
+}
 
-	for _, d := range append(child, parent...) {
-		if excludeDep(exclusions, d) {
-			continue
-		}
-		if _, ok := unique[d.Name()]; ok {
-			continue
-		}
-		unique[d.Name()] = struct{}{}
-		deps = append(deps, d)
-	}
+func (p *Parser) mergeDependencies(child, parent []pomDependency) []pomDependency {
+	return lo.UniqBy(append(child, parent...), func(d pomDependency) string {
+		return d.Name()
+	})
+}
 
-	return deps
+func (p *Parser) filterDependencies(artifacts []artifact, exclusions map[string]struct{}) []artifact {
+	return lo.Filter(artifacts, func(art artifact, _ int) bool {
+		return !excludeDep(exclusions, art)
+	})
 }
 
 func excludeDep(exclusions map[string]struct{}, art artifact) bool {
@@ -497,38 +502,29 @@ func excludeDep(exclusions map[string]struct{}, art artifact) bool {
 	return false
 }
 
-func (p *Parser) parseParent(currentPath string, parent pomParent, rootDepManagement []pomDependency) (analysisResult, error) {
+func (p *Parser) parseParent(currentPath string, parent pomParent) (*pom, error) {
 	// Pass nil properties so that variables in <parent> are not evaluated.
 	target := newArtifact(parent.GroupId, parent.ArtifactId, parent.Version, nil, nil)
 	// if version is property (e.g. ${revision}) - we still need to parse this pom
 	if target.IsEmpty() && !isProperty(parent.Version) {
-		return analysisResult{}, nil
+		return &pom{content: &pomXML{}}, nil
 	}
 
 	logger := p.logger.With("artifact", target.String())
 	logger.Debug("Start parent")
 	defer logger.Debug("Exit parent")
 
-	// If the artifact is found in cache, it is returned.
-	if result := p.cache.get(target); result != nil {
-		return *result, nil
-	}
-
 	parentPOM, err := p.retrieveParent(currentPath, parent.RelativePath, target)
 	if err != nil {
 		logger.Debug("Parent POM not found", log.Err(err))
+		return &pom{content: &pomXML{}}, nil
 	}
 
-	result, err := p.analyze(parentPOM, analysisOptions{
-		depManagement: rootDepManagement,
-	})
-	if err != nil {
-		return analysisResult{}, xerrors.Errorf("analyze error: %w", err)
+	if err = p.resolveParent(parentPOM); err != nil {
+		return nil, xerrors.Errorf("parent pom resolve error: %w", err)
 	}
 
-	p.cache.put(target, result)
-
-	return result, nil
+	return parentPOM, nil
 }
 
 func (p *Parser) retrieveParent(currentPath, relativePath string, target artifact) (*pom, error) {
@@ -565,7 +561,7 @@ func (p *Parser) retrieveParent(currentPath, relativePath string, target artifac
 }
 
 func (p *Parser) tryRelativePath(parentArtifact artifact, currentPath, relativePath string) (*pom, error) {
-	pom, err := p.openRelativePom(currentPath, relativePath)
+	parsedPOM, err := p.openRelativePom(currentPath, relativePath)
 	if err != nil {
 		return nil, err
 	}
@@ -576,19 +572,18 @@ func (p *Parser) tryRelativePath(parentArtifact artifact, currentPath, relativeP
 	// But GroupID can be inherited from parent (`p.analyze` function is required to get the GroupID).
 	// Version can contain a property (`p.analyze` function is required to get the GroupID).
 	// So we can only match ArtifactID's.
-	if pom.artifact().ArtifactID != parentArtifact.ArtifactID {
+	if parsedPOM.artifact().ArtifactID != parentArtifact.ArtifactID {
 		return nil, xerrors.New("'parent.relativePath' points at wrong local POM")
 	}
-	result, err := p.analyze(pom, analysisOptions{})
-	if err != nil {
+	if err := p.resolveParent(parsedPOM); err != nil {
 		return nil, xerrors.Errorf("analyze error: %w", err)
 	}
 
-	if !parentArtifact.Equal(result.artifact) {
+	if !parentArtifact.Equal(parsedPOM.artifact()) {
 		return nil, xerrors.New("'parent.relativePath' points at wrong local POM")
 	}
 
-	return pom, nil
+	return parsedPOM, nil
 }
 
 func (p *Parser) openRelativePom(currentPath, relativePath string) (*pom, error) {
@@ -620,7 +615,7 @@ func (p *Parser) openPom(filePath string) (*pom, error) {
 	}
 	defer f.Close()
 
-	content, err := parsePom(f)
+	content, err := parsePom(f, false)
 	if err != nil {
 		return nil, xerrors.Errorf("failed to parse the local POM: %w", err)
 	}
@@ -777,7 +772,7 @@ func (p *Parser) fetchPOMFromRemoteRepository(repo string, paths []string) (*pom
 	}
 	defer resp.Body.Close()
 
-	content, err := parsePom(resp.Body)
+	content, err := parsePom(resp.Body, false)
 	if err != nil {
 		return nil, xerrors.Errorf("failed to parse the remote POM: %w", err)
 	}
@@ -788,13 +783,19 @@ func (p *Parser) fetchPOMFromRemoteRepository(repo string, paths []string) (*pom
 	}, nil
 }
 
-func parsePom(r io.Reader) (*pomXML, error) {
+func parsePom(r io.Reader, lineNumber bool) (*pomXML, error) {
 	parsed := &pomXML{}
 	decoder := xml.NewDecoder(r)
 	decoder.CharsetReader = charset.NewReaderLabel
 	if err := decoder.Decode(parsed); err != nil {
 		return nil, xerrors.Errorf("xml decode error: %w", err)
 	}
+	if !lineNumber {
+		for i := range parsed.Dependencies.Dependency {
+			parsed.Dependencies.Dependency[i].StartLine = 0
+			parsed.Dependencies.Dependency[i].EndLine = 0
+		}
+	}
 	return parsed, nil
 }