Fix infinite loop in options matching (#56)

This is a bug in `optsMatcher` (i.e. `[OPTIONS]`).
This bug happens when:

* At least one option is declared with an env var name
* The said env var is set
* An invalid option name is passed to the `optsMatcher`

The bug explanation:

`optsMatcher` has a `for {}` loop, which runs until no option matches anymore.
Since options set from an env variables would always match, and to avoid
an infinite loop, a fix was added in #50 where an `optMatcher` would add
its option to an exlusion list when set from env, and `optsMatcher`
would skip options in this exclude list.

This fix was not enough: if called with an invalid option, the code path
to exclude an option is not executed.

This fix moves the exclusion of options to `optsMatcher`, which
incidentally is always run, regardless of which code path was followd.

Fixes #55

matchers.go

@@ -86,10 +86,6 @@ func (o *optMatcher) match(args []string, c *parseContext) (bool, []string) {
 			idx += consumed
 
 		default:
-			if o.theOne.valueSetFromEnv {
-				//  exclude opt
-				c.excludedOpts[o.theOne] = struct{}{}
-			}
 			return o.theOne.valueSetFromEnv, args
 		}
 	}
@@ -235,7 +231,10 @@ func (om optsMatcher) try(args []string, c *parseContext) (bool, []string) {
 			continue
 		}
 		if ok, nargs := (&optMatcher{theOne: o, optionsIdx: om.optionsIndex}).match(args, c); ok {
-			return ok, nargs
+			if o.valueSetFromEnv {
+				c.excludedOpts[o] = struct{}{}
+			}
+			return true, nargs
 		}
 	}
 	return false, args

matchers_test.go

@@ -3,6 +3,8 @@ package cli
 import (
 	"testing"
 
+	"time"
+
 	"github.com/stretchr/testify/require"
 )
 
@@ -192,3 +194,34 @@ func TestOptsMatcher(t *testing.T) {
 		require.False(t, nok, "opts shouldn't match when rejectOptions flag is set")
 	}
 }
+
+// Issue 55
+func TestOptsMatcherInfiniteLoop(t *testing.T) {
+	opts := optsMatcher{
+		options: []*opt{
+			{names: []string{"-g"}, value: newStringValue(new(string), ""), valueSetFromEnv: true},
+		},
+		optionsIndex: map[string]*opt{},
+	}
+
+	for _, o := range opts.options {
+		for _, n := range o.names {
+			opts.optionsIndex[n] = o
+		}
+	}
+
+	done := make(chan struct{}, 1)
+	pc := newParseContext()
+	go func() {
+		opts.match([]string{"-x"}, &pc)
+		done <- struct{}{}
+	}()
+
+	select {
+	case <-done:
+		// nop, everything is good
+	case <-time.After(5 * time.Second):
+		t.Fatalf("Timed out after 5 seconds. Infinite loop in optsMatcher.")
+	}
+
+}