From c2d51aa1a74846c35aca932374c28b47068fb7b5 Mon Sep 17 00:00:00 2001 From: shihanwan Date: Thu, 2 May 2024 13:53:17 -0400 Subject: [PATCH] remove multi line split because it doesn't work welll --- packages/bumpgen-core/src/index.ts | 21 +-- .../src/services/matching/index.test.ts | 112 +++++----------- .../src/services/matching/index.ts | 126 ++++++++++-------- 3 files changed, 104 insertions(+), 155 deletions(-) diff --git a/packages/bumpgen-core/src/index.ts b/packages/bumpgen-core/src/index.ts index d4d4d8c..de4c747 100644 --- a/packages/bumpgen-core/src/index.ts +++ b/packages/bumpgen-core/src/index.ts @@ -298,22 +298,11 @@ const _bumpgen = ({ let fileContents = await services.filesystem.read(planNode.path); for (const replacement of replacements) { - // this accounts for interline replacements but not formatting - let newFileContents = fileContents.replace( - replacement.oldCode.trim(), - replacement.newCode, - ); - - // this accounts for formatting but not interline replacements - if (newFileContents === fileContents) { - newFileContents = services.matching.replacements.fuzzy({ - content: fileContents, - oldCode: replacement.oldCode, - newCode: replacement.newCode, - }); - } - - fileContents = newFileContents; + fileContents = services.matching.replacements.fuzzy({ + content: fileContents, + oldCode: replacement.oldCode, + newCode: replacement.newCode, + }); } const originalSignature = planNode.typeSignature; diff --git a/packages/bumpgen-core/src/services/matching/index.test.ts b/packages/bumpgen-core/src/services/matching/index.test.ts index 25879c8..2d65d8b 100644 --- a/packages/bumpgen-core/src/services/matching/index.test.ts +++ b/packages/bumpgen-core/src/services/matching/index.test.ts @@ -2,10 +2,9 @@ import fs from "fs/promises"; import path from "path"; import { - findSequentialMatchedLinesIndices, + findMatchedBlockIndices, formatNewCode, - searchAndReplace, - splitMultiImportOldCode, + advancedSearchAndReplace, } from "."; const getFileBefore = async (filename: string) => { @@ -24,88 +23,19 @@ const getFileAfter = async (filename: string) => { ).toString(); }; -describe("searchAndReplace", () => { +describe("advancedSearchAndReplace", () => { it("searchAndReplace glob.txt", async () => { const fileBefore = await getFileBefore("glob.txt"); const fileAfter = await getFileAfter("glob.txt"); - const oldCode = "import * as rawGlob from 'glob';"; + const oldCode = "import * as rawGlob from 'glob'"; const newCode = "import { glob as rawGlob } from 'glob';"; - const result = searchAndReplace(fileBefore, oldCode, newCode); - + const result = advancedSearchAndReplace(fileBefore, oldCode, newCode); expect(result).toBe(fileAfter); }); }); -describe("splitMultiImportOldCode", () => { - it("should split cold code with imports", () => { - const oldCode = - "import * as rawGlob from 'glob';\n\n\nconst glob = promisify(rawGlob);"; - - const result = splitMultiImportOldCode(oldCode); - - expect(result).toEqual([ - "import * as rawGlob from 'glob';", - "const glob = promisify(rawGlob);", - ]); - }); - - it("should split cold code with complex imports", () => { - const oldCode =`import * as Sentry from "@sentry/react"; - const module = await import('my-module'); - import 'my-module'; - import Tool from "openai" - import * as X from "openai"; - - import { - ScopeContext, - ScopeContext2, - ScopeContext3, - } from "@sentry/types/types/scope"; - import packageInfo from "../package.json"; - import { Injectable, Inject } from '@graphql-modules/di'; - const hello = require('./hello'); - import myDefault, { myExport1, myExport2 } from 'my-module'; - import express = require('express'); - import x = require('x').default - import { myExport as myRenamedExport } from 'my-module'; - - import type { MyType } from 'my-module'; - - interface BoostProps { - boost: BoostModel; - }`; - - const result = splitMultiImportOldCode(oldCode); - const expectedResult = [ - `import * as Sentry from "@sentry/react";`, - `import 'my-module';`, - `import Tool from "openai"`, - `import * as X from "openai";`, - `import { - ScopeContext, - ScopeContext2, - ScopeContext3, - } from "@sentry/types/types/scope";`, - `import packageInfo from "../package.json";`, - `import { Injectable, Inject } from '@graphql-modules/di';`, - `import myDefault, { myExport1, myExport2 } from 'my-module';`, - `import express = require('express');`, - `import x = require('x').default`, - `import { myExport as myRenamedExport } from 'my-module';`, - `import type { MyType } from 'my-module';`, - `const hello = require('./hello');`, - `const module = await import('my-module');`, - `interface BoostProps { - boost: BoostModel; - }` - ]; - - expectedResult.forEach(line => expect(result).toContain(line)); - }); -}); - describe("formatNewCode", () => { it("should format replacement code correctly for missing indents", () => { const line = " import * as Sentry from '@sentry/line';"; @@ -135,7 +65,7 @@ describe("formatNewCode", () => { }); }); -describe("findSequentialMatchedLinesIndices", () => { +describe("findMatchedBlockIndices", () => { it("should find the right indices for the block of code to replace", () => { const matchedIndices = [ [1, 5, 6], @@ -143,31 +73,51 @@ describe("findSequentialMatchedLinesIndices", () => { [0, 3], ]; - const result = findSequentialMatchedLinesIndices(matchedIndices); + const result = findMatchedBlockIndices(matchedIndices); expect(result).toEqual({"startIndex": 1, "endIndex": 3}); }); - it("should return -1 indices if nothing found", () => { + it("should return -1 indices if no matching block found", () => { const matchedIndices = [ [1, 5, 6], [3, 9, 16], [0, 3], ]; - const result = findSequentialMatchedLinesIndices(matchedIndices); + const result = findMatchedBlockIndices(matchedIndices); expect(result).toEqual({"startIndex": -1, "endIndex": -1}); }); - it("should handle empty lists", () => { + it("should handle empty lists edge case", () => { const matchedIndices = [ [1, 7], [], [2, 3, 4], ]; - const result = findSequentialMatchedLinesIndices(matchedIndices); + const result = findMatchedBlockIndices(matchedIndices); + + expect(result).toEqual({"startIndex": -1, "endIndex": -1}); + }); + + it("should handle only one single line", () => { + const matchedIndices = [ + [1, 7] + ]; + + const result = findMatchedBlockIndices(matchedIndices); + + expect(result).toEqual({"startIndex": 1, "endIndex": 1}); + }); + + it("should handle no matched indices", () => { + const matchedIndices = [ + [] + ]; + + const result = findMatchedBlockIndices(matchedIndices); expect(result).toEqual({"startIndex": -1, "endIndex": -1}); }); diff --git a/packages/bumpgen-core/src/services/matching/index.ts b/packages/bumpgen-core/src/services/matching/index.ts index a9a101b..8bdf91a 100644 --- a/packages/bumpgen-core/src/services/matching/index.ts +++ b/packages/bumpgen-core/src/services/matching/index.ts @@ -38,23 +38,61 @@ export const formatNewCode = (firstMatchedLine: string, newCode: string) => { return formattedCode; }; -const getAllCombinations = (allRefIndexes: number[][]) => { - if (allRefIndexes.length === 1 && allRefIndexes[0]) { - return allRefIndexes[0].map(element => [element]); - } +export const findMatchedBlockIndices = (allRefIndexes: number[][]) => { + const getAllCombinations = (allRefIndexes: number[][]) => { + if (allRefIndexes.length === 1 && allRefIndexes[0]) { + return allRefIndexes[0].map(element => [element]); + } - const restCombinations = getAllCombinations(allRefIndexes.slice(1)); - const allCombinations: number[][] = []; + const restCombinations = getAllCombinations(allRefIndexes.slice(1)); + const allCombinations: number[][] = []; - if (allRefIndexes[0]) { - allRefIndexes[0].forEach(element => { - restCombinations.forEach(combination => { - allCombinations.push([element, ...combination]); + if (allRefIndexes[0]) { + allRefIndexes[0].forEach(element => { + restCombinations.forEach(combination => { + allCombinations.push([element, ...combination]); + }); }); - }); - } + } + + return allCombinations; + }; + + const isSequential = (combination: number[]): boolean => { + if (combination.length === 1) return true; + + for (let i = 0; i < combination.length - 1; i++) { + const current = combination[i]; + const next = combination[i + 1]; + + if (current === undefined || next === undefined) { + continue; + } + + if (next !== current + 1 && current !== -1 && next !== -1) { + return false; + } + } + return true; + }; + + const allCombinations = getAllCombinations(allRefIndexes); + console.log(allCombinations); + + let indices = { startIndex: -1, endIndex: -1 }; - return allCombinations; + for (const combination of allCombinations) { + if (isSequential(combination)) { + const startIndex = combination[0]; + const endIndex = combination[combination.length - 1]; + + if (combination.length > 0 && startIndex && endIndex) { + indices = { startIndex: startIndex, endIndex: endIndex }; + break; + } + }}; + + return indices; }; export const findSequentialMatchedLinesIndices = ( @@ -75,7 +113,6 @@ export const findSequentialMatchedLinesIndices = ( } return true; }; - // recursion black magic const getAllCombinations = ( @@ -134,37 +171,7 @@ export const findSequentialMatchedLinesIndices = ( return getAllCombinations(0, [], { startIndex: -1, endIndex: -1 }); }; -export const splitMultiImportOldCode = (code: string): string[] => { - const regexes = [ - /import\s+([a-zA-Z_$][0-9a-zA-Z_$]*)\s*=\s*require\(['"]([^'"]+)['"]\)(\.[a-zA-Z_$][0-9a-zA-Z_$]*)?\s*(;|\n|$)/g, - /import\s+(['"]([^'"]+)['"]|[\s\S]+?from\s+['"]([^'"]+)['"])\s*(;|\n|$)/g, - /const\s+[a-zA-Z_$][0-9a-zA-Z_$]*\s*=\s*(await\s+)?(require\(['"][^'"]+['"]\)|import\(['"][^'"]+['"]\))\s*(;|\n|$)/g, - ]; - - const imports: string[] = []; - let codeWithoutImports = code.trim(); - - console.log("=== multi import split"); - for (const regex of regexes) { - let match: RegExpExecArray | null; - while ((match = regex.exec(codeWithoutImports)) !== null) { - imports.push(match[0].trim()); - console.log("line:", match[0]); - } - codeWithoutImports = codeWithoutImports.replace(regex, "").trim(); - } - console.log("==="); - - const remainingCodeSections: string[] = []; - - if (codeWithoutImports.length > 0) { - remainingCodeSections.push(codeWithoutImports); - } - - return [...imports, ...remainingCodeSections]; -}; - -export const searchAndReplace = ( +export const advancedSearchAndReplace = ( content: string, oldCode: string, newCode: string, @@ -200,8 +207,7 @@ export const searchAndReplace = ( allMatchedLines.push(matchedLines); }); - const { startIndex, endIndex } = - findSequentialMatchedLinesIndices(allMatchedLines); + const { startIndex, endIndex } = findMatchedBlockIndices(allMatchedLines); console.log(`Looking for this code:`); console.log("====="); @@ -229,13 +235,13 @@ export const searchAndReplace = ( console.log(matchedLines); console.log("=== \n"); - // format the replacing code accordingly then search n replace const firstMatchedLine = splitContent[startIndex]; if (firstMatchedLine === undefined) { console.log("This is a big oopsy"); return content; } - + + // format the replacing code accordingly then search n replace const formattedNewCode = formatNewCode(firstMatchedLine, newCode); console.log("=== replacing with this new code") @@ -251,6 +257,15 @@ export const searchAndReplace = ( return updatedContents; }; +const naiveSearchAndReplace = ( + content: string, + oldCode: string, + newCode: string, +): string => { + console.log("naive replacement done"); + return content.replace(oldCode.trim(), newCode); +}; + export const createMatchingService = () => { return { replacements: { @@ -263,17 +278,12 @@ export const createMatchingService = () => { oldCode: string; newCode: string; }) => { - const multiImportOldCode = splitMultiImportOldCode(oldCode); - - if (multiImportOldCode.length > 1) { - multiImportOldCode.forEach((line: string) => { - content = searchAndReplace(content, line, newCode); - }); + if (content.includes(oldCode)) { + return naiveSearchAndReplace(content, oldCode, newCode); } else { - content = searchAndReplace(content, oldCode, newCode); + return advancedSearchAndReplace(content, oldCode, newCode); } - - return content; + }, }, };