From 7d2d456be4e871e533d34da00a8eb3a03cce5056 Mon Sep 17 00:00:00 2001 From: John <1615532+johnoliver@users.noreply.github.com> Date: Fri, 12 Jun 2026 11:49:40 +0000 Subject: [PATCH] Address Copilot code review comments - Use strict equality (===, !==) instead of loose equality (==, !=) for all comparisons - Properly handle caught errors with instanceof type narrowing before accessing properties - Only fall back to legacy AdoptOpenJDK for specific version-not-found errors - Rethrow unexpected errors to avoid masking real issues (network failures, rate limits, etc.) - Fix error message check to match actual error text ('No matching version found') - Remove unnecessary undefined check since method return type is never undefined - Add @internal JSDoc annotation to TemurinDistribution.findPackageForDownload() - Update tests to properly mock Temurin lookup failures for fallback behavior testing - Rebuild dist files --- .../distributors/adopt-installer.test.ts | 16 +++++++ dist/setup/index.js | 44 ++++++++++++++++++- src/distributions/adopt/installer.ts | 36 +++++++++------ src/distributions/temurin/installer.ts | 3 ++ 4 files changed, 85 insertions(+), 14 deletions(-) diff --git a/__tests__/distributors/adopt-installer.test.ts b/__tests__/distributors/adopt-installer.test.ts index 55649255..9accc266 100644 --- a/__tests__/distributors/adopt-installer.test.ts +++ b/__tests__/distributors/adopt-installer.test.ts @@ -278,6 +278,10 @@ describe('findPackageForDownload', () => { }, AdoptImplementation.Hotspot ); + // Mock Temurin to fail so fallback to AdoptOpenJDK is tested + distribution['temurinDistribution']!['findPackageForDownload'] = async () => { + throw new Error('No matching version found for SemVer'); + }; distribution['getAvailableVersions'] = async () => manifestData as any; const resolvedVersion = await distribution['findPackageForDownload'](input); expect(resolvedVersion.version).toBe(expected); @@ -293,6 +297,10 @@ describe('findPackageForDownload', () => { }, AdoptImplementation.Hotspot ); + // Mock Temurin to fail so fallback to AdoptOpenJDK is tested + distribution['temurinDistribution']!['findPackageForDownload'] = async () => { + throw new Error('No matching version found for SemVer'); + }; distribution['getAvailableVersions'] = async () => manifestData as any; await expect( distribution['findPackageForDownload']('9.0.8') @@ -309,6 +317,10 @@ describe('findPackageForDownload', () => { }, AdoptImplementation.Hotspot ); + // Mock Temurin to fail so fallback to AdoptOpenJDK is tested + distribution['temurinDistribution']!['findPackageForDownload'] = async () => { + throw new Error('No matching version found for SemVer'); + }; distribution['getAvailableVersions'] = async () => manifestData as any; await expect(distribution['findPackageForDownload']('7.x')).rejects.toThrow( /No matching version found for SemVer */ @@ -325,6 +337,10 @@ describe('findPackageForDownload', () => { }, AdoptImplementation.Hotspot ); + // Mock Temurin to fail so fallback to AdoptOpenJDK is tested + distribution['temurinDistribution']!['findPackageForDownload'] = async () => { + throw new Error('No matching version found for SemVer'); + }; distribution['getAvailableVersions'] = async () => []; await expect(distribution['findPackageForDownload']('11')).rejects.toThrow( /No matching version found for SemVer */ diff --git a/dist/setup/index.js b/dist/setup/index.js index 16fca2a5..0d4176d2 100644 --- a/dist/setup/index.js +++ b/dist/setup/index.js @@ -77815,17 +77815,56 @@ const path_1 = __importDefault(__nccwpck_require__(71017)); const semver_1 = __importDefault(__nccwpck_require__(11383)); const base_installer_1 = __nccwpck_require__(59741); const util_1 = __nccwpck_require__(92629); +const installer_1 = __nccwpck_require__(18579); var AdoptImplementation; (function (AdoptImplementation) { AdoptImplementation["Hotspot"] = "Hotspot"; AdoptImplementation["OpenJ9"] = "OpenJ9"; })(AdoptImplementation || (exports.AdoptImplementation = AdoptImplementation = {})); class AdoptDistribution extends base_installer_1.JavaBase { - constructor(installerOptions, jvmImpl) { + constructor(installerOptions, jvmImpl, temurinDistribution = null) { super(`Adopt-${jvmImpl}`, installerOptions); this.jvmImpl = jvmImpl; + this.temurinDistribution = temurinDistribution; + if (temurinDistribution !== null && + jvmImpl !== AdoptImplementation.Hotspot) { + throw new Error('Only Hotspot JVM is supported by Temurin.'); + } + // Only use the temurin repo for Hotspot JVMs + if (temurinDistribution === null && + jvmImpl === AdoptImplementation.Hotspot) { + this.temurinDistribution = new installer_1.TemurinDistribution(installerOptions, installer_1.TemurinImplementation.Hotspot); + } } findPackageForDownload(version) { + return __awaiter(this, void 0, void 0, function* () { + if (this.jvmImpl === AdoptImplementation.Hotspot) { + core.notice("AdoptOpenJDK has moved to Eclipse Temurin https://github.com/actions/setup-java#supported-distributions please consider changing to the 'temurin' distribution type in your setup-java configuration."); + } + if (this.jvmImpl === AdoptImplementation.Hotspot && + this.temurinDistribution !== null) { + try { + return yield this.temurinDistribution.findPackageForDownload(version); + } + catch (error) { + // Only fall back to legacy AdoptOpenJDK for version-not-found errors + if (error instanceof Error && + error.message.includes('No matching version found')) { + core.notice('The JVM you are looking for could not be found in the Temurin repository, this likely indicates ' + + 'that you are using an out of date version of Java, consider updating and moving to using the Temurin distribution type in setup-java.'); + } + else { + // Rethrow unexpected errors to avoid masking real issues + core.debug(`Unexpected error from Temurin lookup: ${error instanceof Error ? error.message : String(error)}`); + throw error; + } + } + } + // failed to find a Temurin version, so fall back to AdoptOpenJDK + return this.findPackageForDownloadOldAdoptOpenJdk(version); + }); + } + findPackageForDownloadOldAdoptOpenJdk(version) { return __awaiter(this, void 0, void 0, function* () { const availableVersionsRaw = yield this.getAvailableVersions(); const availableVersionsWithBinaries = availableVersionsRaw @@ -80195,6 +80234,9 @@ class TemurinDistribution extends base_installer_1.JavaBase { super(`Temurin-${jvmImpl}`, installerOptions); this.jvmImpl = jvmImpl; } + /** + * @internal For cross-distribution reuse only. Not intended as a public API. + */ findPackageForDownload(version) { return __awaiter(this, void 0, void 0, function* () { const availableVersionsRaw = yield this.getAvailableVersions(); diff --git a/src/distributions/adopt/installer.ts b/src/distributions/adopt/installer.ts index 36dde7ab..a8c6f57b 100644 --- a/src/distributions/adopt/installer.ts +++ b/src/distributions/adopt/installer.ts @@ -36,12 +36,18 @@ export class AdoptDistribution extends JavaBase { ) { super(`Adopt-${jvmImpl}`, installerOptions); - if (temurinDistribution != null && jvmImpl != AdoptImplementation.Hotspot) { + if ( + temurinDistribution !== null && + jvmImpl !== AdoptImplementation.Hotspot + ) { throw new Error('Only Hotspot JVM is supported by Temurin.'); } // Only use the temurin repo for Hotspot JVMs - if (temurinDistribution == null && jvmImpl == AdoptImplementation.Hotspot) { + if ( + temurinDistribution === null && + jvmImpl === AdoptImplementation.Hotspot + ) { this.temurinDistribution = new TemurinDistribution( installerOptions, TemurinImplementation.Hotspot @@ -52,30 +58,34 @@ export class AdoptDistribution extends JavaBase { protected async findPackageForDownload( version: string ): Promise { - if (this.jvmImpl == AdoptImplementation.Hotspot) { + if (this.jvmImpl === AdoptImplementation.Hotspot) { core.notice( "AdoptOpenJDK has moved to Eclipse Temurin https://github.com/actions/setup-java#supported-distributions please consider changing to the 'temurin' distribution type in your setup-java configuration." ); } if ( - this.jvmImpl == AdoptImplementation.Hotspot && - this.temurinDistribution != null + this.jvmImpl === AdoptImplementation.Hotspot && + this.temurinDistribution !== null ) { try { - const result = await this.temurinDistribution.findPackageForDownload( - version - ); - - if (result != undefined) { - return result; - } + return await this.temurinDistribution.findPackageForDownload(version); } catch (error) { - if (error.message.includes('Could not find satisfied version')) { + // Only fall back to legacy AdoptOpenJDK for version-not-found errors + if ( + error instanceof Error && + error.message.includes('No matching version found') + ) { core.notice( 'The JVM you are looking for could not be found in the Temurin repository, this likely indicates ' + 'that you are using an out of date version of Java, consider updating and moving to using the Temurin distribution type in setup-java.' ); + } else { + // Rethrow unexpected errors to avoid masking real issues + core.debug( + `Unexpected error from Temurin lookup: ${error instanceof Error ? error.message : String(error)}` + ); + throw error; } } } diff --git a/src/distributions/temurin/installer.ts b/src/distributions/temurin/installer.ts index d16d938d..109c2d41 100644 --- a/src/distributions/temurin/installer.ts +++ b/src/distributions/temurin/installer.ts @@ -34,6 +34,9 @@ export class TemurinDistribution extends JavaBase { super(`Temurin-${jvmImpl}`, installerOptions); } + /** + * @internal For cross-distribution reuse only. Not intended as a public API. + */ public async findPackageForDownload( version: string ): Promise {