Get the direct dependencies while checking for promotion #952

yezhu6 · 2025-12-16T08:37:19Z

add the tool id into the java upgrade prompts to specify the calling tool
Change the getDependencies to getDirectDependencies to exclude the transitive dependencies

src/upgrade/assessmentManager.ts

…t errors

…s in the assessmentManager

Copilot

Pull request overview

This PR refactors the dependency upgrade notification system to focus on direct dependencies instead of all transitive dependencies, aiming to reduce noise in upgrade notifications. The key changes include:

Replacing the workspace-URI-based assessment with a project-dependency-based approach
Adding file-system parsing of pom.xml and Gradle build files to identify direct dependencies
Prioritizing CVE issues in notifications by filtering and handling them separately

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 11 comments.

File	Description
src/upgrade/upgradeManager.ts	Refactored dependency checkup to collect direct dependencies per project and pass structured data to assessmentManager
src/upgrade/assessmentManager.ts	Added extensive logic to parse Maven pom.xml and Gradle build files using regex patterns to filter transitive dependencies; exported getDirectDependencies function
src/upgrade/display/notificationManager.ts	Improved CVE issue prioritization by filtering and extracting CVE issues earlier in the notification rendering logic
package.json	Version bumped from 0.26.5 to 0.27.0 reflecting the new feature

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-26T03:34:54Z

src/upgrade/assessmentManager.ts

+        const shortFormRegex = /(?:implementation|api|compile|compileOnly|runtimeOnly|testImplementation|testCompileOnly|testRuntimeOnly)\s*\(?['"]([^:'"]+):([^:'"]+)(?::[^'"]*)?['"]\)?/g;
+        let match = shortFormRegex.exec(gradleContent);
+        while (match !== null) {
+            const groupId = match[1].trim();
+            const artifactId = match[2].trim();
+            if (!groupId.includes('$') && !artifactId.includes('$')) {
+                directDeps.add(`${groupId}:${artifactId}`);
+            }
+            match = shortFormRegex.exec(gradleContent);
+        }
+
+        // Match map notation: implementation group: 'x', name: 'y', version: 'z'
+        const mapFormRegex = /(?:implementation|api|compile|compileOnly|runtimeOnly|testImplementation|testCompileOnly|testRuntimeOnly)\s*\(?group:\s*['"]([^'"]+)['"]\s*,\s*name:\s*['"]([^'"]+)['"]/g;


The Gradle dependency parser includes test-scoped dependencies (testImplementation, testCompileOnly, testRuntimeOnly) at lines 260 and 272. These test dependencies should typically be excluded from upgrade notifications as they are not part of the production runtime. Consider either filtering out test configurations or making this behavior configurable.

Copilot · 2025-12-26T03:34:54Z

src/upgrade/assessmentManager.ts

+import * as glob from 'glob';
+import { promisify } from 'util';
+
+const globAsync = promisify(glob);


The glob package is imported at line 6 but it's only listed as a devDependency in package.json (line 1103), not as a regular dependency. This will cause runtime errors in production builds since devDependencies are not included. Move glob from devDependencies to dependencies in package.json, or consider using the built-in globby package which is already listed as a regular dependency (line 1118).

Suggested change

import * as glob from 'glob';

import { promisify } from 'util';

const globAsync = promisify(glob);

import globby from 'globby';

import { promisify } from 'util';

const globAsync = globby;

Copilot · 2025-12-26T03:34:54Z

src/upgrade/assessmentManager.ts

+    const dependenciesSet: Set<PackageDescription> = new Set();
+    for (const { projectNode, dependencies } of projectDeps) {
+        issues.push(...getJavaIssues(projectNode));
+        dependencies.forEach(dep => dependenciesSet.add(dep));
+    }
+    issues.push(...await getCVEIssues(Array.from(dependenciesSet)));
+    issues.push(...await getDependencyIssues(Array.from(dependenciesSet)));


Using a Set for deduplication of PackageDescription objects will not work as expected. Set uses identity comparison for objects, so duplicate dependencies with the same groupId, artifactId, and version will not be deduplicated. Consider using a Map with a composite key (e.g., ${groupId}:${artifactId}:${version}) or deduplicate by creating a unique identifier.

Suggested change

const dependenciesSet: Set<PackageDescription> = new Set();

for (const { projectNode, dependencies } of projectDeps) {

issues.push(...getJavaIssues(projectNode));

dependencies.forEach(dep => dependenciesSet.add(dep));

}

issues.push(...await getCVEIssues(Array.from(dependenciesSet)));

issues.push(...await getDependencyIssues(Array.from(dependenciesSet)));

const dependencyMap: Map<string, PackageDescription> = new Map();

for (const { projectNode, dependencies } of projectDeps) {

issues.push(...getJavaIssues(projectNode));

for (const dep of dependencies) {

const key = `${dep.groupId}:${dep.artifactId}:${dep.version ?? ""}`;

if (!dependencyMap.has(key)) {

dependencyMap.set(key, dep);

}

}

}

const uniqueDependencies = Array.from(dependencyMap.values());

issues.push(...await getCVEIssues(uniqueDependencies));

issues.push(...await getDependencyIssues(uniqueDependencies));

Copilot · 2025-12-26T03:34:54Z

src/upgrade/assessmentManager.ts

+        // Match <dependency> blocks and extract groupId and artifactId
+        const dependencyRegex = /<dependency>\s*<groupId>([^<]+)<\/groupId>\s*<artifactId>([^<]+)<\/artifactId>/g;
+        let match = dependencyRegex.exec(withoutDepMgmt);
+        while (match !== null) {
+            const groupId = match[1].trim();
+            const artifactId = match[2].trim();
+            // Skip property references like ${project.groupId}
+            if (!groupId.includes('${') && !artifactId.includes('${')) {
+                directDeps.add(`${groupId}:${artifactId}`);
+            }
+            match = dependencyRegex.exec(withoutDepMgmt);
        }


The regex pattern for parsing Maven dependencies has limitations:

It requires groupId to come before artifactId, but XML allows any order

It doesn't handle extra whitespace or newlines between elements robustly

The pattern assumes no whitespace inside tags, but XML parsers would allow < groupId >

Consider using a proper XML parser (e.g., xml2js or fast-xml-parser) for more reliable parsing of pom.xml files.

Copilot · 2025-12-26T03:34:55Z

src/upgrade/assessmentManager.ts

+        //TODO: fallback to return all dependencies if we cannot parse direct dependencies or just return empty?
+        return dependencies;
+    }
+    // Filter to only direct dependencies if we have build file info
+    if (directDependencyIds && directDependencyIds.size > 0) {
+        dependencies = dependencies.filter(pkg =>
+            directDependencyIds!.has(`${pkg.groupId}:${pkg.artifactId}`)
+        );
+    }
+


This condition is redundant. If directDependencyIds is null at line 367, the function returns at line 372, so directDependencyIds cannot be null when reaching this line. Additionally, the check directDependencyIds.size > 0 at line 375 is problematic: if the size is 0 (meaning no dependencies were parsed), the function should either return an empty array or log a warning, not return unfiltered dependencies.

Suggested change

//TODO: fallback to return all dependencies if we cannot parse direct dependencies or just return empty?

return dependencies;

}

// Filter to only direct dependencies if we have build file info

if (directDependencyIds && directDependencyIds.size > 0) {

dependencies = dependencies.filter(pkg =>

directDependencyIds!.has(`${pkg.groupId}:${pkg.artifactId}`)

);

}

// When we cannot parse direct dependencies, return an empty list instead of all dependencies.

return [];

}

// Filter to only direct dependencies if we have build file info

if (directDependencyIds.size === 0) {

sendInfo("", {

operationName: "java.dependency.assessmentManager.getDirectDependencies.emptyDirectDependencyList"

});

return [];

}

dependencies = dependencies.filter(pkg =>

directDependencyIds.has(`${pkg.groupId}:${pkg.artifactId}`)

);

Copilot · 2025-12-26T03:34:55Z

src/upgrade/assessmentManager.ts

+        // Match <dependency> blocks and extract groupId and artifactId
+        const dependencyRegex = /<dependency>\s*<groupId>([^<]+)<\/groupId>\s*<artifactId>([^<]+)<\/artifactId>/g;
+        let match = dependencyRegex.exec(withoutDepMgmt);
+        while (match !== null) {
+            const groupId = match[1].trim();
+            const artifactId = match[2].trim();
+            // Skip property references like ${project.groupId}
+            if (!groupId.includes('${') && !artifactId.includes('${')) {
+                directDeps.add(`${groupId}:${artifactId}`);
+            }
+            match = dependencyRegex.exec(withoutDepMgmt);


The Maven dependency parser doesn't filter out test-scoped dependencies. Test dependencies should typically be excluded from upgrade notifications since they're not part of the production runtime. Consider adding logic to parse and exclude dependencies with <scope>test</scope> or other non-runtime scopes (provided, system, etc.).

Suggested change

// Match <dependency> blocks and extract groupId and artifactId

const dependencyRegex = /<dependency>\s*<groupId>([^<]+)<\/groupId>\s*<artifactId>([^<]+)<\/artifactId>/g;

let match = dependencyRegex.exec(withoutDepMgmt);

while (match !== null) {

const groupId = match[1].trim();

const artifactId = match[2].trim();

// Skip property references like ${project.groupId}

if (!groupId.includes('${') && !artifactId.includes('${')) {

directDeps.add(`${groupId}:${artifactId}`);

}

match = dependencyRegex.exec(withoutDepMgmt);

// Match full <dependency> blocks so we can inspect scope as well as coordinates

const dependencyBlockRegex = /<dependency>([\s\S]*?)<\/dependency>/g;

let blockMatch = dependencyBlockRegex.exec(withoutDepMgmt);

while (blockMatch !== null) {

const dependencyBlock = blockMatch[1];

const groupIdMatch = /<groupId>\s*([^<]+?)\s*<\/groupId>/.exec(dependencyBlock);

const artifactIdMatch = /<artifactId>\s*([^<]+?)\s*<\/artifactId>/.exec(dependencyBlock);

if (groupIdMatch && artifactIdMatch) {

const groupId = groupIdMatch[1].trim();

const artifactId = artifactIdMatch[1].trim();

// Skip property references like ${project.groupId}

if (!groupId.includes('${') && !artifactId.includes('${')) {

// Extract scope if present; default (no scope) is treated as compile/runtime

const scopeMatch = /<scope>\s*([^<]+?)\s*<\/scope>/.exec(dependencyBlock);

const scope = scopeMatch ? scopeMatch[1].trim() : undefined;

const nonRuntimeScopes = new Set(['test', 'provided', 'system']);

if (!scope || !nonRuntimeScopes.has(scope)) {

directDeps.add(`${groupId}:${artifactId}`);

}

}

}

blockMatch = dependencyBlockRegex.exec(withoutDepMgmt);

Copilot · 2025-12-26T03:34:55Z

src/upgrade/assessmentManager.ts

+        const shortFormRegex = /(?:implementation|api|compile|compileOnly|runtimeOnly|testImplementation|testCompileOnly|testRuntimeOnly)\s*\(?['"]([^:'"]+):([^:'"]+)(?::[^'"]*)?['"]\)?/g;
+        let match = shortFormRegex.exec(gradleContent);
+        while (match !== null) {
+            const groupId = match[1].trim();
+            const artifactId = match[2].trim();
+            if (!groupId.includes('$') && !artifactId.includes('$')) {
+                directDeps.add(`${groupId}:${artifactId}`);
+            }
+            match = shortFormRegex.exec(gradleContent);
+        }
+
+        // Match map notation: implementation group: 'x', name: 'y', version: 'z'
+        const mapFormRegex = /(?:implementation|api|compile|compileOnly|runtimeOnly|testImplementation|testCompileOnly|testRuntimeOnly)\s*\(?group:\s*['"]([^'"]+)['"]\s*,\s*name:\s*['"]([^'"]+)['"]/g;
+        match = mapFormRegex.exec(gradleContent);
+        while (match !== null) {
+            const groupId = match[1].trim();
+            const artifactId = match[2].trim();
+            if (!groupId.includes('$') && !artifactId.includes('$')) {
+                directDeps.add(`${groupId}:${artifactId}`);
+            }
+            match = mapFormRegex.exec(gradleContent);
+        }


The regex patterns for parsing Gradle dependencies have several limitations:

They don't handle multiline dependency declarations (e.g., when group, name, and version are on separate lines)

The Kotlin DSL syntax differs from Groovy (uses parentheses and different string syntax)

Complex scenarios like dependency constraints, platform dependencies, or dependencies with closures will be missed

Consider using Gradle's built-in dependency reporting (e.g., parsing output from gradle dependencies) or a more robust parser.

Copilot · 2025-12-26T03:34:56Z

src/upgrade/assessmentManager.ts

+        } catch {
+            // Ignore errors


The catch blocks at lines 362-364 silently ignore all errors when parsing build files. This makes debugging difficult when the parsing logic fails. Consider logging errors or sending telemetry with error details to help diagnose why direct dependency parsing might fail in production.

Suggested change

} catch {

// Ignore errors

} catch (error) {

// Log parsing errors for diagnostics while preserving fallback behavior

let errorName: string | undefined;

let errorMessage: string | undefined;

let errorStack: string | undefined;

if (error instanceof Error) {

errorName = error.name;

errorMessage = error.message;

errorStack = error.stack;

} else if (error !== undefined && error !== null) {

errorMessage = String(error);

}

sendInfo("", {

operationName: "java.dependency.assessmentManager.getDirectDependencies.parseDirectDependencies.error",

buildTool: isMaven ? "maven" : "gradle",

projectUri: projectNode.uri,

errorName,

errorMessage,

// Truncate stack to avoid oversized telemetry; rely on default batching limits.

errorStack,

});

Copilot · 2025-12-26T03:34:56Z

src/upgrade/assessmentManager.ts

+
+        // Extract dependencies from <dependencies> section (not inside <dependencyManagement>)
+        // First, remove dependencyManagement sections to avoid including managed deps
+        const withoutDepMgmt = pomContent.replace(/<dependencyManagement>[\s\S]*?<\/dependencyManagement>/g, '');


The regex pattern /<dependencyManagement>[\s\S]*?<\/dependencyManagement>/g uses a non-greedy match, but this can fail with nested structures. If the XML contains CDATA sections, comments with these tags, or multiple dependencyManagement blocks, the regex might remove content incorrectly. Also, the regex won't handle self-closing tags or handle namespace prefixes (e.g., <maven:dependencyManagement>). Consider using a proper XML parser instead of regex-based string manipulation.

Copilot · 2025-12-26T03:34:56Z

src/upgrade/assessmentManager.ts

+    if (!dependencies) {
+        sendInfo("", {
+            operationName: "java.dependency.assessmentManager.getDirectDependencies.noDependencyInfo"
+        });
+        return [];
+    }


The check if (!dependencies) at line 343 is incorrect. The expression fulfilled.map(x => x.value).flat() will always return an array (possibly empty), never null or undefined. This condition will never be true. If you want to check for an empty array, use if (dependencies.length === 0) instead.

yezhu6 requested review from chagong, testforstephen and wenytang-ms as code owners December 16, 2025 08:37

wangmingliang-ms reviewed Dec 17, 2025

View reviewed changes

src/upgrade/assessmentManager.ts Outdated Show resolved Hide resolved

wangmingliang-ms reviewed Dec 17, 2025

View reviewed changes

src/upgrade/assessmentManager.ts Outdated Show resolved Hide resolved

yezhu6 added 2 commits December 24, 2025 13:51

get the direct dependencies

eedc65e

fix comments

1d4ca96

yezhu6 force-pushed the yezhu/get-direct-deps branch from 0fc93e4 to 1d4ca96 Compare December 24, 2025 05:51

yezhu6 added 3 commits December 24, 2025 15:02

Put the projectype checking in the getDirectDependencies && fix tslin…

d112421

…t errors

Move getDirectDeps into upgradeManager and just check the dependencie…

ea52c38

…s in the assessmentManager

update package version

0398525

chagong previously approved these changes Dec 26, 2025

View reviewed changes

wangmingliang-ms requested a review from Copilot December 26, 2025 03:28

Copilot started reviewing on behalf of wangmingliang-ms December 26, 2025 03:29 View session

Copilot AI reviewed Dec 26, 2025

View reviewed changes

wangmingliang-ms previously approved these changes Dec 26, 2025

View reviewed changes

Defer the upgrade scan

4fa1c05

yezhu6 dismissed stale reviews from wangmingliang-ms and chagong via 4fa1c05 December 26, 2025 04:46

fix comments

44b2781

chagong previously approved these changes Dec 26, 2025

View reviewed changes

Fix tslint errors

c248b7e

yezhu6 dismissed chagong’s stale review via c248b7e December 26, 2025 06:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Get the direct dependencies while checking for promotion #952

Get the direct dependencies while checking for promotion #952

yezhu6 commented Dec 16, 2025

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 26, 2025

Uh oh!

Copilot AI Dec 26, 2025

Uh oh!

Copilot AI Dec 26, 2025

Uh oh!

Copilot AI Dec 26, 2025

Uh oh!

Copilot AI Dec 26, 2025

Uh oh!

Copilot AI Dec 26, 2025

Uh oh!

Copilot AI Dec 26, 2025

Uh oh!

Copilot AI Dec 26, 2025

Uh oh!

Copilot AI Dec 26, 2025

Uh oh!

Copilot AI Dec 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

-        // Match <dependency> blocks and extract groupId and artifactId
-        const dependencyRegex = /<dependency>\s*<groupId>([^<]+)<\/groupId>\s*<artifactId>([^<]+)<\/artifactId>/g;
-        let match = dependencyRegex.exec(withoutDepMgmt);
-        while (match !== null) {
-            const groupId = match[1].trim();
-            const artifactId = match[2].trim();
-            // Skip property references like ${project.groupId}
-            if (!groupId.includes('${') && !artifactId.includes('${')) {
-                directDeps.add(`${groupId}:${artifactId}`);
-            }
-            match = dependencyRegex.exec(withoutDepMgmt);
+        // Match full <dependency> blocks so we can inspect scope as well as coordinates
+        const dependencyBlockRegex = /<dependency>([\s\S]*?)<\/dependency>/g;
+        let blockMatch = dependencyBlockRegex.exec(withoutDepMgmt);
+        while (blockMatch !== null) {
+            const dependencyBlock = blockMatch[1];
+            const groupIdMatch = /<groupId>\s*([^<]+?)\s*<\/groupId>/.exec(dependencyBlock);
+            const artifactIdMatch = /<artifactId>\s*([^<]+?)\s*<\/artifactId>/.exec(dependencyBlock);
+            if (groupIdMatch && artifactIdMatch) {
+                const groupId = groupIdMatch[1].trim();
+                const artifactId = artifactIdMatch[1].trim();
+                // Skip property references like ${project.groupId}
+                if (!groupId.includes('${') && !artifactId.includes('${')) {
+                    // Extract scope if present; default (no scope) is treated as compile/runtime
+                    const scopeMatch = /<scope>\s*([^<]+?)\s*<\/scope>/.exec(dependencyBlock);
+                    const scope = scopeMatch ? scopeMatch[1].trim() : undefined;
+                    const nonRuntimeScopes = new Set(['test', 'provided', 'system']);
+                    if (!scope || !nonRuntimeScopes.has(scope)) {
+                        directDeps.add(`${groupId}:${artifactId}`);
+                    }
+                }
+            }
+            blockMatch = dependencyBlockRegex.exec(withoutDepMgmt);

-        } catch {
-            // Ignore errors
+        } catch (error) {
+            // Log parsing errors for diagnostics while preserving fallback behavior
+            let errorName: string | undefined;
+            let errorMessage: string | undefined;
+            let errorStack: string | undefined;
+            if (error instanceof Error) {
+                errorName = error.name;
+                errorMessage = error.message;
+                errorStack = error.stack;
+            } else if (error !== undefined && error !== null) {
+                errorMessage = String(error);
+            }
+            sendInfo("", {
+                operationName: "java.dependency.assessmentManager.getDirectDependencies.parseDirectDependencies.error",
+                buildTool: isMaven ? "maven" : "gradle",
+                projectUri: projectNode.uri,
+                errorName,
+                errorMessage,
+                // Truncate stack to avoid oversized telemetry; rely on default batching limits.
+                errorStack,
+            });

Get the direct dependencies while checking for promotion #952

Are you sure you want to change the base?

Get the direct dependencies while checking for promotion #952

Conversation

yezhu6 commented Dec 16, 2025

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants