From 0b8ecc87e61ef55ae72dae6bd22c011c36abc0b0 Mon Sep 17 00:00:00 2001 From: James Garbutt <43081j@users.noreply.github.com> Date: Sat, 15 Feb 2025 21:24:34 +0000 Subject: [PATCH 1/3] test: migrate `plugin-options-string` tests Copies the tests from prettier. Notable changes: - A failed plugin load throws an uncaught exception before this change. This catches it and logs an error to stderr - A `snapshot-diff` package was being used to create a `git diff` like diff as a jest snapshot. We can pick the unique lines out instead of using this Skipped tests: - `--help {option}` is not supported in prettier CLI - `--config-path` with stdin is not yet supported (see #21) --- src/bin.ts | 10 ++- src/utils.ts | 14 +++- .../plugin-options-string/config.json | 4 ++ .../plugin-options-string/plugin.cjs | 31 ++++++++ .../plugin-options-string.js.snap | 19 +++++ test/__tests__/plugin-options-string.js | 72 +++++++++++++++++++ 6 files changed, 146 insertions(+), 4 deletions(-) create mode 100644 test/__fixtures__/plugin-options-string/config.json create mode 100644 test/__fixtures__/plugin-options-string/plugin.cjs create mode 100644 test/__tests__/__snapshots__/plugin-options-string.js.snap create mode 100644 test/__tests__/plugin-options-string.js diff --git a/src/bin.ts b/src/bin.ts index d566abb..925f237 100644 --- a/src/bin.ts +++ b/src/bin.ts @@ -5,7 +5,7 @@ import { bin, color, exit, parseArgv } from "specialist"; import { PRETTIER_VERSION, DEFAULT_PARSERS } from "./constants.js"; import { getPlugin, isBoolean, isNumber, isIntegerInRange, isString } from "./utils.js"; import { normalizeOptions, normalizeFormatOptions, normalizePluginOptions } from "./utils.js"; -import type { Bin, PluginsOptions } from "./types.js"; +import type { Bin, PluginsOptions, PrettierPlugin } from "./types.js"; const makeBin = (): Bin => { return ( @@ -208,7 +208,13 @@ const makePluggableBin = async (): Promise => { for (let i = 0, l = pluginsNames.length; i < l; i++) { const pluginName = pluginsNames[i]; - const plugin = await getPlugin(pluginName); + let plugin: PrettierPlugin; + + try { + plugin = await getPlugin(pluginName); + } catch { + exit(`The plugin "${pluginName}" could not be loaded`); + } for (const option in plugin.options) { optionsNames.push(option); diff --git a/src/utils.ts b/src/utils.ts index 88d05b2..44563a5 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -146,9 +146,19 @@ function getPluginVersion(name: string): string | null { } } -function getPlugins(names: string[]): PromiseMaybe { +async function getPlugins(names: string[]): Promise { if (!names.length) return []; - return Promise.all(names.map(getPlugin)); + return ( + await Promise.all( + names.map(async (name) => { + try { + return await getPlugin(name); + } catch { + return null; + } + }), + ) + ).filter((plugin): plugin is PrettierPlugin => plugin !== null); } const getPluginsBuiltin = once(async (): Promise => { diff --git a/test/__fixtures__/plugin-options-string/config.json b/test/__fixtures__/plugin-options-string/config.json new file mode 100644 index 0000000..a1d5192 --- /dev/null +++ b/test/__fixtures__/plugin-options-string/config.json @@ -0,0 +1,4 @@ +{ + "plugins": ["./plugin.cjs"], + "fooString": "baz" +} diff --git a/test/__fixtures__/plugin-options-string/plugin.cjs b/test/__fixtures__/plugin-options-string/plugin.cjs new file mode 100644 index 0000000..85f5cc5 --- /dev/null +++ b/test/__fixtures__/plugin-options-string/plugin.cjs @@ -0,0 +1,31 @@ +"use strict"; + +module.exports = { + languages: [ + { + name: "foo", + parsers: ["foo-parser"], + extensions: [".foo"], + since: "1.0.0", + }, + ], + options: { + fooString: { + type: "string", + default: "bar", + description: "foo description", + }, + }, + parsers: { + "foo-parser": { + parse: (text) => ({ text }), + astFormat: "foo-ast", + }, + }, + printers: { + "foo-ast": { + print: (path, options) => + options.fooString ? `foo:${options.fooString}` : path.getValue().text, + }, + }, +}; diff --git a/test/__tests__/__snapshots__/plugin-options-string.js.snap b/test/__tests__/__snapshots__/plugin-options-string.js.snap new file mode 100644 index 0000000..5a47c59 --- /dev/null +++ b/test/__tests__/__snapshots__/plugin-options-string.js.snap @@ -0,0 +1,19 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`show detailed external option with \`--help foo-string\` (stderr) 1`] = `""`; + +exports[`show detailed external option with \`--help foo-string\` (stdout) 1`] = ` +"--foo-string + + foo description + +Default: bar" +`; + +exports[`show detailed external option with \`--help foo-string\` (write) 1`] = `[]`; + +exports[`show external options with \`--help\` 1`] = ` +" --parser + --foo-string foo description + Defaults to "bar"" +`; diff --git a/test/__tests__/plugin-options-string.js b/test/__tests__/plugin-options-string.js new file mode 100644 index 0000000..b2dd2a3 --- /dev/null +++ b/test/__tests__/plugin-options-string.js @@ -0,0 +1,72 @@ +import { runCli } from "../utils"; + +test("show external options with `--help`", async () => { + const originalStdout = await runCli("plugin-options-string", ["--help"]) + .stdout; + const pluggedStdout = await runCli("plugin-options-string", [ + "--help", + "--plugin=./plugin.cjs", + ]).stdout; + const originalLines = originalStdout.split("\n"); + const pluggedLines = pluggedStdout.split("\n"); + const differentLines = pluggedLines.filter((line) => + !originalLines.includes(line)); + expect(differentLines.join("\n")).toMatchSnapshot(); +}); + +// Note (43081j); we don't currently support `--help {optionName}` +describe.skip("show detailed external option with `--help foo-string`", () => { + runCli("plugin-options-string", [ + "--plugin=./plugin.cjs", + "--help", + "foo-string", + ]).test({ + status: 0, + }); +}); + +describe("external options from CLI should work", () => { + runCli( + "plugin-options-string", + [ + "--plugin=./plugin.cjs", + "--stdin-filepath", + "example.foo", + "--foo-string", + "baz", + ], + { input: "hello-world" }, + ).test({ + stdout: "foo:baz", + stderr: "", + status: 0, + write: [], + }); +}); + +// TODO (43081j): this won't work until we fix #21 +describe.skip("external options from config file should work", () => { + runCli( + "plugin-options-string", + ["--config-path=./config.json", "--stdin-filepath", "example.foo"], + { input: "hello-world" }, + ).test({ + stdout: "foo:baz", + stderr: "", + status: 0, + write: [], + }); +}); + +describe("Non exists plugin", () => { + runCli( + "plugin-options-string", + ["--plugin=--foo--", "--stdin-filepath", "example.foo"], + { input: "hello-world" }, + ).test({ + stdout: "", + stderr: expect.stringMatching(/The plugin "--foo--" could not be loaded/u), + status: 1, + write: [], + }); +}); From c4f61666483fcb216c8f6868baf8a4eeff0a0cfb Mon Sep 17 00:00:00 2001 From: James Garbutt <43081j@users.noreply.github.com> Date: Sun, 23 Feb 2025 17:51:17 +0000 Subject: [PATCH 2/3] chore: add getPluginOrExit helper --- src/bin.ts | 10 ++-------- src/utils.ts | 9 +++++++++ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/bin.ts b/src/bin.ts index 925f237..e74cf4a 100644 --- a/src/bin.ts +++ b/src/bin.ts @@ -3,7 +3,7 @@ import { toKebabCase } from "kasi"; import { bin, color, exit, parseArgv } from "specialist"; import { PRETTIER_VERSION, DEFAULT_PARSERS } from "./constants.js"; -import { getPlugin, isBoolean, isNumber, isIntegerInRange, isString } from "./utils.js"; +import { getPluginOrExit, isBoolean, isNumber, isIntegerInRange, isString } from "./utils.js"; import { normalizeOptions, normalizeFormatOptions, normalizePluginOptions } from "./utils.js"; import type { Bin, PluginsOptions, PrettierPlugin } from "./types.js"; @@ -208,13 +208,7 @@ const makePluggableBin = async (): Promise => { for (let i = 0, l = pluginsNames.length; i < l; i++) { const pluginName = pluginsNames[i]; - let plugin: PrettierPlugin; - - try { - plugin = await getPlugin(pluginName); - } catch { - exit(`The plugin "${pluginName}" could not be loaded`); - } + const plugin = await getPluginOrExit(pluginName); for (const option in plugin.options) { optionsNames.push(option); diff --git a/src/utils.ts b/src/utils.ts index 44563a5..fe00d6d 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -129,6 +129,14 @@ const getPlugin = memoize((name: string): Promise => { return plugin; }); +async function getPluginOrExit(name: string): Promise { + try { + return await getPlugin(name); + } catch { + exit(`The plugin "${name}" could not be loaded`); + } +} + function getPluginPath(name: string): string { const rootPath = path.join(process.cwd(), "index.js"); const pluginPath = getModulePath(name, rootPath); @@ -730,6 +738,7 @@ export { getModule, getModulePath, getPlugin, + getPluginOrExit, getPluginPath, getPluginVersion, getPlugins, From 9011cb7e47ec138c91fcdee4ff2fde21fc250b0b Mon Sep 17 00:00:00 2001 From: James Garbutt <43081j@users.noreply.github.com> Date: Mon, 24 Feb 2025 11:16:40 +0000 Subject: [PATCH 3/3] chore: exit if getPlugins fails --- src/prettier_serial.ts | 4 ++-- src/utils.ts | 20 ++++++++++++-------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/prettier_serial.ts b/src/prettier_serial.ts index 685fa4d..2a1f05f 100644 --- a/src/prettier_serial.ts +++ b/src/prettier_serial.ts @@ -1,7 +1,7 @@ import { readFile, writeFile } from "atomically"; import process from "node:process"; import prettier from "prettier/standalone"; -import { getPlugins, getPluginsBuiltin, resolve } from "./utils.js"; +import { getPluginsOrExit, getPluginsBuiltin, resolve } from "./utils.js"; import type { ContextOptions, LazyFormatOptions, PluginsOptions } from "./types.js"; async function check( @@ -37,7 +37,7 @@ async function format( ): Promise { formatOptions = await resolve(formatOptions); const pluginsBuiltin = await getPluginsBuiltin(); - const plugins = await getPlugins(formatOptions.plugins || []); + const plugins = await getPluginsOrExit(formatOptions.plugins || []); const pluginsOverride = contextOptions.configPrecedence !== "file-override"; const options = { diff --git a/src/utils.ts b/src/utils.ts index fe00d6d..40dc72a 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -158,15 +158,18 @@ async function getPlugins(names: string[]): Promise { if (!names.length) return []; return ( await Promise.all( - names.map(async (name) => { - try { - return await getPlugin(name); - } catch { - return null; - } - }), + names.map((name) => getPlugin(name)) ) - ).filter((plugin): plugin is PrettierPlugin => plugin !== null); + ); +} + +async function getPluginsOrExit(names: string[]): Promise { + if (!names.length) return []; + return ( + await Promise.all( + names.map((name) => getPluginOrExit(name)) + ) + ); } const getPluginsBuiltin = once(async (): Promise => { @@ -743,6 +746,7 @@ export { getPluginVersion, getPlugins, getPluginsBuiltin, + getPluginsOrExit, getPluginsPaths, getPluginsVersions, getProjectPath,