Cli: v2 bug: ctx.NArg() panics on Command.BashComplete

[unRob]

Checklist

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

Describe the bug

ctx.flagSet is nil during cli.Command.BashComplete when a malformed flag is passed, either misspelled or with an invalid value, leading to a panic when calling ctx.NArg(). For example:

# these panic
go run main.go test --undefined --generate-bash-completion
go run main.go test --number fourty-two --generate-bash-completion
# these don't panic
go run main.go --he --generate-bash-completion
go run main.go test -- --generate-bash-completion
go run main.go test --number 42 --generate-bash-completion

To reproduce

With released v2.0.0 on go version go1.12.5 darwin/amd64 the following example results in panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x38 pc=0x11752c2]

package main

import (
    "fmt"

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

func main() {
    app := cli.NewApp()
    app.EnableBashCompletion = true

    app.Commands = []*cli.Command{
        &cli.Command{
            Name: "test",
            BashComplete: func(c *cli.Context) {
                fmt.Println(c.NArg())
            },
        },
    }

    app.RunAndExitOnError()
}

Expected behavior

0 output to stdout. Noticed after upgrade to v2.0.0 from d3ae77c26ac8, according to my go.mod.

Additional context

I started working my way through parser.go@06e3d38d88, which is where the nil value for my command's flagSet seems to be last seen, but this is still more of a hunch than a fact. I also feel like there's might be some initialization that I should be doing inside the command's BashComplete that I'm missing?

Complete panic:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x38 pc=0x11752c2]

goroutine 1 [running]:
flag.(*FlagSet).Args(...)
    /usr/local/Cellar/go/1.12.5/libexec/src/flag/flag.go:615
github.com/urfave/cli/v2.(*Context).Args(...)
    /Users/rob/src/go/pkg/mod/github.com/urfave/cli/[email protected]/context.go:124
github.com/urfave/cli/v2.(*Context).NArg(...)
    /Users/rob/src/go/pkg/mod/github.com/urfave/cli/[email protected]/context.go:130
main.main.func1(0xc0000a4500)
    /Users/rob/Work/cli-panic/main.go:17 +0x42
github.com/urfave/cli/v2.ShowCommandCompletions(0xc0000a4500, 0x11d5476, 0x4)
    /Users/rob/src/go/pkg/mod/github.com/urfave/cli/[email protected]/help.go:252 +0x64
github.com/urfave/cli/v2.checkCommandCompletions(...)
    /Users/rob/src/go/pkg/mod/github.com/urfave/cli/[email protected]/help.go:366
github.com/urfave/cli/v2.(*Command).Run(0xc000120000, 0xc0000a4400, 0x1213800, 0xc0000a6c40)
    /Users/rob/src/go/pkg/mod/github.com/urfave/cli/[email protected]/command.go:107 +0x66e
github.com/urfave/cli/v2.(*App).Run(0xc0000ac180, 0xc0000a4000, 0x4, 0x4, 0x0, 0x0)
    /Users/rob/src/go/pkg/mod/github.com/urfave/cli/[email protected]/app.go:295 +0x6c2
github.com/urfave/cli/v2.(*App).RunAndExitOnError(0xc0000ac180)
    /Users/rob/src/go/pkg/mod/github.com/urfave/cli/[email protected]/app.go:316 +0x53
main.main()
    /Users/rob/Work/cli-panic/main.go:22 +0xc1
exit status 2

Want to fix this yourself?

Happy to take a stab at if this is a valid use case, and some guidance on an approach to doing so would be much appreciated! Or maybe this is more of a documentation issue, and I should try something like what's done in DefaultCompleteWithFlags instead of using ctx.NArg()? in which case I can send a PR to the manual.

Run go env and paste its output here

GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/rob/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/rob/src/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.12.5/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.12.5/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/rob/Work/cli-panic/go.mod"
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/mj/xhg1t1sx2tj_2g6pd0snwvf4k7p1w7/T/go-build964702892=/tmp/go-build -gno-record-gcc-switches -fno-common"

[lynncyrin]

🔍 I believe what we want here is a PR to fix the cases that panic. The PR will want to start with writing some test cases.

@unRob thanks for the very detailed issue! Also, you're free to work on the PR, if you're interested 🙏

[unRob]

That's really helpful, thanks @lynncyrin. Submitted #946 to that effect, turns out it was easier than I thought, which probably means we might need to work a bit more on it 🙃.