cue/cmd: ensure flags are added if they are used

Problem: trim used the dry-run flag but did not add it.

This change ensures that when a value is fetched for any flag, we ensure
that the flag has been added to the flagset. We panic if it has not
been.

Although better than the status quo, this is not quite perfect because
it is only when the value is fetched at runtime that we perform this
checking, and not all code paths require every flag value. So in the
case of trim, if trim exits early due to some bug or divergence, then
the dry-run flag value is never inspected and so we never do this
checking.

Trim has been fixed: it now adds the dry-run flag to the flagSet.

There are quite a few cases of commands "using" flags which have not
been added, much of which is because of some flags being "used" in
common code (e.g. in common.go). So this change whitelists all these
existing uses so that we can slowly work towards eliminating them all,
and ensures the situation doesn't get worse in the mean time.

Change-Id: I21263c4da9f7550e994d4bd4cbe0195f8946b2c3
Signed-off-by: Matthew Sackman <[email protected]>
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1198283
Reviewed-by: Daniel Martí <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>

cmd/cue/cmd/flags.go

@@ -15,6 +15,8 @@
 package cmd
 
 import (
+	"fmt"
+
 	"github.com/spf13/pflag"
 )
 
@@ -114,17 +116,61 @@ func addInjectionFlags(f *pflag.FlagSet, auto, hidden bool) {
 
 type flagName string
 
+type unaddedFlagUse struct {
+	cmd  string
+	flag flagName
+}
+
+// TODO(ms) consider each of these in turn and either remove their
+// use, or add the flag to the flagset.
+var todoUnaddedFlagUse = map[unaddedFlagUse]struct{}{
+	{cmd: "def", flag: flagEscape}:           {},
+	{cmd: "def", flag: flagFiles}:            {},
+	{cmd: "eval", flag: flagEscape}:          {},
+	{cmd: "eval", flag: flagFiles}:           {},
+	{cmd: "eval", flag: flagInlineImports}:   {},
+	{cmd: "export", flag: flagFiles}:         {},
+	{cmd: "export", flag: flagInlineImports}: {},
+	{cmd: "import", flag: flagEscape}:        {},
+	{cmd: "import", flag: flagExpression}:    {},
+	{cmd: "import", flag: flagInlineImports}: {},
+	{cmd: "import", flag: flagOut}:           {},
+	{cmd: "vet", flag: flagEscape}:           {},
+	{cmd: "vet", flag: flagExpression}:       {},
+	{cmd: "vet", flag: flagFiles}:            {},
+	{cmd: "vet", flag: flagForce}:            {},
+	{cmd: "vet", flag: flagInlineImports}:    {},
+	{cmd: "vet", flag: flagOut}:              {},
+	{cmd: "vet", flag: flagOutFile}:          {},
+}
+
+// ensureAdded detects if a flag is being used without it first being
+// added to the flagSet. Because flagNames are global, it is quite
+// easy to accidentally use a flag in a command without adding it to
+// the flagSet.
+func (f flagName) ensureAdded(cmd *Command) {
+	if cmd.Flags().Lookup(string(f)) == nil {
+		if _, found := todoUnaddedFlagUse[unaddedFlagUse{cmd.Name(), f}]; found {
+			return
+		}
+		panic(fmt.Sprintf("Cmd %q uses flag %q without adding it", cmd.Name(), f))
+	}
+}
+
 func (f flagName) Bool(cmd *Command) bool {
+	f.ensureAdded(cmd)
 	v, _ := cmd.Flags().GetBool(string(f))
 	return v
 }
 
 func (f flagName) String(cmd *Command) string {
+	f.ensureAdded(cmd)
 	v, _ := cmd.Flags().GetString(string(f))
 	return v
 }
 
 func (f flagName) StringArray(cmd *Command) []string {
+	f.ensureAdded(cmd)
 	v, _ := cmd.Flags().GetStringArray(string(f))
 	return v
 }

cmd/cue/cmd/trim.go

@@ -87,6 +87,7 @@ removal.
 	}
 
 	addOutFlags(cmd.Flags(), false)
+	cmd.Flags().BoolP(string(flagDryRun), "n", false, "only run simulation")
 
 	return cmd
 }