Missing required flag error uses alias

[nirhaas]

My urfave/cli version is

v2.25.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 <-- The link is broken, but I read the documentation.
• 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

The error message for a missing required flag uses the alias instead of the main flag name, which is not untrue, but confusing.

To reproduce

package main

import (
	"fmt"
	"log"

	"github.com/urfave/cli/v2"
)

func main() {
	app := &cli.App{
		Flags: []cli.Flag{
			&cli.StringFlag{
				Name:     "collection",
				Aliases:  []string{"c"},
				Required: true,
			},
		},
		Action: func(cCtx *cli.Context) error {
			fmt.Println(cCtx.String("collection"))
			return nil
		},
	}

	if err := app.Run([]string{""}); err != nil {
		log.Fatal(err)
	}
}

Observed behavior

Required flag "c" not set

Expected behavior

Required flag "collection" not set

Additional context

It seems like checkRequiredFlags is iterating over the the flag names (which is the append(name, aliases...)) and sets flagName = key. After the loop is over, flagName is the last alias.

Want to fix this yourself?

I can submit a PR if you want, but according to the guidelines you are not accepting v2 fixes. Let me know if this is ok, and I'll move forward with a fix.

A fix could be:

func (cCtx *Context) checkRequiredFlags(flags []Flag) requiredFlagsErr {
	var missingFlags []string
	for _, f := range flags {
		if rf, ok := f.(RequiredFlag); ok && rf.IsRequired() {
			var flagPresent bool
			var flagName string

			flagNames := f.Names()
			flagName = flagNames[0]

			for _, key := range flagNames {
				if cCtx.IsSet(strings.TrimSpace(key)) {
					flagPresent = true
				}
			}

			if !flagPresent && flagName != "" {
				missingFlags = append(missingFlags, flagName)
			}
		}
	}

	if len(missingFlags) != 0 {
		return &errRequiredFlags{missingFlags: missingFlags}
	}

	return nil
}

Run go version and paste its output here

go version go1.20.2 darwin/amd64

Run go env and paste its output here

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/nhaas/Library/Caches/go-build"
GOENV="/Users/nhaas/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/nhaas/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/nhaas/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.20.2"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK="/Users/something"
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/t8/83vffqv11_bbfxbbdc3fsfx40000gn/T/go-build1340975921=/tmp/go-build -gno-record-gcc-switches -fno-common"

[dearchap]

@nirhaas The behaviour you requested seems to make sense. Please submit a PR against v2. I'll be happy to review and merge. Thank you

[nirhaas]

@dearchap #1709 is ready for review. Thanks.

[nirhaas]

Fixed by #1709