V2: Improve parameter parsing

docs/writing_plugins.md

@@ -369,13 +369,13 @@ automatically generate the correct export for CommonJS.
 
 ### Parsing plugin options
 
-The plugin framework recognizes a set of pre-defined key/value pairs that can be passed to all plugins when executed (i.e. `target`, `keep_empty_files`, etc.), but if your plugin needs to be passed additional parameters, you can specify a `parseOption` function as part of your plugin initialization.  
+The plugin framework recognizes a set of pre-defined key/value pairs that can be passed to all plugins when executed (i.e. `target`, `keep_empty_files`, etc.), but if your plugin needs to be passed additional parameters, you can specify a `parseOptions` function as part of your plugin initialization.  
 
 ```ts
-parseOption(key: string, value: string | undefined): void;
+parseOptions(rawOptions: {key: string, value: string}[]): T;
 ```
 
-This function will be invoked by the framework, passing in any key/value pairs that it does not recognize from its pre-defined list.
+This function will be invoked by the framework, passing in any key/value pairs that it does not recognize from its pre-defined list. The returned option will be merged with the pre-defined options and passed to the generate functions along via the `options` property of the schema.
 
 ### Using custom Protobuf options
 

packages/protoplugin-test/src/file-preamble.test.ts

@@ -187,8 +187,9 @@ describe("GeneratedFile.preamble", () => {
           "const placeholder = 1; // ensure file is not considered empty",
         );
       },
-      parseOption() {
+      parseOptions() {
         // accept all options
+        return {};
       },
       returnLinesOfFirstFile: true,
     });

packages/protoplugin-test/src/helpers.ts

@@ -34,7 +34,9 @@ import assert from "node:assert";
 
 let upstreamProtobuf: UpstreamProtobuf | undefined;
 
-type PluginInit = Parameters<typeof createEcmaScriptPlugin>[0];
+type PluginInit = Parameters<
+  typeof createEcmaScriptPlugin<Record<string, never>>
+>[0];
 
 // prettier-ignore
 type CreateTestPluginAndRunOptions<ReturnLinesOfFirstFile extends boolean | undefined> =
@@ -48,7 +50,7 @@ type CreateTestPluginAndRunOptions<ReturnLinesOfFirstFile extends boolean | unde
     parameter?: string;
     name?: PluginInit["name"];
     version?: PluginInit["version"];
-    parseOption?: PluginInit["parseOption"];
+    parseOptions?: PluginInit["parseOptions"];
     minimumEdition?: PluginInit["minimumEdition"];
     maximumEdition?: PluginInit["maximumEdition"];
   }

packages/protoplugin-test/src/parse-option.test.ts

@@ -12,86 +12,94 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import { beforeEach, describe, expect, test } from "@jest/globals";
+import { describe, expect, test } from "@jest/globals";
 import { CodeGeneratorRequestDesc } from "@bufbuild/protobuf/wkt";
-import { create } from "@bufbuild/protobuf";
-import type { Plugin } from "@bufbuild/protoplugin";
+import { create, type MessageInitShape } from "@bufbuild/protobuf";
+import type { Schema } from "@bufbuild/protoplugin";
 import { createEcmaScriptPlugin } from "@bufbuild/protoplugin";
 
 describe("parse custom plugin option", () => {
-  let foo: number | undefined;
-  let bar = false;
-  let baz: string[] = [];
-  let plugin: Plugin;
-  beforeEach(() => {
-    foo = undefined;
-    bar = false;
-    baz = [];
-    const noop = () => {
-      //
-    };
-    plugin = createEcmaScriptPlugin({
+  interface Options {
+    foo?: number;
+    bar: boolean;
+    baz: string[];
+  }
+  const runPlugin = (
+    onOptions: (options: Options) => void,
+    req: MessageInitShape<typeof CodeGeneratorRequestDesc>,
+  ) => {
+    const generate = ({ options }: Schema<Options>) => onOptions(options);
+    createEcmaScriptPlugin({
       name: "test",
       version: "v1",
-      parseOption(key, value) {
-        switch (key) {
-          case "foo":
-            foo = parseInt(value);
-            if (isNaN(foo)) {
-              throw "please provide an integer for foo";
+      parseOptions(options): Options {
+        const parsed: Options = { bar: false, baz: [] };
+        for (const { key, value } of options) {
+          switch (key) {
+            case "foo": {
+              const foo = parseInt(value);
+              if (isNaN(foo)) {
+                throw "please provide an integer for foo";
+              }
+              parsed.foo = foo;
+              break;
             }
-            break;
-          case "bar":
-            if (value.length > 0) {
-              throw "bar does not take a value";
-            }
-            bar = true;
-            break;
-          case "baz":
-            if (value.length == 0) {
-              throw "please provide a value";
-            }
-            baz.push(value);
-            break;
-          default:
-            throw new Error();
+            case "bar":
+              if (value.length > 0) {
+                throw "bar does not take a value";
+              }
+              parsed.bar = true;
+              break;
+            case "baz":
+              if (value.length == 0) {
+                throw "please provide a value";
+              }
+              parsed.baz.push(value);
+              break;
+            default:
+              throw new Error();
+          }
         }
+        return parsed;
       },
-      generateTs: noop,
-      generateJs: noop,
-      generateDts: noop,
-    });
-  });
+      generateTs: generate,
+      generateJs: generate,
+      generateDts: generate,
+    }).run(create(CodeGeneratorRequestDesc, req));
+  };
   test("parse as expected on the happy path", () => {
-    plugin.run(
+    runPlugin(
+      (options) => {
+        expect(options.foo).toBe(123);
+        expect(options.bar).toBe(true);
+        expect(options.baz).toStrictEqual(["a", "b"]);
+      },
       create(CodeGeneratorRequestDesc, {
         parameter: "foo=123,bar,baz=a,baz=b",
       }),
     );
-    expect(foo).toBe(123);
-    expect(bar).toBe(true);
-    expect(baz).toStrictEqual(["a", "b"]);
   });
   test("error from parseOption is wrapped", () => {
-    const req = create(CodeGeneratorRequestDesc, {
-      parameter: "foo=abc",
-    });
-    expect(() => plugin.run(req)).toThrowError(
+    expect(() =>
+      runPlugin(() => {}, {
+        parameter: "foo=abc",
+      }),
+    ).toThrowError(
       /^invalid option "foo=abc": please provide an integer for foo$/,
     );
   });
   test("unknown option raises an error", () => {
-    const req = create(CodeGeneratorRequestDesc, {
-      parameter: "unknown",
-    });
-    expect(() => plugin.run(req)).toThrowError(/^invalid option "unknown"$/);
+    expect(() =>
+      runPlugin(() => {}, {
+        parameter: "unknown",
+      }),
+    ).toThrowError(/^invalid option "unknown"$/);
   });
   test("unknown option with value raises an error", () => {
-    const req = create(CodeGeneratorRequestDesc, {
-      parameter: "unknown=bar",
-    });
-    expect(() => plugin.run(req)).toThrowError(
-      /^invalid option "unknown=bar"$/,
-    );
+    expect(() =>
+      runPlugin(() => {}, {
+        parameter: "unknown=bar",
+      }),
+    ).toThrowError(/^invalid option "unknown=bar"$/);
   });
 });

packages/protoplugin-test/src/target.test.ts

@@ -22,7 +22,9 @@ import type {
 import type { CodeGeneratorResponse } from "@bufbuild/protobuf/wkt";
 
 describe("target", () => {
-  type PluginInit = Parameters<typeof createEcmaScriptPlugin>[0];
+  type PluginInit = Parameters<
+    typeof createEcmaScriptPlugin<Record<string, never>>
+  >[0];
   let generateTs: jest.Mock<PluginInit["generateTs"]>;
   let generateJs: jest.Mock<Required<PluginInit>["generateJs"]>;
   let generateDts: jest.Mock<Required<PluginInit>["generateDts"]>;

packages/protoplugin/src/create-es-plugin.ts

@@ -30,7 +30,7 @@ import type { Plugin } from "./plugin.js";
 import { transpile } from "./transpile.js";
 import { parseParameter } from "./parameter.js";
 
-interface PluginInit {
+interface PluginInit<Options extends object> {
   /**
    * Name of this code generator plugin.
    */
@@ -45,7 +45,7 @@ interface PluginInit {
    * An optional parsing function which can be used to parse your own plugin
    * options.
    */
-  parseOption?: (key: string, value: string) => void;
+  parseOptions?: (rawOptions: { key: string; value: string }[]) => Options;
 
   /**
    * The earliest edition supported by this plugin. Defaults to the minimum
@@ -65,7 +65,7 @@ interface PluginInit {
    *
    * Note that this is required to be provided for plugin initialization.
    */
-  generateTs: (schema: Schema, target: "ts") => void;
+  generateTs: (schema: Schema<Options>, target: "ts") => void;
 
   /**
    * A optional function which will generate JavaScript files based on proto
@@ -77,7 +77,7 @@ interface PluginInit {
    * JavaScript files.  If not, the plugin framework will transpile the files
    * itself.
    */
-  generateJs?: (schema: Schema, target: "js") => void;
+  generateJs?: (schema: Schema<Options>, target: "js") => void;
 
   /**
    * A optional function which will generate TypeScript declaration files
@@ -89,7 +89,7 @@ interface PluginInit {
    * declaration files.  If not, the plugin framework will transpile the files
    * itself.
    */
-  generateDts?: (schema: Schema, target: "dts") => void;
+  generateDts?: (schema: Schema<Options>, target: "dts") => void;
 
   /**
    * An optional function which will transpile a given set of files.
@@ -117,7 +117,9 @@ interface PluginInit {
  * The plugin can generate JavaScript, TypeScript, or TypeScript declaration
  * files.
  */
-export function createEcmaScriptPlugin(init: PluginInit): Plugin {
+export function createEcmaScriptPlugin<Options extends object = object>(
+  init: PluginInit<Options>,
+): Plugin {
   let transpileJs = false;
   let transpileDts = false;
   return {
@@ -126,7 +128,7 @@ export function createEcmaScriptPlugin(init: PluginInit): Plugin {
     run(req) {
       const minimumEdition = init.minimumEdition ?? minimumEditionUpstream;
       const maximumEdition = init.maximumEdition ?? maximumEditionUpstream;
-      const parameter = parseParameter(req.parameter, init.parseOption);
+      const parameter = parseParameter(req.parameter, init.parseOptions);
       const schema = createSchema(
         req,
         parameter,
@@ -208,7 +210,7 @@ export function createEcmaScriptPlugin(init: PluginInit): Plugin {
           tsFiles,
           transpileJs,
           transpileDts,
-          parameter.jsImportStyle,
+          parameter.parsed.jsImportStyle,
         );
         files.push(...transpiledFiles);
       }

packages/protoplugin/src/index.ts

@@ -24,6 +24,7 @@ export {
 
 export type { Target } from "./target.js";
 export type { Schema } from "./schema.js";
+export type { EcmaScriptPluginParameters } from "./parameter.js";
 export type { GeneratedFile, FileInfo } from "./generated-file.js";
 export type { ImportSymbol } from "./import-symbol.js";
 export { createImportSymbol } from "./import-symbol.js";

packages/protoplugin/src/parameter.ts

@@ -16,28 +16,36 @@ import type { Target } from "./target.js";
 import type { RewriteImports } from "./import-path.js";
 import { PluginOptionError } from "./error.js";
 
-export interface ParsedParameter {
+export interface EcmaScriptPluginParameters {
   targets: Target[];
   tsNocheck: boolean;
   bootstrapWkt: boolean;
   keepEmptyFiles: boolean;
   rewriteImports: RewriteImports;
   importExtension: string;
   jsImportStyle: "module" | "legacy_commonjs";
-  sanitizedParameter: string;
 }
 
-export function parseParameter(
+export interface ParsedParameter<T> {
+  parsed: T & EcmaScriptPluginParameters;
+  sanitized: string;
+}
+
+export function parseParameter<T extends object>(
   parameter: string,
-  parseExtraOption: ((key: string, value: string) => void) | undefined,
-): ParsedParameter {
+  parseExtraOptions:
+    | ((rawOptions: { key: string; value: string }[]) => T)
+    | undefined,
+): ParsedParameter<T> {
   let targets: Target[] = ["js", "dts"];
   let tsNocheck = false;
   let bootstrapWkt = false;
   let keepEmptyFiles = false;
   const rewriteImports: RewriteImports = [];
   let importExtension = "";
   let jsImportStyle: "module" | "legacy_commonjs" = "module";
+  const extraParameters: { key: string; value: string }[] = [];
+  const extraParametersRaw: string[] = [];
   const rawParameters: string[] = [];
   for (const { key, value, raw } of splitParameter(parameter)) {
     // Whether this key/value plugin parameter pair should be
@@ -136,33 +144,44 @@ export function parseParameter(
         break;
       }
       default:
-        if (parseExtraOption === undefined) {
+        if (parseExtraOptions === undefined) {
           throw new PluginOptionError(raw);
         }
-        try {
-          parseExtraOption(key, value);
-        } catch (e) {
-          throw new PluginOptionError(raw, e);
-        }
+        extraParameters.push({ key, value });
+        extraParametersRaw.push(raw);
         break;
     }
     if (!sanitize) {
       rawParameters.push(raw);
     }
   }
-
-  const sanitizedParameter = rawParameters.join(",");
-
-  return {
+  const sanitizedParameters = rawParameters.join(",");
+  const ecmaScriptPluginParameters = {
     targets,
     tsNocheck,
     bootstrapWkt,
     rewriteImports,
     importExtension,
     jsImportStyle,
     keepEmptyFiles,
-    sanitizedParameter,
   };
+  if (parseExtraOptions === undefined || extraParameters.length === 0) {
+    return {
+      parsed: ecmaScriptPluginParameters as T & EcmaScriptPluginParameters,
+      sanitized: sanitizedParameters,
+    };
+  }
+  try {
+    return {
+      parsed: Object.assign(
+        ecmaScriptPluginParameters,
+        parseExtraOptions(extraParameters),
+      ),
+      sanitized: sanitizedParameters,
+    };
+  } catch (err) {
+    throw new PluginOptionError(extraParametersRaw.join(","), err);
+  }
 }
 
 function splitParameter(