From 5c2c1d627d35a00153764a3d37400efc66eaca1c Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Sat, 18 May 2024 16:41:31 +0300 Subject: [PATCH 1/3] Consistent annotation names (#2140) Add `Annotation` suffix to the private annotations to allow nicer code using the constants. For example one can use the current annotation names as a temporary variable instead of unclear shortcut. Instead of this: rag := flagsFromAnnotation(c, f, requiredAsGroup) me := flagsFromAnnotation(c, f, mutuallyExclusive) or := flagsFromAnnotation(c, f, oneRequired) We can use now: requiredAsGrop := flagsFromAnnotation(c, f, requiredAsGroupAnnotation) mutuallyExclusive := flagsFromAnnotation(c, f, mutuallyExclusiveAnnotation) oneRequired := flagsFromAnnotation(c, f, oneRequiredAnnotation) Example taken from #2105. --- flag_groups.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/flag_groups.go b/flag_groups.go index 2be3b18b..560612fd 100644 --- a/flag_groups.go +++ b/flag_groups.go @@ -23,9 +23,9 @@ import ( ) const ( - requiredAsGroup = "cobra_annotation_required_if_others_set" - oneRequired = "cobra_annotation_one_required" - mutuallyExclusive = "cobra_annotation_mutually_exclusive" + requiredAsGroupAnnotation = "cobra_annotation_required_if_others_set" + oneRequiredAnnotation = "cobra_annotation_one_required" + mutuallyExclusiveAnnotation = "cobra_annotation_mutually_exclusive" ) // MarkFlagsRequiredTogether marks the given flags with annotations so that Cobra errors @@ -37,7 +37,7 @@ func (c *Command) MarkFlagsRequiredTogether(flagNames ...string) { if f == nil { panic(fmt.Sprintf("Failed to find flag %q and mark it as being required in a flag group", v)) } - if err := c.Flags().SetAnnotation(v, requiredAsGroup, append(f.Annotations[requiredAsGroup], strings.Join(flagNames, " "))); err != nil { + if err := c.Flags().SetAnnotation(v, requiredAsGroupAnnotation, append(f.Annotations[requiredAsGroupAnnotation], strings.Join(flagNames, " "))); err != nil { // Only errs if the flag isn't found. panic(err) } @@ -53,7 +53,7 @@ func (c *Command) MarkFlagsOneRequired(flagNames ...string) { if f == nil { panic(fmt.Sprintf("Failed to find flag %q and mark it as being in a one-required flag group", v)) } - if err := c.Flags().SetAnnotation(v, oneRequired, append(f.Annotations[oneRequired], strings.Join(flagNames, " "))); err != nil { + if err := c.Flags().SetAnnotation(v, oneRequiredAnnotation, append(f.Annotations[oneRequiredAnnotation], strings.Join(flagNames, " "))); err != nil { // Only errs if the flag isn't found. panic(err) } @@ -70,7 +70,7 @@ func (c *Command) MarkFlagsMutuallyExclusive(flagNames ...string) { panic(fmt.Sprintf("Failed to find flag %q and mark it as being in a mutually exclusive flag group", v)) } // Each time this is called is a single new entry; this allows it to be a member of multiple groups if needed. - if err := c.Flags().SetAnnotation(v, mutuallyExclusive, append(f.Annotations[mutuallyExclusive], strings.Join(flagNames, " "))); err != nil { + if err := c.Flags().SetAnnotation(v, mutuallyExclusiveAnnotation, append(f.Annotations[mutuallyExclusiveAnnotation], strings.Join(flagNames, " "))); err != nil { panic(err) } } @@ -91,9 +91,9 @@ func (c *Command) ValidateFlagGroups() error { oneRequiredGroupStatus := map[string]map[string]bool{} mutuallyExclusiveGroupStatus := map[string]map[string]bool{} flags.VisitAll(func(pflag *flag.Flag) { - processFlagForGroupAnnotation(flags, pflag, requiredAsGroup, groupStatus) - processFlagForGroupAnnotation(flags, pflag, oneRequired, oneRequiredGroupStatus) - processFlagForGroupAnnotation(flags, pflag, mutuallyExclusive, mutuallyExclusiveGroupStatus) + processFlagForGroupAnnotation(flags, pflag, requiredAsGroupAnnotation, groupStatus) + processFlagForGroupAnnotation(flags, pflag, oneRequiredAnnotation, oneRequiredGroupStatus) + processFlagForGroupAnnotation(flags, pflag, mutuallyExclusiveAnnotation, mutuallyExclusiveGroupStatus) }) if err := validateRequiredFlagGroups(groupStatus); err != nil { @@ -232,9 +232,9 @@ func (c *Command) enforceFlagGroupsForCompletion() { oneRequiredGroupStatus := map[string]map[string]bool{} mutuallyExclusiveGroupStatus := map[string]map[string]bool{} c.Flags().VisitAll(func(pflag *flag.Flag) { - processFlagForGroupAnnotation(flags, pflag, requiredAsGroup, groupStatus) - processFlagForGroupAnnotation(flags, pflag, oneRequired, oneRequiredGroupStatus) - processFlagForGroupAnnotation(flags, pflag, mutuallyExclusive, mutuallyExclusiveGroupStatus) + processFlagForGroupAnnotation(flags, pflag, requiredAsGroupAnnotation, groupStatus) + processFlagForGroupAnnotation(flags, pflag, oneRequiredAnnotation, oneRequiredGroupStatus) + processFlagForGroupAnnotation(flags, pflag, mutuallyExclusiveAnnotation, mutuallyExclusiveGroupStatus) }) // If a flag that is part of a group is present, we make all the other flags From 8003b74a10ef0d0d84fe3c408d3939d86fdeb210 Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Sun, 19 May 2024 05:12:02 +0300 Subject: [PATCH 2/3] Remove fully inactivated linters (#2148) * Remove fully inactivated linters Currently golangci-lint fails with these errors: ERRO [linters_context] golint: This linter is fully inactivated: it will not produce any reports. ERRO [linters_context] interfacer: This linter is fully inactivated: it will not produce any reports. ERRO [linters_context] maligned: This linter is fully inactivated: it will not produce any reports. I could not find any docs explaining what "fully inactivated" mean, but based this PR[1] it seems that these linters do nothing now. Removing the linters fixes this issue without changing linting, as they did not produce any report. Looking in the linters docs[2] I did not find a replacement for "interfacer" and "malinged" linters. "stylecheck" seems to be a replacement for "golint", but we need to fix the code to enable it. [1] https://github.com/golangci/golangci-lint/pull/4436 [2] https://golangci-lint.run/usage/linters/ * Add stylecheck linter, replacement for golint This revealed 2 capitalized error messages. https://golangci-lint.run/usage/linters/#stylecheck --- .golangci.yml | 5 +---- command.go | 2 +- completions.go | 2 +- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 22ae622e..7b6d3021 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -33,16 +33,13 @@ linters: #- gocyclo #- gofmt - goimports - - golint #- gomnd #- goprintffuncname #- gosec - gosimple - govet - ineffassign - - interfacer #- lll - - maligned - megacheck #- misspell #- nakedret @@ -52,7 +49,7 @@ linters: #- scopelint #- staticcheck #- structcheck ! deprecated since v1.49.0; replaced by 'unused' - #- stylecheck + - stylecheck #- typecheck - unconvert #- unparam diff --git a/command.go b/command.go index b6f8f4b1..b31e22c8 100644 --- a/command.go +++ b/command.go @@ -875,7 +875,7 @@ func (c *Command) ArgsLenAtDash() int { func (c *Command) execute(a []string) (err error) { if c == nil { - return fmt.Errorf("Called Execute() on a nil Command") + return fmt.Errorf("called Execute() on a nil Command") } if len(c.Deprecated) > 0 { diff --git a/completions.go b/completions.go index ad7b6d0a..c0c08b05 100644 --- a/completions.go +++ b/completions.go @@ -298,7 +298,7 @@ func (c *Command) getCompletions(args []string) (*Command, []string, ShellCompDi } if err != nil { // Unable to find the real command. E.g., someInvalidCmd - return c, []string{}, ShellCompDirectiveDefault, fmt.Errorf("Unable to find a command for arguments: %v", trimmedArgs) + return c, []string{}, ShellCompDirectiveDefault, fmt.Errorf("unable to find a command for arguments: %v", trimmedArgs) } finalCmd.ctx = c.ctx From e94f6d0dd9a5e5738dca6bce03c4b1207ffbc0ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Skytt=C3=A4?= Date: Sat, 1 Jun 2024 13:31:11 +0300 Subject: [PATCH 3/3] Address golangci-lint deprecation warnings, enable some more linters (#2152) * Address golangci-lint linter deprecation warnings 1.59.0 outputs: WARN [lintersdb] The name "gas" is deprecated. The linter has been renamed to: gosec. WARN [lintersdb] The linter named "megacheck" is deprecated. It has been split into: gosimple, staticcheck, unused. * Enable some more linters, address finding --- .golangci.yml | 12 +++++------- command.go | 1 - 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 7b6d3021..2c8f4808 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -26,28 +26,26 @@ linters: - errcheck #- exhaustive #- funlen - - gas #- gochecknoinits - goconst - gocritic #- gocyclo - #- gofmt + - gofmt - goimports #- gomnd #- goprintffuncname - #- gosec + - gosec - gosimple - govet - ineffassign #- lll - - megacheck - #- misspell + - misspell #- nakedret #- noctx - #- nolintlint + - nolintlint #- rowserrcheck #- scopelint - #- staticcheck + - staticcheck #- structcheck ! deprecated since v1.49.0; replaced by 'unused' - stylecheck #- typecheck diff --git a/command.go b/command.go index b31e22c8..54748fc6 100644 --- a/command.go +++ b/command.go @@ -1460,7 +1460,6 @@ func (c *Command) UseLine() string { // DebugFlags used to determine which flags have been assigned to which commands // and which persist. -// nolint:goconst func (c *Command) DebugFlags() { c.Println("DebugFlags called on", c.Name()) var debugflags func(*Command)