diff --git a/commands/deploy_command_test.go b/commands/deploy_command_test.go index 75a8b47..b35a425 100644 --- a/commands/deploy_command_test.go +++ b/commands/deploy_command_test.go @@ -207,23 +207,35 @@ var _ = Describe("DeployCommand", func() { command.FileUrlReadTimeout = time.Second }) - // unknown flag - error - Context("with argument that is not a directory or MTA", func() { + // with argument that is not a directory or MTA and a valid and unknown flag (unknown flag - error) + Context("with argument that is not a directory or MTA and a valid and unknown flag", func() { It("should print incorrect usage, call cf help, and exit with a non-zero status", func() { output, status := oc.CaptureOutputAndStatus(func() int { return command.Execute([]string{"x", "-l"}).ToInt() }) - ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flag") + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: -l") Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) }) }) + // with argument that is a directory or MTA and with unknown flag (unknown flag - error) Context("with argument that is a directory or MTA and with unknown flag", func() { It("should print incorrect usage, call cf help, and exit with a non-zero status", func() { output, status := oc.CaptureOutputAndStatus(func() int { return command.Execute([]string{mtaArchivePath, "-l"}).ToInt() }) - ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flag") + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: -l") + Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) + }) + }) + + // with argument that is a directory or MTA and with unknown flags - some even duplicated (unknown flag - error) + Context("with argument that is a directory or MTA and with unknown flags - some even duplicated", func() { + It("should print incorrect usage (and print the duplicating flags only once), call cf help, and exit with a non-zero status", func() { + output, status := oc.CaptureOutputAndStatus(func() int { + return command.Execute([]string{mtaArchivePath, "-nonValidFlag", "-u", "-nonValidFlag", "-nonValidFlag", "--flagInvalid"}).ToInt() + }) + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: -nonValidFlag, --flagInvalid") Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) }) }) @@ -276,7 +288,40 @@ var _ = Describe("DeployCommand", func() { output, status := oc.CaptureOutputAndStatus(func() int { return command.Execute([]string{mtaArchivePath, "--strategy"}).ToInt() }) - ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flag") + ex.ExpectFailure(status, output, "Incorrect usage. Parsing of arguments has failed") + Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) + }) + }) + + // more than one unknown flag and one valid flag, which is a boolean, but its value is missing + Context("with argument that is a directory or MTA and with more than one unknown flag and one valid flag, which is a boolean, but its value is missing", func() { + It("should print incorrect usage, call cf help, and exit with a non-zero status", func() { + output, status := oc.CaptureOutputAndStatus(func() int { + return command.Execute([]string{mtaArchivePath, "--nonValidFlag", "--apply-namespace-app-names", "-notAValidOne", "-not-a-valid-one"}).ToInt() + }) + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: --nonValidFlag, -notAValidOne, -not-a-valid-one") + Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) + }) + }) + + // more than one unknown flag and one valid flag, which is a boolean and its value is set + Context("with argument that is a directory or MTA and with more than one unknown flag and one valid flag, which is a boolean and its value is set", func() { + It("should print incorrect usage, call cf help, and exit with a non-zero status", func() { + output, status := oc.CaptureOutputAndStatus(func() int { + return command.Execute([]string{mtaArchivePath, "--nonValidFlag", "--apply-namespace-app-names", "true", "-notAValidOne", "-not-a-valid-one"}).ToInt() + }) + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: --nonValidFlag, -notAValidOne, -not-a-valid-one") + Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) + }) + }) + + // more than one unknown flag and one valid flag that is not boolean and has a value set + Context("with argument that is a directory or MTA and with more than one unknown flag and one valid flag that is not boolean and has a value set", func() { + It("should print incorrect usage, call cf help, and exit with a non-zero status", func() { + output, status := oc.CaptureOutputAndStatus(func() int { + return command.Execute([]string{mtaArchivePath, "--nonValidFlag", "--apps-stage-timeout", "8000", "-notAValidOne", "-inavalid-flag"}).ToInt() + }) + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: --nonValidFlag, -notAValidOne, -inavalid-flag") Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) }) }) diff --git a/commands/download_mta_op_logs_command_test.go b/commands/download_mta_op_logs_command_test.go index 54f58e5..e246b6c 100644 --- a/commands/download_mta_op_logs_command_test.go +++ b/commands/download_mta_op_logs_command_test.go @@ -88,7 +88,18 @@ var _ = Describe("DownloadMtaOperationLogsCommand", func() { output, status := oc.CaptureOutputAndStatus(func() int { return command.Execute([]string{"-a"}).ToInt() }) - ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flag") + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: -a") + Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) + }) + }) + + // with an unknown flag and one valid flag + Context("with an unknown flag and one valid flag", func() { + It("should print incorrect usage, call cf help, and exit with a non-zero status", func() { + output, status := oc.CaptureOutputAndStatus(func() int { + return command.Execute([]string{"-a", "--mta"}).ToInt() + }) + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: -a") Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) }) }) diff --git a/commands/flags_parser.go b/commands/flags_parser.go index 84bb25f..57d55bd 100644 --- a/commands/flags_parser.go +++ b/commands/flags_parser.go @@ -88,7 +88,11 @@ func (p DefaultCommandFlagsParser) ParseFlags(flags *flag.FlagSet, args []string // Parse the arguments err := flags.Parse(args[positionalArgsCount:]) if err != nil { - return errors.New("Unknown or wrong flag") + if unknownFlags := collectUnknownFlags(flags, args); len(unknownFlags) > 0 { + return fmt.Errorf("Unknown or wrong flags: %s", strings.Join(unknownFlags, ", ")) + } else { + return errors.New("Parsing of arguments has failed") + } } // Check for wrong arguments @@ -124,3 +128,67 @@ func (v DefaultCommandFlagsValidator) ValidateParsedFlags(flags *flag.FlagSet) e return nil } + +func collectUnknownFlags(flags *flag.FlagSet, args []string) []string { + var unknownFlags []string + alreadySeenUnknownFLags := make(map[string]int) + + for i := 0; i < len(args); i++ { + currentArgument := args[i] + + if !strings.HasPrefix(currentArgument, "-") { + continue + } + + currentFlag := currentArgument + flagName := strings.TrimLeft(currentFlag, "-") + + if flagName == "" { + continue + } + + isFlagKnown := flags.Lookup(flagName) + if isFlagKnown != nil { + isBoolean := isBoolFlag(isFlagKnown) + if !isBoolean { + i = tryToGetNext(args, i) + } + continue + } + + appendOnlyWhenCountIsOne(alreadySeenUnknownFLags, currentFlag, &unknownFlags) + i = tryToGetNext(args, i) + } + + return unknownFlags +} + +func isBoolFlag(flag *flag.Flag) bool { + type boolFlagInterface interface{ IsBoolFlag() bool } + + boolFlag, isInterfaceImplemented := flag.Value.(boolFlagInterface) + if !isInterfaceImplemented { + return false + } + + return boolFlag.IsBoolFlag() +} + +func tryToGetNext(args []string, currentIndex int) int { + nextIndex := currentIndex + 1 + if nextIndex < len(args) { + nextArgument := args[nextIndex] + nextHasPrefixDash := strings.HasPrefix(nextArgument, "-") + if !nextHasPrefixDash { + return nextIndex + } + } + return currentIndex +} + +func appendOnlyWhenCountIsOne(alreadySeenUnknownFLags map[string]int, currentFlag string, unknownFlags *[]string) { + alreadySeenUnknownFLags[currentFlag]++ + if alreadySeenUnknownFLags[currentFlag] == 1 { + *unknownFlags = append(*unknownFlags, currentFlag) + } +} diff --git a/commands/mta_command_test.go b/commands/mta_command_test.go index d6fa0d3..d6bbd59 100644 --- a/commands/mta_command_test.go +++ b/commands/mta_command_test.go @@ -90,6 +90,17 @@ var _ = Describe("MtaCommand", func() { }) }) + // with unknown flags and one valid flag - error + Context("with unknown flags and one valid flag", func() { + It("should print incorrect usage, call cf help, and exit with a non-zero status", func() { + output, status := oc.CaptureOutputAndStatus(func() int { + return command.Execute([]string{"fakeMta", "--nonValidFlag", "--namespace", "-u"}).ToInt() + }) + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: --nonValidFlag") + Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) + }) + }) + // can't connect to backend - error Context("when can't connect to backend", func() { const host = "x" diff --git a/commands/mta_operations_command_test.go b/commands/mta_operations_command_test.go index 008f257..9721d8e 100644 --- a/commands/mta_operations_command_test.go +++ b/commands/mta_operations_command_test.go @@ -56,17 +56,29 @@ var _ = Describe("MtaOperationsCommand", func() { command.InitializeAll(name, cliConnection, testutil.NewCustomTransport(200), clientFactory, testTokenFactory, deployServiceURLCalculator) }) + // with an unknown flag - error Context("with an unknown flag", func() { It("should print incorrect usage, call cf help, and exit with a non-zero status", func() { output, status := oc.CaptureOutputAndStatus(func() int { return command.Execute([]string{"-a"}).ToInt() }) - ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flag") + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: -a") Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) }) }) - // // wrong arguments + // with an unknown flag and one valid flag + Context("with an unknown flag and one valid flag", func() { + It("should print incorrect usage, call cf help, and exit with a non-zero status", func() { + output, status := oc.CaptureOutputAndStatus(func() int { + return command.Execute([]string{"-a", "--last"}).ToInt() + }) + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: -a") + Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) + }) + }) + + // wrong arguments Context("with wrong arguments", func() { It("should print incorrect usage, call cf help, and exit with a non-zero status", func() { output, status := oc.CaptureOutputAndStatus(func() int { diff --git a/commands/mtas_command_test.go b/commands/mtas_command_test.go index 6397f75..4c216b2 100644 --- a/commands/mtas_command_test.go +++ b/commands/mtas_command_test.go @@ -60,13 +60,24 @@ var _ = Describe("MtasCommand", func() { command.InitializeAll(name, cliConnection, testutil.NewCustomTransport(200), clientFactory, testTokenFactory, deployServiceURLCalculator) }) - // unknown flag - error + // with an unknown flag - error Context("with an unknown flag", func() { It("should print incorrect usage, call cf help, and exit with a non-zero status", func() { output, status := oc.CaptureOutputAndStatus(func() int { return command.Execute([]string{"-a"}).ToInt() }) - ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flag") + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: -a") + Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) + }) + }) + + // with an unknown flag and one valid flag + Context("with an unknown flag and one valid flag", func() { + It("should print incorrect usage, call cf help, and exit with a non-zero status", func() { + output, status := oc.CaptureOutputAndStatus(func() int { + return command.Execute([]string{"-nonValidFlag", "-u"}).ToInt() + }) + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: -nonValidFlag") Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) }) }) diff --git a/commands/purge_config_command_test.go b/commands/purge_config_command_test.go index 90d367c..30f0b57 100644 --- a/commands/purge_config_command_test.go +++ b/commands/purge_config_command_test.go @@ -54,7 +54,17 @@ var _ = Describe("PurgeConfigCommand", func() { output, status := oc.CaptureOutputAndStatus(func() int { return command.Execute([]string{"-a"}).ToInt() }) - ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flag") + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: -a") + Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) + }) + }) + + Context("with an unknown flag and one valid flag", func() { + It("should print incorrect usage, call cf help, and exit with a non-zero status", func() { + output, status := oc.CaptureOutputAndStatus(func() int { + return command.Execute([]string{"-a", "-u"}).ToInt() + }) + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: -a") Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) }) }) diff --git a/commands/undeploy_command_test.go b/commands/undeploy_command_test.go index 8c5f16b..0e64ae1 100644 --- a/commands/undeploy_command_test.go +++ b/commands/undeploy_command_test.go @@ -101,7 +101,7 @@ var _ = Describe("UndeployCommand", func() { output, status := oc.CaptureOutputAndStatus(func() int { return command.Execute([]string{"test-mta-id", "-unknown-flag"}).ToInt() }) - ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flag") + ex.ExpectFailure(status, output, "Incorrect usage. Unknown or wrong flags: -unknown-flag") Expect(cliConnection.CliCommandArgsForCall(0)).To(Equal([]string{"help", name})) }) })