Skip to content

feat: add Deprecated field to flags and commands #2429

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.

Sign up for GitHub

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

Open
anandghegde wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: add Deprecated field to flags and commands #2429

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ type Command struct {
InvalidFlagAccessHandler InvalidFlagAccessFunc `json:"-"`
// Boolean to hide this command from help or completion
Hidden bool `json:"hidden"`
// Deprecation message for this command. If non-empty, a warning is
// printed to ErrWriter when the command is invoked. Setting it does not
// hide the command; set Hidden as well for that.
Deprecated string `json:"deprecated"`
// List of all authors who contributed (string or fmt.Stringer)
// TODO: ~string | fmt.Stringer when interface unions are available
Authors []any `json:"authors"`
Expand Down
28 changes: 28 additions & 0 deletions command_run.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,8 @@ func (cmd *Command) run(ctx context.Context, osArgs []string) (_ context.Context
// First, resolve the chain of nested commands up to the parent.
cmdChain := commandChain(cmd)

printDeprecationWarnings(cmdChain)

// Run ArgValidator from the nearest ancestor that sets one.
if validator := findArgValidator(cmd); validator != nil {
if err := validator(ctx, cmd); err != nil {
Expand Down Expand Up @@ -421,6 +423,32 @@ func commandChain(cmd *Command) []*Command {
return cmdChain
}

// printDeprecationWarnings writes a warning to the root ErrWriter for each
// deprecated command in the chain and for each deprecated flag that has been
// set, whether on the command line or from one of its Sources.
func printDeprecationWarnings(cmdChain []*Command) {
w := cmdChain[0].ErrWriter
warned := map[Flag]struct{}{}
for _, cmd := range cmdChain {
if cmd.Deprecated != "" {
fmt.Fprintf(w, "Command %q is deprecated, %s\n", cmd.Name, cmd.Deprecated)
}
for _, fl := range cmd.appliedFlags {
if _, inSet := cmd.setFlags[fl]; !inSet {
continue
}
if _, done := warned[fl]; done {
continue
}
if df, ok := fl.(DeprecatedFlag); ok && df.GetDeprecated() != "" {
warned[fl] = struct{}{}
name := fl.Names()[0]
fmt.Fprintf(w, "Flag %s%s has been deprecated, %s\n", prefixFor(name), name, df.GetDeprecated())
}
}
}
}

func findArgValidator(cmd *Command) ArgValidatorFunc {
for c := cmd; c != nil; c = c.parent {
if c.ArgValidator != nil {
Expand Down
160 changes: 160 additions & 0 deletions command_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5410,6 +5410,7 @@ func TestJSONExportCommand(t *testing.T) {
"usage": "",
"required": false,
"hidden": false,
"deprecated": "",
"hideDefault": false,
"local": false,
"defaultValue": "",
Expand All @@ -5431,6 +5432,7 @@ func TestJSONExportCommand(t *testing.T) {
"usage": "some usage text",
"required": false,
"hidden": false,
"deprecated": "",
"hideDefault": false,
"local": false,
"defaultValue": false,
Expand All @@ -5449,6 +5451,7 @@ func TestJSONExportCommand(t *testing.T) {
"hideHelpCommand": false,
"hideVersion": false,
"hidden": false,
"deprecated": "",
"authors": null,
"copyright": "",
"metadata": null,
Expand All @@ -5474,6 +5477,7 @@ func TestJSONExportCommand(t *testing.T) {
"usage": "",
"required": false,
"hidden": false,
"deprecated": "",
"hideDefault": false,
"local": false,
"defaultValue": "",
Expand All @@ -5495,6 +5499,7 @@ func TestJSONExportCommand(t *testing.T) {
"usage": "another usage text",
"required": false,
"hidden": false,
"deprecated": "",
"hideDefault": false,
"local": false,
"defaultValue": false,
Expand All @@ -5513,6 +5518,7 @@ func TestJSONExportCommand(t *testing.T) {
"hideHelpCommand": false,
"hideVersion": false,
"hidden": false,
"deprecated": "",
"authors": null,
"copyright": "",
"metadata": null,
Expand Down Expand Up @@ -5548,6 +5554,7 @@ func TestJSONExportCommand(t *testing.T) {
"hideHelpCommand": false,
"hideVersion": false,
"hidden": false,
"deprecated": "",
"authors": null,
"copyright": "",
"metadata": null,
Expand Down Expand Up @@ -5580,6 +5587,7 @@ func TestJSONExportCommand(t *testing.T) {
"hideHelpCommand": false,
"hideVersion": false,
"hidden": false,
"deprecated": "",
"authors": null,
"copyright": "",
"metadata": null,
Expand Down Expand Up @@ -5615,6 +5623,7 @@ func TestJSONExportCommand(t *testing.T) {
"usage": "",
"required": false,
"hidden": false,
"deprecated": "",
"hideDefault": false,
"local": false,
"defaultValue": false,
Expand All @@ -5631,6 +5640,7 @@ func TestJSONExportCommand(t *testing.T) {
"hideHelpCommand": false,
"hideVersion": false,
"hidden": true,
"deprecated": "",
"authors": null,
"copyright": "",
"metadata": null,
Expand Down Expand Up @@ -5681,6 +5691,7 @@ func TestJSONExportCommand(t *testing.T) {
"usage": "some usage text",
"required": false,
"hidden": false,
"deprecated": "",
"hideDefault": false,
"local": false,
"defaultValue": false,
Expand All @@ -5699,6 +5710,7 @@ func TestJSONExportCommand(t *testing.T) {
"hideHelpCommand": false,
"hideVersion": false,
"hidden": false,
"deprecated": "",
"authors": null,
"copyright": "",
"metadata": null,
Expand All @@ -5724,6 +5736,7 @@ func TestJSONExportCommand(t *testing.T) {
"usage": "",
"required": false,
"hidden": false,
"deprecated": "",
"hideDefault": false,
"local": false,
"defaultValue": "",
Expand All @@ -5745,6 +5758,7 @@ func TestJSONExportCommand(t *testing.T) {
"usage": "another usage text",
"required": false,
"hidden": false,
"deprecated": "",
"hideDefault": false,
"local": false,
"defaultValue": false,
Expand All @@ -5763,6 +5777,7 @@ func TestJSONExportCommand(t *testing.T) {
"hideHelpCommand": false,
"hideVersion": false,
"hidden": false,
"deprecated": "",
"authors": null,
"copyright": "",
"metadata": null,
Expand All @@ -5788,6 +5803,7 @@ func TestJSONExportCommand(t *testing.T) {
"usage": "some 'usage' text",
"required": false,
"hidden": false,
"deprecated": "",
"hideDefault": false,
"local": false,
"defaultValue": "value",
Expand All @@ -5808,6 +5824,7 @@ func TestJSONExportCommand(t *testing.T) {
"usage": "",
"required": false,
"hidden": false,
"deprecated": "",
"hideDefault": false,
"local": false,
"defaultValue": "",
Expand All @@ -5829,6 +5846,7 @@ func TestJSONExportCommand(t *testing.T) {
"usage": "another usage text",
"required": false,
"hidden": false,
"deprecated": "",
"hideDefault": false,
"local": false,
"defaultValue": false,
Expand All @@ -5849,6 +5867,7 @@ func TestJSONExportCommand(t *testing.T) {
"usage": "",
"required": false,
"hidden": true,
"deprecated": "",
"hideDefault": false,
"local": false,
"defaultValue": false,
Expand All @@ -5865,6 +5884,7 @@ func TestJSONExportCommand(t *testing.T) {
"hideHelpCommand": false,
"hideVersion": false,
"hidden": false,
"deprecated": "",
"authors": [
"Harrison <harrison@lolwut.example.com>",
{
Expand Down Expand Up @@ -6612,3 +6632,143 @@ func TestRunWithNoOsArgs(t *testing.T) {
})
}
}

func TestCommand_Deprecated(t *testing.T) {
newCmd := func() *Command {
return &Command{
Name: "app",
Flags: []Flag{
&StringFlag{Name: "old-flag", Aliases: []string{"o"}, Deprecated: "use --new-flag instead"},
&StringFlag{Name: "x", Deprecated: "use --new-flag instead"},
&StringFlag{Name: "env-flag", Sources: EnvVars("APP_ENV_FLAG"), Deprecated: "use --new-flag instead"},
&BoolWithInverseFlag{Name: "color", Deprecated: "it is always on"},
&StringFlag{Name: "new-flag"},
},
Commands: []*Command{
{
Name: "old",
Deprecated: "use \"new\" instead",
},
{
Name: "new",
},
{
Name: "group",
Deprecated: "use \"new\" instead",
Commands: []*Command{
{Name: "sub"},
},
},
},
Action: func(context.Context, *Command) error { return nil },
}
}

tests := []struct {
name string
args []string
env map[string]string
wantErr string
}{
{
name: "nothing deprecated used",
args: []string{"app", "--new-flag", "v", "new"},
},
{
name: "deprecated command",
args: []string{"app", "old"},
wantErr: "Command \"old\" is deprecated, use \"new\" instead\n",
},
{
name: "deprecated parent command",
args: []string{"app", "group", "sub"},
wantErr: "Command \"group\" is deprecated, use \"new\" instead\n",
},
{
name: "deprecated flag",
args: []string{"app", "--old-flag", "v"},
wantErr: "Flag --old-flag has been deprecated, use --new-flag instead\n",
},
{
name: "deprecated flag set by alias",
args: []string{"app", "-o", "v"},
wantErr: "Flag --old-flag has been deprecated, use --new-flag instead\n",
},
{
name: "deprecated short flag",
args: []string{"app", "-x", "v"},
wantErr: "Flag -x has been deprecated, use --new-flag instead\n",
},
{
name: "deprecated inverse bool flag",
args: []string{"app", "--no-color"},
wantErr: "Flag --color has been deprecated, it is always on\n",
},
{
name: "deprecated flag set from env",
args: []string{"app"},
env: map[string]string{"APP_ENV_FLAG": "v"},
wantErr: "Flag --env-flag has been deprecated, use --new-flag instead\n",
},
{
name: "deprecated persistent flag set on subcommand",
args: []string{"app", "new", "--old-flag", "v"},
wantErr: "Flag --old-flag has been deprecated, use --new-flag instead\n",
},
{
name: "deprecated persistent flag set on parent and subcommand warns once",
args: []string{"app", "--old-flag", "v", "new", "--old-flag", "w"},
wantErr: "Flag --old-flag has been deprecated, use --new-flag instead\n",
},
{
name: "deprecated command and flag",
args: []string{"app", "old", "--old-flag", "v"},
wantErr: "Command \"old\" is deprecated, use \"new\" instead\n" +
"Flag --old-flag has been deprecated, use --new-flag instead\n",
},
{
name: "help for deprecated command",
args: []string{"app", "old", "--help"},
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
for k, v := range test.env {
t.Setenv(k, v)
}

var errBuf bytes.Buffer
cmd := newCmd()
cmd.Writer = io.Discard
cmd.ErrWriter = &errBuf

require.NoError(t, cmd.Run(buildTestContext(t), test.args))
assert.Equal(t, test.wantErr, errBuf.String())
})
}
}

func TestCommand_DeprecatedStillRuns(t *testing.T) {
var got string
cmd := &Command{
Name: "app",
ErrWriter: io.Discard,
Commands: []*Command{
{
Name: "old",
Deprecated: "use \"new\" instead",
Flags: []Flag{
&StringFlag{Name: "old-flag", Deprecated: "use --new-flag instead"},
},
Action: func(_ context.Context, cmd *Command) error {
got = cmd.String("old-flag")
return nil
},
},
},
}

require.NoError(t, cmd.Run(buildTestContext(t), []string{"app", "old", "--old-flag", "v"}))
assert.Equal(t, "v", got)
}
Loading