Skip to content

Optimize artifact lookup in DefaultProjectDependencyAnalyzer #270

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sanjana2505006 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Optimize artifact lookup in DefaultProjectDependencyAnalyzer #270

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
90cdf73
Optimize artifact lookup and improve code quality in DefaultProjectDe…
sanjana2505006 Feb 2, 2026
8f213b7
Use putIfAbsent in buildClassToArtifactMap to preserve first-match se…
sanjana2505006 Mar 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 25 additions & 23 deletions ...in/java/org/apache/maven/shared/dependency/analyzer/DefaultProjectDependencyAnalyzer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@
import org.apache.maven.project.MavenProject;

/**
* <p>DefaultProjectDependencyAnalyzer class.</p>
* <p>
* DefaultProjectDependencyAnalyzer class.
* </p>
*
* @author <a href="mailto:markhobson@gmail.com">Mark Hobson</a>
*/
Expand All @@ -69,6 +71,7 @@ public ProjectDependencyAnalysis analyze(MavenProject project, Collection<String
try {
ClassesPatterns excludedClassesPatterns = new ClassesPatterns(excludedClasses);
Map<Artifact, Set<String>> artifactClassMap = buildArtifactClassMap(project, excludedClassesPatterns);
Map<String, Artifact> classToArtifactMap = buildClassToArtifactMap(artifactClassMap);

Set<DependencyUsage> mainDependencyClasses = new HashSet<>();
for (MainDependencyClassesProvider provider : mainDependencyClassesProviders) {
Expand All @@ -84,14 +87,14 @@ public ProjectDependencyAnalysis analyze(MavenProject project, Collection<String
dependencyClasses.addAll(mainDependencyClasses);
dependencyClasses.addAll(testDependencyClasses);

Set<DependencyUsage> testOnlyDependencyClasses =
buildTestOnlyDependencyClasses(mainDependencyClasses, testDependencyClasses);
Set<DependencyUsage> testOnlyDependencyClasses = buildTestOnlyDependencyClasses(mainDependencyClasses,
testDependencyClasses);

Map<Artifact, Set<DependencyUsage>> usedArtifacts = buildUsedArtifacts(artifactClassMap, dependencyClasses);
Set<Artifact> mainUsedArtifacts =
buildUsedArtifacts(artifactClassMap, mainDependencyClasses).keySet();
Map<Artifact, Set<DependencyUsage>> usedArtifacts = buildUsedArtifacts(classToArtifactMap,
dependencyClasses);
Set<Artifact> mainUsedArtifacts = buildUsedArtifacts(classToArtifactMap, mainDependencyClasses).keySet();

Set<Artifact> testArtifacts = buildUsedArtifacts(artifactClassMap, testOnlyDependencyClasses)
Set<Artifact> testArtifacts = buildUsedArtifacts(classToArtifactMap, testOnlyDependencyClasses)
.keySet();
Set<Artifact> testOnlyArtifacts = removeAll(testArtifacts, mainUsedArtifacts);

Expand All @@ -105,8 +108,8 @@ public ProjectDependencyAnalysis analyze(MavenProject project, Collection<String
}

Map<Artifact, Set<DependencyUsage>> usedUndeclaredArtifactsWithClasses = new LinkedHashMap<>(usedArtifacts);
Set<Artifact> usedUndeclaredArtifacts =
removeAll(usedUndeclaredArtifactsWithClasses.keySet(), declaredArtifacts);
Set<Artifact> usedUndeclaredArtifacts = removeAll(usedUndeclaredArtifactsWithClasses.keySet(),
declaredArtifacts);

usedUndeclaredArtifactsWithClasses.keySet().retainAll(usedUndeclaredArtifacts);

Expand All @@ -124,7 +127,8 @@ public ProjectDependencyAnalysis analyze(MavenProject project, Collection<String
}

/**
* This method defines a new way to remove the artifacts by using the conflict id. We don't care about the version
* This method defines a new way to remove the artifacts by using the conflict
* id. We don't care about the version
* here because there can be only 1 for a given artifact anyway.
*
* @param start initial set
Expand Down Expand Up @@ -156,7 +160,7 @@ private static Set<Artifact> getTestArtifactsWithNonTestScope(Set<Artifact> test
Set<Artifact> nonTestScopeArtifacts = new LinkedHashSet<>();

for (Artifact artifact : testOnlyArtifacts) {
if (artifact.getScope().equals("compile")) {
if (Artifact.SCOPE_COMPILE.equals(artifact.getScope())) {
nonTestScopeArtifacts.add(artifact);
}
}
Expand Down Expand Up @@ -226,19 +230,14 @@ private static Set<Artifact> buildDeclaredArtifacts(MavenProject project) {
}

private static Map<Artifact, Set<DependencyUsage>> buildUsedArtifacts(
Map<Artifact, Set<String>> artifactClassMap, Set<DependencyUsage> dependencyClasses) {
Map<String, Artifact> classToArtifactMap, Set<DependencyUsage> dependencyClasses) {
Map<Artifact, Set<DependencyUsage>> usedArtifacts = new HashMap<>();

for (DependencyUsage classUsage : dependencyClasses) {
Artifact artifact = findArtifactForClassName(artifactClassMap, classUsage.getDependencyClass());
Artifact artifact = classToArtifactMap.get(classUsage.getDependencyClass());

if (artifact != null && !includedInJDK(artifact)) {
Set<DependencyUsage> classesFromArtifact = usedArtifacts.get(artifact);
if (classesFromArtifact == null) {
classesFromArtifact = new HashSet<>();
usedArtifacts.put(artifact, classesFromArtifact);
}
classesFromArtifact.add(classUsage);
usedArtifacts.computeIfAbsent(artifact, k -> new HashSet<>()).add(classUsage);
}
}
Comment on lines 232 to 242
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no unit tests covering the new class-to-artifact resolution path (including duplicate-class scenarios and ensuring the chosen artifact is deterministic). Adding focused tests around buildUsedArtifacts/buildClassToArtifactMap would help prevent regressions in dependency classification as this is core analysis logic.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@sanjana2505006 sanjana2505006 Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a fair point. I'll add focused unit tests covering the new class-to-artifact resolution path, including duplicate-class scenarios, to ensure the chosen artifact is deterministic. Will update it.


Expand All @@ -260,13 +259,16 @@ private static boolean includedInJDK(Artifact artifact) {
return false;
}

private static Artifact findArtifactForClassName(Map<Artifact, Set<String>> artifactClassMap, String className) {
private static Map<String, Artifact> buildClassToArtifactMap(Map<Artifact, Set<String>> artifactClassMap) {
Map<String, Artifact> classToArtifactMap = new HashMap<>();

for (Map.Entry<Artifact, Set<String>> entry : artifactClassMap.entrySet()) {
if (entry.getValue().contains(className)) {
return entry.getKey();
Artifact artifact = entry.getKey();
for (String className : entry.getValue()) {
classToArtifactMap.putIfAbsent(className, artifact);
}
}

return null;
return classToArtifactMap;
}
}