From b64870419cdd8b8598f85969fdf279e6e5e4e170 Mon Sep 17 00:00:00 2001 From: Michael Kriese Date: Wed, 30 Sep 2026 13:04:49 +0200 Subject: [PATCH] refactor: tighten the tool map types Type ResolverMap and DeprecatedTools without undefined values and look them up through getToolType, which only matches own keys. Co-Authored-By: Claude Opus 5.5 Co-Authored-By: Claude Sonnet 5.5 --- src/cli/command/install-tool.ts | 6 +++--- src/cli/command/uninstall-tool.ts | 4 ++-- src/cli/install-tool/index.ts | 4 ++-- src/cli/tools/index.spec.ts | 24 ++++++++++++++++++++++++ src/cli/tools/index.ts | 18 ++++++++++++++++-- 5 files changed, 47 insertions(+), 9 deletions(-) create mode 100644 src/cli/tools/index.spec.ts diff --git a/src/cli/command/install-tool.ts b/src/cli/command/install-tool.ts index 384f329237..27b9c7e4be 100644 --- a/src/cli/command/install-tool.ts +++ b/src/cli/command/install-tool.ts @@ -2,7 +2,7 @@ import { isNonEmptyStringAndNotWhitespace } from '@sindresorhus/is'; import { Command, Option } from 'clipanion'; import prettyMilliseconds from 'pretty-ms'; import { installTool, resolveVersion } from '../install-tool/index.ts'; -import { DeprecatedTools, ResolverMap } from '../tools/index.ts'; +import { DeprecatedTools, ResolverMap, getToolType } from '../tools/index.ts'; import type { InstallToolType } from '../utils'; import { MissingVersion } from '../utils/codes.ts'; import { logger } from '../utils/index.ts'; @@ -46,14 +46,14 @@ export class InstallToolCommand extends Command { let version = this.version?.replace(/^v/, ''); // trim optional 'v' prefix - let type = DeprecatedTools[this.name]; + let type = getToolType(DeprecatedTools, this.name); if (type) { logger.warn( `The 'install-tool ${this.name}' command is deprecated. Please use the 'install-${type} ${this.name}'.`, ); } else { - type = ResolverMap[this.name] ?? this.type; + type = getToolType(ResolverMap, this.name) ?? this.type; } if (!isNonEmptyStringAndNotWhitespace(version)) { diff --git a/src/cli/command/uninstall-tool.ts b/src/cli/command/uninstall-tool.ts index 211d907365..5986e2e850 100644 --- a/src/cli/command/uninstall-tool.ts +++ b/src/cli/command/uninstall-tool.ts @@ -2,7 +2,7 @@ import { isNonEmptyStringAndNotWhitespace } from '@sindresorhus/is'; import { Command, Option } from 'clipanion'; import prettyMilliseconds from 'pretty-ms'; import { uninstallTool } from '../install-tool/index.ts'; -import { ResolverMap } from '../tools/index.ts'; +import { ResolverMap, getToolType } from '../tools/index.ts'; import type { InstallToolType } from '../utils'; import { MissingVersion } from '../utils/codes.ts'; import { logger } from '../utils/index.ts'; @@ -55,7 +55,7 @@ export class UninstallToolCommand extends Command { let version = this.version; - const type = ResolverMap[tool] ?? this.type; + const type = getToolType(ResolverMap, tool) ?? this.type; if (!isNonEmptyStringAndNotWhitespace(version) && !all) { logger.error(`No version found for ${tool}`); diff --git a/src/cli/install-tool/index.ts b/src/cli/install-tool/index.ts index 983f24bb31..d1e5f18650 100644 --- a/src/cli/install-tool/index.ts +++ b/src/cli/install-tool/index.ts @@ -39,7 +39,7 @@ import { CabalInstallService } from '../tools/haskell/cabal.ts'; import { GhcInstallService } from '../tools/haskell/ghc.ts'; import { HelmInstallService } from '../tools/helm.ts'; import { HelmfileInstallService } from '../tools/helmfile.ts'; -import { ResolverMap } from '../tools/index.ts'; +import { ResolverMap, getToolType } from '../tools/index.ts'; import { AndroidSdkCmdlineToolsInstallService, AndroidSdkCmdlineToolsVersionResolver, @@ -332,7 +332,7 @@ export async function installTool( // some pip packages may not have a `--version` flag await super.test(version); } catch (err) { - if (ResolverMap[tool] === 'pip') { + if (getToolType(ResolverMap, tool) === 'pip') { // those tools are known and should work throw err; } diff --git a/src/cli/tools/index.spec.ts b/src/cli/tools/index.spec.ts new file mode 100644 index 0000000000..094e2c4d8f --- /dev/null +++ b/src/cli/tools/index.spec.ts @@ -0,0 +1,24 @@ +import { describe, expect, test } from 'vitest'; +import { getToolType } from './index.ts'; + +describe('cli/tools/index', () => { + describe('getToolType', () => { + const map = { poetry: 'pip', pnpm: 'npm' } as const; + + test('returns the type of a mapped tool', () => { + expect(getToolType(map, 'poetry')).toBe('pip'); + expect(getToolType(map, 'pnpm')).toBe('npm'); + }); + + test('returns undefined for an unknown tool', () => { + expect(getToolType(map, 'unknown')).toBeUndefined(); + }); + + test.each(['constructor', 'toString', '__proto__'])( + 'returns undefined for the prototype key %s', + (name) => { + expect(getToolType(map, name)).toBeUndefined(); + }, + ); + }); +}); diff --git a/src/cli/tools/index.ts b/src/cli/tools/index.ts index 7e0b817fc6..439c7003ff 100644 --- a/src/cli/tools/index.ts +++ b/src/cli/tools/index.ts @@ -67,7 +67,7 @@ export const NoInitTools = [ * Tools in this map are implicit mapped from `install-tool` to `install-`. * So no need for an extra install service. */ -export const ResolverMap: Record = { +export const ResolverMap: Record = { bundler: 'gem', checkov: 'pip', copier: 'pip', @@ -87,7 +87,21 @@ export const ResolverMap: Record = { * This tools are deprecated and should not be used anymore via `install-tool`. * They are implicit mapped from `install-tool` to `install-`. */ -export const DeprecatedTools: Record = { +export const DeprecatedTools: Record = { bower: 'npm', lerna: 'npm', }; + +/** + * Looks up the install type of a tool in the given map. + * Only own keys resolve, so names like `constructor` never match. + * @param map - tool to install type map + * @param name - tool name + * @returns the mapped install type or `undefined` + */ +export function getToolType( + map: Record, + name: string, +): InstallToolType | undefined { + return Object.hasOwn(map, name) ? map[name] : undefined; +}