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

@@ -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<any>) => {
+const defaultFlagToCached = async (flag: CompletableOptionFlag<any>, 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<any>) => {
   // 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<any>): Promise<any> => {
+const defaultArgToCached = async (arg: Arg<any>, isWritingManifest = false): Promise<any> => {
   // 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<any>): Promise<any> => {
   // 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<Command.Cached> {
+export async function toCached(c: Command.Class, plugin?: IPlugin | undefined, isWritingManifest?: boolean): Promise<Command.Cached> {
   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<Comm
         dependsOn: flag.dependsOn,
         relationships: flag.relationships,
         exclusive: flag.exclusive,
-        default: await defaultFlagToCached(flag),
+        default: await defaultFlagToCached(flag, isWritingManifest),
         deprecated: flag.deprecated,
         deprecateAliases: c.deprecateAliases,
         aliases: flag.aliases,
@@ -914,7 +914,7 @@ export async function toCached(c: Command.Class, plugin?: IPlugin): Promise<Comm
       description: arg.description,
       required: arg.required,
       options: arg.options,
-      default: await defaultArgToCached(arg),
+      default: await defaultArgToCached(arg, isWritingManifest),
       hidden: arg.hidden,
     }
   }

src/config/plugin.ts

@@ -136,7 +136,13 @@ export class Plugin implements IPlugin {
 
   constructor(public options: PluginOptions) {}
 
-  public async load(): Promise<void> {
+  /**
+   * Loads a plugin
+   * @param isWritingManifest - if true, exclude selected data from manifest
+   * default is false to maintain backwards compatibility
+   * @returns Promise<void>
+   */
+  public async load(isWritingManifest?: boolean): Promise<void> {
     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<Manifest> {
+  protected async _manifest(ignoreManifest: boolean, errorOnManifestCreate = false, isWritingManifest = false): Promise<Manifest> {
     const readManifest = async (dotfile = false): Promise<Manifest | undefined> => {
       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)

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)

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<T> = {
   flags: Record<string, string>;
 }
 
-export type FlagDefault<T, P = CustomOptions> = T | ((context: DefaultContext<P & OptionFlag<T, P>>) => Promise<T>)
-export type FlagDefaultHelp<T, P = CustomOptions> = T | ((context: DefaultContext<P & OptionFlag<T, P>>) => Promise<string | undefined>)
-
-export type ArgDefault<T, P = CustomOptions> = T | ((context: DefaultContext<Arg<T, P>>) => Promise<T>)
-export type ArgDefaultHelp<T, P = CustomOptions> = T | ((context: DefaultContext<Arg<T, P>>) => Promise<string | undefined>)
+/**
+ * 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, P = CustomOptions> = T | ((context: DefaultContext<P & OptionFlag<T, P>>, isWritingManifest?: boolean) => Promise<T>)
+
+/**
+ * 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, P = CustomOptions> = T | ((context: DefaultContext<P & OptionFlag<T, P>>, isWritingManifest?: boolean) => Promise<string | undefined>)
+
+/**
+ * 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, P = CustomOptions> = T | ((context: DefaultContext<Arg<T, P>>, isWritingManifest?: boolean) => Promise<T>)
+
+/**
+ * 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, P = CustomOptions> = T | ((context: DefaultContext<Arg<T, P>>, isWritingManifest?: boolean) => Promise<string | undefined>)
 
 export type FlagRelationship = string | {name: string; when: (flags: Record<string, unknown>) => Promise<boolean>};
 export type Relationship = {

src/interfaces/plugin.ts

@@ -73,5 +73,5 @@ export interface Plugin {
 
   findCommand(id: string, opts: { must: true }): Promise<Command.Class>;
   findCommand(id: string, opts?: { must: boolean }): Promise<Command.Class> | undefined;
-  load(): Promise<void>;
+  load(isWritingManifest: boolean): Promise<void>;
 }

src/parser/parse.ts

@@ -217,6 +217,14 @@ export class Parser<T extends ParserInput, TFlags extends OutputFlags<T['flags']
       const flag = this.input.flags[token.flag]
 
       if (!flag) throw new CLIError(`Unexpected flag ${token.flag}`)
+
+      // if flag has defaultHelp, capture its value into metadata
+      if (Reflect.has(flag, 'defaultHelp')) {
+        const defaultHelpProperty = Reflect.get(flag, 'defaultHelp')
+        const defaultHelp = (typeof defaultHelpProperty === 'function' ? await defaultHelpProperty({options: flag, flags, ...this.context}) : defaultHelpProperty)
+        this.metaData.flags[token.flag] = {...this.metaData.flags[token.flag], defaultHelp}
+      }
+
       if (flag.type === 'boolean') {
         if (token.input === `--no-${flag.name}`) {
           flags[token.flag] = false
@@ -256,7 +264,7 @@ export class Parser<T extends ParserInput, TFlags extends OutputFlags<T['flags']
     for (const k of Object.keys(this.input.flags)) {
       const flag = this.input.flags[k]
       if (flags[k]) continue
-      if (flag.env && Object.prototype.hasOwnProperty.call(process.env, flag.env)) {
+      if (flag.env && Reflect.has(process.env, flag.env)) {
         const input = process.env[flag.env]
         if (flag.type === 'option') {
           if (input) {
@@ -271,7 +279,7 @@ export class Parser<T extends ParserInput, TFlags extends OutputFlags<T['flags']
       }
 
       if (!(k in flags) && flag.default !== undefined) {
-        this.metaData.flags[k] = {setFromDefault: true}
+        this.metaData.flags[k] = {...this.metaData.flags[k], setFromDefault: true}
         const defaultValue = (typeof flag.default === 'function' ? await flag.default({options: flag, flags, ...this.context}) : flag.default)
         flags[k] = defaultValue
       }

src/parser/validate.ts

@@ -118,7 +118,8 @@ export async function validate(parse: {
       if (parse.output.metadata.flags && parse.output.metadata.flags[name]?.setFromDefault)
         continue
       if (parse.output.flags[flag] !== undefined) {
-        return {...base, status: 'failed', reason: `--${flag}=${parse.output.flags[flag]} cannot also be provided when using --${name}`}
+        const flagValue = parse.output.metadata.flags?.[flag]?.defaultHelp ?? parse.output.flags[flag]
+        return {...base, status: 'failed', reason: `--${flag}=${flagValue} cannot also be provided when using --${name}`}
       }
     }
 

test/command/command.test.ts

@@ -103,7 +103,7 @@ describe('command', () => {
       }
       }
 
-      const c = await toCached(C)
+      const c = await toCached(C, undefined, false)
 
       expect(c).to.deep.equal({
         id: 'foo:bar',

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)

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<TestClass>({
+                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<TestClass>({
+                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: {