From fc741f55be25fb23fd10e9beb3c53f3360b30483 Mon Sep 17 00:00:00 2001 From: Eduardo Villalpando Mello Date: Fri, 13 Nov 2026 10:18:01 -0800 Subject: [PATCH] Add typed tool definitions and settings Add opt-in settings contracts and generic resolver return types without changing runtime behavior. Preserve legacy callers and document source-relative adoption. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 85ef72cc-9e26-4bc4-b1c3-55256d9bef91 --- README.md | 4 + typescript/README.md | 79 +++++++++ typescript/src/index.ts | 10 +- typescript/src/settings.ts | 19 +- typescript/src/types.ts | 34 +++- typescript/tests/settingsWorkspace.test.ts | 64 ++++++- typescript/tests/toolDefinition.typecheck.ts | 174 +++++++++++++++++++ 7 files changed, 379 insertions(+), 5 deletions(-) create mode 100644 typescript/tests/toolDefinition.typecheck.ts diff --git a/README.md b/README.md index 3dced4f..506bd10 100644 --- a/README.md +++ b/README.md @@ -197,6 +197,10 @@ notes, but they do not trigger or gate the submodule updates. ### Optional configuration +For an opt-in typed settings contract without changing runtime behavior, see +the [typed tool definition example](typescript/README.md#typed-tool-definitions). +Existing `ToolConfig` callers do not need to migrate. + Extensions whose servers run tools using the interpreter in each resolved settings entry can opt in to per-project environments: diff --git a/typescript/README.md b/typescript/README.md index 8969abb..e8b1e0b 100644 --- a/typescript/README.md +++ b/typescript/README.md @@ -9,3 +9,82 @@ published package or a supported bare-module import. See the [main README](../README.md#consuming-in-extensions) for build order, consumer type-checking and bundling instructions. + +## Typed tool definitions + +Adoption is optional. Annotate a definition with `ToolDefinition` to +check custom defaults and retain the declared types through all three settings +resolvers. The example belongs in an extension's `src/extension.ts`, after +building the checkout under `external/` as described above. Ordinary interfaces +work without an index signature: + +```typescript +import { WorkspaceFolder } from 'vscode'; +import { + getExtensionSettings, + getGlobalSettings, + getWorkspaceSettings, + IInitOptions, + ToolConfig, + ToolDefinition, +} from '../external/vscode-common-python-lsp/typescript/dist'; + +interface LintSettings { + ignorePatterns: string[]; + severity: Record; + enabled: boolean; + optionalLabel?: string; +} + +const tool: ToolDefinition = { + toolId: 'example', + toolDisplayName: 'Example', + toolModule: 'example', + minimumPythonVersion: { major: 3, minor: 10 }, + configFiles: ['tool.toml'], + serverScript: 'bundled/tool/lsp_server.py', // Use the extension's server path. + settingsDefaults: { + ignorePatterns: [], + severity: { E: 'Error' }, + enabled: false, + }, + trackedSettings: ['args', 'path', 'interpreter', 'ignorePatterns', 'severity', 'envFile'], +}; + +// Also accepted directly as createToolContext({ ..., toolConfig: tool }). +export const legacyCompatible: ToolConfig = tool; + +export async function resolveExampleSettings(workspace: WorkspaceFolder) { + const resolved = await getWorkspaceSettings('example', workspace, tool); + const patterns: string[] = resolved.ignorePatterns; + const enabled: boolean = resolved.enabled; + const optionalLabel: string | undefined = resolved.optionalLabel; + const futureOption: unknown = resolved.futureOption; // Narrow before use. + + const initializationOptions: IInitOptions = { + settings: await getExtensionSettings('example', tool, undefined, [workspace]), + globalSettings: await getGlobalSettings('example', tool), + }; + return { patterns, enabled, optionalLabel, futureOption, initializationOptions }; +} +``` + +Use the explicit annotation to retain the full declared types: `false` and `[]` +are defaults, not literal restrictions on resolved values. Required custom +fields require defaults. Optional fields remain optional; omitting a default +also omits that field from resolution, even if it is configured. Common fields +keep their existing `IBaseSettings` types and cannot be contradicted by the +custom declaration. `ToolSettings` names the resulting open settings +type; future wire fields stay `unknown`. + +The resolver signatures infer custom fields from the supplied configuration's +`settingsDefaults`. An unparameterized `ToolConfig` retains the legacy +`IBaseSettings` surface. Definitions also pass directly to existing lifecycle +APIs; no generic lifecycle classes or migration are required. + +This is a compile-time contract, not runtime validation or a schema. +Configuration values, shallow replacement, variable resolution, manifest +defaults and the `{ settings, globalSettings }` initialization payload are +unchanged. Only custom keys in `settingsDefaults` are projected. Keep +`trackedSettings` explicit: a client-only key such as `envFile` may be watched +without being serialized. diff --git a/typescript/src/index.ts b/typescript/src/index.ts index f88d5b1..8a11c0c 100644 --- a/typescript/src/index.ts +++ b/typescript/src/index.ts @@ -8,7 +8,15 @@ */ // Types -export { IBaseSettings, IInitOptions, IResolvedPythonEnvironment, IServerInfo, ToolConfig } from './types'; +export { + IBaseSettings, + IInitOptions, + IResolvedPythonEnvironment, + IServerInfo, + ToolConfig, + ToolDefinition, + ToolSettings, +} from './types'; // Logging export { registerLogger, traceError, traceInfo, traceLog, traceVerbose, traceWarn } from './logging'; diff --git a/typescript/src/settings.ts b/typescript/src/settings.ts index 33355a1..0b0bec5 100644 --- a/typescript/src/settings.ts +++ b/typescript/src/settings.ts @@ -14,7 +14,7 @@ import * as os from 'os'; import * as path from 'path'; import { ConfigurationChangeEvent, Uri, WorkspaceConfiguration, WorkspaceFolder } from 'vscode'; import { traceLog, traceWarn } from './logging'; -import { IBaseSettings, ToolConfig } from './types'; +import { IBaseSettings, ToolConfig, ToolSettings } from './types'; import { getConfiguration, getWorkspaceFolder, getWorkspaceFolders } from './vscodeapi'; // Re-export for convenience — callers that used these from the old @@ -165,6 +165,12 @@ function getInterpreterSettingValue(namespace: string, workspace?: WorkspaceFold * When `workspace` is a Python project nested inside a VS Code workspace, * variable substitution still uses the containing VS Code workspace folder. */ +export function getWorkspaceSettings( + namespace: string, + workspace: WorkspaceFolder, + toolConfig: TConfig, + resolveInterpreter?: (resource?: Uri) => Promise<{ path?: string[] }>, +): Promise>; export async function getWorkspaceSettings( namespace: string, workspace: WorkspaceFolder, @@ -233,6 +239,11 @@ export async function getWorkspaceSettings( /** * Resolve global (user-level) settings. */ +export function getGlobalSettings( + namespace: string, + toolConfig: TConfig, + resolveInterpreter?: () => Promise<{ path?: string[] }>, +): Promise>; export async function getGlobalSettings( namespace: string, toolConfig: ToolConfig, @@ -278,6 +289,12 @@ export async function getGlobalSettings( * default. Consumers may supply Python project roots to enable per-project * interpreter resolution while retaining a singleton language server. */ +export function getExtensionSettings( + namespace: string, + toolConfig: TConfig, + resolveInterpreter?: (resource?: Uri) => Promise<{ path?: string[] }>, + workspaces?: readonly WorkspaceFolder[], +): Promise[]>; export function getExtensionSettings( namespace: string, toolConfig: ToolConfig, diff --git a/typescript/src/types.ts b/typescript/src/types.ts index a98ad53..d0a7516 100644 --- a/typescript/src/types.ts +++ b/typescript/src/types.ts @@ -11,8 +11,7 @@ export interface IServerInfo { module: string; } -/** Base settings resolved per workspace by the settings module. */ -export interface IBaseSettings { +interface ICommonSettings { cwd: string; workspace: string; args: string[]; @@ -21,6 +20,10 @@ export interface IBaseSettings { importStrategy: string; showNotifications: string; extraPaths?: string[]; +} + +/** Base settings resolved per workspace by the settings module. */ +export interface IBaseSettings extends ICommonSettings { [key: string]: unknown; } @@ -111,6 +114,33 @@ export interface ToolConfig { isFormatter?: boolean; } +type CompatibleSettings = { + [K in keyof TSettings]: K extends keyof ICommonSettings ? ICommonSettings[K] : unknown; +}; + +/** + * Opt-in tool configuration with declared custom settings. + * + * Every required custom field needs a default because only keys in + * `settingsDefaults` are projected by the resolvers. Optional fields may + * be omitted and remain optional in resolved settings. + */ +export type ToolDefinition> = Omit< + ToolConfig, + 'settingsDefaults' +> & { + // A mapped shape accepts ordinary interfaces while remaining compatible + // with ToolConfig's open settingsDefaults record. + settingsDefaults: { [K in keyof TSettings]: TSettings[K] }; +}; + +/** + * Resolved custom settings with the common fields and an open, unknown-valued + * wire boundary. Common fields retain their existing types, even when a + * definition supplies narrower defaults. + */ +export type ToolSettings = IBaseSettings & Omit; + /** * Canonical resolved Python environment — API-agnostic. * diff --git a/typescript/tests/settingsWorkspace.test.ts b/typescript/tests/settingsWorkspace.test.ts index e7456eb..be45cea 100644 --- a/typescript/tests/settingsWorkspace.test.ts +++ b/typescript/tests/settingsWorkspace.test.ts @@ -7,6 +7,7 @@ import * as path from 'path'; import * as sinon from 'sinon'; import { pathToFileURL } from 'url'; import { Uri, WorkspaceFolder } from 'vscode'; +import { ToolDefinition } from '../src'; import { checkIfConfigurationChanged, expandTilde, @@ -82,6 +83,10 @@ suite('settings — workspace & global resolution', () => { 'settings', 'producer.json', ); + interface FixtureSettings { + severity: Record; + extensionOption: { enabled: boolean; label?: string }; + } const fixture = JSON.parse(readFileSync(fixturePath, 'utf8'), (_key: string, value: unknown) => { if (typeof value !== 'string') { return value; @@ -92,7 +97,7 @@ suite('settings — workspace & global resolution', () => { } return expanded; }) as { - toolDefaults: Record; + toolDefaults: ToolDefinition['settingsDefaults']; configuration: { workspace: Record; global: Record }; initializationOptions: IInitOptions; }; @@ -109,6 +114,63 @@ suite('settings — workspace & global resolution', () => { }; assert.deepEqual(actual, fixture.initializationOptions); + + const typedToolConfig: ToolDefinition = { + ...toolConfig, + settingsDefaults: fixture.toolDefaults, + }; + const typed: IInitOptions = { + settings: await getExtensionSettings('flake8', typedToolConfig, undefined, [root]), + globalSettings: await getGlobalSettings('flake8', typedToolConfig), + }; + assert.deepEqual(typed, actual); + assert.deepEqual(await getWorkspaceSettings('flake8', root, typedToolConfig), actual.settings[0]); + }); + + test('typed definitions preserve defaults and project only declared custom keys', async () => { + interface OptionalSettings { + enabled: boolean; + ignorePatterns: string[]; + optionalLabel?: string; + } + const typed: ToolDefinition = { + ...makeToolConfig(), + settingsDefaults: { enabled: false, ignorePatterns: [] }, + trackedSettings: ['enabled', 'ignorePatterns', 'envFile'], + }; + const configured: Record = { + optionalLabel: 'not projected without a default', + envFile: 'client-only.env', + futureOption: true, + }; + getConfigurationStub.returns({ + get: (key: string, fallback?: unknown) => configured[key] ?? fallback, + inspect: (key: string) => ({ globalValue: configured[key] }), + }); + const legacy: ToolConfig = typed; + const typedInitialization: IInitOptions = { + settings: await getExtensionSettings('flake8', typed), + globalSettings: await getGlobalSettings('flake8', typed), + }; + assert.deepEqual(typedInitialization, { + settings: await getExtensionSettings('flake8', legacy), + globalSettings: await getGlobalSettings('flake8', legacy), + }); + for (const resolved of [...typedInitialization.settings, typedInitialization.globalSettings]) { + assert.isFalse(resolved.enabled); + assert.deepEqual(resolved.ignorePatterns, []); + assert.notProperty(resolved, 'optionalLabel'); + assert.notProperty(resolved, 'envFile'); + assert.notProperty(resolved, 'futureOption'); + } + assert.deepEqual(Object.keys(typedInitialization).sort(), ['globalSettings', 'settings']); + assert.isTrue( + checkIfConfigurationChanged( + { affectsConfiguration: (key: string) => key === 'flake8.envFile' }, + 'flake8', + typed.trackedSettings, + ), + ); }); suite('getWorkspaceSettings', () => { diff --git a/typescript/tests/toolDefinition.typecheck.ts b/typescript/tests/toolDefinition.typecheck.ts new file mode 100644 index 0000000..6157f8f --- /dev/null +++ b/typescript/tests/toolDefinition.typecheck.ts @@ -0,0 +1,174 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { WorkspaceFolder } from 'vscode'; +import { + createToolContext, + CreateToolContextOptions, + getExtensionSettings, + getGlobalSettings, + getWorkspaceSettings, + IBaseSettings, + IInitOptions, + ToolConfig, + ToolDefinition, + ToolSettings, +} from '../src'; + +declare function expectType(value: T): void; + +interface LintSettings { + ignorePatterns: string[]; + severity: Record; + enabled: boolean; + optionalLabel?: string; +} + +// Compiled by the existing pretest (tsconfig.test.json), not executed by Mocha. +export async function checkToolDefinitions( + workspace: WorkspaceFolder, + contextOptions: Omit, +): Promise { + const tool: ToolDefinition = { + toolId: 'example', + toolDisplayName: 'Example', + toolModule: 'example', + minimumPythonVersion: { major: 3, minor: 10 }, + configFiles: ['tool.toml'], + serverScript: 'bundled/tool/lsp_server.py', + settingsDefaults: { + ignorePatterns: [], + severity: { error: 'Error' }, + enabled: false, + }, + trackedSettings: ['args', 'path', 'interpreter', 'ignorePatterns', 'severity', 'envFile'], + }; + + const legacyCompatible: ToolConfig = tool; + const options: CreateToolContextOptions = { ...contextOptions, toolConfig: tool }; + createToolContext(options).dispose(); + + const workspaceSettings = await getWorkspaceSettings('example', workspace, tool, async () => ({ path: [] })); + const globalSettings = await getGlobalSettings('example', tool, async () => ({ path: [] })); + const extensionSettings = await getExtensionSettings('example', tool, undefined, [workspace]); + + expectType>(workspaceSettings); + expectType(workspaceSettings); + expectType(globalSettings); + expectType(extensionSettings[0]); + expectType(workspaceSettings.ignorePatterns); + expectType(globalSettings.enabled); + expectType>(extensionSettings[0].severity); + expectType(workspaceSettings.optionalLabel); + expectType(globalSettings.optionalLabel); + expectType(extensionSettings[0].optionalLabel); + expectType({ settings: extensionSettings, globalSettings }); + expectType(workspaceSettings.args); + expectType(globalSettings.importStrategy); + expectType(extensionSettings[0].extraPaths); + + // @ts-expect-error Workspace custom fields must not fall back to unknown or any. + expectType(workspaceSettings.ignorePatterns); + // @ts-expect-error Global custom fields must not fall back to unknown or any. + expectType(globalSettings.enabled); + // @ts-expect-error Multi-root custom fields must not fall back to unknown or any. + expectType(extensionSettings[0].severity); + // @ts-expect-error The declared boolean is not narrowed to its false default. + expectType(workspaceSettings.enabled); + // @ts-expect-error The declared string array is not narrowed to never[]. + expectType(globalSettings.ignorePatterns); + // @ts-expect-error A custom field without a required default may be absent. + expectType(workspaceSettings.optionalLabel); + // @ts-expect-error Global settings do not materialize omitted optional fields. + expectType(globalSettings.optionalLabel); + // @ts-expect-error Multi-root settings do not materialize omitted optional fields. + expectType(extensionSettings[0].optionalLabel); + // @ts-expect-error Common fields retain their existing types. + expectType(workspaceSettings.args); + + expectType(workspaceSettings.futureOption); + // @ts-expect-error Future wire fields require narrowing, not implicit any. + expectType(workspaceSettings.futureOption); + // @ts-expect-error Global future fields also remain unknown. + expectType(globalSettings.futureOption); + // @ts-expect-error Multi-root future fields also remain unknown. + expectType(extensionSettings[0].futureOption); + if (typeof workspaceSettings.futureOption === 'string') { + expectType(workspaceSettings.futureOption); + } + + const invalidValue: ToolDefinition = { + ...tool, + settingsDefaults: { + ...tool.settingsDefaults, + // @ts-expect-error A declared boolean default cannot be a string. + enabled: 'false', + }, + }; + const invalidKey: ToolDefinition = { + ...tool, + settingsDefaults: { + ...tool.settingsDefaults, + // @ts-expect-error Explicitly typed defaults reject undeclared settings. + misspelled: true, + }, + }; + const missingDefault: ToolDefinition = { + ...tool, + // @ts-expect-error Required custom results need defaults for every required key. + settingsDefaults: { enabled: false, ignorePatterns: [] }, + }; + // @ts-expect-error Custom declarations cannot contradict a common field. + type ConflictingBase = ToolDefinition<{ args: number }>; + // @ts-expect-error Optional declarations cannot contradict a common field either. + type ConflictingOptionalBase = ToolDefinition<{ cwd?: number }>; + expectType(invalidValue); + expectType(invalidKey); + expectType(missingDefault); + + const commonDefaults: ToolDefinition<{ cwd: 'fixed'; args: ['--fixed']; extraPaths: string[] }> = { + ...tool, + settingsDefaults: { cwd: 'fixed', args: ['--fixed'], extraPaths: [] }, + }; + const commonSettings = await getWorkspaceSettings('example', workspace, commonDefaults); + expectType(commonSettings.cwd); + expectType(commonSettings.args); + expectType(commonSettings.extraPaths); + // @ts-expect-error Common fields must not be narrowed by literal defaults. + expectType<'fixed'>(commonSettings.cwd); + // @ts-expect-error Common array fields must not be narrowed to default tuples. + expectType<['--fixed']>(commonSettings.args); + // @ts-expect-error Common optional fields retain their existing optionality. + expectType(commonSettings.extraPaths); + + const legacyLinter: ToolConfig = { + ...legacyCompatible, + settingsDefaults: { enabled: true, severity: { error: 'Error' }, ignorePatterns: [] }, + }; + const legacyFormatter: ToolConfig = { + ...legacyCompatible, + toolId: 'formatter', + isFormatter: true, + restartDelay: 250, + pythonUtf8: true, + settingsDefaults: { serverArgs: ['--workers', '1'], useDaemon: false }, + trackedSettings: ['args', 'path', 'interpreter', 'serverArgs', 'useDaemon', 'envFile'], + }; + expectType({ ...contextOptions, toolConfig: legacyFormatter }); + expectType>(getWorkspaceSettings('example', workspace, legacyLinter)); + expectType>(getGlobalSettings('formatter', legacyFormatter)); + expectType>(getExtensionSettings('formatter', legacyFormatter)); + const legacySettings = await getWorkspaceSettings('example', workspace, legacyCompatible); + // @ts-expect-error Unparameterized ToolConfig keeps the legacy unknown-valued surface. + expectType(legacySettings.enabled); + + interface FormatterSettings { + serverArgs: string[]; + useDaemon: boolean; + } + const formatter: ToolDefinition = { + ...legacyFormatter, + settingsDefaults: { serverArgs: [], useDaemon: false }, + }; + expectType(await getGlobalSettings('formatter', formatter)); +}