Use dependency injection for TestCaseRecorder (#2002)

Use dependency injection for getting TestCaseRecorder into
CursorlessEngine.

A future PR should perhaps move TestCaseRecorder into a new package,
@cursorless/cursorless-dev or similar, to isolate developer-only
dependencies that are needed in production code.

This is an alternate approach to
#2001 requested in a
comment.

It is also the most TypeScript I've written to date, so I probably did
some things stylistically wrong. Class
`TestCaseRecorderCommandRunnerDecorator` in particular feels very
C++-like, not very TS-like, to me.

---------

Co-authored-by: Barry Jaspan <bjaspan@google.com>
Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com>
Co-authored-by: Pokey Rule <755842+pokey@users.noreply.github.com>

Author: 4 people

packages/cursorless-engine/src/api/CursorlessEngineApi.ts

@@ -1,19 +1,22 @@
 import { Command, HatTokenMap, IDE } from "@cursorless/common";
 import { Snippets } from "../core/Snippets";
 import { StoredTargetMap } from "../core/StoredTargets";
-import { TestCaseRecorder } from "../testCaseRecorder/TestCaseRecorder";
 import { ScopeProvider } from "@cursorless/common";
+import { CommandRunner } from "../CommandRunner";
+import { ReadOnlyHatMap } from "@cursorless/common";
 
 export interface CursorlessEngine {
   commandApi: CommandApi;
   scopeProvider: ScopeProvider;
   customSpokenFormGenerator: CustomSpokenFormGenerator;
-  testCaseRecorder: TestCaseRecorder;
   storedTargets: StoredTargetMap;
   hatTokenMap: HatTokenMap;
   snippets: Snippets;
   injectIde: (ide: IDE | undefined) => void;
   runIntegrationTests: () => Promise<void>;
+  addCommandRunnerDecorator: (
+    commandRunnerDecorator: CommandRunnerDecorator,
+  ) => void;
 }
 
 export interface CustomSpokenFormGenerator {
@@ -39,3 +42,16 @@ export interface CommandApi {
    */
   runCommandSafe(...args: unknown[]): Promise<unknown>;
 }
+
+export interface CommandRunnerDecorator {
+  /**
+   * @param commandRunner: A CommandRunner.
+   * @param readableHatMap: A ReadOnlyHatMap.
+   * @returns A new CommandRunner that invokes the provided CommandRunner in
+   *   addition to performing some other work.
+   */
+  wrapCommandRunner: (
+    readableHatMap: ReadOnlyHatMap,
+    commandRunner: CommandRunner,
+  ) => CommandRunner;
+}

packages/cursorless-engine/src/cursorlessEngine.ts

@@ -6,8 +6,11 @@ import {
   IDE,
   ScopeProvider,
 } from "@cursorless/common";
-import { StoredTargetMap, TestCaseRecorder, TreeSitter } from ".";
-import { CursorlessEngine } from "./api/CursorlessEngineApi";
+import { StoredTargetMap, TreeSitter } from ".";
+import {
+  CommandRunnerDecorator,
+  CursorlessEngine,
+} from "./api/CursorlessEngineApi";
 import { Debug } from "./core/Debug";
 import { HatTokenMapImpl } from "./core/HatTokenMapImpl";
 import { Snippets } from "./core/Snippets";
@@ -53,8 +56,6 @@ export function createCursorlessEngine(
 
   const storedTargets = new StoredTargetMap();
 
-  const testCaseRecorder = new TestCaseRecorder(hatTokenMap, storedTargets);
-
   const languageDefinitions = new LanguageDefinitions(fileSystem, treeSitter);
 
   const talonSpokenForms = new TalonSpokenFormsJsonReader(fileSystem);
@@ -65,18 +66,20 @@ export function createCursorlessEngine(
 
   ide.disposeOnExit(rangeUpdater, languageDefinitions, hatTokenMap, debug);
 
+  const commandRunnerDecorators: CommandRunnerDecorator[] = [];
+
   return {
     commandApi: {
       runCommand(command: Command) {
         return runCommand(
           treeSitter,
           debug,
           hatTokenMap,
-          testCaseRecorder,
           snippets,
           storedTargets,
           languageDefinitions,
           rangeUpdater,
+          commandRunnerDecorators,
           command,
         );
       },
@@ -86,11 +89,11 @@ export function createCursorlessEngine(
           treeSitter,
           debug,
           hatTokenMap,
-          testCaseRecorder,
           snippets,
           storedTargets,
           languageDefinitions,
           rangeUpdater,
+          commandRunnerDecorators,
           ensureCommandShape(args),
         );
       },
@@ -101,13 +104,15 @@ export function createCursorlessEngine(
       customSpokenFormGenerator,
     ),
     customSpokenFormGenerator,
-    testCaseRecorder,
     storedTargets,
     hatTokenMap,
     snippets,
     injectIde,
     runIntegrationTests: () =>
       runIntegrationTests(treeSitter, languageDefinitions),
+    addCommandRunnerDecorator: (decorator: CommandRunnerDecorator) => {
+      commandRunnerDecorators.push(decorator);
+    },
   };
 }
 

packages/cursorless-engine/src/runCommand.ts

@@ -6,12 +6,13 @@ import { Snippets } from "./core/Snippets";
 import { CommandRunnerImpl } from "./core/commandRunner/CommandRunnerImpl";
 import { canonicalizeAndValidateCommand } from "./core/commandVersionUpgrades/canonicalizeAndValidateCommand";
 import { RangeUpdater } from "./core/updateSelections/RangeUpdater";
-import { StoredTargetMap, TestCaseRecorder, TreeSitter } from "./index";
+import { StoredTargetMap, TreeSitter } from "./index";
 import { LanguageDefinitions } from "./languages/LanguageDefinitions";
 import { TargetPipelineRunner } from "./processTargets";
 import { MarkStageFactoryImpl } from "./processTargets/MarkStageFactoryImpl";
 import { ModifierStageFactoryImpl } from "./processTargets/ModifierStageFactoryImpl";
 import { ScopeHandlerFactoryImpl } from "./processTargets/modifiers/scopeHandlers";
+import { CommandRunnerDecorator } from "./api/CursorlessEngineApi";
 
 /**
  * Entry point for Cursorless commands. We proceed as follows:
@@ -29,11 +30,11 @@ export async function runCommand(
   treeSitter: TreeSitter,
   debug: Debug,
   hatTokenMap: HatTokenMap,
-  testCaseRecorder: TestCaseRecorder,
   snippets: Snippets,
   storedTargets: StoredTargetMap,
   languageDefinitions: LanguageDefinitions,
   rangeUpdater: RangeUpdater,
+  commandRunnerDecorators: CommandRunnerDecorator[],
   command: Command,
 ): Promise<unknown> {
   if (debug.active) {
@@ -57,11 +58,8 @@ export async function runCommand(
     rangeUpdater,
   );
 
-  if (testCaseRecorder.isActive()) {
-    commandRunner = testCaseRecorder.wrapCommandRunner(
-      readableHatMap,
-      commandRunner,
-    );
+  for (const decorator of commandRunnerDecorators) {
+    commandRunner = decorator.wrapCommandRunner(readableHatMap, commandRunner);
   }
 
   return await commandRunner.run(commandComplete);

packages/cursorless-engine/src/testCaseRecorder/TestCaseRecorder.ts

@@ -459,6 +459,9 @@ export class TestCaseRecorder {
     readableHatMap: ReadOnlyHatMap,
     runner: CommandRunner,
   ): CommandRunner {
+    if (!this.isActive()) {
+      return runner;
+    }
     return {
       run: async (commandComplete: CommandComplete) => {
         try {

packages/cursorless-vscode/src/extension.ts

@@ -48,6 +48,7 @@ import {
 import { StatusBarItem } from "./StatusBarItem";
 import { vscodeApi } from "./vscodeApi";
 import { mkdir } from "fs/promises";
+import { TestCaseRecorder } from "@cursorless/cursorless-engine";
 
 /**
  * Extension entrypoint called by VSCode on Cursorless startup.
@@ -82,13 +83,13 @@ export async function activate(
 
   const {
     commandApi,
-    testCaseRecorder,
     storedTargets,
     hatTokenMap,
     scopeProvider,
     snippets,
     injectIde,
     runIntegrationTests,
+    addCommandRunnerDecorator,
     customSpokenFormGenerator,
   } = createCursorlessEngine(
     treeSitter,
@@ -98,6 +99,9 @@ export async function activate(
     fileSystem,
   );
 
+  const testCaseRecorder = new TestCaseRecorder(hatTokenMap, storedTargets);
+  addCommandRunnerDecorator(testCaseRecorder);
+
   const statusBarItem = StatusBarItem.create("cursorless.showQuickPick");
   const keyboardCommands = KeyboardCommands.create(context, statusBarItem);
   const scopeVisualizer = createScopeVisualizer(normalizedIde, scopeProvider);