Use of obsolete v1 aliases goes undetected

[TomOnTime]

my urfave/cli version is

2.2.0

Checklist

• Are you running the latest v2 release? The list of releases is here.
• Did you check the manual for your release? The v2 manual is here
• Did you perform a search about this problem? Here's the Github guide about searching.

Dependency Management

• My project is using go modules.

Describe the bug

There is no warning if old-style v1 aliases are used. The alias simply doesn't exist.

To reproduce

Create a flag using the v1 method:

cli.StringFlag{
  Name: "config, cfg"
}

Observed behavior

The flag --config works; --cfg does not.

Expected behavior

The system should output an error or warning.

Additional context

Ideally if Name includes a space or a comma, the code should output an
message about "Old style alias being used. Please see docs/migrate-v1-to-v2.md"
then the program should exit.

Want to fix this yourself?

no

Run go version and paste its output here

go1.14.1

Run go env and paste its output here

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/qw/qp8v2j353wz7q57_jymyxj0h0000gn/T/go-build854616664=/tmp/go-build -gno-record-gcc-switches -fno-common"

[coilysiren]

@TomOnTime can you add a title to the issue?

[stale]

This issue or PR has been automatically marked as stale because it has not had recent activity. Please add a comment bumping this if you're still interested in it's resolution! Thanks for your help, please let us know if you need anything else.

[stale]

Closing this as it has become stale.

[stale]

This issue or PR has been bumped and is no longer marked as stale! Feel free to bump it again in the future, if it's still relevant.

[stale]

This issue or PR has been automatically marked as stale because it has not had recent activity. Please add a comment bumping this if you're still interested in it's resolution! Thanks for your help, please let us know if you need anything else.

[stale]

Closing this as it has become stale.

[meatballhat]

From what I can tell, at the time of v1 ➡️ v2 we made the call to silently trim right rather than break everyone's Name values 🙁

cli/flag.go

Lines 262 to 274 in 6033c00

	func flagNames(name string, aliases []string) []string {
	var ret []string
	for _, part := range append([]string{name}, aliases...) {
	// v1 -> v2 migration warning zone:
	// Strip off anything after the first found comma or space, which
	// *hopefully* makes it a tiny bit more obvious that unexpected behavior is
	// caused by using the v1 form of stringly typed "Name".
	ret = append(ret, commaWhitespace.ReplaceAllString(part, ""))
	}
	return ret
	}

Given that we don't really have a compile-time or type-enforced way of preventing v1 style values, I think I'd like to incorporate some validation feedback as part of reworking Before/After/"lifecycle hooks" (see #1273).

[wxiaoguang]

It's not that difficult but I think the framework should report such low-level mistakes to developers.

Without framework support, I have to do:

func reflectGet(v any, fieldName string) any {
	e := reflect.ValueOf(v).Elem()
	return e.FieldByName(fieldName).Interface()
}

// https://cli.urfave.org/migrate-v1-to-v2/#flag-aliases-are-done-differently
// Sadly v2 doesn't warn you if a comma is in the name. (https://github.com/urfave/cli/issues/1103)
func checkCommandFlags(c any) bool {
	var cmds []*cli.Command
	if app, ok := c.(*cli.App); ok {
		cmds = app.Commands
	} else {
		cmds = c.(*cli.Command).Subcommands
	}
	ok := true
	for _, cmd := range cmds {
		for _, flag := range cmd.Flags {
			flagName := reflectGet(flag, "Name").(string)
			if strings.Contains(flagName, ",") {
				ok = false
				log.Error("cli.Flag can't have comma in its Name: %q, use Aliases instead", flagName)
			}
		}
		if !checkCommandFlags(cmd) {
			ok = false
		}
	}
	return ok
}