More cli sanity verifications

cmd/clicheck/check_cli_conventions.go

@@ -31,16 +31,23 @@ var (
 )
 
 func main() {
-	errors := []error{}
+	var errorCount int
 
 	kubectl := cmd.NewKubectlCommand(cmdutil.NewFactory(nil), os.Stdin, ioutil.Discard, ioutil.Discard)
-	result := cmdsanity.CheckCmdTree(kubectl, cmdsanity.AllCmdChecks, []string{})
+	errors := cmdsanity.RunCmdChecks(kubectl, cmdsanity.AllCmdChecks, []string{})
-	errors = append(errors, result...)
+	for _, err := range errors {
+		errorCount++
+		fmt.Fprintf(os.Stderr, "     %d. %s\n", errorCount, err)
+	}
 
-	if len(errors) > 0 {
+	errors = cmdsanity.RunGlobalChecks(cmdsanity.AllGlobalChecks)
-		for i, err := range errors {
+	for _, err := range errors {
-			fmt.Fprintf(os.Stderr, "%d. %s\n\n", i+1, err)
+		errorCount++
-		}
+		fmt.Fprintf(os.Stderr, "     %d. %s\n", errorCount, err)
+	}
+
+	if errorCount > 0 {
+		fmt.Fprintf(os.Stdout, "Found %d errors.\n", errorCount)
 		os.Exit(1)
 	}
 

hack/verify-cli-conventions.sh

@@ -32,9 +32,12 @@ clicheck=$(kube::util::find-binary "clicheck")
 
 if ! output=`$clicheck 2>&1`
 then
-	echo "FAILURE: CLI is not following one or more required conventions:"
 	echo "$output"
+	echo
+	echo "FAILURE: CLI is not following one or more required conventions."
 	exit 1
 else
+	echo "$output"
+	echo
   echo "SUCCESS: CLI is following all tested conventions."
 fi

pkg/kubectl/cmd/util/sanity/BUILD

@@ -15,6 +15,7 @@ go_library(
     deps = [
         "//pkg/kubectl/cmd/templates:go_default_library",
         "//vendor/github.com/spf13/cobra:go_default_library",
+        "//vendor/github.com/spf13/pflag:go_default_library",
     ],
 )
 

pkg/kubectl/cmd/util/sanity/cmd_sanity.go

@@ -19,27 +19,43 @@ package sanity
 import (
 	"fmt"
 	"os"
+	"regexp"
 	"strings"
 
 	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
 	"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
 )
 
 type CmdCheck func(cmd *cobra.Command) []error
+type GlobalCheck func() []error
 
 var (
 	AllCmdChecks = []CmdCheck{
 		CheckLongDesc,
 		CheckExamples,
+		CheckFlags,
+	}
+	AllGlobalChecks = []GlobalCheck{
+		CheckGlobalVarFlags,
 	}
 )
 
-func CheckCmdTree(cmd *cobra.Command, checks []CmdCheck, skip []string) []error {
+func RunGlobalChecks(globalChecks []GlobalCheck) []error {
+	fmt.Fprint(os.Stdout, "---+ RUNNING GLOBAL CHECKS\n")
+	errors := []error{}
+	for _, check := range globalChecks {
+		errors = append(errors, check()...)
+	}
+	return errors
+}
+
+func RunCmdChecks(cmd *cobra.Command, cmdChecks []CmdCheck, skipCmd []string) []error {
 	cmdPath := cmd.CommandPath()
 
-	for _, skipCmdPath := range skip {
+	for _, skipCmdPath := range skipCmd {
 		if cmdPath == skipCmdPath {
-			fmt.Fprintf(os.Stdout, "-----+ skipping command %s\n", cmdPath)
+			fmt.Fprintf(os.Stdout, "---+ skipping command %s\n", cmdPath)
 			return []error{}
 		}
 	}
@@ -48,13 +64,13 @@ func CheckCmdTree(cmd *cobra.Command, checks []CmdCheck, skip []string) []error
 
 	if cmd.HasSubCommands() {
 		for _, subCmd := range cmd.Commands() {
-			errors = append(errors, CheckCmdTree(subCmd, checks, skip)...)
+			errors = append(errors, RunCmdChecks(subCmd, cmdChecks, skipCmd)...)
 		}
 	}
 
-	fmt.Fprintf(os.Stdout, "-----+ checking command %s\n", cmdPath)
+	fmt.Fprintf(os.Stdout, "---+ RUNNING COMMAND CHECKS on %q\n", cmdPath)
 
-	for _, check := range checks {
+	for _, check := range cmdChecks {
 		if err := check(cmd); err != nil && len(err) > 0 {
 			errors = append(errors, err...)
 		}
@@ -64,26 +80,26 @@ func CheckCmdTree(cmd *cobra.Command, checks []CmdCheck, skip []string) []error
 }
 
 func CheckLongDesc(cmd *cobra.Command) []error {
+	fmt.Fprint(os.Stdout, "   ↳ checking long description\n")
 	cmdPath := cmd.CommandPath()
 	long := cmd.Long
 	if len(long) > 0 {
 		if strings.Trim(long, " \t\n") != long {
-			return []error{fmt.Errorf(`command %q: long description is not normalized
+			return []error{fmt.Errorf(`command %q: long description is not normalized, make sure you are calling templates.LongDesc (from pkg/cmd/templates) before assigning cmd.Long`, cmdPath)}
- ↳ make sure you are calling templates.LongDesc (from pkg/cmd/templates) before assigning cmd.Long`, cmdPath)}
 		}
 	}
 	return nil
 }
 
 func CheckExamples(cmd *cobra.Command) []error {
+	fmt.Fprint(os.Stdout, "   ↳ checking examples\n")
 	cmdPath := cmd.CommandPath()
 	examples := cmd.Example
 	errors := []error{}
 	if len(examples) > 0 {
 		for _, line := range strings.Split(examples, "\n") {
 			if !strings.HasPrefix(line, templates.Indentation) {
-				errors = append(errors, fmt.Errorf(`command %q: examples are not normalized
+				errors = append(errors, fmt.Errorf(`command %q: examples are not normalized, make sure you are calling templates.Examples (from pkg/cmd/templates) before assigning cmd.Example`, cmdPath))
- ↳ make sure you are calling templates.Examples (from pkg/cmd/templates) before assigning cmd.Example`, cmdPath))
 			}
 			if trimmed := strings.TrimSpace(line); strings.HasPrefix(trimmed, "//") {
 				errors = append(errors, fmt.Errorf(`command %q: we use # to start comments in examples instead of //`, cmdPath))
@@ -92,3 +108,42 @@ func CheckExamples(cmd *cobra.Command) []error {
 	}
 	return errors
 }
+
+func CheckFlags(cmd *cobra.Command) []error {
+	allFlagsSlice := []*pflag.Flag{}
+
+	cmd.Flags().VisitAll(func(f *pflag.Flag) {
+		allFlagsSlice = append(allFlagsSlice, f)
+	})
+	cmd.PersistentFlags().VisitAll(func(f *pflag.Flag) {
+		allFlagsSlice = append(allFlagsSlice, f)
+	})
+
+	fmt.Fprintf(os.Stdout, "   ↳ checking %d flags\n", len(allFlagsSlice))
+
+	errors := []error{}
+
+	// check flags long names
+	regex, err := regexp.Compile(`^[a-z]+[a-z\-]*$`)
+	if err != nil {
+		errors = append(errors, fmt.Errorf("command %q: unable to compile regex to check flags", cmd.CommandPath()))
+		return errors
+	}
+	for _, flag := range allFlagsSlice {
+		name := flag.Name
+		if !regex.MatchString(name) {
+			errors = append(errors, fmt.Errorf("command %q: flag name %q is invalid, long form of flag names can only contain lowercase characters or dash (must match %v)", cmd.CommandPath(), name, regex))
+		}
+	}
+
+	return errors
+}
+
+func CheckGlobalVarFlags() []error {
+	fmt.Fprint(os.Stdout, "   ↳ checking flags from global vars\n")
+	errors := []error{}
+	pflag.CommandLine.VisitAll(func(f *pflag.Flag) {
+		errors = append(errors, fmt.Errorf("flag %q is invalid, please don't register any flag under the global variable \"CommandLine\"", f.Name))
+	})
+	return errors
+}