diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b2ec4e..5402731 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,40 @@ documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.7.0] - 2026-06-17 + +### Added + +- **TypeScript** `ToolConfig.isFormatter` — optional boolean flag for tool + extensions that provide LSP-based document formatting + (`textDocument/formatting`, `rangeFormatting`, `rangesFormatting`). + +- **TypeScript** `NullFormatter` class — a lifecycle-aware placeholder + `DocumentFormattingEditProvider`. It registers a no-op formatter at + activation time (so VS Code lists the extension in the formatter picker + *before* the LSP has finished starting) and exposes `register()`, + `unregister()`, `isRegistered()`, and `dispose()` for manual control. + Exported from the package root. + +- **TypeScript** `createToolContext` — when `ToolConfig.isFormatter` is + `true`, the shared activation pattern now automatically: + - registers the `NullFormatter` placeholder immediately at activation, + - disposes it when the language client transitions to `State.Running` + (including the "already Running" case where the initial transition would + otherwise be missed), + - re-registers it if the client later transitions to `State.Stopped` or + `State.Starting` (e.g. during a restart), and disposes it again on the + next `Running`, + - disposes it on `ctx.dispose()` / `deactivateServer()`. + + This fixes the "duplicate formatter entry" regression reported in + [microsoft/vscode-black-formatter#752](https://github.com/microsoft/vscode-black-formatter/issues/752) + and unblocks the same fix for other formatter extensions + (`vscode-autopep8`, `vscode-isort`, …). + + Linter-only extensions (pylint, flake8, mypy, …) are unaffected — the + placeholder is **not** registered when `isFormatter` is `false` or unset. + ## [0.6.1] - 2026-06-17 ### Fixed diff --git a/VERSION b/VERSION index ee6cdce..faef31a 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.6.1 +0.7.0 diff --git a/python/pyproject.toml b/python/pyproject.toml index 6a78cc6..1777daf 100644 --- a/python/pyproject.toml +++ b/python/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "vscode-common-python-lsp" -version = "0.6.1" +version = "0.7.0" description = "Shared Python utilities for VS Code Python tool extensions" readme = "README.md" license = "MIT" diff --git a/typescript/package-lock.json b/typescript/package-lock.json index 5031021..957776c 100644 --- a/typescript/package-lock.json +++ b/typescript/package-lock.json @@ -1,12 +1,12 @@ { "name": "@vscode/common-python-lsp", - "version": "0.6.1", + "version": "0.7.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@vscode/common-python-lsp", - "version": "0.6.1", + "version": "0.7.0", "license": "MIT", "dependencies": { "@vscode/python-environments": "https://pkgs.dev.azure.com/azure-public/vside/_packaging/msft_consumption/npm/registry/@vscode/python-environments/-/python-environments-1.0.0.tgz", diff --git a/typescript/package.json b/typescript/package.json index 7c8c03b..2a3f42e 100644 --- a/typescript/package.json +++ b/typescript/package.json @@ -1,6 +1,6 @@ { "name": "@vscode/common-python-lsp", - "version": "0.6.1", + "version": "0.7.0", "description": "Shared TypeScript utilities for VS Code Python tool extensions", "main": "dist/index.js", "types": "dist/index.d.ts", diff --git a/typescript/src/activation.ts b/typescript/src/activation.ts index 592ef14..65ea8e5 100644 --- a/typescript/src/activation.ts +++ b/typescript/src/activation.ts @@ -11,9 +11,11 @@ */ import * as vscode from 'vscode'; +import { State } from 'vscode-languageclient'; import { LanguageClient } from 'vscode-languageclient/node'; import { createConfigFileWatchers } from './configWatcher'; import { traceError, traceLog, traceVerbose } from './logging'; +import { NullFormatter } from './nullFormatter'; import { PythonEnvironmentsProvider } from './python'; import { restartServer, RestartServerOptions } from './server'; import { checkIfConfigurationChanged, getWorkspaceSettings } from './settings'; @@ -100,6 +102,9 @@ export function createToolContext(options: CreateToolContextOptions): ToolExtens let disposed = false; let serverDisposables: vscode.Disposable[] = []; + const nullFormatter = toolConfig.isFormatter ? new NullFormatter() : undefined; + nullFormatter?.register(); + const ctx: ToolExtensionContext = { lsClient: undefined, @@ -116,6 +121,25 @@ export function createToolContext(options: CreateToolContextOptions): ToolExtens } isRestarting = true; try { + // Re-register the placeholder at the start of each restart + // cycle when there is no healthy running client. This covers + // two gaps the per-state listener misses: + // 1. Extension-driven restarts (config/interpreter change): + // runServer() disposes the old state listener *before* + // restartServer() stops the previous client, so + // Stopped/Starting transitions fire into a dead listener. + // 2. Failure paths: if restartServer() throws or returns + // client: undefined the state-listener block is skipped + // entirely. + // The guard avoids re-introducing the duplicate-formatter + // symptom: if the old client is still Running (serving + // formatting requests), re-registering the placeholder would + // make the extension appear twice in the formatter picker + // until restartServer() stops the old client. + if (nullFormatter && (!ctx.lsClient || ctx.lsClient.state !== State.Running)) { + nullFormatter.register(); + } + const projectRoot = await getProjectRoot(); if (disposed) { return; @@ -149,6 +173,11 @@ export function createToolContext(options: CreateToolContextOptions): ToolExtens } serverDisposables = []; + // Re-register the placeholder so the extension stays + // visible in the formatter picker while there is no + // interpreter (and therefore no running LSP formatter). + nullFormatter?.register(); + updateStatus( vscode.l10n.t('Please select a Python interpreter.'), vscode.LanguageStatusSeverity.Error, @@ -207,9 +236,45 @@ export function createToolContext(options: CreateToolContextOptions): ToolExtens ctx.lsClient = result.client; serverDisposables = result.disposables; + + if (nullFormatter && result.client) { + if (result.client.state === State.Running) { + nullFormatter.unregister(); + } else { + // New client not yet Running — ensure placeholder is + // visible while the server finishes starting. This + // covers the extension-driven restart path where the + // guard at the top skipped register() because the + // *old* client was still Running at that point. + nullFormatter.register(); + } + serverDisposables.push( + result.client.onDidChangeState((e) => { + switch (e.newState) { + case State.Running: + nullFormatter.unregister(); + break; + case State.Stopped: + case State.Starting: + nullFormatter.register(); + break; + } + }), + ); + } else if (nullFormatter && !result.client) { + // No client available — ensure placeholder is visible + // so the extension remains in the formatter picker. + nullFormatter.register(); + } } } catch (ex) { traceError(`Server restart failed: ${ex}`); + // Ensure placeholder stays visible after a failure — but only + // when there is no healthy running client (same guard as the + // top of runServer to avoid the duplicate-formatter symptom). + if (nullFormatter && (!ctx.lsClient || ctx.lsClient.state !== State.Running)) { + nullFormatter.register(); + } } finally { isRestarting = false; } @@ -244,6 +309,7 @@ export function createToolContext(options: CreateToolContextOptions): ToolExtens } } serverDisposables = []; + nullFormatter?.dispose(); }, }; diff --git a/typescript/src/index.ts b/typescript/src/index.ts index b46e099..2282511 100644 --- a/typescript/src/index.ts +++ b/typescript/src/index.ts @@ -69,6 +69,9 @@ export { RestartServerResult, } from './server'; +// Null formatter helper +export { NullFormatter } from './nullFormatter'; + // Activation / deactivation export { createToolContext, diff --git a/typescript/src/nullFormatter.ts b/typescript/src/nullFormatter.ts new file mode 100644 index 0000000..6ac812e --- /dev/null +++ b/typescript/src/nullFormatter.ts @@ -0,0 +1,63 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { Disposable } from 'vscode'; +import { traceLog } from './logging'; +import { getDocumentSelector } from './utilities'; +import { registerDocumentFormattingEditProvider } from './vscodeapi'; + +/** + * Lifecycle-aware placeholder DocumentFormattingEditProvider. + * + * A tool extension that provides LSP-based formatting needs to appear in + * VS Code's formatter picker *before* the language client has finished + * starting, otherwise the first "Format Document" invocation after + * activation is treated as "no formatter installed". + * + * This helper registers a no-op provider for `getDocumentSelector()` and + * lets callers dispose it (typically as soon as the real LSP provider takes + * over). + * + * **Important:** never leave both the placeholder and the LSP provider + * registered at the same time — VS Code will show the extension twice in + * the formatter picker (one entry per provider). The shared activation + * pattern in `activation.ts` handles this automatically when + * `ToolConfig.isFormatter` is `true`. + */ +export class NullFormatter implements Disposable { + private disposable: Disposable | undefined; + + /** No-op if already registered. */ + register(): void { + if (this.disposable) { + return; + } + this.disposable = registerDocumentFormattingEditProvider(getDocumentSelector(), { + provideDocumentFormattingEdits: () => { + traceLog('Formatting requested before server has started.'); + return Promise.resolve(undefined); + }, + }); + } + + /** No-op if not registered. */ + unregister(): void { + if (!this.disposable) { + return; + } + try { + this.disposable.dispose(); + } finally { + this.disposable = undefined; + } + } + + /** True iff the placeholder is currently registered. */ + isRegistered(): boolean { + return this.disposable !== undefined; + } + + dispose(): void { + this.unregister(); + } +} diff --git a/typescript/src/types.ts b/typescript/src/types.ts index a8670e4..049dbff 100644 --- a/typescript/src/types.ts +++ b/typescript/src/types.ts @@ -53,6 +53,31 @@ export interface ToolConfig { // Environment extraEnvVars?: Record; + + /** + * Set to `true` for tools that provide LSP formatting (textDocument/formatting, + * rangeFormatting, rangesFormatting). + * + * When enabled, the common activation pattern will: + * - register a placeholder DocumentFormattingEditProvider at activation time + * (so VS Code lists the extension as a formatter *before* the LSP has + * finished starting), + * - dispose it automatically when the language client transitions to + * `Running` (so VS Code does not see two providers for the same selector + * and list the extension twice in the formatter picker), + * - re-register it if the client transitions back to `Stopped` / `Starting` + * during a crash/recovery restart, then dispose it again on the next + * `Running`, + * - re-register it at the start of each extension-driven restart cycle + * (config change, interpreter change, restart command) when the previous + * client is no longer `Running`, ensuring the placeholder is visible + * while the new server starts — and in failure paths where + * `restartServer()` throws or returns no client. + * + * Defaults to `false`. Linter-only tools (pylint, flake8, mypy, …) should + * leave this unset. + */ + isFormatter?: boolean; } /** diff --git a/typescript/tests/_languageclient_mock.ts b/typescript/tests/_languageclient_mock.ts index 1e09a41..caa968e 100644 --- a/typescript/tests/_languageclient_mock.ts +++ b/typescript/tests/_languageclient_mock.ts @@ -36,6 +36,10 @@ export class LanguageClient { this.clientOptions = clientOptions; } + get state(): State { + return this._state; + } + onDidChangeState(listener: (e: { oldState: State; newState: State }) => void) { this._stateListeners.push(listener); return { @@ -46,6 +50,12 @@ export class LanguageClient { }; } + simulateStateChange(newState: State): void { + const oldState = this._state; + this._state = newState; + this._stateListeners.forEach((l) => l({ oldState, newState })); + } + async start(): Promise { this._state = State.Running; } diff --git a/typescript/tests/activation.test.ts b/typescript/tests/activation.test.ts index 5fd592c..1b15a5a 100644 --- a/typescript/tests/activation.test.ts +++ b/typescript/tests/activation.test.ts @@ -5,6 +5,7 @@ import { assert } from 'chai'; import * as sinon from 'sinon'; import * as vscode from 'vscode'; import { LanguageClient } from 'vscode-languageclient/node'; +import { State } from 'vscode-languageclient'; import { createToolContext, CreateToolContextOptions, @@ -376,3 +377,239 @@ suite('deactivateServer', () => { assert.isTrue(dispose.calledOnce); }); }); + +// --------------------------------------------------------------------------- +// NullFormatter lifecycle (via createToolContext with isFormatter: true) +// --------------------------------------------------------------------------- + +suite('createToolContext – NullFormatter lifecycle', () => { + let sandbox: sinon.SinonSandbox; + let registerFormattingProviderStub: sinon.SinonStub; + let providerDispose: sinon.SinonStub; + + setup(() => { + sandbox = sinon.createSandbox(); + sandbox.stub(utilities, 'getProjectRoot').resolves(undefined); + sandbox.stub(utilities, 'getInterpreterFromSetting').returns(['/usr/bin/python3']); + sandbox.stub(settingsModule, 'getWorkspaceSettings').resolves({ + cwd: '/workspace', + workspace: 'file:///workspace', + args: [], + path: [], + interpreter: ['/usr/bin/python3'], + importStrategy: 'useBundled', + showNotifications: 'off', + }); + + providerDispose = sandbox.stub(); + registerFormattingProviderStub = sandbox + .stub(vscodeapi, 'registerDocumentFormattingEditProvider') + .returns({ dispose: providerDispose }); + }); + + teardown(() => { + sandbox.restore(); + }); + + function makeFormatterOptions(overrides?: Partial): CreateToolContextOptions { + return { + serverInfo: { name: 'Black', module: 'black' }, + outputChannel: { + logLevel: vscode.LogLevel.Info, + show: sinon.stub(), + onDidChangeLogLevel: sinon.stub().returns({ dispose: sinon.stub() }), + } as unknown as vscode.LogOutputChannel, + toolConfig: { + toolId: 'black', + toolDisplayName: 'Black', + toolModule: 'black', + minimumPythonVersion: { major: 3, minor: 8 }, + configFiles: ['pyproject.toml'], + settingsDefaults: {}, + trackedSettings: ['args'], + serverScript: '/path/to/server.py', + isFormatter: true, + }, + pythonProvider: { + getInterpreterDetails: sandbox.stub().resolves({ path: ['/usr/bin/python3'] }), + getDebuggerPath: sandbox.stub().resolves(undefined), + initializePython: sandbox.stub().resolves(), + onDidChangeInterpreter: sinon.stub().returns({ dispose: sinon.stub() }), + } as unknown as PythonEnvironmentsProvider, + ...overrides, + }; + } + + // Uses the shared LanguageClient mock from _languageclient_mock.ts + // (wired in by _setup.ts) so production and test getter semantics + // stay in sync. At compile-time, LanguageClient resolves to the real + // vscode-languageclient type; at runtime it is replaced by the mock + // (which exposes simulateStateChange). We import the mock type for + // the helper return type so callers get proper type-checking. + type MockClient = import('./_languageclient_mock').LanguageClient; + + function makeMockClient(initialState: State = State.Stopped): MockClient { + const MockLC = LanguageClient as unknown as typeof import('./_languageclient_mock').LanguageClient; + const client = new MockLC('test', 'Test', {}, {}); + // Drive the mock to the desired initial state. + if (initialState !== State.Stopped) { + client.simulateStateChange(initialState); + } + return client; + } + + /** Cast a mock client to the real LanguageClient type for stub return values. */ + function asLC(mock: MockClient): LanguageClient { + return mock as unknown as LanguageClient; + } + + // Test 1: provider registered once at createToolContext, disposed on State.Running + test('registers placeholder once at activation and disposes it on State.Running', async () => { + const mockClient = makeMockClient(State.Stopped); + sandbox.stub(serverModule, 'restartServer').resolves({ client: asLC(mockClient), disposables: [] }); + + const ctx = createToolContext(makeFormatterOptions()); + + assert.isTrue(registerFormattingProviderStub.calledOnce, 'provider registered at activation'); + assert.isFalse(providerDispose.called, 'not yet disposed before server starts'); + + await ctx.runServer(); + + // Client is Stopped — "already Running" guard does not fire + assert.isFalse(providerDispose.called, 'not disposed before Running transition'); + + // Emit Running — state listener should dispose the placeholder + mockClient.simulateStateChange(State.Running); + assert.isTrue(providerDispose.calledOnce, 'placeholder disposed on State.Running'); + }); + + // Test 2: crash/recovery — server stops while running, placeholder restored + test('re-registers placeholder on server crash (Stopped while running) and disposes on recovery', async () => { + const mockClient = makeMockClient(State.Stopped); + sandbox.stub(serverModule, 'restartServer').resolves({ client: asLC(mockClient), disposables: [] }); + + const ctx = createToolContext(makeFormatterOptions()); + assert.isTrue(registerFormattingProviderStub.calledOnce, 'provider registered at activation'); + + await ctx.runServer(); + + // Simulate Running — placeholder disposed + mockClient.simulateStateChange(State.Running); + assert.strictEqual(providerDispose.callCount, 1, 'placeholder disposed on Running'); + + // Simulate Stopped — placeholder re-registered + mockClient.simulateStateChange(State.Stopped); + assert.strictEqual(registerFormattingProviderStub.callCount, 2, 'placeholder re-registered on Stopped'); + + // Simulate Starting — placeholder already registered, no double-registration + mockClient.simulateStateChange(State.Starting); + assert.strictEqual(registerFormattingProviderStub.callCount, 2, 'no double-registration on Starting'); + + // Simulate Running again — placeholder disposed again + mockClient.simulateStateChange(State.Running); + assert.strictEqual(providerDispose.callCount, 2, 'placeholder disposed again on Running'); + }); + + // Test 3: already-Running guard — client is Running when runServer() returns + test('disposes placeholder immediately when client is already Running at runServer return', async () => { + const mockClient = makeMockClient(State.Running); + sandbox.stub(serverModule, 'restartServer').resolves({ client: asLC(mockClient), disposables: [] }); + + const ctx = createToolContext(makeFormatterOptions()); + assert.isTrue(registerFormattingProviderStub.calledOnce, 'provider registered at activation'); + + await ctx.runServer(); + + assert.isTrue( + providerDispose.calledOnce, + 'placeholder must be disposed even when client is already Running (missed initial transition)', + ); + }); + + // Test 4: isFormatter: false — no placeholder, no state listener + test('does not register placeholder when isFormatter is false', async () => { + const options = makeFormatterOptions(); + options.toolConfig = { ...options.toolConfig, isFormatter: false }; + sandbox.stub(serverModule, 'restartServer').resolves({ client: undefined, disposables: [] }); + + createToolContext(options); + + assert.isFalse( + registerFormattingProviderStub.called, + 'should not register provider when isFormatter is false', + ); + }); + + // Test 5: isFormatter unset — no placeholder + test('does not register placeholder when isFormatter is unset', async () => { + const options = makeFormatterOptions(); + const { isFormatter, ...configWithoutFormatter } = options.toolConfig; + options.toolConfig = configWithoutFormatter as ToolConfig; + sandbox.stub(serverModule, 'restartServer').resolves({ client: undefined, disposables: [] }); + + createToolContext(options); + + assert.isFalse( + registerFormattingProviderStub.called, + 'should not register provider when isFormatter is unset', + ); + }); + + // Test 6: ctx.dispose() disposes placeholder + test('ctx.dispose() disposes the placeholder when it is registered', () => { + const ctx = createToolContext(makeFormatterOptions()); + assert.isTrue(registerFormattingProviderStub.calledOnce, 'provider registered at activation'); + + ctx.dispose(); + assert.isTrue(providerDispose.calledOnce, 'placeholder disposed on ctx.dispose()'); + }); + + // Test 7: extension-driven restart — second runServer() is called while + // the first client is still Running. The guard at the top of runServer + // skips re-registration to avoid the transient duplicate-formatter issue. + // The placeholder is re-registered only after restartServer returns the + // new (not-yet-Running) client. + test('re-registers placeholder on extension-driven restart (second runServer call)', async () => { + const firstClient = makeMockClient(State.Stopped); + const secondClient = makeMockClient(State.Stopped); + const restartStub = sandbox.stub(serverModule, 'restartServer'); + restartStub.onFirstCall().resolves({ client: asLC(firstClient), disposables: [] }); + restartStub.onSecondCall().resolves({ client: asLC(secondClient), disposables: [] }); + + const ctx = createToolContext(makeFormatterOptions()); + assert.strictEqual(registerFormattingProviderStub.callCount, 1, 'registered at activation'); + + // First run — transition to Running disposes placeholder + await ctx.runServer(); + firstClient.simulateStateChange(State.Running); + assert.strictEqual(providerDispose.callCount, 1, 'placeholder disposed after first Running'); + + // Second runServer() — simulates config/interpreter-driven restart. + // firstClient is still Running, so the guard at the top of runServer + // skips register (avoiding transient duplicate). After restartServer + // returns the new client (Stopped), the placeholder is re-registered. + await ctx.runServer(); + assert.strictEqual( + registerFormattingProviderStub.callCount, 2, + 'placeholder re-registered after restartServer returns non-Running client', + ); + + // Transition second client to Running — placeholder disposed again + secondClient.simulateStateChange(State.Running); + assert.strictEqual(providerDispose.callCount, 2, 'placeholder disposed after second Running'); + }); + + // Test 8: restartServer returns no client — placeholder stays registered + test('keeps placeholder registered when restartServer returns no client', async () => { + sandbox.stub(serverModule, 'restartServer').resolves({ client: undefined, disposables: [] }); + + const ctx = createToolContext(makeFormatterOptions()); + assert.strictEqual(registerFormattingProviderStub.callCount, 1, 'registered at activation'); + + await ctx.runServer(); + + // Placeholder should still be registered (register() called again at + // top of runServer as a no-op, and never unregistered). + assert.isFalse(providerDispose.called, 'placeholder must stay registered when no client'); + }); +});