Commit 4701cca0 authored by Elwyn Benson's avatar Elwyn Benson 2️⃣
Browse files

feat: add format middleware when applying workflow edits

parent adedfb41
Loading
Loading
Loading
Loading
+4 −35
Original line number Diff line number Diff line
@@ -3,6 +3,8 @@ const { EventEmitter } = require('../desktop/test_utils/event_emitter');
const { FileType } = require('../desktop/test_utils/file_type');
const { FileSystemError } = require('../desktop/test_utils/file_system_error');
const { Position } = require('../common/test_utils/position');
const { Selection } = require('../common/test_utils/selection');
const { Range } = require('../common/test_utils/range');
const { TabInputText } = require('../common/test_utils/tab_input_text');

const returnsDisposable = () => jest.fn().mockReturnValue({ dispose: jest.fn() });
@@ -122,41 +124,8 @@ module.exports = {
  CommentThreadCollapsibleState: { Collapsed: 0, Expanded: 1 },
  CommentThreadState: { Unresolved: 0, Resolved: 1 },
  Position,
  // eslint-disable-next-line max-params
  Selection: function Selection(anchorLine, anchorCharacter, activeLine, activeCharacter) {
    const anchor = new Position(anchorLine, anchorCharacter);
    const active = new Position(activeLine, activeCharacter);

    return {
      anchor,
      active,
      start: anchor.isBefore(active) ? anchor : active,
      end: anchor.isBefore(active) ? active : anchor,
      get isEmpty() {
        return this.start.isEqual(this.end);
      },
    };
  },
  Range: function Range(...args) {
    let range;
    if (typeof args[0] === 'number') {
      const start = new Position(args[0], args[1]);
      const end = new Position(args[2], args[3]);
      range = { start, end };
    } else {
      range = { start: args[0], end: args[1] };
    }

    Object.defineProperty(range, 'isEqual', {
      value(other) {
        return this.start.isEqual(other.start) && this.end.isEqual(other.end);
      },
      // Add the isEqual method as non-enumerable so it doesn't mess up `expect(range1).isEqual(range2)` comparisons
      enumerable: false,
    });

    return range;
  },
  Selection,
  Range,
  CancellationTokenSource: function CancellationTokenSource() {
    const controller = new AbortController();

+2 −0
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ export enum FeatureFlag {
  FixWithDuoQuickChatCodeActions = 'fixWithDuoQuickChatCodeActions',
  LsCredentialsSync = 'lsCredentialsSync',
  AgenticTabs = 'agenticTabs',
  FormatEdits = 'formatEdits',
}

// Set the feature flag default value here
@@ -35,6 +36,7 @@ export const FEATURE_FLAGS_DEFAULT_VALUES = {
  [FeatureFlag.FixWithDuoQuickChatCodeActions]: true,
  [FeatureFlag.LsCredentialsSync]: true,
  [FeatureFlag.AgenticTabs]: false,
  [FeatureFlag.FormatEdits]: false,
};

// PLEASE NOTE: We can only query 20 flags at a time so this list shouldn't grow past that.
+3 −0
Original line number Diff line number Diff line
@@ -6,6 +6,7 @@ import {
} from 'vscode-languageclient';
import { createConverter } from 'vscode-languageclient/lib/common/protocolConverter';
import { Semaphore } from '../utils/semaphore';
import { log } from '../log';

export interface ApplyEditMiddleware {
  process(
@@ -22,6 +23,7 @@ export class ApplyEditClientWrapper {
  #workspaceEditLock: Semaphore<vscode.WorkspaceEdit> = new Semaphore(1);

  handleApplyWorkspaceEdit = async (params: ApplyWorkspaceEditParams) => {
    log.debug(`[ApplyEditClientWrapper] Running middlewares`);
    return this.#processWithMiddlewares(params, 0);
  };

@@ -34,6 +36,7 @@ export class ApplyEditClientWrapper {
    index: number,
  ): Promise<ApplyWorkspaceEditResult> {
    if (index >= this.#middlewares.length) {
      log.debug(`[ApplyEditClientWrapper] Applying edit`);
      return this.#applyEdit(params);
    }

+4 −6
Original line number Diff line number Diff line
@@ -80,12 +80,10 @@ export class DiffMiddleware implements ApplyEditMiddleware {
  async #openDiffView(fileUri: vscode.Uri): Promise<void> {
    const fileName = fileUri.path.split('/').pop() || 'untitled';
    const snapshotUri = this.#fileSnapshotProvider.snapshotUri(fileUri);
    const diffTitle = `${fileName}: Original ↔ Edited`;

    await vscode.commands.executeCommand(
      VS_COMMANDS.DIFF,
      snapshotUri,
      fileUri,
      `${fileName}: Original ↔ Edited`,
    );
    await vscode.commands.executeCommand(VS_COMMANDS.DIFF, snapshotUri, fileUri, diffTitle);

    log.debug(`[DiffMiddleware] Opened diff view: "${diffTitle}"`);
  }
}
+428 −0
Original line number Diff line number Diff line
import * as vscode from 'vscode';
import { ApplyWorkspaceEditParams, ApplyWorkspaceEditResult } from 'vscode-languageclient';
import { createFakePartial } from '../test_utils/create_fake_partial';
import { getLocalFeatureFlagService } from '../feature_flags/local_feature_flag_service';
import { FormatEditsMiddleware } from './format_edits_middleware';

jest.mock('../log');
jest.mock('../feature_flags/local_feature_flag_service');

describe('FormatEditsMiddleware', () => {
  let middleware: FormatEditsMiddleware;
  let mockNext: jest.Mock;
  let mockEditor: vscode.TextEditor;
  let mockDocument: vscode.TextDocument;
  let defaultParams: ApplyWorkspaceEditParams;

  const mockUri = vscode.Uri.parse('file:///test/file.ts');
  const mockSelection = new vscode.Selection(5, 10, 5, 10);

  beforeEach(() => {
    mockDocument = createFakePartial<vscode.TextDocument>({
      uri: mockUri,
    });

    let currentSelection = mockSelection;
    mockEditor = createFakePartial<vscode.TextEditor>({
      document: mockDocument,
      get selection() {
        return currentSelection;
      },
      set selection(value) {
        currentSelection = value;
      },
    });

    vscode.workspace.openTextDocument = jest.fn().mockResolvedValue(mockDocument);
    vscode.window.showTextDocument = jest.fn().mockResolvedValue(mockEditor);
    vscode.commands.executeCommand = jest.fn().mockResolvedValue(undefined);
    Object.defineProperty(vscode.window, 'visibleTextEditors', {
      value: [],
      configurable: true,
    });

    middleware = new FormatEditsMiddleware();

    mockNext = jest.fn().mockResolvedValue(
      createFakePartial<ApplyWorkspaceEditResult>({
        applied: true,
      }),
    );

    defaultParams = createFakePartial<ApplyWorkspaceEditParams>({
      edit: {
        documentChanges: [
          {
            textDocument: { uri: mockUri.toString(), version: 1 },
            edits: [
              {
                range: { start: { line: 0, character: 0 }, end: { line: 3, character: 5 } },
                newText: 'formatted content',
              },
            ],
          },
        ],
      },
    });
  });

  describe('when the feature flag is disabled', () => {
    beforeEach(() => {
      jest.mocked(getLocalFeatureFlagService).mockReturnValue(
        createFakePartial({
          isEnabled: jest.fn().mockReturnValue(false),
        }),
      );
    });

    it('should not format edits', async () => {
      const result = await middleware.process(defaultParams, mockNext);

      expect(mockNext).toHaveBeenCalledWith(defaultParams);
      expect(result).toEqual({ applied: true });

      expect(vscode.commands.executeCommand).not.toHaveBeenCalled();
    });
  });

  describe('when the feature flag is enabled', () => {
    beforeEach(() => {
      jest.mocked(getLocalFeatureFlagService).mockReturnValue(
        createFakePartial({
          isEnabled: jest.fn().mockReturnValue(true),
        }),
      );
    });

    describe('when edit is successfully applied', () => {
      let selectionBeforeFormat: vscode.Selection | null = null;

      beforeEach(() => {
        const originalExecuteCommand = vscode.commands.executeCommand;
        vscode.commands.executeCommand = jest.fn().mockImplementation(command => {
          if (command === 'editor.action.formatSelection') {
            selectionBeforeFormat = mockEditor.selection;
          }
          return originalExecuteCommand(command);
        });
      });

      it('should format edited ranges', async () => {
        const result = await middleware.process(defaultParams, mockNext);

        expect(mockNext).toHaveBeenCalledWith(defaultParams);

        expect(result).toEqual({ applied: true });

        expect(vscode.commands.executeCommand).toHaveBeenCalledWith(
          'editor.action.formatSelection',
        );
      });

      it('should handle multiple edits in the same document', async () => {
        const params = createFakePartial<ApplyWorkspaceEditParams>({
          edit: {
            documentChanges: [
              {
                textDocument: { uri: mockUri.toString(), version: 1 },
                edits: [
                  {
                    range: { start: { line: 0, character: 0 }, end: { line: 3, character: 5 } },
                    newText: 'formatted content',
                  },
                ],
              },
            ],
          },
        });

        const result = await middleware.process(params, mockNext);

        expect(mockNext).toHaveBeenCalledWith(params);

        expect(result).toEqual({ applied: true });

        expect(vscode.commands.executeCommand).toHaveBeenCalledWith(
          'editor.action.formatSelection',
        );
      });

      it('should format inserted text with zero-width range (insertion)', async () => {
        const params = createFakePartial<ApplyWorkspaceEditParams>({
          edit: {
            documentChanges: [
              {
                textDocument: { uri: mockUri.toString(), version: 1 },
                edits: [
                  {
                    // Zero-width range - this is an insertion, not a replacement
                    range: { start: { line: 0, character: 5 }, end: { line: 0, character: 5 } },
                    newText: ' Beautiful',
                  },
                ],
              },
            ],
          },
        });

        await middleware.process(params, mockNext);

        expect(vscode.commands.executeCommand).toHaveBeenCalledWith(
          'editor.action.formatSelection',
        );

        expect(selectionBeforeFormat).not.toBeNull();
        expect(selectionBeforeFormat!.start.line).toBe(0);
        expect(selectionBeforeFormat!.start.character).toBe(5);
        expect(selectionBeforeFormat!.end.line).toBe(0);
        expect(selectionBeforeFormat!.end.character).toBe(15); // 5 + 10 characters in " Beautiful"
      });

      it('should format multi-line inserted text with zero-width range', async () => {
        const params = createFakePartial<ApplyWorkspaceEditParams>({
          edit: {
            documentChanges: [
              {
                textDocument: { uri: mockUri.toString(), version: 1 },
                edits: [
                  {
                    // Zero-width range - this is an insertion, not a replacement
                    range: { start: { line: 2, character: 10 }, end: { line: 2, character: 10 } },
                    newText: 'function hello() {\n  console.log("world");\n}',
                  },
                ],
              },
            ],
          },
        });

        await middleware.process(params, mockNext);

        expect(mockNext).toHaveBeenCalledWith(params);
        expect(vscode.commands.executeCommand).toHaveBeenCalledWith(
          'editor.action.formatSelection',
        );

        // Verify the selection covered the inserted multi-line text before formatting
        expect(selectionBeforeFormat).not.toBeNull();
        expect(selectionBeforeFormat!.start.line).toBe(2);
        expect(selectionBeforeFormat!.start.character).toBe(10);
        expect(selectionBeforeFormat!.end.line).toBe(4); // 2 + 2 newlines
        expect(selectionBeforeFormat!.end.character).toBe(1); // Length of final "}"
      });

      it('should handle edits across multiple documents', async () => {
        const mockUri2 = vscode.Uri.parse('file:///test/file2.ts');
        const mockDocument2 = createFakePartial<vscode.TextDocument>({
          uri: mockUri2,
        });
        const currentSelection2 = new vscode.Selection(0, 0, 0, 0);
        const mockEditor2 = createFakePartial<vscode.TextEditor>({
          document: mockDocument2,
          selection: currentSelection2,
        });

        vscode.workspace.openTextDocument = jest
          .fn()
          .mockResolvedValueOnce(mockDocument)
          .mockResolvedValueOnce(mockDocument2);
        vscode.window.showTextDocument = jest
          .fn()
          .mockResolvedValueOnce(mockEditor)
          .mockResolvedValueOnce(mockEditor2);

        const params = createFakePartial<ApplyWorkspaceEditParams>({
          edit: {
            documentChanges: [
              {
                textDocument: { uri: mockUri.toString(), version: 1 },
                edits: [
                  {
                    range: { start: { line: 0, character: 0 }, end: { line: 3, character: 5 } },
                    newText: 'edit1',
                  },
                ],
              },
              {
                textDocument: { uri: mockUri2.toString(), version: 1 },
                edits: [
                  {
                    range: { start: { line: 5, character: 0 }, end: { line: 7, character: 0 } },
                    newText: 'edit2',
                  },
                ],
              },
            ],
          },
        });

        await middleware.process(params, mockNext);

        expect(vscode.workspace.openTextDocument).toHaveBeenCalledTimes(2);
        expect(vscode.window.showTextDocument).toHaveBeenCalledTimes(2);
        expect(vscode.commands.executeCommand).toHaveBeenCalledTimes(2);
      });

      it('should preserve existing user selection when document was already open', async () => {
        const existingSelection = new vscode.Selection(10, 5, 10, 15);
        const existingEditor = createFakePartial<vscode.TextEditor>({
          document: createFakePartial<vscode.TextDocument>({
            uri: mockUri,
          }),
          selection: existingSelection,
        });

        Object.defineProperty(vscode.window, 'visibleTextEditors', {
          value: [existingEditor],
          configurable: true,
        });

        await middleware.process(defaultParams, mockNext);

        expect(mockEditor.selection.start.line).toBe(10);
        expect(mockEditor.selection.start.character).toBe(5);
        expect(mockEditor.selection.end.line).toBe(10);
        expect(mockEditor.selection.end.character).toBe(15);
      });

      it('should continue processing even if formatting one range fails', async () => {
        vscode.commands.executeCommand = jest
          .fn()
          .mockRejectedValueOnce(new Error('Format failed'))
          .mockResolvedValueOnce(undefined);

        const params = createFakePartial<ApplyWorkspaceEditParams>({
          edit: {
            documentChanges: [
              {
                textDocument: { uri: mockUri.toString(), version: 1 },
                edits: [
                  {
                    range: { start: { line: 0, character: 0 }, end: { line: 3, character: 5 } },
                    newText: 'edit1',
                  },
                  {
                    range: { start: { line: 10, character: 0 }, end: { line: 12, character: 0 } },
                    newText: 'edit2',
                  },
                ],
              },
            ],
          },
        });

        await middleware.process(params, mockNext);

        expect(vscode.commands.executeCommand).toHaveBeenCalledTimes(2);
      });

      it('should process edits in reverse document order', async () => {
        const formatSelectionCalls: vscode.Selection[] = [];
        vscode.commands.executeCommand = jest.fn().mockImplementation(command => {
          if (command === 'editor.action.formatSelection') {
            formatSelectionCalls.push(mockEditor.selection);
          }
        });

        const params = createFakePartial<ApplyWorkspaceEditParams>({
          edit: {
            documentChanges: [
              {
                textDocument: { uri: mockUri.toString(), version: 1 },
                edits: [
                  {
                    range: { start: { line: 5, character: 0 }, end: { line: 5, character: 10 } },
                    newText: 'first edit',
                  },
                  {
                    range: { start: { line: 10, character: 5 }, end: { line: 10, character: 15 } },
                    newText: 'second edit',
                  },
                  {
                    range: { start: { line: 10, character: 0 }, end: { line: 10, character: 5 } },
                    newText: 'third edit',
                  },
                ],
              },
            ],
          },
        });

        await middleware.process(params, mockNext);

        expect(formatSelectionCalls).toHaveLength(3);
        // Should process in reverse order: line 10 char 5, then line 10 char 0, then line 5 char 0
        expect(formatSelectionCalls[0].start.line).toBe(10);
        expect(formatSelectionCalls[0].start.character).toBe(5);
        expect(formatSelectionCalls[1].start.line).toBe(10);
        expect(formatSelectionCalls[1].start.character).toBe(0);
        expect(formatSelectionCalls[2].start.line).toBe(5);
        expect(formatSelectionCalls[2].start.character).toBe(0);
      });
    });

    describe('when edit is not applied', () => {
      it('should not format anything', async () => {
        const params = createFakePartial<ApplyWorkspaceEditParams>({
          edit: {
            documentChanges: [
              {
                textDocument: { uri: mockUri.toString(), version: 1 },
                edits: [
                  {
                    range: { start: { line: 0, character: 0 }, end: { line: 3, character: 5 } },
                    newText: 'formatted content',
                  },
                ],
              },
            ],
          },
        });
        mockNext.mockResolvedValue(
          createFakePartial<ApplyWorkspaceEditResult>({
            applied: false,
          }),
        );

        await middleware.process(params, mockNext);

        expect(mockNext).toHaveBeenCalledWith(params);

        expect(vscode.workspace.openTextDocument).not.toHaveBeenCalled();
        expect(vscode.window.showTextDocument).not.toHaveBeenCalled();
        expect(vscode.commands.executeCommand).not.toHaveBeenCalled();
      });
    });

    describe('edge cases', () => {
      it('should handle workspace edit without documentChanges', async () => {
        const params = createFakePartial<ApplyWorkspaceEditParams>({
          edit: {},
        });

        await middleware.process(params, mockNext);

        expect(mockNext).toHaveBeenCalledWith(params);
        expect(vscode.commands.executeCommand).not.toHaveBeenCalled();
      });

      it('should handle empty edits array', async () => {
        const params = createFakePartial<ApplyWorkspaceEditParams>({
          edit: {
            documentChanges: [
              {
                textDocument: { uri: mockUri.toString(), version: 1 },
                edits: [],
              },
            ],
          },
        });

        await middleware.process(params, mockNext);

        expect(mockNext).toHaveBeenCalledWith(params);
        expect(vscode.commands.executeCommand).not.toHaveBeenCalled();
      });
    });
  });
});
Loading