Skip to content

Commit 6b5f248

Browse files
DonJayamanneDonJayamanne
authored
Ensure decorators cannot be applied to async functions (microsoft#4053)
For microsoft#4055
1 parent d535738 commit 6b5f248

7 files changed

Lines changed: 34 additions & 18 deletions

File tree

‎news/3 Code Health/4055.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Ensure `debounce` decorator cannot be applied to async functions.

‎src/client/activation/languageServer/analysisOptions.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -171,11 +171,14 @@ export class LanguageServerAnalysisOptions implements ILanguageServerAnalysisOpt
171171
if (e && !e.affectsConfiguration('python', this.resource)) {
172172
return;
173173
}
174-
this.onSettingsChanged().catch(ex => traceError('Failed to detect changes', ex));
174+
this.onSettingsChanged();
175175
}
176-
@traceDecorators.verbose('Changes in python settings detected in analysis options')
177176
@debounce(1000)
178-
protected async onSettingsChanged(): Promise<void> {
177+
protected onSettingsChanged(): void {
178+
this.notifyIfSettingsChanged().ignoreErrors();
179+
}
180+
@traceDecorators.verbose('Changes in python settings detected in analysis options')
181+
protected async notifyIfSettingsChanged(): Promise<void> {
179182
const idata = await this.interpreterDataService.getInterpreterData(this.resource);
180183
if (!idata || idata.hash !== this.interpreterHash) {
181184
this.interpreterHash = idata ? idata.hash : '';

‎src/client/activation/languageServer/manager.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import { inject, injectable } from 'inversify';
77
import { ICommandManager } from '../../common/application/types';
8+
import '../../common/extensions';
89
import { traceDecorators } from '../../common/logger';
910
import { IDisposable, Resource } from '../../common/types';
1011
import { debounce } from '../../common/utils/decorators';
@@ -39,7 +40,7 @@ export class LanguageServerManager implements ILanguageServerManager {
3940
}
4041
this.registerCommandHandler();
4142
this.resource = resource;
42-
this.analysisOptions.onDidChange(this.restartLanguageServer, this, this.disposables);
43+
this.analysisOptions.onDidChange(this.restartLanguageServerDebounced, this, this.disposables);
4344

4445
await this.analysisOptions.initialize(resource);
4546
await this.startLanguageServer();
@@ -56,9 +57,12 @@ export class LanguageServerManager implements ILanguageServerManager {
5657
this.languageServer.loadExtension(LanguageServerManager.loadExtensionArgs);
5758
}
5859
}
60+
@debounce(1000)
61+
protected restartLanguageServerDebounced(): void {
62+
this.restartLanguageServer().ignoreErrors();
63+
}
5964
@traceDecorators.error('Failed to restart Language Server')
6065
@traceDecorators.verbose('Restarting Language Server')
61-
@debounce(1000)
6266
protected async restartLanguageServer(): Promise<void> {
6367
if (this.languageServer) {
6468
this.languageServer.dispose();

‎src/client/common/logger.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,19 @@ function argsToLogString(args: any[]): string {
5858
try {
5959
return (args || [])
6060
.map((item, index) => {
61+
if (item === undefined) {
62+
return `Arg ${index + 1}: undefined`;
63+
}
64+
if (item === null) {
65+
return `Arg ${index + 1}: null`;
66+
}
6167
try {
62-
if (item.fsPath) {
68+
if (item && item.fsPath) {
6369
return `Arg ${index + 1}: <Uri:${item.fsPath}>`;
6470
}
6571
return `Arg ${index + 1}: ${JSON.stringify(item)}`;
6672
} catch {
67-
return `Arg ${index + 1}: UNABLE TO DETERMINE VALUE`;
73+
return `Arg ${index + 1}: <argument cannot be serialized for logging>`;
6874
}
6975
})
7076
.join(', ');

‎src/client/common/utils/decorators.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { InMemoryInterpreterSpecificCache } from './cacheUtils';
1010
// tslint:disable-next-line:no-require-imports no-var-requires
1111
const _debounce = require('lodash/debounce') as typeof import('lodash/debounce');
1212

13+
type VoidFunction = (...any: any[]) => void;
1314
/**
1415
* Debounces a function execution. Function must return either a void or a promise that resolves to a void.
1516
* @export
@@ -18,12 +19,12 @@ const _debounce = require('lodash/debounce') as typeof import('lodash/debounce')
1819
*/
1920
export function debounce(wait?: number) {
2021
// tslint:disable-next-line:no-any no-function-expression
21-
return function (_target: any, _propertyName: string, descriptor: TypedPropertyDescriptor<any>) {
22+
return function (_target: any, _propertyName: string, descriptor: TypedPropertyDescriptor<VoidFunction>) {
2223
const originalMethod = descriptor.value!;
2324
// If running tests, lets not debounce (so tests run fast).
2425
wait = wait && isTestExecution() ? undefined : wait;
2526
// tslint:disable-next-line:no-invalid-this no-any
26-
(descriptor as any).value = _debounce(function () { return originalMethod.apply(this, arguments); }, wait);
27+
(descriptor as any).value = _debounce(function () { return originalMethod.apply(this, arguments as any); }, wait);
2728
};
2829
}
2930

‎src/client/sourceMapSupport.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,11 @@ export class SourceMapSupport {
2727
await this.enableSourceMaps(true);
2828
const localize = require('./common/utils/localize') as typeof import('./common/utils/localize');
2929
const disable = localize.Diagnostics.disableSourceMaps();
30-
const selection = await this.vscode.window.showWarningMessage(localize.Diagnostics.warnSourceMaps(), disable);
31-
if (selection === disable) {
32-
await this.disable();
33-
}
30+
this.vscode.window.showWarningMessage(localize.Diagnostics.warnSourceMaps(), disable).then(selection => {
31+
if (selection === disable) {
32+
this.disable().ignoreErrors();
33+
}
34+
});
3435
}
3536
public get enabled(): boolean {
3637
return this.config.get<boolean>(setting, false);

‎src/test/activation/languageServer/analysisOptions.unit.test.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ suite('Language Server - Analysis Options', () => {
3939
public getTypeshedPaths(): string[] {
4040
return super.getTypeshedPaths();
4141
}
42-
public async onSettingsChanged(): Promise<void> {
42+
public onSettingsChanged(): void {
4343
return super.onSettingsChanged();
4444
}
4545
public async notifyIfValuesHaveChanged(oldArray: string[], newArray: string[]): Promise<void> {
@@ -128,7 +128,7 @@ suite('Language Server - Analysis Options', () => {
128128
let eventFired = false;
129129
analysisOptions.onDidChange(() => eventFired = true);
130130

131-
await analysisOptions.onSettingsChanged();
131+
analysisOptions.onSettingsChanged();
132132
await sleep(1);
133133

134134
expect(eventFired).to.be.equal(false);
@@ -142,7 +142,7 @@ suite('Language Server - Analysis Options', () => {
142142
let eventFired = false;
143143
analysisOptions.onDidChange(() => eventFired = true);
144144

145-
await analysisOptions.onSettingsChanged();
145+
analysisOptions.onSettingsChanged();
146146
await sleep(1);
147147

148148
expect(eventFired).to.be.equal(true);
@@ -156,7 +156,7 @@ suite('Language Server - Analysis Options', () => {
156156
let eventFired = false;
157157
analysisOptions.onDidChange(() => eventFired = true);
158158

159-
await analysisOptions.onSettingsChanged();
159+
analysisOptions.onSettingsChanged();
160160
await sleep(1);
161161

162162
expect(eventFired).to.be.equal(true);
@@ -168,7 +168,7 @@ suite('Language Server - Analysis Options', () => {
168168
let eventFired = false;
169169
analysisOptions.onDidChange(() => eventFired = true);
170170

171-
await analysisOptions.onSettingsChanged();
171+
analysisOptions.onSettingsChanged();
172172
await sleep(1);
173173

174174
expect(eventFired).to.be.equal(true);

0 commit comments

Comments
 (0)