Skip to content

Commit b4a738e

Browse files
authored
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
1 parent 252a176 commit b4a738e

File tree

10 files changed

+165
-25
lines changed

10 files changed

+165
-25
lines changed

src/config/config.ts

+9-9
Original file line numberDiff line numberDiff line change
@@ -813,11 +813,11 @@ export class Config implements IConfig {
813813
}
814814

815815
// when no manifest exists, the default is calculated. This may throw, so we need to catch it
816-
const defaultFlagToCached = async (flag: CompletableOptionFlag<any>) => {
816+
const defaultFlagToCached = async (flag: CompletableOptionFlag<any>, isWritingManifest = false) => {
817817
// Prefer the helpDefaultValue function (returns a friendly string for complex types)
818818
if (typeof flag.defaultHelp === 'function') {
819819
try {
820-
return await flag.defaultHelp()
820+
return await flag.defaultHelp({options: flag, flags: {}}, isWritingManifest)
821821
} catch {
822822
return
823823
}
@@ -826,18 +826,18 @@ const defaultFlagToCached = async (flag: CompletableOptionFlag<any>) => {
826826
// if not specified, try the default function
827827
if (typeof flag.default === 'function') {
828828
try {
829-
return await flag.default({options: {}, flags: {}})
829+
return await flag.default({options: {}, flags: {}}, isWritingManifest)
830830
} catch {}
831831
} else {
832832
return flag.default
833833
}
834834
}
835835

836-
const defaultArgToCached = async (arg: Arg<any>): Promise<any> => {
836+
const defaultArgToCached = async (arg: Arg<any>, isWritingManifest = false): Promise<any> => {
837837
// Prefer the helpDefaultValue function (returns a friendly string for complex types)
838838
if (typeof arg.defaultHelp === 'function') {
839839
try {
840-
return await arg.defaultHelp()
840+
return await arg.defaultHelp({options: arg, flags: {}}, isWritingManifest)
841841
} catch {
842842
return
843843
}
@@ -846,14 +846,14 @@ const defaultArgToCached = async (arg: Arg<any>): Promise<any> => {
846846
// if not specified, try the default function
847847
if (typeof arg.default === 'function') {
848848
try {
849-
return await arg.default({options: {}, flags: {}})
849+
return await arg.default({options: arg, flags: {}}, isWritingManifest)
850850
} catch {}
851851
} else {
852852
return arg.default
853853
}
854854
}
855855

856-
export async function toCached(c: Command.Class, plugin?: IPlugin): Promise<Command.Cached> {
856+
export async function toCached(c: Command.Class, plugin?: IPlugin | undefined, isWritingManifest?: boolean): Promise<Command.Cached> {
857857
const flags = {} as {[k: string]: Command.Flag.Cached}
858858

859859
for (const [name, flag] of Object.entries(c.flags || {})) {
@@ -894,7 +894,7 @@ export async function toCached(c: Command.Class, plugin?: IPlugin): Promise<Comm
894894
dependsOn: flag.dependsOn,
895895
relationships: flag.relationships,
896896
exclusive: flag.exclusive,
897-
default: await defaultFlagToCached(flag),
897+
default: await defaultFlagToCached(flag, isWritingManifest),
898898
deprecated: flag.deprecated,
899899
deprecateAliases: c.deprecateAliases,
900900
aliases: flag.aliases,
@@ -914,7 +914,7 @@ export async function toCached(c: Command.Class, plugin?: IPlugin): Promise<Comm
914914
description: arg.description,
915915
required: arg.required,
916916
options: arg.options,
917-
default: await defaultArgToCached(arg),
917+
default: await defaultArgToCached(arg, isWritingManifest),
918918
hidden: arg.hidden,
919919
}
920920
}

src/config/plugin.ts

+10-4
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,13 @@ export class Plugin implements IPlugin {
136136

137137
constructor(public options: PluginOptions) {}
138138

139-
public async load(): Promise<void> {
139+
/**
140+
* Loads a plugin
141+
* @param isWritingManifest - if true, exclude selected data from manifest
142+
* default is false to maintain backwards compatibility
143+
* @returns Promise<void>
144+
*/
145+
public async load(isWritingManifest?: boolean): Promise<void> {
140146
this.type = this.options.type || 'core'
141147
this.tag = this.options.tag
142148
const root = await findRoot(this.options.name, this.options.root)
@@ -160,7 +166,7 @@ export class Plugin implements IPlugin {
160166

161167
this.hooks = mapValues(this.pjson.oclif.hooks || {}, i => Array.isArray(i) ? i : [i])
162168

163-
this.manifest = await this._manifest(Boolean(this.options.ignoreManifest), Boolean(this.options.errorOnManifestCreate))
169+
this.manifest = await this._manifest(Boolean(this.options.ignoreManifest), Boolean(this.options.errorOnManifestCreate), isWritingManifest)
164170
this.commands = Object
165171
.entries(this.manifest.commands)
166172
.map(([id, c]) => ({
@@ -244,7 +250,7 @@ export class Plugin implements IPlugin {
244250
return cmd
245251
}
246252

247-
protected async _manifest(ignoreManifest: boolean, errorOnManifestCreate = false): Promise<Manifest> {
253+
protected async _manifest(ignoreManifest: boolean, errorOnManifestCreate = false, isWritingManifest = false): Promise<Manifest> {
248254
const readManifest = async (dotfile = false): Promise<Manifest | undefined> => {
249255
try {
250256
const p = path.join(this.root, `${dotfile ? '.' : ''}oclif.manifest.json`)
@@ -279,7 +285,7 @@ export class Plugin implements IPlugin {
279285
version: this.version,
280286
commands: (await Promise.all(this.commandIDs.map(async id => {
281287
try {
282-
return [id, await toCached(await this.findCommand(id, {must: true}), this)]
288+
return [id, await toCached(await this.findCommand(id, {must: true}), this, isWritingManifest)]
283289
} catch (error: any) {
284290
const scope = 'toCached'
285291
if (Boolean(errorOnManifestCreate) === false) this.warn(error, scope)

src/help/index.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ export class Help extends HelpBase {
106106
const command = this.config.findCommand(subject)
107107
if (command) {
108108
if (command.hasDynamicHelp && command.pluginType !== 'jit') {
109-
const dynamicCommand = await toCached(await command.load())
109+
const dynamicCommand = await toCached(await command.load(), undefined, false)
110110
await this.showCommandHelp(dynamicCommand)
111111
} else {
112112
await this.showCommandHelp(command)

src/interfaces/parser.ts

+89-5
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ export type Metadata = {
4343

4444
type MetadataFlag = {
4545
setFromDefault?: boolean;
46+
defaultHelp?: unknown;
4647
}
4748

4849
export type ListItem = [string, string | undefined]
@@ -55,11 +56,94 @@ export type DefaultContext<T> = {
5556
flags: Record<string, string>;
5657
}
5758

58-
export type FlagDefault<T, P = CustomOptions> = T | ((context: DefaultContext<P & OptionFlag<T, P>>) => Promise<T>)
59-
export type FlagDefaultHelp<T, P = CustomOptions> = T | ((context: DefaultContext<P & OptionFlag<T, P>>) => Promise<string | undefined>)
60-
61-
export type ArgDefault<T, P = CustomOptions> = T | ((context: DefaultContext<Arg<T, P>>) => Promise<T>)
62-
export type ArgDefaultHelp<T, P = CustomOptions> = T | ((context: DefaultContext<Arg<T, P>>) => Promise<string | undefined>)
59+
/**
60+
* Type to define a default value for a flag.
61+
* @param context The context of the flag.
62+
* @param isWritingManifest Informs the function that a manifest file is being written.
63+
* The manifest file is used to store the flag definitions, with a default value if present, for a command and is published to npm.
64+
* When a manifest file is being written, the default value may contain data that should not be included in the manifest.
65+
* The plugin developer can use isWritingManifest to determine if the default value should be omitted from the manifest.
66+
* in the function's implementation.
67+
* @example
68+
* static flags = {
69+
* foo: flags.string({
70+
* defaultHelp: async (context, isWritingManifest) => {
71+
* if (isWritingManifest) {
72+
* return undefined
73+
* }
74+
* return 'value that is used outside a manifest'
75+
* },
76+
* }),
77+
* }
78+
*/
79+
export type FlagDefault<T, P = CustomOptions> = T | ((context: DefaultContext<P & OptionFlag<T, P>>, isWritingManifest?: boolean) => Promise<T>)
80+
81+
/**
82+
* Type to define a defaultHelp value for a flag.
83+
* The defaultHelp value is used in the help output for the flag and when writing a manifest.
84+
* It is also can be used to provide a value for the flag when issuing certain error messages.
85+
*
86+
* @param context The context of the flag.
87+
* @param isWritingManifest Informs the function that a manifest file is being written.
88+
* 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.
89+
* When a manifest file is being written, the default value may contain data that should not be included in the manifest.
90+
* The plugin developer can use isWritingManifest to determine if the defaultHelp value should be omitted from the manifest.
91+
* in the function's implementation.
92+
* @example
93+
* static flags = {
94+
* foo: flags.string({
95+
* defaultHelp: async (context, isWritingManifest) => {
96+
* if (isWritingManifest) {
97+
* return undefined
98+
* }
99+
* return 'value that is used outside a manifest'
100+
* },
101+
* }),
102+
* }
103+
*/
104+
export type FlagDefaultHelp<T, P = CustomOptions> = T | ((context: DefaultContext<P & OptionFlag<T, P>>, isWritingManifest?: boolean) => Promise<string | undefined>)
105+
106+
/**
107+
* Type to define a default value for an arg.
108+
* @param context The context of the arg.
109+
* @param isWritingManifest Informs the function that a manifest file is being written.
110+
* The manifest file is used to store the arg definitions, with a default value if present, for a command and is published to npm.
111+
* When a manifest file is being written, the default value may contain data that should not be included in the manifest.
112+
* The plugin developer can use isWritingManifest to determine if the default value should be omitted from the manifest.
113+
* in the function's implementation.
114+
* @example
115+
* public static readonly args = {
116+
* one: Args.string({
117+
* default: async (context, isWritingManifest) => {
118+
* if (isWritingManifest) {
119+
* return undefined
120+
* }
121+
* return 'value that is used outside a manifest'
122+
* }),
123+
* };
124+
*/
125+
export type ArgDefault<T, P = CustomOptions> = T | ((context: DefaultContext<Arg<T, P>>, isWritingManifest?: boolean) => Promise<T>)
126+
127+
/**
128+
* Type to define a defaultHelp value for an arg.
129+
* @param context The context of the arg.
130+
* @param isWritingManifest Informs the function that a manifest file is being written.
131+
* 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.
132+
* When a manifest file is being written, the default value may contain data that should not be included in the manifest.
133+
* The plugin developer can use isWritingManifest to determine if the default value should be omitted from the manifest.
134+
* in the function's implementation.
135+
* @example
136+
* public static readonly args = {
137+
* one: Args.string({
138+
* defaultHelp: async (context, isWritingManifest) => {
139+
* if (isWritingManifest) {
140+
* return undefined
141+
* }
142+
* return 'value that is used outside a manifest'
143+
* }),
144+
* };
145+
*/
146+
export type ArgDefaultHelp<T, P = CustomOptions> = T | ((context: DefaultContext<Arg<T, P>>, isWritingManifest?: boolean) => Promise<string | undefined>)
63147

64148
export type FlagRelationship = string | {name: string; when: (flags: Record<string, unknown>) => Promise<boolean>};
65149
export type Relationship = {

src/interfaces/plugin.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -73,5 +73,5 @@ export interface Plugin {
7373

7474
findCommand(id: string, opts: { must: true }): Promise<Command.Class>;
7575
findCommand(id: string, opts?: { must: boolean }): Promise<Command.Class> | undefined;
76-
load(): Promise<void>;
76+
load(isWritingManifest: boolean): Promise<void>;
7777
}

src/parser/parse.ts

+10-2
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,14 @@ export class Parser<T extends ParserInput, TFlags extends OutputFlags<T['flags']
217217
const flag = this.input.flags[token.flag]
218218

219219
if (!flag) throw new CLIError(`Unexpected flag ${token.flag}`)
220+
221+
// if flag has defaultHelp, capture its value into metadata
222+
if (Reflect.has(flag, 'defaultHelp')) {
223+
const defaultHelpProperty = Reflect.get(flag, 'defaultHelp')
224+
const defaultHelp = (typeof defaultHelpProperty === 'function' ? await defaultHelpProperty({options: flag, flags, ...this.context}) : defaultHelpProperty)
225+
this.metaData.flags[token.flag] = {...this.metaData.flags[token.flag], defaultHelp}
226+
}
227+
220228
if (flag.type === 'boolean') {
221229
if (token.input === `--no-${flag.name}`) {
222230
flags[token.flag] = false
@@ -256,7 +264,7 @@ export class Parser<T extends ParserInput, TFlags extends OutputFlags<T['flags']
256264
for (const k of Object.keys(this.input.flags)) {
257265
const flag = this.input.flags[k]
258266
if (flags[k]) continue
259-
if (flag.env && Object.prototype.hasOwnProperty.call(process.env, flag.env)) {
267+
if (flag.env && Reflect.has(process.env, flag.env)) {
260268
const input = process.env[flag.env]
261269
if (flag.type === 'option') {
262270
if (input) {
@@ -271,7 +279,7 @@ export class Parser<T extends ParserInput, TFlags extends OutputFlags<T['flags']
271279
}
272280

273281
if (!(k in flags) && flag.default !== undefined) {
274-
this.metaData.flags[k] = {setFromDefault: true}
282+
this.metaData.flags[k] = {...this.metaData.flags[k], setFromDefault: true}
275283
const defaultValue = (typeof flag.default === 'function' ? await flag.default({options: flag, flags, ...this.context}) : flag.default)
276284
flags[k] = defaultValue
277285
}

src/parser/validate.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,8 @@ export async function validate(parse: {
118118
if (parse.output.metadata.flags && parse.output.metadata.flags[name]?.setFromDefault)
119119
continue
120120
if (parse.output.flags[flag] !== undefined) {
121-
return {...base, status: 'failed', reason: `--${flag}=${parse.output.flags[flag]} cannot also be provided when using --${name}`}
121+
const flagValue = parse.output.metadata.flags?.[flag]?.defaultHelp ?? parse.output.flags[flag]
122+
return {...base, status: 'failed', reason: `--${flag}=${flagValue} cannot also be provided when using --${name}`}
122123
}
123124
}
124125

test/command/command.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ describe('command', () => {
103103
}
104104
}
105105

106-
const c = await toCached(C)
106+
const c = await toCached(C, undefined, false)
107107

108108
expect(c).to.deep.equal({
109109
id: 'foo:bar',

test/help/help-test-utils.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ export class TestHelp extends Help {
4747

4848
export const commandHelp = (command?: any) => ({
4949
async run(ctx: {help: TestHelp; commandHelp: string; expectation: string}) {
50-
const cached = await toCached(command!, {} as any)
50+
const cached = await toCached(command!, {} as any, false)
5151
const help = ctx.help.formatCommand(cached)
5252
if (process.env.TEST_OUTPUT === '1') {
5353
console.log(help)

test/parser/parse.test.ts

+41
Original file line numberDiff line numberDiff line change
@@ -684,6 +684,10 @@ See more help with --help`)
684684
constructor(input: string) {
685685
this.prop = input
686686
}
687+
688+
public toString(): string {
689+
return this.prop
690+
}
687691
}
688692
it('uses default via value', async () => {
689693
const out = await parse([], {
@@ -707,6 +711,43 @@ See more help with --help`)
707711
})
708712
expect(out.flags.foo?.prop).to.equal('baz')
709713
})
714+
it('should error with exclusive flag violation', async () => {
715+
try {
716+
const out = await parse(['--foo', 'baz', '--bar'], {
717+
flags: {
718+
foo: Flags.custom<TestClass>({
719+
parse: async input => new TestClass(input),
720+
defaultHelp: new TestClass('bar'),
721+
})(),
722+
bar: Flags.boolean({
723+
exclusive: ['foo'],
724+
}),
725+
},
726+
})
727+
expect.fail(`Should have thrown an error ${JSON.stringify(out)}`)
728+
} catch (error) {
729+
assert(error instanceof Error)
730+
expect(error.message).to.include('--foo=bar cannot also be provided when using --bar')
731+
}
732+
})
733+
it('should error with exclusive flag violation and defaultHelp value', async () => {
734+
try {
735+
const out = await parse(['--foo', 'baz', '--bar'], {
736+
flags: {
737+
foo: Flags.custom<TestClass>({
738+
parse: async input => new TestClass(input),
739+
})(),
740+
bar: Flags.boolean({
741+
exclusive: ['foo'],
742+
}),
743+
},
744+
})
745+
expect.fail(`Should have thrown an error ${JSON.stringify(out)}`)
746+
} catch (error) {
747+
assert(error instanceof Error)
748+
expect(error.message).to.include('--foo=baz cannot also be provided when using --bar')
749+
}
750+
})
710751
it('uses parser when value provided', async () => {
711752
const out = await parse(['--foo=bar'], {
712753
flags: {

0 commit comments

Comments
 (0)