Skip to content

Commit

Permalink
Add common service for logging deprecated api usage
Browse files Browse the repository at this point in the history 
For microsoft#88391

Adds a new `ExtHostApiDeprecationService`. This service logs a warning and telemetry when a deprecated API is used.

Updates some of the more simple deprecated apis to use this new service
  • Loading branch information
@mjbvz
mjbvz committed Jan 14, 2020
1 parent 2bbe9c5 commit 0fa8a81
Show file tree
Hide file tree
Showing 7 changed files with 130 additions and 21 deletions.
23 changes: 16 additions & 7 deletions src/vs/workbench/api/common/extHost.api.impl.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ import { IExtHostRpcService } from 'vs/workbench/api/common/extHostRpcService';
import { IExtHostInitDataService } from 'vs/workbench/api/common/extHostInitDataService';
import { ExtHostTheming } from 'vs/workbench/api/common/extHostTheming';
import { IExtHostTunnelService } from 'vs/workbench/api/common/extHostTunnelService';
import { IExtHostApiDeprecationService } from 'vs/workbench/api/common/extHostApiDeprecationService';

export interface IExtensionApiFactory {
(extension: IExtensionDescription, registry: ExtensionDescriptionRegistry, configProvider: ExtHostConfigProvider): typeof vscode;
Expand All @@ -89,6 +90,7 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I
const extHostStorage = accessor.get(IExtHostStorage);
const extHostLogService = accessor.get(ILogService);
const extHostTunnelService = accessor.get(IExtHostTunnelService);
const extHostApiDeprecation = accessor.get(IExtHostApiDeprecationService);

// register addressable instances
rpcProtocol.set(ExtHostContext.ExtHostLogService, <ExtHostLogServiceShape><any>extHostLogService);
Expand Down Expand Up @@ -118,7 +120,7 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I
const extHostTreeViews = rpcProtocol.set(ExtHostContext.ExtHostTreeViews, new ExtHostTreeViews(rpcProtocol.getProxy(MainContext.MainThreadTreeViews), extHostCommands, extHostLogService));
const extHostEditorInsets = rpcProtocol.set(ExtHostContext.ExtHostEditorInsets, new ExtHostEditorInsets(rpcProtocol.getProxy(MainContext.MainThreadEditorInsets), extHostEditors, initData.environment));
const extHostDiagnostics = rpcProtocol.set(ExtHostContext.ExtHostDiagnostics, new ExtHostDiagnostics(rpcProtocol, extHostLogService));
const extHostLanguageFeatures = rpcProtocol.set(ExtHostContext.ExtHostLanguageFeatures, new ExtHostLanguageFeatures(rpcProtocol, uriTransformer, extHostDocuments, extHostCommands, extHostDiagnostics, extHostLogService));
const extHostLanguageFeatures = rpcProtocol.set(ExtHostContext.ExtHostLanguageFeatures, new ExtHostLanguageFeatures(rpcProtocol, uriTransformer, extHostDocuments, extHostCommands, extHostDiagnostics, extHostLogService, extHostApiDeprecation));
const extHostFileSystem = rpcProtocol.set(ExtHostContext.ExtHostFileSystem, new ExtHostFileSystem(rpcProtocol, extHostLanguageFeatures));
const extHostFileSystemEvent = rpcProtocol.set(ExtHostContext.ExtHostFileSystemEventService, new ExtHostFileSystemEventService(rpcProtocol, extHostLogService, extHostDocumentsAndEditors));
const extHostQuickOpen = rpcProtocol.set(ExtHostContext.ExtHostQuickOpen, new ExtHostQuickOpen(rpcProtocol, extHostWorkspace, extHostCommands));
Expand Down Expand Up @@ -381,7 +383,7 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I
return extHostLanguageFeatures.registerCallHierarchyProvider(extension, selector, provider);
},
setLanguageConfiguration: (language: string, configuration: vscode.LanguageConfiguration): vscode.Disposable => {
return extHostLanguageFeatures.setLanguageConfiguration(language, configuration);
return extHostLanguageFeatures.setLanguageConfiguration(extension, language, configuration);
}
};

Expand Down Expand Up @@ -501,6 +503,9 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I
return extHostStatusBar.setStatusBarMessage(text, timeoutOrThenable);
},
withScmProgress<R>(task: (progress: vscode.Progress<number>) => Thenable<R>) {
extHostApiDeprecation.report('window.withScmProgress', extension,
`Use 'withProgress' instead.`);

return extHostProgress.withProgress(extension, { location: extHostTypes.ProgressLocation.SourceControl }, (progress, token) => task({ report(n: number) { /*noop*/ } }));
},
withProgress<R>(options: vscode.ProgressOptions, task: (progress: vscode.Progress<{ message?: string; worked?: number }>, token: vscode.CancellationToken) => Thenable<R>) {
Expand Down Expand Up @@ -562,13 +567,11 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I
};

// namespace: workspace
let warnedRootPathDeprecated = false;

const workspace: typeof vscode.workspace = {
get rootPath() {
if (extension.isUnderDevelopment && !warnedRootPathDeprecated) {
warnedRootPathDeprecated = true;
extHostLogService.warn(`[Deprecation Warning] 'workspace.rootPath' is deprecated and should no longer be used. Please use 'workspace.workspaceFolders' instead. More details: https://aka.ms/vscode-eliminating-rootpath`);
}
extHostApiDeprecation.report('workspace.rootPath', extension,
`Please use 'workspace.workspaceFolders' instead. More details: https://aka.ms/vscode-eliminating-rootpath`);

return extHostWorkspace.getPath();
},
Expand Down Expand Up @@ -682,6 +685,9 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I
return extHostDocumentContentProviders.registerTextDocumentContentProvider(scheme, provider);
},
registerTaskProvider: (type: string, provider: vscode.TaskProvider) => {
extHostApiDeprecation.report('window.registerTaskProvider', extension,
`Use the corresponding function on the 'tasks' namespace instead`);

return extHostTask.registerTaskProvider(extension, type, provider);
},
registerFileSystemProvider(scheme, provider, options) {
Expand Down Expand Up @@ -733,6 +739,9 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I
// namespace: scm
const scm: typeof vscode.scm = {
get inputBox() {
extHostApiDeprecation.report('scm.inputBox', extension,
`Use 'SourceControl.inputBox' instead`);

return extHostSCM.getLastInputBox(extension)!; // Strict null override - Deprecated api
},
createSourceControl(id: string, label: string, rootUri?: vscode.Uri) {
Expand Down
71 changes: 71 additions & 0 deletions src/vs/workbench/api/common/extHostApiDeprecationService.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { IExtensionDescription } from 'vs/platform/extensions/common/extensions';
import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
import { ILogService } from 'vs/platform/log/common/log';
import * as extHostProtocol from 'vs/workbench/api/common/extHost.protocol';
import { IExtHostRpcService } from 'vs/workbench/api/common/extHostRpcService';

export interface IExtHostApiDeprecationService {
_serviceBrand: undefined;

report(apiId: string, extension: IExtensionDescription, migrationSuggestion: string): void;
}

export const IExtHostApiDeprecationService = createDecorator<IExtHostApiDeprecationService>('IExtHostApiDeprecationService');

export class ExtHostApiDeprecationService implements IExtHostApiDeprecationService {

_serviceBrand: undefined;

private readonly _reportedUsages = new Set<string>();
private readonly _telemetryShape: extHostProtocol.MainThreadTelemetryShape;

constructor(
@IExtHostRpcService rpc: IExtHostRpcService,
@ILogService private readonly _extHostLogService: ILogService,
) {
this._telemetryShape = rpc.getProxy(extHostProtocol.MainContext.MainThreadTelemetry);
}

public report(apiId: string, extension: IExtensionDescription, migrationSuggestion: string): void {
const key = this.getUsageKey(apiId, extension);
if (this._reportedUsages.has(key)) {
return;
}
this._reportedUsages.add(key);

if (extension.isUnderDevelopment) {
this._extHostLogService.warn(`[Deprecation Warning] '${apiId}' is deprecated. ${migrationSuggestion}`);
}

type DeprecationTelemetry = {
extensionId: string | undefined;
apiId: string;
};
type DeprecationTelemetryMeta = {
extensionId: { classification: 'SystemMetaData', purpose: 'PerformanceAndHealth' };
apiId: { classification: 'SystemMetaData', purpose: 'PerformanceAndHealth' };
};
this._telemetryShape.$publicLog2<DeprecationTelemetry, DeprecationTelemetryMeta>('resolveCompletionItem/invalid', {
extensionId: extension.identifier.value,
apiId: apiId,
});
}

private getUsageKey(apiId: string, extension: IExtensionDescription): string {
return `${apiId}-${extension.identifier.value}`;
}
}


export const NullApiDeprecationService = Object.freeze(new class implements IExtHostApiDeprecationService {
_serviceBrand: undefined;

public report(_apiId: string, _extension: IExtensionDescription | undefined, _warningMessage: string): void {
// noop
}
}());
47 changes: 35 additions & 12 deletions src/vs/workbench/api/common/extHostLanguageFeatures.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { DisposableStore, dispose } from 'vs/base/common/lifecycle';
import { VSBuffer } from 'vs/base/common/buffer';
import { encodeSemanticTokensDto } from 'vs/workbench/api/common/shared/semanticTokens';
import { IdGenerator } from 'vs/base/common/idGenerator';
import { IExtHostApiDeprecationService } from 'vs/workbench/api/common/extHostApiDeprecationService';

// --- adapter

Expand Down Expand Up @@ -326,7 +327,8 @@ class CodeActionAdapter {
private readonly _diagnostics: ExtHostDiagnostics,
private readonly _provider: vscode.CodeActionProvider,
private readonly _logService: ILogService,
private readonly _extensionId: ExtensionIdentifier
private readonly _extension: IExtensionDescription,
private readonly _apiDeprecation: IExtHostApiDeprecationService,
) { }

provideCodeActions(resource: URI, rangeOrSelection: IRange | ISelection, context: modes.CodeActionContext, token: CancellationToken): Promise<extHostProtocol.ICodeActionListDto | undefined> {
Expand Down Expand Up @@ -367,6 +369,10 @@ class CodeActionAdapter {
}
if (CodeActionAdapter._isCommand(candidate)) {
// old school: synthetic code action

this._apiDeprecation.report('CodeActionProvider.provideCodeActions - return commands', this._extension,
`Return 'CodeAction' instances instead.`);

actions.push({
_isSynthetic: true,
title: candidate.title,
Expand All @@ -375,9 +381,9 @@ class CodeActionAdapter {
} else {
if (codeActionContext.only) {
if (!candidate.kind) {
this._logService.warn(`${this._extensionId.value} - Code actions of kind '${codeActionContext.only.value} 'requested but returned code action does not have a 'kind'. Code action will be dropped. Please set 'CodeAction.kind'.`);
this._logService.warn(`${this._extension.identifier.value} - Code actions of kind '${codeActionContext.only.value} 'requested but returned code action does not have a 'kind'. Code action will be dropped. Please set 'CodeAction.kind'.`);
} else if (!codeActionContext.only.contains(candidate.kind)) {
this._logService.warn(`${this._extensionId.value} - Code actions of kind '${codeActionContext.only.value} 'requested but returned code action is of kind '${candidate.kind.value}'. Code action will be dropped. Please check 'CodeActionContext.only' to only return requested code actions.`);
this._logService.warn(`${this._extension.identifier.value} - Code actions of kind '${codeActionContext.only.value} 'requested but returned code action is of kind '${candidate.kind.value}'. Code action will be dropped. Please check 'CodeActionContext.only' to only return requested code actions.`);
}
}

Expand Down Expand Up @@ -748,8 +754,9 @@ class SuggestAdapter {
private readonly _commands: CommandsConverter,
private readonly _provider: vscode.CompletionItemProvider,
private readonly _logService: ILogService,
private readonly _apiDeprecation: IExtHostApiDeprecationService,
private readonly _telemetry: extHostProtocol.MainThreadTelemetryShape,
private readonly _extensionId: ExtensionIdentifier
private readonly _extension: IExtensionDescription,
) { }

provideCompletionItems(resource: URI, position: IPosition, context: modes.CompletionContext, token: CancellationToken): Promise<extHostProtocol.ISuggestResultDto | undefined> {
Expand Down Expand Up @@ -838,15 +845,15 @@ class SuggestAdapter {

let _mustNotChangeIndex = !this._didWarnMust && SuggestAdapter._mustNotChangeDiff(_mustNotChange, resolvedItem);
if (typeof _mustNotChangeIndex === 'string') {
this._logService.warn(`[${this._extensionId.value}] INVALID result from 'resolveCompletionItem', extension MUST NOT change any of: label, sortText, filterText, insertText, or textEdit`);
this._telemetry.$publicLog2<BlameExtension, BlameExtensionMeta>('resolveCompletionItem/invalid', { extensionId: this._extensionId.value, kind: 'must', index: _mustNotChangeIndex });
this._logService.warn(`[${this._extension.identifier.value}] INVALID result from 'resolveCompletionItem', extension MUST NOT change any of: label, sortText, filterText, insertText, or textEdit`);
this._telemetry.$publicLog2<BlameExtension, BlameExtensionMeta>('resolveCompletionItem/invalid', { extensionId: this._extension.identifier.value, kind: 'must', index: _mustNotChangeIndex });
this._didWarnMust = true;
}

let _mayNotChangeIndex = !this._didWarnShould && SuggestAdapter._mayNotChangeDiff(_mayNotChange, resolvedItem);
if (typeof _mayNotChangeIndex === 'string') {
this._logService.info(`[${this._extensionId.value}] UNSAVE result from 'resolveCompletionItem', extension SHOULD NOT change any of: additionalTextEdits, or command`);
this._telemetry.$publicLog2<BlameExtension, BlameExtensionMeta>('resolveCompletionItem/invalid', { extensionId: this._extensionId.value, kind: 'should', index: _mayNotChangeIndex });
this._logService.info(`[${this._extension.identifier.value}] UNSAVE result from 'resolveCompletionItem', extension SHOULD NOT change any of: additionalTextEdits, or command`);
this._telemetry.$publicLog2<BlameExtension, BlameExtensionMeta>('resolveCompletionItem/invalid', { extensionId: this._extension.identifier.value, kind: 'should', index: _mayNotChangeIndex });
this._didWarnShould = true;
}

Expand Down Expand Up @@ -892,6 +899,9 @@ class SuggestAdapter {

// 'insertText'-logic
if (item.textEdit) {
this._apiDeprecation.report('CompletionItem.textEdit', this._extension,
`Use 'CompletionItem.insertText' and 'CompletionItem.range' instead.`);

result[extHostProtocol.ISuggestDataDtoField.insertText] = item.textEdit.newText;

} else if (typeof item.insertText === 'string') {
Expand Down Expand Up @@ -1339,14 +1349,16 @@ export class ExtHostLanguageFeatures implements extHostProtocol.ExtHostLanguageF
private _diagnostics: ExtHostDiagnostics;
private _adapter = new Map<number, AdapterData>();
private readonly _logService: ILogService;
private readonly _apiDeprecation: IExtHostApiDeprecationService;

constructor(
mainContext: extHostProtocol.IMainContext,
uriTransformer: IURITransformer | null,
documents: ExtHostDocuments,
commands: ExtHostCommands,
diagnostics: ExtHostDiagnostics,
logService: ILogService
logService: ILogService,
apiDeprecationService: IExtHostApiDeprecationService,
) {
this._uriTransformer = uriTransformer;
this._proxy = mainContext.getProxy(extHostProtocol.MainContext.MainThreadLanguageFeatures);
Expand All @@ -1355,6 +1367,7 @@ export class ExtHostLanguageFeatures implements extHostProtocol.ExtHostLanguageF
this._commands = commands;
this._diagnostics = diagnostics;
this._logService = logService;
this._apiDeprecation = apiDeprecationService;
}

private _transformDocumentSelector(selector: vscode.DocumentSelector): Array<extHostProtocol.IDocumentFilterDto> {
Expand Down Expand Up @@ -1562,7 +1575,7 @@ export class ExtHostLanguageFeatures implements extHostProtocol.ExtHostLanguageF
// --- quick fix

registerCodeActionProvider(extension: IExtensionDescription, selector: vscode.DocumentSelector, provider: vscode.CodeActionProvider, metadata?: vscode.CodeActionProviderMetadata): vscode.Disposable {
const handle = this._addNewAdapter(new CodeActionAdapter(this._documents, this._commands.converter, this._diagnostics, provider, this._logService, extension.identifier), extension);
const handle = this._addNewAdapter(new CodeActionAdapter(this._documents, this._commands.converter, this._diagnostics, provider, this._logService, extension, this._apiDeprecation), extension);
this._proxy.$registerQuickFixSupport(handle, this._transformDocumentSelector(selector), (metadata && metadata.providedCodeActionKinds) ? metadata.providedCodeActionKinds.map(kind => kind.value) : undefined);
return this._createDisposable(handle);
}
Expand Down Expand Up @@ -1665,7 +1678,7 @@ export class ExtHostLanguageFeatures implements extHostProtocol.ExtHostLanguageF
// --- suggestion

registerCompletionItemProvider(extension: IExtensionDescription, selector: vscode.DocumentSelector, provider: vscode.CompletionItemProvider, triggerCharacters: string[]): vscode.Disposable {
const handle = this._addNewAdapter(new SuggestAdapter(this._documents, this._commands.converter, provider, this._logService, this._telemetryShape, extension.identifier), extension);
const handle = this._addNewAdapter(new SuggestAdapter(this._documents, this._commands.converter, provider, this._logService, this._apiDeprecation, this._telemetryShape, extension), extension);
this._proxy.$registerSuggestSupport(handle, this._transformDocumentSelector(selector), triggerCharacters, SuggestAdapter.supportsResolving(provider), extension.identifier);
return this._createDisposable(handle);
}
Expand Down Expand Up @@ -1813,7 +1826,7 @@ export class ExtHostLanguageFeatures implements extHostProtocol.ExtHostLanguageF
return onEnterRules.map(ExtHostLanguageFeatures._serializeOnEnterRule);
}

setLanguageConfiguration(languageId: string, configuration: vscode.LanguageConfiguration): vscode.Disposable {
setLanguageConfiguration(extension: IExtensionDescription, languageId: string, configuration: vscode.LanguageConfiguration): vscode.Disposable {
let { wordPattern } = configuration;

// check for a valid word pattern
Expand All @@ -1828,6 +1841,16 @@ export class ExtHostLanguageFeatures implements extHostProtocol.ExtHostLanguageF
this._documents.setWordDefinitionFor(languageId, undefined);
}

if (configuration.__electricCharacterSupport) {
this._apiDeprecation.report('LanguageConfiguration.__electricCharacterSupport', extension,
`Do not use.`);
}

if (configuration.__characterPairSupport) {
this._apiDeprecation.report('LanguageConfiguration.__characterPairSupport', extension,
`Do not use.`);
}

const handle = this._nextHandle();
const serializedConfiguration: extHostProtocol.ILanguageConfigurationDto = {
comments: configuration.comments,
Expand Down
2 changes: 2 additions & 0 deletions src/vs/workbench/api/node/extHost.services.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,11 @@ import { ILogService } from 'vs/platform/log/common/log';
import { ExtHostLogService } from 'vs/workbench/api/node/extHostLogService';
import { IExtHostTunnelService } from 'vs/workbench/api/common/extHostTunnelService';
import { ExtHostTunnelService } from 'vs/workbench/api/node/extHostTunnelService';
import { IExtHostApiDeprecationService, ExtHostApiDeprecationService } from 'vs/workbench/api/common/extHostApiDeprecationService';

// register singleton services
registerSingleton(ILogService, ExtHostLogService);
registerSingleton(IExtHostApiDeprecationService, ExtHostApiDeprecationService);
registerSingleton(IExtHostOutputService, ExtHostOutputService2);
registerSingleton(IExtHostWorkspace, ExtHostWorkspace);
registerSingleton(IExtHostDecorations, ExtHostDecorations);
Expand Down
2 changes: 2 additions & 0 deletions src/vs/workbench/services/extensions/worker/extHost.services.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,11 @@ import { ServiceIdentifier } from 'vs/platform/instantiation/common/instantiatio
import { ILogService } from 'vs/platform/log/common/log';
import { ExtHostLogService } from 'vs/workbench/api/worker/extHostLogService';
import { IExtHostTunnelService, ExtHostTunnelService } from 'vs/workbench/api/common/extHostTunnelService';
import { IExtHostApiDeprecationService, ExtHostApiDeprecationService, } from 'vs/workbench/api/common/extHostApiDeprecationService';

// register singleton services
registerSingleton(ILogService, ExtHostLogService);
registerSingleton(IExtHostApiDeprecationService, ExtHostApiDeprecationService);
registerSingleton(IExtHostOutputService, ExtHostOutputService);
registerSingleton(IExtHostWorkspace, ExtHostWorkspace);
registerSingleton(IExtHostDecorations, ExtHostDecorations);
Expand Down

0 comments on commit 0fa8a81

Please sign in to comment.