From b4a738e40735c40dbf633546011c4a860ebff46c Mon Sep 17 00:00:00 2001 From: peternhale Date: Wed, 22 Mar 2023 09:19:04 -0600 Subject: [PATCH] feat: add param noSensitiveData to Plugin.load (#665) * feat: add new param noSensitiveData to Plugin.load @W-12513575@ * chore: add param to plugin interface * chore: add param to plugin interface * chore: address review concerns * fix: add plugin load param that indicates menifest write underway @W-12513575@ @W-11868418@ * chore: remove redundant T from type --- src/config/config.ts | 18 +++---- src/config/plugin.ts | 14 ++++-- src/help/index.ts | 2 +- src/interfaces/parser.ts | 94 ++++++++++++++++++++++++++++++++++-- src/interfaces/plugin.ts | 2 +- src/parser/parse.ts | 12 ++++- src/parser/validate.ts | 3 +- test/command/command.test.ts | 2 +- test/help/help-test-utils.ts | 2 +- test/parser/parse.test.ts | 41 ++++++++++++++++ 10 files changed, 165 insertions(+), 25 deletions(-) diff --git a/src/config/config.ts b/src/config/config.ts index 6f3aafba8..d4a6d8beb 100644 --- a/src/config/config.ts +++ b/src/config/config.ts @@ -813,11 +813,11 @@ export class Config implements IConfig { } // when no manifest exists, the default is calculated. This may throw, so we need to catch it -const defaultFlagToCached = async (flag: CompletableOptionFlag) => { +const defaultFlagToCached = async (flag: CompletableOptionFlag, isWritingManifest = false) => { // Prefer the helpDefaultValue function (returns a friendly string for complex types) if (typeof flag.defaultHelp === 'function') { try { - return await flag.defaultHelp() + return await flag.defaultHelp({options: flag, flags: {}}, isWritingManifest) } catch { return } @@ -826,18 +826,18 @@ const defaultFlagToCached = async (flag: CompletableOptionFlag) => { // if not specified, try the default function if (typeof flag.default === 'function') { try { - return await flag.default({options: {}, flags: {}}) + return await flag.default({options: {}, flags: {}}, isWritingManifest) } catch {} } else { return flag.default } } -const defaultArgToCached = async (arg: Arg): Promise => { +const defaultArgToCached = async (arg: Arg, isWritingManifest = false): Promise => { // Prefer the helpDefaultValue function (returns a friendly string for complex types) if (typeof arg.defaultHelp === 'function') { try { - return await arg.defaultHelp() + return await arg.defaultHelp({options: arg, flags: {}}, isWritingManifest) } catch { return } @@ -846,14 +846,14 @@ const defaultArgToCached = async (arg: Arg): Promise => { // if not specified, try the default function if (typeof arg.default === 'function') { try { - return await arg.default({options: {}, flags: {}}) + return await arg.default({options: arg, flags: {}}, isWritingManifest) } catch {} } else { return arg.default } } -export async function toCached(c: Command.Class, plugin?: IPlugin): Promise { +export async function toCached(c: Command.Class, plugin?: IPlugin | undefined, isWritingManifest?: boolean): Promise { const flags = {} as {[k: string]: Command.Flag.Cached} for (const [name, flag] of Object.entries(c.flags || {})) { @@ -894,7 +894,7 @@ export async function toCached(c: Command.Class, plugin?: IPlugin): Promise { + /** + * Loads a plugin + * @param isWritingManifest - if true, exclude selected data from manifest + * default is false to maintain backwards compatibility + * @returns Promise + */ + public async load(isWritingManifest?: boolean): Promise { this.type = this.options.type || 'core' this.tag = this.options.tag const root = await findRoot(this.options.name, this.options.root) @@ -160,7 +166,7 @@ export class Plugin implements IPlugin { this.hooks = mapValues(this.pjson.oclif.hooks || {}, i => Array.isArray(i) ? i : [i]) - this.manifest = await this._manifest(Boolean(this.options.ignoreManifest), Boolean(this.options.errorOnManifestCreate)) + this.manifest = await this._manifest(Boolean(this.options.ignoreManifest), Boolean(this.options.errorOnManifestCreate), isWritingManifest) this.commands = Object .entries(this.manifest.commands) .map(([id, c]) => ({ @@ -244,7 +250,7 @@ export class Plugin implements IPlugin { return cmd } - protected async _manifest(ignoreManifest: boolean, errorOnManifestCreate = false): Promise { + protected async _manifest(ignoreManifest: boolean, errorOnManifestCreate = false, isWritingManifest = false): Promise { const readManifest = async (dotfile = false): Promise => { try { const p = path.join(this.root, `${dotfile ? '.' : ''}oclif.manifest.json`) @@ -279,7 +285,7 @@ export class Plugin implements IPlugin { version: this.version, commands: (await Promise.all(this.commandIDs.map(async id => { try { - return [id, await toCached(await this.findCommand(id, {must: true}), this)] + return [id, await toCached(await this.findCommand(id, {must: true}), this, isWritingManifest)] } catch (error: any) { const scope = 'toCached' if (Boolean(errorOnManifestCreate) === false) this.warn(error, scope) diff --git a/src/help/index.ts b/src/help/index.ts index a768286e9..d5572dc5f 100644 --- a/src/help/index.ts +++ b/src/help/index.ts @@ -106,7 +106,7 @@ export class Help extends HelpBase { const command = this.config.findCommand(subject) if (command) { if (command.hasDynamicHelp && command.pluginType !== 'jit') { - const dynamicCommand = await toCached(await command.load()) + const dynamicCommand = await toCached(await command.load(), undefined, false) await this.showCommandHelp(dynamicCommand) } else { await this.showCommandHelp(command) diff --git a/src/interfaces/parser.ts b/src/interfaces/parser.ts index c7ad4c0a5..5b2f15fb6 100644 --- a/src/interfaces/parser.ts +++ b/src/interfaces/parser.ts @@ -43,6 +43,7 @@ export type Metadata = { type MetadataFlag = { setFromDefault?: boolean; + defaultHelp?: unknown; } export type ListItem = [string, string | undefined] @@ -55,11 +56,94 @@ export type DefaultContext = { flags: Record; } -export type FlagDefault = T | ((context: DefaultContext

>) => Promise) -export type FlagDefaultHelp = T | ((context: DefaultContext

>) => Promise) - -export type ArgDefault = T | ((context: DefaultContext>) => Promise) -export type ArgDefaultHelp = T | ((context: DefaultContext>) => Promise) +/** + * Type to define a default value for a flag. + * @param context The context of the flag. + * @param isWritingManifest Informs the function that a manifest file is being written. + * The manifest file is used to store the flag definitions, with a default value if present, for a command and is published to npm. + * When a manifest file is being written, the default value may contain data that should not be included in the manifest. + * The plugin developer can use isWritingManifest to determine if the default value should be omitted from the manifest. + * in the function's implementation. + * @example + * static flags = { + * foo: flags.string({ + * defaultHelp: async (context, isWritingManifest) => { + * if (isWritingManifest) { + * return undefined + * } + * return 'value that is used outside a manifest' + * }, + * }), + * } + */ +export type FlagDefault = T | ((context: DefaultContext

>, isWritingManifest?: boolean) => Promise) + +/** + * Type to define a defaultHelp value for a flag. + * The defaultHelp value is used in the help output for the flag and when writing a manifest. + * It is also can be used to provide a value for the flag when issuing certain error messages. + * + * @param context The context of the flag. + * @param isWritingManifest Informs the function that a manifest file is being written. + * The manifest file is used to store the flag definitions, with a default value if present via defaultHelp, for a command and is published to npm. + * When a manifest file is being written, the default value may contain data that should not be included in the manifest. + * The plugin developer can use isWritingManifest to determine if the defaultHelp value should be omitted from the manifest. + * in the function's implementation. + * @example + * static flags = { + * foo: flags.string({ + * defaultHelp: async (context, isWritingManifest) => { + * if (isWritingManifest) { + * return undefined + * } + * return 'value that is used outside a manifest' + * }, + * }), + * } + */ +export type FlagDefaultHelp = T | ((context: DefaultContext

>, isWritingManifest?: boolean) => Promise) + +/** + * Type to define a default value for an arg. + * @param context The context of the arg. + * @param isWritingManifest Informs the function that a manifest file is being written. + * The manifest file is used to store the arg definitions, with a default value if present, for a command and is published to npm. + * When a manifest file is being written, the default value may contain data that should not be included in the manifest. + * The plugin developer can use isWritingManifest to determine if the default value should be omitted from the manifest. + * in the function's implementation. + * @example + * public static readonly args = { + * one: Args.string({ + * default: async (context, isWritingManifest) => { + * if (isWritingManifest) { + * return undefined + * } + * return 'value that is used outside a manifest' + * }), + * }; + */ +export type ArgDefault = T | ((context: DefaultContext>, isWritingManifest?: boolean) => Promise) + +/** + * Type to define a defaultHelp value for an arg. + * @param context The context of the arg. + * @param isWritingManifest Informs the function that a manifest file is being written. + * The manifest file is used to store the arg definitions, with a default value if present via defaultHelp, for a command and is published to npm. + * When a manifest file is being written, the default value may contain data that should not be included in the manifest. + * The plugin developer can use isWritingManifest to determine if the default value should be omitted from the manifest. + * in the function's implementation. + * @example + * public static readonly args = { + * one: Args.string({ + * defaultHelp: async (context, isWritingManifest) => { + * if (isWritingManifest) { + * return undefined + * } + * return 'value that is used outside a manifest' + * }), + * }; + */ +export type ArgDefaultHelp = T | ((context: DefaultContext>, isWritingManifest?: boolean) => Promise) export type FlagRelationship = string | {name: string; when: (flags: Record) => Promise}; export type Relationship = { diff --git a/src/interfaces/plugin.ts b/src/interfaces/plugin.ts index ea61b66e6..303f20846 100644 --- a/src/interfaces/plugin.ts +++ b/src/interfaces/plugin.ts @@ -73,5 +73,5 @@ export interface Plugin { findCommand(id: string, opts: { must: true }): Promise; findCommand(id: string, opts?: { must: boolean }): Promise | undefined; - load(): Promise; + load(isWritingManifest: boolean): Promise; } diff --git a/src/parser/parse.ts b/src/parser/parse.ts index 09064fd29..8bf7e01b1 100644 --- a/src/parser/parse.ts +++ b/src/parser/parse.ts @@ -217,6 +217,14 @@ export class Parser { } } - const c = await toCached(C) + const c = await toCached(C, undefined, false) expect(c).to.deep.equal({ id: 'foo:bar', diff --git a/test/help/help-test-utils.ts b/test/help/help-test-utils.ts index 4f636b272..a58ca8a89 100644 --- a/test/help/help-test-utils.ts +++ b/test/help/help-test-utils.ts @@ -47,7 +47,7 @@ export class TestHelp extends Help { export const commandHelp = (command?: any) => ({ async run(ctx: {help: TestHelp; commandHelp: string; expectation: string}) { - const cached = await toCached(command!, {} as any) + const cached = await toCached(command!, {} as any, false) const help = ctx.help.formatCommand(cached) if (process.env.TEST_OUTPUT === '1') { console.log(help) diff --git a/test/parser/parse.test.ts b/test/parser/parse.test.ts index c5d34f9f2..aa9246de7 100644 --- a/test/parser/parse.test.ts +++ b/test/parser/parse.test.ts @@ -684,6 +684,10 @@ See more help with --help`) constructor(input: string) { this.prop = input } + + public toString(): string { + return this.prop + } } it('uses default via value', async () => { const out = await parse([], { @@ -707,6 +711,43 @@ See more help with --help`) }) expect(out.flags.foo?.prop).to.equal('baz') }) + it('should error with exclusive flag violation', async () => { + try { + const out = await parse(['--foo', 'baz', '--bar'], { + flags: { + foo: Flags.custom({ + parse: async input => new TestClass(input), + defaultHelp: new TestClass('bar'), + })(), + bar: Flags.boolean({ + exclusive: ['foo'], + }), + }, + }) + expect.fail(`Should have thrown an error ${JSON.stringify(out)}`) + } catch (error) { + assert(error instanceof Error) + expect(error.message).to.include('--foo=bar cannot also be provided when using --bar') + } + }) + it('should error with exclusive flag violation and defaultHelp value', async () => { + try { + const out = await parse(['--foo', 'baz', '--bar'], { + flags: { + foo: Flags.custom({ + parse: async input => new TestClass(input), + })(), + bar: Flags.boolean({ + exclusive: ['foo'], + }), + }, + }) + expect.fail(`Should have thrown an error ${JSON.stringify(out)}`) + } catch (error) { + assert(error instanceof Error) + expect(error.message).to.include('--foo=baz cannot also be provided when using --bar') + } + }) it('uses parser when value provided', async () => { const out = await parse(['--foo=bar'], { flags: {