From 4e0f93d3933230fb27bac498b9db8a71d95fce30 Mon Sep 17 00:00:00 2001 From: Alexander Lyon Date: Fri, 2 Jun 2023 10:34:35 +0100 Subject: [PATCH 1/4] feat: add basic codemod for common glob syntax issues --- .../clean-globs/clean-globs/package.json | 1 + .../clean-globs/clean-globs/turbo.json | 27 ++++ .../__tests__/clean-globs.test.ts | 133 ++++++++++++++++++ .../src/transforms/clean-globs.ts | 106 ++++++++++++++ 4 files changed, 267 insertions(+) create mode 100644 packages/turbo-codemod/__tests__/__fixtures__/clean-globs/clean-globs/package.json create mode 100644 packages/turbo-codemod/__tests__/__fixtures__/clean-globs/clean-globs/turbo.json create mode 100644 packages/turbo-codemod/__tests__/clean-globs.test.ts create mode 100644 packages/turbo-codemod/src/transforms/clean-globs.ts diff --git a/packages/turbo-codemod/__tests__/__fixtures__/clean-globs/clean-globs/package.json b/packages/turbo-codemod/__tests__/__fixtures__/clean-globs/clean-globs/package.json new file mode 100644 index 0000000000000..0967ef424bce6 --- /dev/null +++ b/packages/turbo-codemod/__tests__/__fixtures__/clean-globs/clean-globs/package.json @@ -0,0 +1 @@ +{} diff --git a/packages/turbo-codemod/__tests__/__fixtures__/clean-globs/clean-globs/turbo.json b/packages/turbo-codemod/__tests__/__fixtures__/clean-globs/clean-globs/turbo.json new file mode 100644 index 0000000000000..c879014c37587 --- /dev/null +++ b/packages/turbo-codemod/__tests__/__fixtures__/clean-globs/clean-globs/turbo.json @@ -0,0 +1,27 @@ +{ + "$schema": "../../../../../../docs/public/schema.json", + "pipeline": { + "case_1": { + "inputs": ["../../app-store/**/**", "**/**/result.json"], + "outputs": ["../../app-store/**/**", "**/**/result.json"] + }, + "case_2": { + "inputs": ["!**/dist", "!**/node_modules"], + "outputs": ["!**/dist", "!**/node_modules"] + }, + "case_3": { + "inputs": [ + "cypress/integration/**.test.ts", + "src/types/generated/**.ts", + "scripts/**.mjs", + "scripts/**.js" + ], + "outputs": [ + "cypress/integration/**.test.ts", + "src/types/generated/**.ts", + "scripts/**.mjs", + "scripts/**.js" + ] + } + } +} diff --git a/packages/turbo-codemod/__tests__/clean-globs.test.ts b/packages/turbo-codemod/__tests__/clean-globs.test.ts new file mode 100644 index 0000000000000..186a1e9f4c9de --- /dev/null +++ b/packages/turbo-codemod/__tests__/clean-globs.test.ts @@ -0,0 +1,133 @@ +import { transformer, fixGlobPattern } from "../src/transforms/clean-globs"; +import { setupTestFixtures } from "@turbo/test-utils"; +import getTransformerHelpers from "../src/utils/getTransformerHelpers"; + +describe("clean-globs", () => { + const { useFixture } = setupTestFixtures({ + directory: __dirname, + test: "clean-globs", + }); + + test("basic", () => { + // load the fixture for the test + const { root, read, readJson } = useFixture({ + fixture: "clean-globs", + }); + + // run the transformer + const result = transformer({ + root, + options: { force: false, dry: false, print: false }, + }); + + // result should be correct + expect(result.fatalError).toBeUndefined(); + expect(result.changes).toMatchInlineSnapshot(` + Object { + "turbo.json": Object { + "action": "modified", + "additions": 6, + "deletions": 6, + }, + } + `); + }); + + test("collapses back-to-back doublestars", () => { + let badGlobPatterns = [ + ["../../app-store/**/**", "../../app-store/**"], + ["**/**/result.json", "**/result.json"], + ["**/**/**/**", "**"], + ["**/foo/**/**/bar/**", "**/foo/**/bar/**"], + ["**/foo/**/**/**/bar/**/**", "**/foo/**/bar/**"], + ["**/foo/**/**/**/**/bar/**/**/**", "**/foo/**/bar/**"], + ]; + + const { log } = getTransformerHelpers({ + transformer: "test", + rootPath: ".", + options: { force: false, dry: false, print: false }, + }); + + // Now let's test the function + badGlobPatterns.forEach(([input, output]) => { + expect(fixGlobPattern(input, log)).toBe(output); + }); + }); + + test("doesn't update valid globs and prints a message", () => { + const { log } = getTransformerHelpers({ + transformer: "test", + rootPath: ".", + options: { force: false, dry: false, print: false }, + }); + + // Now let's test the function + expect(fixGlobPattern("a/b/c/*", log)).toBe("a/b/c/*"); + }); + + test("transforms '!**/folder' to '**/[!folder]'", () => { + let badGlobPatterns = [ + ["!**/dist", "**/[!dist]"], + ["!**/node_modules", "**/[!node_modules]"], + ["!**/foo/bar", "**/[!foo/bar]"], + ["!**/foo/bar/baz", "**/[!foo/bar/baz]"], + ["!**/foo/bar/baz/qux", "**/[!foo/bar/baz/qux]"], + ]; + + const { log } = getTransformerHelpers({ + transformer: "test", + rootPath: ".", + options: { force: false, dry: false, print: false }, + }); + + // Now let's test the function + badGlobPatterns.forEach(([input, output]) => { + expect(fixGlobPattern(input, log)).toBe(output); + }); + }); + + test("transforms '**ext' to '**/*ext'", () => { + let badGlobPatterns = [ + ["cypress/integration/**.test.ts", "cypress/integration/**/*.test.ts"], + ["scripts/**.mjs", "scripts/**/*.mjs"], + ["scripts/**.js", "scripts/**/*.js"], + ["src/types/generated/**.ts", "src/types/generated/**/*.ts"], + ["**md", "**/*md"], + ["**txt", "**/*txt"], + ["**html", "**/*html"], + ]; + + const { log } = getTransformerHelpers({ + transformer: "test", + rootPath: ".", + options: { force: false, dry: false, print: false }, + }); + + // Now let's test the function + badGlobPatterns.forEach(([input, output]) => { + expect(fixGlobPattern(input, log)).toBe(output); + }); + }); + + test("transforms 'pre**' to pre*/**", () => { + let badGlobPatterns = [ + ["pre**", "pre*/**"], + ["pre**/foo", "pre*/**/foo"], + ["pre**/foo/bar", "pre*/**/foo/bar"], + ["pre**/foo/bar/baz", "pre*/**/foo/bar/baz"], + ["pre**/foo/bar/baz/qux", "pre*/**/foo/bar/baz/qux"], + ]; + + const { log } = getTransformerHelpers({ + transformer: "test", + rootPath: ".", + options: { force: false, dry: false, print: false }, + }); + + // Now let's test the function + badGlobPatterns.forEach(([input, output]) => { + expect(fixGlobPattern(input, log)).toBe(output); + }); + }); +}); diff --git a/packages/turbo-codemod/src/transforms/clean-globs.ts b/packages/turbo-codemod/src/transforms/clean-globs.ts new file mode 100644 index 0000000000000..03441a8770725 --- /dev/null +++ b/packages/turbo-codemod/src/transforms/clean-globs.ts @@ -0,0 +1,106 @@ +import { TransformerArgs } from "../types"; +import type { Schema as TurboJsonSchema } from "@turbo/types"; +import { TransformerResults } from "../runner"; +import path from "path"; +import fs from "fs-extra"; +import getTransformerHelpers from "../utils/getTransformerHelpers"; +import { getTurboConfigs } from "@turbo/utils"; +import Logger from "../utils/logger"; + +// transformer details +const TRANSFORMER = "clean-globs"; +const DESCRIPTION = + "Automatically clean up invalid globs from your `turbo.json` file"; +const INTRODUCED_IN = "1.11.0"; + +export function transformer({ + root, + options, +}: TransformerArgs): TransformerResults { + const { log, runner } = getTransformerHelpers({ + transformer: TRANSFORMER, + rootPath: root, + options, + }); + + const turboConfigPath = path.join(root, "turbo.json"); + + const turboJson: TurboJsonSchema = fs.readJsonSync(turboConfigPath); + runner.modifyFile({ + filePath: turboConfigPath, + after: migrateConfig(turboJson, log), + }); + + // find and migrate any workspace configs + const workspaceConfigs = getTurboConfigs(root); + workspaceConfigs.forEach((workspaceConfig) => { + const { config, turboConfigPath } = workspaceConfig; + runner.modifyFile({ + filePath: turboConfigPath, + after: migrateConfig(config, log), + }); + }); + + return runner.finish(); +} + +function migrateConfig(config: TurboJsonSchema, log: Logger) { + const mapGlob = (glob: string) => fixGlobPattern(glob, log); + for (const [_, taskDef] of Object.entries(config.pipeline)) { + taskDef.inputs = taskDef.inputs?.map(mapGlob); + taskDef.outputs = taskDef.outputs?.map(mapGlob); + } + + return config; +} + +export function fixGlobPattern(pattern: string, log: Logger): string { + let oldPattern = pattern; + // For '../../app-store/**/**' and '**/**/result.json' + // Collapse back-to-back doublestars '**/**' to a single doublestar '**' + let newPattern = pattern.replace(/\*\*\/\*\*/g, "**"); + while (newPattern !== pattern) { + log.modified(`${pattern} to ${newPattern}`); + pattern = newPattern; + newPattern = pattern.replace(/\*\*\/\*\*/g, "**"); + } + + // For '!**/dist' and '!**/node_modules' + // Change '!**/dist' and '!**/node_modules' to '**/[!dist]' and '**/[!node_modules]' + newPattern = pattern.replace(/^!\*\*\/([a-z_\/]+)$/g, "**/[!$1]"); + if (newPattern !== pattern) { + log.modified(`${pattern} to ${newPattern}`); + log.info("please make the previous transform is correct"); + pattern = newPattern; + } + + // For 'cypress/integration/**.test.ts', 'scripts/**.mjs', 'scripts/**.js', 'src/types/generated/**.ts' + // Change '**.ext' to '**/*.ext' where 'ext' is 'test.ts', 'mjs', 'js', 'ts' + newPattern = pattern.replace(/(\*\*)([a-z\.]+)/g, "$1/*$2"); + if (newPattern !== pattern) { + log.modified(`${pattern} to ${newPattern}`); + pattern = newPattern; + } + + // For 'test/prefix**' change to 'test/prefix*/**' + newPattern = pattern.replace(/([a-z_]+)(\*\*)/g, "$1*/$2"); + if (newPattern !== pattern) { + log.modified(`${pattern} to ${newPattern}`); + pattern = newPattern; + } + + if (oldPattern === pattern) { + log.unchanged(pattern); + } + + return pattern; +} + +const transformerMeta = { + name: `${TRANSFORMER}: ${DESCRIPTION}`, + value: TRANSFORMER, + introducedIn: INTRODUCED_IN, + transformer, +}; + +export default transformerMeta; From 7e90a805970b5f4bf6e29f942788951cfc4f5124 Mon Sep 17 00:00:00 2001 From: Alexander Lyon Date: Sat, 3 Jun 2023 05:34:21 +0100 Subject: [PATCH 2/4] remove bogus behaviour regarding ! --- .../__tests__/clean-globs.test.ts | 21 ------------------- .../src/transforms/clean-globs.ts | 9 -------- 2 files changed, 30 deletions(-) diff --git a/packages/turbo-codemod/__tests__/clean-globs.test.ts b/packages/turbo-codemod/__tests__/clean-globs.test.ts index 186a1e9f4c9de..c6dcd91d8cdba 100644 --- a/packages/turbo-codemod/__tests__/clean-globs.test.ts +++ b/packages/turbo-codemod/__tests__/clean-globs.test.ts @@ -66,27 +66,6 @@ describe("clean-globs", () => { expect(fixGlobPattern("a/b/c/*", log)).toBe("a/b/c/*"); }); - test("transforms '!**/folder' to '**/[!folder]'", () => { - let badGlobPatterns = [ - ["!**/dist", "**/[!dist]"], - ["!**/node_modules", "**/[!node_modules]"], - ["!**/foo/bar", "**/[!foo/bar]"], - ["!**/foo/bar/baz", "**/[!foo/bar/baz]"], - ["!**/foo/bar/baz/qux", "**/[!foo/bar/baz/qux]"], - ]; - - const { log } = getTransformerHelpers({ - transformer: "test", - rootPath: ".", - options: { force: false, dry: false, print: false }, - }); - - // Now let's test the function - badGlobPatterns.forEach(([input, output]) => { - expect(fixGlobPattern(input, log)).toBe(output); - }); - }); - test("transforms '**ext' to '**/*ext'", () => { let badGlobPatterns = [ ["cypress/integration/**.test.ts", "cypress/integration/**/*.test.ts"], diff --git a/packages/turbo-codemod/src/transforms/clean-globs.ts b/packages/turbo-codemod/src/transforms/clean-globs.ts index 03441a8770725..01f2195982dc0 100644 --- a/packages/turbo-codemod/src/transforms/clean-globs.ts +++ b/packages/turbo-codemod/src/transforms/clean-globs.ts @@ -65,15 +65,6 @@ export function fixGlobPattern(pattern: string, log: Logger): string { newPattern = pattern.replace(/\*\*\/\*\*/g, "**"); } - // For '!**/dist' and '!**/node_modules' - // Change '!**/dist' and '!**/node_modules' to '**/[!dist]' and '**/[!node_modules]' - newPattern = pattern.replace(/^!\*\*\/([a-z_\/]+)$/g, "**/[!$1]"); - if (newPattern !== pattern) { - log.modified(`${pattern} to ${newPattern}`); - log.info("please make the previous transform is correct"); - pattern = newPattern; - } - // For 'cypress/integration/**.test.ts', 'scripts/**.mjs', 'scripts/**.js', 'src/types/generated/**.ts' // Change '**.ext' to '**/*.ext' where 'ext' is 'test.ts', 'mjs', 'js', 'ts' newPattern = pattern.replace(/(\*\*)([a-z\.]+)/g, "$1/*$2"); From 38f666695cca056f458f75e268593804ebf3a5f5 Mon Sep 17 00:00:00 2001 From: Alexander Lyon Date: Sat, 3 Jun 2023 05:44:38 +0100 Subject: [PATCH 3/4] relax the charset of the globs significantly, and add more tests --- .../__tests__/clean-globs.test.ts | 91 ++++++++++++++----- .../src/transforms/clean-globs.ts | 11 ++- 2 files changed, 73 insertions(+), 29 deletions(-) diff --git a/packages/turbo-codemod/__tests__/clean-globs.test.ts b/packages/turbo-codemod/__tests__/clean-globs.test.ts index c6dcd91d8cdba..5c53235bbc604 100644 --- a/packages/turbo-codemod/__tests__/clean-globs.test.ts +++ b/packages/turbo-codemod/__tests__/clean-globs.test.ts @@ -33,6 +33,12 @@ describe("clean-globs", () => { `); }); + const { log } = getTransformerHelpers({ + transformer: "test", + rootPath: ".", + options: { force: false, dry: false, print: false }, + }); + test("collapses back-to-back doublestars", () => { let badGlobPatterns = [ ["../../app-store/**/**", "../../app-store/**"], @@ -43,12 +49,6 @@ describe("clean-globs", () => { ["**/foo/**/**/**/**/bar/**/**/**", "**/foo/**/bar/**"], ]; - const { log } = getTransformerHelpers({ - transformer: "test", - rootPath: ".", - options: { force: false, dry: false, print: false }, - }); - // Now let's test the function badGlobPatterns.forEach(([input, output]) => { expect(fixGlobPattern(input, log)).toBe(output); @@ -56,12 +56,6 @@ describe("clean-globs", () => { }); test("doesn't update valid globs and prints a message", () => { - const { log } = getTransformerHelpers({ - transformer: "test", - rootPath: ".", - options: { force: false, dry: false, print: false }, - }); - // Now let's test the function expect(fixGlobPattern("a/b/c/*", log)).toBe("a/b/c/*"); }); @@ -77,12 +71,6 @@ describe("clean-globs", () => { ["**html", "**/*html"], ]; - const { log } = getTransformerHelpers({ - transformer: "test", - rootPath: ".", - options: { force: false, dry: false, print: false }, - }); - // Now let's test the function badGlobPatterns.forEach(([input, output]) => { expect(fixGlobPattern(input, log)).toBe(output); @@ -98,15 +86,70 @@ describe("clean-globs", () => { ["pre**/foo/bar/baz/qux", "pre*/**/foo/bar/baz/qux"], ]; - const { log } = getTransformerHelpers({ - transformer: "test", - rootPath: ".", - options: { force: false, dry: false, print: false }, - }); - // Now let's test the function badGlobPatterns.forEach(([input, output]) => { expect(fixGlobPattern(input, log)).toBe(output); }); }); + + it("should collapse back-to-back doublestars to a single doublestar", () => { + expect(fixGlobPattern("../../app-store/**/**", log)).toBe( + "../../app-store/**" + ); + expect(fixGlobPattern("**/**/result.json", log)).toBe("**/result.json"); + }); + + it("should change **.ext to **/*.ext", () => { + expect(fixGlobPattern("**.js", log)).toBe("**/*.js"); + expect(fixGlobPattern("**.json", log)).toBe("**/*.json"); + expect(fixGlobPattern("**.ext", log)).toBe("**/*.ext"); + }); + + it("should change prefix** to prefix*/**", () => { + expect(fixGlobPattern("app**", log)).toBe("app*/**"); + expect(fixGlobPattern("src**", log)).toBe("src*/**"); + expect(fixGlobPattern("prefix**", log)).toBe("prefix*/**"); + }); + + it("should collapse back-to-back doublestars and change **.ext to **/*.ext", () => { + expect(fixGlobPattern("../../app-store/**/**/*.js", log)).toBe( + "../../app-store/**/*.js" + ); + expect(fixGlobPattern("**/**/result.json", log)).toBe("**/result.json"); + }); + + it("should collapse back-to-back doublestars and change prefix** to prefix*/**", () => { + expect(fixGlobPattern("../../app-store/**/**prefix**", log)).toBe( + "../../app-store/**/*prefix*/**" + ); + expect(fixGlobPattern("**/**/prefix**", log)).toBe("**/prefix*/**"); + }); + + it("should not modify valid glob patterns", () => { + expect(fixGlobPattern("src/**/*.js", log)).toBe("src/**/*.js"); + expect(fixGlobPattern("src/**/test/*.js", log)).toBe("src/**/test/*.js"); + expect(fixGlobPattern("src/**/test/**/*.js", log)).toBe( + "src/**/test/**/*.js" + ); + expect(fixGlobPattern("src/**/test/**/result.json", log)).toBe( + "src/**/test/**/result.json" + ); + }); + + it("should handle glob patterns with non-ASCII characters", () => { + expect(fixGlobPattern("src/日本語/**/*.js", log)).toBe( + "src/日本語/**/*.js" + ); + expect(fixGlobPattern("src/中文/**/*.json", log)).toBe( + "src/中文/**/*.json" + ); + expect(fixGlobPattern("src/русский/**/*.ts", log)).toBe( + "src/русский/**/*.ts" + ); + }); + it("should handle glob patterns with emojis", () => { + expect(fixGlobPattern("src/👋**/*.js", log)).toBe("src/👋*/**/*.js"); + expect(fixGlobPattern("src/🌎**/*.json", log)).toBe("src/🌎*/**/*.json"); + expect(fixGlobPattern("src/🚀**/*.ts", log)).toBe("src/🚀*/**/*.ts"); + }); }); diff --git a/packages/turbo-codemod/src/transforms/clean-globs.ts b/packages/turbo-codemod/src/transforms/clean-globs.ts index 01f2195982dc0..11fd3dabce98d 100644 --- a/packages/turbo-codemod/src/transforms/clean-globs.ts +++ b/packages/turbo-codemod/src/transforms/clean-globs.ts @@ -65,16 +65,17 @@ export function fixGlobPattern(pattern: string, log: Logger): string { newPattern = pattern.replace(/\*\*\/\*\*/g, "**"); } - // For 'cypress/integration/**.test.ts', 'scripts/**.mjs', 'scripts/**.js', 'src/types/generated/**.ts' - // Change '**.ext' to '**/*.ext' where 'ext' is 'test.ts', 'mjs', 'js', 'ts' - newPattern = pattern.replace(/(\*\*)([a-z\.]+)/g, "$1/*$2"); + // For '**.ext' change to '**/*.ext' + // 'ext' is a filename or extension and can contain almost any character except '*' and '/' + newPattern = pattern.replace(/(\*\*)([^*/]+)/g, "$1/*$2"); if (newPattern !== pattern) { log.modified(`${pattern} to ${newPattern}`); pattern = newPattern; } - // For 'test/prefix**' change to 'test/prefix*/**' - newPattern = pattern.replace(/([a-z_]+)(\*\*)/g, "$1*/$2"); + // For 'prefix**' change to 'prefix*/**' + // 'prefix' is a folder name and can contain almost any character except '*' and '/' + newPattern = pattern.replace(/([^*/]+)(\*\*)/g, "$1*/$2"); if (newPattern !== pattern) { log.modified(`${pattern} to ${newPattern}`); pattern = newPattern; From 5cfe14da5739e39a3ab7ca69054c76c1443f3d06 Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Mon, 5 Jun 2023 10:38:11 -0700 Subject: [PATCH 4/4] remove log statements --- packages/turbo-codemod/src/transforms/clean-globs.ts | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/packages/turbo-codemod/src/transforms/clean-globs.ts b/packages/turbo-codemod/src/transforms/clean-globs.ts index 11fd3dabce98d..93c242f367e76 100644 --- a/packages/turbo-codemod/src/transforms/clean-globs.ts +++ b/packages/turbo-codemod/src/transforms/clean-globs.ts @@ -10,7 +10,7 @@ import Logger from "../utils/logger"; // transformer details const TRANSFORMER = "clean-globs"; const DESCRIPTION = - "Automatically clean up invalid globs from your `turbo.json` file"; + "Automatically clean up invalid globs from your 'turbo.json' file"; const INTRODUCED_IN = "1.11.0"; export function transformer({ @@ -60,7 +60,6 @@ export function fixGlobPattern(pattern: string, log: Logger): string { // Collapse back-to-back doublestars '**/**' to a single doublestar '**' let newPattern = pattern.replace(/\*\*\/\*\*/g, "**"); while (newPattern !== pattern) { - log.modified(`${pattern} to ${newPattern}`); pattern = newPattern; newPattern = pattern.replace(/\*\*\/\*\*/g, "**"); } @@ -69,7 +68,6 @@ export function fixGlobPattern(pattern: string, log: Logger): string { // 'ext' is a filename or extension and can contain almost any character except '*' and '/' newPattern = pattern.replace(/(\*\*)([^*/]+)/g, "$1/*$2"); if (newPattern !== pattern) { - log.modified(`${pattern} to ${newPattern}`); pattern = newPattern; } @@ -77,14 +75,9 @@ export function fixGlobPattern(pattern: string, log: Logger): string { // 'prefix' is a folder name and can contain almost any character except '*' and '/' newPattern = pattern.replace(/([^*/]+)(\*\*)/g, "$1*/$2"); if (newPattern !== pattern) { - log.modified(`${pattern} to ${newPattern}`); pattern = newPattern; } - if (oldPattern === pattern) { - log.unchanged(pattern); - } - return pattern; }