Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: warn about alias deprecations #540

Merged
merged 5 commits into from Oct 28, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 23 additions & 1 deletion src/command.ts
Expand Up @@ -9,7 +9,7 @@ import {PrettyPrintableError} from './errors'
import * as Parser from './parser'
import * as Flags from './flags'
import {Deprecation} from './interfaces/parser'
import {formatCommandDeprecationWarning, formatFlagDeprecationWarning} from './help/util'
import {formatCommandDeprecationWarning, formatFlagDeprecationWarning, normalizeArgv} from './help/util'

const pjson = require('../package.json')

Expand Down Expand Up @@ -62,6 +62,11 @@ export default abstract class Command {

static deprecationOptions?: Deprecation;

/**
* Emit deprecation warning when a command alias is used
*/
static deprecateAliases?: boolean

/**
* An override string (or strings) for the default usage documentation.
*/
Expand Down Expand Up @@ -255,10 +260,27 @@ export default abstract class Command {
if (deprecated) {
this.warn(formatFlagDeprecationWarning(flag, deprecated))
}

const deprecateAliases = this.ctor.flags[flag]?.deprecateAliases
const aliases = this.ctor.flags[flag]?.aliases ?? []
if (deprecateAliases && aliases) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aliases will always be true since it's ?? [] (empty arrays are truthy).

Did you maybe want aliases.length?

const foundAliases = this.argv.filter(a => aliases.includes(a.replace(/-/g, '')))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's happening with the - ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

argv has the --/- on all the flag names e.g., ['--foo', '--bar', '-b']

But the flag aliases don't have the prefix, e.g. ['foo', 'bar', 'b']

So in order to find the intersection, we have to remove all the -s first

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens for target-org and no-prompt and the like?

for (const alias of foundAliases) {
this.warn(formatFlagDeprecationWarning(alias, {to: this.ctor.flags[flag]?.name}))
}
}
}
}

protected warnIfCommandDeprecated(): void {
const [id] = normalizeArgv(this.config)

if (this.ctor.deprecateAliases && this.ctor.aliases.includes(id)) {
const cmdName = toConfiguredId(this.ctor.id, this.config)
const aliasName = toConfiguredId(id, this.config)
this.warn(formatCommandDeprecationWarning(aliasName, {to: cmdName}))
}

if (this.ctor.state === 'deprecated') {
const cmdName = toConfiguredId(this.ctor.id, this.config)
this.warn(formatCommandDeprecationWarning(cmdName, this.ctor.deprecationOptions))
Expand Down
3 changes: 3 additions & 0 deletions src/config/config.ts
Expand Up @@ -754,6 +754,7 @@ export async function toCached(c: Command.Class, plugin?: IPlugin): Promise<Comm
relationships: flag.relationships,
exclusive: flag.exclusive,
deprecated: flag.deprecated,
deprecateAliases: c.deprecateAliases,
aliases: flag.aliases,
}
} else {
Expand All @@ -775,6 +776,7 @@ export async function toCached(c: Command.Class, plugin?: IPlugin): Promise<Comm
exclusive: flag.exclusive,
default: await defaultToCached(flag),
deprecated: flag.deprecated,
deprecateAliases: c.deprecateAliases,
aliases: flag.aliases,
}
// a command-level placeholder in the manifest so that oclif knows it should regenerate the command during help-time
Expand Down Expand Up @@ -809,6 +811,7 @@ export async function toCached(c: Command.Class, plugin?: IPlugin): Promise<Comm
aliases: c.aliases || [],
examples: c.examples || (c as any).example,
deprecationOptions: c.deprecationOptions,
deprecateAliases: c.deprecateAliases,
flags,
args,
}
Expand Down
2 changes: 1 addition & 1 deletion src/flags.ts
Expand Up @@ -37,7 +37,7 @@ export const help = (opts: Partial<BooleanFlag<boolean>> = {}) => {
description: 'Show CLI help.',
...opts,
parse: async (_: any, cmd: Command) => {
new Help(cmd.config).showHelp(cmd.argv)
new Help(cmd.config).showHelp([cmd.id!, ...cmd.argv])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if it matters or not, but I have seen scenarios where a command doesn't have an ID, particularly when we do weird things with them in UT.

https://github.com/salesforcecli/plugin-source/blob/f5f681b19bcae6a98c25e45e7b461a624f1c2920/test/commands/source/cancel.test.ts#L43

One of those "I'm super cautious about asserting things to TS"

cmd.exit(0)
},
})
Expand Down
2 changes: 1 addition & 1 deletion src/help/index.ts
Expand Up @@ -9,7 +9,7 @@ import {formatCommandDeprecationWarning, getHelpFlagAdditions, standardizeIDFrom
import {HelpFormatter} from './formatter'
import {toCached} from '../config/config'
export {CommandHelp} from './command'
export {standardizeIDFromArgv, loadHelpClass, getHelpFlagAdditions} from './util'
export {standardizeIDFromArgv, loadHelpClass, getHelpFlagAdditions, normalizeArgv} from './util'

function getHelpSubject(args: string[], config: Interfaces.Config): string | undefined {
// for each help flag that starts with '--' create a new flag with same name sans '--'
Expand Down
7 changes: 6 additions & 1 deletion src/help/util.ts
Expand Up @@ -108,7 +108,7 @@ export function formatFlagDeprecationWarning(flag: string, opts: true | Deprecat
message += ` and will be removed in version ${opts.version}`
}

message += opts.to ? `. Use "${opts.to}" instead.` : '.'
message += opts.to ? `. Use "--${opts.to}" instead.` : '.'

return message
}
Expand All @@ -127,3 +127,8 @@ export function formatCommandDeprecationWarning(command: string, opts?: Deprecat

return message
}

export function normalizeArgv(config: IConfig, argv = process.argv.slice(2)): string[] {
if (config.topicSeparator !== ':' && !argv[0]?.includes(':')) argv = standardizeIDFromArgv(argv, config)
return argv
}
5 changes: 5 additions & 0 deletions src/interfaces/command.ts
Expand Up @@ -22,6 +22,11 @@ export interface CommandProps {
*/
deprecationOptions?: Deprecation;

/**
* Emit a deprecation warning when a command alias is used.
*/
deprecateAliases?: boolean

/** An array of aliases for this command. */
aliases: string[];

Expand Down
4 changes: 4 additions & 0 deletions src/interfaces/parser.ts
Expand Up @@ -157,6 +157,10 @@ export type FlagProps = {
* Alternate names that can be used for this flag.
*/
aliases?: string[];
/**
* Emit deprecation warning when a flag alias is provided
*/
deprecateAliases?: boolean
}

export type BooleanFlagProps = FlagProps & {
Expand Down
5 changes: 2 additions & 3 deletions src/main.ts
Expand Up @@ -5,7 +5,7 @@ import {format, inspect} from 'util'
import * as Interfaces from './interfaces'
import {URL} from 'url'
import {Config} from './config'
import {getHelpFlagAdditions, loadHelpClass, standardizeIDFromArgv} from './help'
import {getHelpFlagAdditions, loadHelpClass, normalizeArgv} from './help'

const log = (message = '', ...args: any[]) => {
// tslint:disable-next-line strict-type-predicates
Expand Down Expand Up @@ -41,8 +41,7 @@ export async function run(argv = process.argv.slice(2), options?: Interfaces.Loa
// return Main.run(argv, options)
const config = await Config.load(options || (module.parent && module.parent.parent && module.parent.parent.filename) || __dirname) as Config

if (config.topicSeparator !== ':' && !argv[0]?.includes(':')) argv = standardizeIDFromArgv(argv, config)
let [id, ...argvSlice] = argv
let [id, ...argvSlice] = normalizeArgv(config, argv)
// run init hook
await config.runHook('init', {id, argv: argvSlice})

Expand Down
8 changes: 6 additions & 2 deletions src/parser/parse.ts
Expand Up @@ -79,8 +79,12 @@ export class Parser<T extends ParserInput, TFlags extends OutputFlags<T['flags']
}
}

const findShortFlag = (arg: string) => {
return Object.keys(this.input.flags).find(k => this.input.flags[k].char === arg[1])
const findShortFlag = ([_, char]: string) => {
if (this.flagAliases[char]) {
return this.flagAliases[char].name
}

return Object.keys(this.input.flags).find(k => this.input.flags[k].char === char)
}

const parseFlag = (arg: string): boolean => {
Expand Down