Make sure we cancel code action requests for mainThreadSave

If a code action provider takes too long to compute a code action during save, we skip that code action. However we do not cancel the request itself

With this change, we now cancel `getCodeActions` when the request times out

src/vs/editor/contrib/codeAction/codeAction.ts

@@ -18,8 +18,8 @@ import { CodeActionFilter, CodeActionKind, CodeActionTrigger } from './codeActio
 export function getCodeActions(
  model: ITextModel,
  rangeOrSelection: Range | Selection,
- trigger?: CodeActionTrigger,
- token: CancellationToken = CancellationToken.None
+ trigger: CodeActionTrigger | undefined,
+ token: CancellationToken
 ): Promise<CodeAction[]> {
  const codeActionContext: CodeActionContext = {
  only: trigger && trigger.filter && trigger.filter.kind ? trigger.filter.kind.value : undefined,
@@ -114,5 +114,9 @@ registerLanguageCommand('_executeCodeActionProvider', function (accessor, args)
  throw illegalArgument();
  }
 
- return getCodeActions(model, model.validateRange(range), { type: 'manual', filter: { includeSourceActions: true } });
+ return getCodeActions(
+ model,
+ model.validateRange(range),
+ { type: 'manual', filter: { includeSourceActions: true } },
+ CancellationToken.None);
 });

src/vs/editor/contrib/codeAction/test/codeAction.test.ts

@@ -11,6 +11,7 @@ import { CodeAction, CodeActionContext, CodeActionProvider, CodeActionProviderRe
 import { getCodeActions } from 'vs/editor/contrib/codeAction/codeAction';
 import { CodeActionKind } from 'vs/editor/contrib/codeAction/codeActionTrigger';
 import { IMarkerData, MarkerSeverity } from 'vs/platform/markers/common/markers';
+import { CancellationToken } from 'vs/base/common/cancellation';
 
 suite('CodeAction', () => {
 
@@ -115,7 +116,7 @@ suite('CodeAction', () => {
  testData.tsLint.abc
  ];
 
- const actions = await getCodeActions(model, new Range(1, 1, 2, 1));
+ const actions = await getCodeActions(model, new Range(1, 1, 2, 1), undefined, CancellationToken.None);
  assert.equal(actions.length, 6);
  assert.deepEqual(actions, expected);
  });
@@ -134,20 +135,20 @@ suite('CodeAction', () => {
  disposables.push(CodeActionProviderRegistry.register('fooLang', provider));
 
  {
- const actions = await getCodeActions(model, new Range(1, 1, 2, 1), { type: 'auto', filter: { kind: new CodeActionKind('a') } });
+ const actions = await getCodeActions(model, new Range(1, 1, 2, 1), { type: 'auto', filter: { kind: new CodeActionKind('a') } }, CancellationToken.None);
  assert.equal(actions.length, 2);
  assert.strictEqual(actions[0].title, 'a');
  assert.strictEqual(actions[1].title, 'a.b');
  }
 
  {
- const actions = await getCodeActions(model, new Range(1, 1, 2, 1), { type: 'auto', filter: { kind: new CodeActionKind('a.b') } });
+ const actions = await getCodeActions(model, new Range(1, 1, 2, 1), { type: 'auto', filter: { kind: new CodeActionKind('a.b') } }, CancellationToken.None);
  assert.equal(actions.length, 1);
  assert.strictEqual(actions[0].title, 'a.b');
  }
 
  {
- const actions = await getCodeActions(model, new Range(1, 1, 2, 1), { type: 'auto', filter: { kind: new CodeActionKind('a.b.c') } });
+ const actions = await getCodeActions(model, new Range(1, 1, 2, 1), { type: 'auto', filter: { kind: new CodeActionKind('a.b.c') } }, CancellationToken.None);
  assert.equal(actions.length, 0);
  }
  });
@@ -163,7 +164,7 @@ suite('CodeAction', () => {
 
  disposables.push(CodeActionProviderRegistry.register('fooLang', provider));
 
- const actions = await getCodeActions(model, new Range(1, 1, 2, 1), { type: 'auto', filter: { kind: new CodeActionKind('a') } });
+ const actions = await getCodeActions(model, new Range(1, 1, 2, 1), { type: 'auto', filter: { kind: new CodeActionKind('a') } }, CancellationToken.None);
  assert.equal(actions.length, 1);
  assert.strictEqual(actions[0].title, 'a');
  });
@@ -181,13 +182,13 @@ suite('CodeAction', () => {
  disposables.push(CodeActionProviderRegistry.register('fooLang', provider));
 
  {
- const actions = await getCodeActions(model, new Range(1, 1, 2, 1), { type: 'auto' });
+ const actions = await getCodeActions(model, new Range(1, 1, 2, 1), { type: 'auto' }, CancellationToken.None);
  assert.equal(actions.length, 1);
  assert.strictEqual(actions[0].title, 'b');
  }
 
  {
- const actions = await getCodeActions(model, new Range(1, 1, 2, 1), { type: 'auto', filter: { kind: CodeActionKind.Source, includeSourceActions: true } });
+ const actions = await getCodeActions(model, new Range(1, 1, 2, 1), { type: 'auto', filter: { kind: CodeActionKind.Source, includeSourceActions: true } }, CancellationToken.None);
  assert.equal(actions.length, 1);
  assert.strictEqual(actions[0].title, 'a');
  }
@@ -211,7 +212,7 @@ suite('CodeAction', () => {
  filter: {
  kind: CodeActionKind.QuickFix
  }
- });
+ }, CancellationToken.None);
  assert.strictEqual(actions.length, 0);
  assert.strictEqual(wasInvoked, false);
  });

src/vs/workbench/api/electron-browser/mainThreadSaveParticipant.ts

@@ -5,7 +5,7 @@
 
 import { isNonEmptyArray } from 'vs/base/common/arrays';
 import { IdleValue, sequence } from 'vs/base/common/async';
-import { CancellationTokenSource } from 'vs/base/common/cancellation';
+import { CancellationTokenSource, CancellationToken } from 'vs/base/common/cancellation';
 import * as strings from 'vs/base/common/strings';
 import { ICodeEditor } from 'vs/editor/browser/editorBrowser';
 import { IBulkEditService } from 'vs/editor/browser/services/bulkEditService';
@@ -316,18 +316,28 @@ class CodeActionOnParticipant implements ISaveParticipant {
  return undefined;
  }
 
+ const tokenSource = new CancellationTokenSource();
+
  const timeout = this._configurationService.getValue<number>('editor.codeActionsOnSaveTimeout', settingsOverrides);
 
  return Promise.race([
  new Promise<void>((_resolve, reject) =>
- setTimeout(() => reject(localize('codeActionsOnSave.didTimeout', "Aborted codeActionsOnSave after {0}ms", timeout)), timeout)),
- this.applyOnSaveActions(model, codeActionsOnSave)
- ]).then(() => undefined);
+ setTimeout(() => {
+ tokenSource.cancel();
+ reject(localize('codeActionsOnSave.didTimeout', "Aborted codeActionsOnSave after {0}ms", timeout));
+ }, timeout)),
+ this.applyOnSaveActions(model, codeActionsOnSave, tokenSource.token)
+ ]).then(() => {
+ tokenSource.cancel();
+ }, (e) => {
+ tokenSource.cancel();
+ return Promise.reject(e);
+ });
  }
 
- private async applyOnSaveActions(model: ITextModel, codeActionsOnSave: CodeActionKind[]): Promise<void> {
+ private async applyOnSaveActions(model: ITextModel, codeActionsOnSave: CodeActionKind[], token: CancellationToken): Promise<void> {
  for (const codeActionKind of codeActionsOnSave) {
- const actionsToRun = await this.getActionsToRun(model, codeActionKind);
+ const actionsToRun = await this.getActionsToRun(model, codeActionKind, token);
  try {
  await this.applyCodeActions(actionsToRun);
  } catch {
@@ -342,11 +352,11 @@ class CodeActionOnParticipant implements ISaveParticipant {
  }
  }
 
- private getActionsToRun(model: ITextModel, codeActionKind: CodeActionKind) {
+ private getActionsToRun(model: ITextModel, codeActionKind: CodeActionKind, token: CancellationToken) {
  return getCodeActions(model, model.getFullModelRange(), {
  type: 'auto',
  filter: { kind: codeActionKind, includeSourceActions: true },
- });
+ }, token);
  }
 }
 

src/vs/workbench/parts/markers/electron-browser/markers.ts

@@ -24,6 +24,7 @@ import { Range } from 'vs/editor/common/core/range';
 import { getCodeActions } from 'vs/editor/contrib/codeAction/codeAction';
 import { CodeActionKind } from 'vs/editor/contrib/codeAction/codeActionTrigger';
 import { timeout } from 'vs/base/common/async';
+import { CancellationToken } from 'vs/base/common/cancellation';
 
 export const IMarkersWorkbenchService = createDecorator<IMarkersWorkbenchService>('markersWorkbenchService');
 
@@ -159,8 +160,7 @@ export class MarkersWorkbenchService extends Disposable implements IMarkersWorkb
  private async _getFixes(uri: URI, range?: Range): Promise<CodeAction[]> {
  const model = this.modelService.getModel(uri);
  if (model) {
- const codeActions = await getCodeActions(model, range ? range : model.getFullModelRange(), { type: 'manual', filter: { kind: CodeActionKind.QuickFix } });
- return codeActions;
+ return getCodeActions(model, range ? range : model.getFullModelRange(), { type: 'manual', filter: { kind: CodeActionKind.QuickFix } }, CancellationToken.None /* TODO: use cancellation here */);
  }
  return [];
  }

src/vs/workbench/test/electron-browser/api/extHostLanguageFeatures.test.ts

@@ -670,7 +670,7 @@ suite('ExtHostLanguageFeatures', function () {
  }));
 
  return rpcProtocol.sync().then(() => {
- return getCodeActions(model, model.getFullModelRange(), undefined).then(value => {
+ return getCodeActions(model, model.getFullModelRange(), undefined, CancellationToken.None).then(value => {
  assert.equal(value.length, 2);
 
  const [first, second] = value;
@@ -697,7 +697,7 @@ suite('ExtHostLanguageFeatures', function () {
  }));
 
  return rpcProtocol.sync().then(() => {
- return getCodeActions(model, model.getFullModelRange(), undefined).then(value => {
+ return getCodeActions(model, model.getFullModelRange(), undefined, CancellationToken.None).then(value => {
  assert.equal(value.length, 1);
 
  const [first] = value;
@@ -723,7 +723,7 @@ suite('ExtHostLanguageFeatures', function () {
  }));
 
  return rpcProtocol.sync().then(() => {
- return getCodeActions(model, model.getFullModelRange(), undefined).then(value => {
+ return getCodeActions(model, model.getFullModelRange(), undefined, CancellationToken.None).then(value => {
  assert.equal(value.length, 1);
  });
  });
@@ -743,7 +743,7 @@ suite('ExtHostLanguageFeatures', function () {
  }));
 
  return rpcProtocol.sync().then(() => {
- return getCodeActions(model, model.getFullModelRange(), undefined).then(value => {
+ return getCodeActions(model, model.getFullModelRange(), undefined, CancellationToken.None).then(value => {
  assert.equal(value.length, 1);
  });
  });