fix: multiple bug fixes in one pr (#12)

zaripych · Jan 8, 2024 · 9131738 · 9131738
1 parent 301bca2
commit 9131738
Show file tree

Hide file tree

Showing 15 changed files with 164 additions and 37 deletions.
diff --git a/.changeset/cool-pumpkins-drum.md b/.changeset/cool-pumpkins-drum.md
@@ -0,0 +1,6 @@
+---
+'refactor-bot': patch
+---
+
+fix: sanitize results of the function calls when they fail removing full paths
+to repository
diff --git a/.changeset/gold-hairs-share.md b/.changeset/gold-hairs-share.md
@@ -0,0 +1,5 @@
+---
+'refactor-bot': patch
+---
+
+fix: default to gpt-3.5-turbo-1106 in the config
diff --git a/.changeset/quiet-walls-add.md b/.changeset/quiet-walls-add.md
@@ -0,0 +1,5 @@
+---
+'refactor-bot': patch
+---
+
+fix: fail at the start of the refactor when prettier cannot be found
diff --git a/packages/refactor-bot/src/functions/executeFunction.ts b/packages/refactor-bot/src/functions/executeFunction.ts
@@ -3,6 +3,7 @@ import { realpath } from 'fs/promises';
 import { findRepositoryRoot } from '../file-system/findRepositoryRoot';
 import { logger } from '../logger/logger';
 import type { functions } from './registry';
+import { sanitizeFunctionResult } from './sanitizeFunctionResult';
 import type { FunctionsConfig } from './types';
 
 type FunctionNames = (typeof functions)[number]['name'];
@@ -67,19 +68,24 @@ export async function executeFunction(
         () => repositoryRoot
     );
 
+    const initializedConfig = {
+        ...config,
+        repositoryRoot: realRepositoryRoot,
+    };
+
     try {
-        return (await fn(opts.arguments as never, {
-            ...config,
-            repositoryRoot: realRepositoryRoot,
-        })) as object;
+        return (await fn(opts.arguments as never, initializedConfig)) as object;
     } catch (err: unknown) {
         if (err instanceof Error) {
             logger.debug(err);
-            return {
-                error: {
-                    message: err.message,
+            return await sanitizeFunctionResult({
+                result: {
+                    error: {
+                        message: err.message,
+                    },
                 },
-            };
+                config: initializedConfig,
+            });
         }
         throw err;
     }

diff --git a/packages/refactor-bot/src/functions/makeFunction.ts b/packages/refactor-bot/src/functions/makeFunction.ts
@@ -41,7 +41,7 @@ export const makeFunction = <
                     result
                 )) as unknown;
 
-                return sanitizeFunctionResult({
+                return await sanitizeFunctionResult({
                     result: validatedResult,
                     config,
                 });

diff --git a/packages/refactor-bot/src/functions/sanitizeFunctionResult.ts b/packages/refactor-bot/src/functions/sanitizeFunctionResult.ts
@@ -1,37 +1,67 @@
+import { realpath } from 'fs/promises';
 import { relative } from 'path';
 
+import { escapeRegExp } from '../utils/escapeRegExp';
 import type { FunctionsConfig } from './types';
 
-function sanitizeText(text: string, config: FunctionsConfig) {
-    if (text.startsWith(config.repositoryRoot)) {
-        return relative(config.repositoryRoot, text);
+type SanitizationParameters = {
+    repositoryRoot: string;
+    realRepositoryRoot: string;
+};
+
+async function evaluateParameters(config: FunctionsConfig) {
+    const realRepositoryRoot = await realpath(config.repositoryRoot).catch(
+        () => {
+            return config.repositoryRoot;
+        }
+    );
+    return {
+        repositoryRoot: config.repositoryRoot,
+        realRepositoryRoot,
+    };
+}
+
+function sanitizeText(text: string, config: SanitizationParameters) {
+    if (
+        text.includes(config.repositoryRoot) ||
+        text.includes(config.realRepositoryRoot)
+    ) {
+        return text
+            .replaceAll(
+                new RegExp(`(${escapeRegExp(config.repositoryRoot)})`, 'g'),
+                (match) => '.' + relative(config.repositoryRoot, match)
+            )
+            .replaceAll(
+                new RegExp(`(${escapeRegExp(config.realRepositoryRoot)})`, 'g'),
+                (match) => '.' + relative(config.repositoryRoot, match)
+            );
     }
     return text;
 }
 
-export function sanitizeFunctionResult(opts: {
+function sanitizeFunctionResultWithParams(opts: {
     result: unknown;
-    config: FunctionsConfig;
+    params: SanitizationParameters;
 }): unknown {
-    const { result, config } = opts;
+    const { result, params } = opts;
 
     if (typeof result === 'string') {
-        return sanitizeText(result, config);
+        return sanitizeText(result, params);
     }
 
     if (typeof result === 'object' && result !== null) {
         if (Array.isArray(result)) {
             const array = result as unknown[];
             return array.map((item) =>
-                sanitizeFunctionResult({ result: item, config })
+                sanitizeFunctionResultWithParams({ result: item, params })
             );
         }
 
         const sanitized: Record<string, unknown> = {};
         for (const [key, value] of Object.entries(result)) {
-            sanitized[key] = sanitizeFunctionResult({
+            sanitized[key] = sanitizeFunctionResultWithParams({
                 result: value,
-                config,
+                params,
             });
         }
 
@@ -40,3 +70,17 @@ export function sanitizeFunctionResult(opts: {
 
     return result;
 }
+
+export async function sanitizeFunctionResult<T>(opts: {
+    result: T;
+    config: FunctionsConfig;
+}): Promise<T> {
+    const { result, config } = opts;
+
+    const params = await evaluateParameters(config);
+
+    return sanitizeFunctionResultWithParams({
+        result,
+        params,
+    }) as T;
+}
diff --git a/packages/refactor-bot/src/prettier/prettier.ts b/packages/refactor-bot/src/prettier/prettier.ts
@@ -7,8 +7,9 @@ import { z } from 'zod';
 import { spawnResult } from '../child-process/spawnResult';
 import { findRefactorBotPackageRoot } from '../file-system/findRefactorBotPackageRoot';
 import { logger } from '../logger/logger';
+import { line } from '../text/line';
 
-async function findPrettierScriptLocation(opts: { location: string }) {
+export async function findPrettierScriptLocation(opts: { location: string }) {
     const { location } = opts;
 
     const npmPackage = join(location, 'node_modules', 'prettier');
@@ -32,9 +33,15 @@ async function findPrettierScriptLocation(opts: { location: string }) {
 
         return join(npmPackage, pkg.bin);
     } catch (err) {
-        throw new Error(`Could not find prettier package at "${npmPackage}"`, {
-            cause: err,
-        });
+        if (
+            typeof err === 'object' &&
+            err &&
+            'code' in err &&
+            err.code === 'ENOENT'
+        ) {
+            return undefined;
+        }
+        throw err;
     }
 }
 
@@ -47,16 +54,31 @@ const defaultDeps = {
 
 const prettierFormat = async (
     opts: {
-        prettierLocation?: string;
+        prettierScriptLocation?: string;
         repositoryRoot: string;
         input: string;
         filePath: string;
     },
     deps = defaultDeps
 ) => {
-    const scriptLocation = await findPrettierScriptLocation({
-        location: opts.prettierLocation ?? findRefactorBotPackageRoot(),
-    });
+    const scriptLocation = opts.prettierScriptLocation
+        ? opts.prettierScriptLocation
+        : (await findPrettierScriptLocation({
+              location: opts.repositoryRoot,
+          })) ||
+          (await findPrettierScriptLocation({
+              location: findRefactorBotPackageRoot(),
+          }));
+
+    if (!scriptLocation) {
+        throw new Error(
+            line`
+                Cannot find prettier script location in the sandbox repository
+                root "${opts.repositoryRoot}" or in the refactor-bot package
+                root "${findRefactorBotPackageRoot()}
+            `
+        );
+    }
 
     const child = spawn(
         process.execPath,
@@ -97,7 +119,7 @@ const prettierFormat = async (
 
 export async function prettierMarkdown(
     opts: {
-        prettierLocation?: string;
+        prettierScriptLocation?: string;
         repositoryRoot: string;
         filePath?: string;
         md: string;
@@ -107,7 +129,7 @@ export async function prettierMarkdown(
     return await prettierFormat(
         {
             filePath: opts.filePath ?? 'output.md',
-            prettierLocation: opts.prettierLocation,
+            prettierScriptLocation: opts.prettierScriptLocation,
             repositoryRoot: opts.repositoryRoot,
             input: opts.md,
         },
@@ -117,7 +139,7 @@ export async function prettierMarkdown(
 
 export async function prettierTypescript(
     opts: {
-        prettierLocation?: string;
+        prettierScriptLocation?: string;
         repositoryRoot: string;
         filePath?: string;
         ts: string;
@@ -127,7 +149,7 @@ export async function prettierTypescript(
     return await prettierFormat(
         {
             filePath: opts.filePath ?? 'output.ts',
-            prettierLocation: opts.prettierLocation,
+            prettierScriptLocation: opts.prettierScriptLocation,
             repositoryRoot: opts.repositoryRoot,
             input: opts.ts,
         },

diff --git a/packages/refactor-bot/src/refactor/edit.ts b/packages/refactor-bot/src/refactor/edit.ts
@@ -30,6 +30,7 @@ export const editInputSchema = refactorConfigSchema
         fileContents: z.string(),
         sandboxDirectoryPath: z.string(),
         eslintAutoFixScriptArgs: z.array(z.string()).nonempty().optional(),
+        prettierScriptLocation: z.string().optional(),
         choices: z.number().optional(),
     });
 
@@ -166,10 +167,11 @@ export const edit = makeCachedFunction({
                     const codeChunk = codeChunks[0];
 
                     const formattedCodeChunk = await prettierTypescript({
-                        prettierLocation: input.sandboxDirectoryPath,
+                        prettierScriptLocation: input.prettierScriptLocation,
                         repositoryRoot: input.sandboxDirectoryPath,
                         ts: codeChunk,
                     });
+
                     const eslintFixed = input.eslintAutoFixScriptArgs
                         ? (
                               await autoFixIssuesContents(

diff --git a/packages/refactor-bot/src/refactor/planAndRefactor.ts b/packages/refactor-bot/src/refactor/planAndRefactor.ts
@@ -18,6 +18,7 @@ export const planAndRefactorInputSchema = refactorConfigSchema.augment({
     startCommit: z.string(),
     sandboxDirectoryPath: z.string(),
     scripts: z.array(scriptSchema),
+    prettierScriptLocation: z.string().optional(),
     filesToEdit: z.array(z.string()),
 });
 

diff --git a/packages/refactor-bot/src/refactor/prompt.ts b/packages/refactor-bot/src/refactor/prompt.ts
@@ -21,6 +21,7 @@ import { GptRequestError } from '../errors/gptRequestError';
 import { OutOfContextBoundsError } from '../errors/outOfContextBoundsError';
 import { executeFunction } from '../functions/executeFunction';
 import { includeFunctions } from '../functions/includeFunctions';
+import { sanitizeFunctionResult } from '../functions/sanitizeFunctionResult';
 import { functionsConfigSchema } from '../functions/types';
 import { ensureHasOneElement } from '../utils/hasOne';
 import { isTruthy } from '../utils/isTruthy';
@@ -171,7 +172,10 @@ const exec = makeCachedFunction({
             return {
                 message: {
                     role: 'system' as const,
-                    content: e instanceof Error ? e.message : String(e),
+                    content: await sanitizeFunctionResult({
+                        result: e instanceof Error ? e.message : String(e),
+                        config: functionsConfig,
+                    }),
                 },
             };
         }

diff --git a/packages/refactor-bot/src/refactor/refactorBatch.ts b/packages/refactor-bot/src/refactor/refactorBatch.ts
@@ -43,6 +43,7 @@ export const refactorBatchInputSchema = refactorConfigSchema
             )
             .optional(),
         scripts: z.array(scriptSchema),
+        prettierScriptLocation: z.string().optional(),
     });
 
 export const refactorBatch = async (

diff --git a/packages/refactor-bot/src/refactor/refactorFile.ts b/packages/refactor-bot/src/refactor/refactorFile.ts
@@ -48,6 +48,7 @@ export const refactorFileInputSchema = refactorConfigSchema
         startCommit: z.string(),
         sandboxDirectoryPath: z.string(),
         scripts: z.array(scriptSchema),
+        prettierScriptLocation: z.string().optional(),
     })
     .transform(async (input) => ({
         ...input,
@@ -202,6 +203,7 @@ export const refactorFile = makeCachedFunction({
             eslintAutoFixScriptArgs: input.scripts.find((script) =>
                 script.args.includes('eslint')
             )?.args,
+            prettierScriptLocation: input.prettierScriptLocation,
         };
 
         const advice: string[] = [];

diff --git a/packages/refactor-bot/src/refactor/refactorGoal.ts b/packages/refactor-bot/src/refactor/refactorGoal.ts
@@ -1,11 +1,13 @@
 import { z } from 'zod';
 
 import { ConfigurationError } from '../errors/configurationError';
+import { findRefactorBotPackageRoot } from '../file-system/findRefactorBotPackageRoot';
 import { gitResetHard } from '../git/gitResetHard';
 import { gitRevParse } from '../git/gitRevParse';
 import { gitStatus } from '../git/gitStatus';
 import { logger } from '../logger/logger';
 import { determinePackageManager } from '../package-manager/determinePackageManager';
+import { findPrettierScriptLocation } from '../prettier/prettier';
 import { line } from '../text/line';
 import { check, checkScriptsFromConfig } from './check';
 import { discoverCheckDependencies } from './discoverDependencies';
@@ -38,6 +40,31 @@ export const refactorGoal = async (
         );
     }
 
+    let prettierScriptLocation = await findPrettierScriptLocation({
+        location: input.sandboxDirectoryPath,
+    });
+    if (!prettierScriptLocation) {
+        prettierScriptLocation = await findPrettierScriptLocation({
+            location: findRefactorBotPackageRoot(),
+        });
+        if (!prettierScriptLocation) {
+            throw new Error(line`
+                Cannot find prettier script location, this might mean the
+                dependencies are not installed
+            `);
+        }
+        logger.warn(
+            line`
+                Cannot find prettier script location in the sandbox repository
+                root "${input.sandboxDirectoryPath}" - this means that we might
+                use a different version of prettier than the one used in the
+                sandbox repository. This can lead to unexpected formatting
+                changes. To fix this, please add prettier to the repository
+                dependencies before using the refactor-bot.
+            `
+        );
+    }
+
     const scripts = checkScriptsFromConfig(input, checks);
 
     const currentCommit = await gitRevParse({
@@ -90,6 +117,7 @@ export const refactorGoal = async (
     const result = await planAndRefactor({
         ...input,
         scripts,
+        prettierScriptLocation,
     });
 
     await resetToLastAcceptedCommit({